Simplify newNode()
The newNode() macro can be turned into a static inline function, which
makes it a lot simpler. See attached. This was not possible when the
macro was originally written, as we didn't require compiler to have
static inline support, but nowadays we do.
This was last discussed in 2008, see discussion at
/messages/by-id/26133.1220037409@sss.pgh.pa.us.
In those tests, Tom observed that gcc refused to inline the static
inline function. That was weird, the function is very small and doesn't
do anything special. Whatever the problem was, I think we can dismiss it
with modern compilers. It does get inlined on gcc 12 and clang 14 that I
have installed.
--
Heikki Linnakangas
Neon (https://neon.tech)
Attachments:
0001-Convert-newNode-into-a-static-inline-function.patchtext/x-patch; charset=UTF-8; name=0001-Convert-newNode-into-a-static-inline-function.patchDownload
From 6ad4c4cf49ef5b3f7ed22acc258a868f1a13f6f4 Mon Sep 17 00:00:00 2001
From: Heikki Linnakangas <heikki.linnakangas@iki.fi>
Date: Thu, 14 Dec 2023 01:08:16 +0100
Subject: [PATCH 1/1] Convert newNode() into a static inline function
Because it's simpler. We didn't require static inline support from
compiler when this was originally written, but now we do.
One complication is that the static inline function doesn't work in
the frontend, because it calls palloc0fast() which isn't included
frontend programs. That's OK, the old macro also didn't work in
frontend programs if you actually tried to call it, but we now need to
explicitly put it in an #ifndef FRONTEND block to keep the compiler
happy.
---
src/backend/nodes/Makefile | 1 -
src/backend/nodes/meson.build | 1 -
src/backend/nodes/nodes.c | 31 -------------------------
src/include/nodes/nodes.h | 43 +++++++++++------------------------
4 files changed, 13 insertions(+), 63 deletions(-)
delete mode 100644 src/backend/nodes/nodes.c
diff --git a/src/backend/nodes/Makefile b/src/backend/nodes/Makefile
index ebbe9052cb7..66bbad8e6e0 100644
--- a/src/backend/nodes/Makefile
+++ b/src/backend/nodes/Makefile
@@ -23,7 +23,6 @@ OBJS = \
makefuncs.o \
multibitmapset.o \
nodeFuncs.o \
- nodes.o \
outfuncs.o \
params.o \
print.o \
diff --git a/src/backend/nodes/meson.build b/src/backend/nodes/meson.build
index 31467a12d3b..1efbf2c11ca 100644
--- a/src/backend/nodes/meson.build
+++ b/src/backend/nodes/meson.build
@@ -7,7 +7,6 @@ backend_sources += files(
'makefuncs.c',
'multibitmapset.c',
'nodeFuncs.c',
- 'nodes.c',
'params.c',
'print.c',
'read.c',
diff --git a/src/backend/nodes/nodes.c b/src/backend/nodes/nodes.c
deleted file mode 100644
index 1913a4bdf7d..00000000000
--- a/src/backend/nodes/nodes.c
+++ /dev/null
@@ -1,31 +0,0 @@
-/*-------------------------------------------------------------------------
- *
- * nodes.c
- * support code for nodes (now that we have removed the home-brew
- * inheritance system, our support code for nodes is much simpler)
- *
- * Portions Copyright (c) 1996-2023, PostgreSQL Global Development Group
- * Portions Copyright (c) 1994, Regents of the University of California
- *
- *
- * IDENTIFICATION
- * src/backend/nodes/nodes.c
- *
- * HISTORY
- * Andrew Yu Oct 20, 1994 file creation
- *
- *-------------------------------------------------------------------------
- */
-#include "postgres.h"
-
-#include "nodes/nodes.h"
-
-/*
- * Support for newNode() macro
- *
- * In a GCC build there is no need for the global variable newNodeMacroHolder.
- * However, we create it anyway, to support the case of a non-GCC-built
- * loadable module being loaded into a GCC-built backend.
- */
-
-Node *newNodeMacroHolder;
diff --git a/src/include/nodes/nodes.h b/src/include/nodes/nodes.h
index 4c32682e4ce..dbb8eed122c 100644
--- a/src/include/nodes/nodes.h
+++ b/src/include/nodes/nodes.h
@@ -132,6 +132,8 @@ typedef struct Node
#define nodeTag(nodeptr) (((const Node*)(nodeptr))->type)
+#ifndef FRONTEND
+
/*
* newNode -
* create a new node of the specified size and tag the node with the
@@ -139,43 +141,24 @@ typedef struct Node
*
* !WARNING!: Avoid using newNode directly. You should be using the
* macro makeNode. eg. to create a Query node, use makeNode(Query)
- *
- * Note: the size argument should always be a compile-time constant, so the
- * apparent risk of multiple evaluation doesn't matter in practice.
- */
-#ifdef __GNUC__
-
-/* With GCC, we can use a compound statement within an expression */
-#define newNode(size, tag) \
-({ Node *_result; \
- AssertMacro((size) >= sizeof(Node)); /* need the tag, at least */ \
- _result = (Node *) palloc0fast(size); \
- _result->type = (tag); \
- _result; \
-})
-#else
-
-/*
- * There is no way to dereference the palloc'ed pointer to assign the
- * tag, and also return the pointer itself, so we need a holder variable.
- * Fortunately, this macro isn't recursive so we just define
- * a global variable for this purpose.
*/
-extern PGDLLIMPORT Node *newNodeMacroHolder;
+static inline Node *
+newNode(size_t size, NodeTag tag)
+{
+ Node *result;
-#define newNode(size, tag) \
-( \
- AssertMacro((size) >= sizeof(Node)), /* need the tag, at least */ \
- newNodeMacroHolder = (Node *) palloc0fast(size), \
- newNodeMacroHolder->type = (tag), \
- newNodeMacroHolder \
-)
-#endif /* __GNUC__ */
+ Assert(size >= sizeof(Node)); /* need the tag, at least */
+ result = (Node *) palloc0fast(size);
+ result->type = tag;
+ return result;
+}
#define makeNode(_type_) ((_type_ *) newNode(sizeof(_type_),T_##_type_))
#define NodeSetTag(nodeptr,t) (((Node*)(nodeptr))->type = (t))
+#endif /* FRONTEND */
+
#define IsA(nodeptr,_type_) (nodeTag(nodeptr) == T_##_type_)
/*
--
2.39.2
Hi,
LGTM.
+ Assert(size >= sizeof(Node)); /* need the tag, at least */
+ result = (Node *) palloc0fast(size);
+ result->type = tag;+ return result;
+}
How about moving the comments /* need the tag, at least */ after result->type = tag; by the way?
Zhang Mingli
www.hashdata.xyz
On Thu, Dec 14, 2023 at 9:34 AM Zhang Mingli <zmlpostgres@gmail.com> wrote:
Hi,
LGTM.
+ Assert(size >= sizeof(Node)); /* need the tag, at least */ + result = (Node *) palloc0fast(size); + result->type = tag;+ return result;
+}How about moving the comments /* need the tag, at least */ after result->type = tag; by the way?
I don't think so, the comment has the meaning of the requested size
should at least the size
of Node, which contains just a NodeTag.
typedef struct Node
{
NodeTag type;
} Node;
Zhang Mingli
www.hashdata.xyz
--
Regards
Junwang Zhao
On 14.12.23 01:48, Heikki Linnakangas wrote:
The newNode() macro can be turned into a static inline function, which
makes it a lot simpler. See attached. This was not possible when the
macro was originally written, as we didn't require compiler to have
static inline support, but nowadays we do.This was last discussed in 2008, see discussion at
/messages/by-id/26133.1220037409@sss.pgh.pa.us.
In those tests, Tom observed that gcc refused to inline the static
inline function. That was weird, the function is very small and doesn't
do anything special. Whatever the problem was, I think we can dismiss it
with modern compilers. It does get inlined on gcc 12 and clang 14 that I
have installed.
I notice that the existing comments point out that the size argument
should be a compile-time constant, but that is no longer the case for
ExtensibleNode(). Also, newNode() is the only caller of palloc0fast(),
which also points out that the size argument should be a compile-time
constant, and palloc0fast() is the only caller of MemSetTest(). I can
see how an older compiler might have gotten too confused by all that.
But if we think that compilers are now smart enough, maybe we can unwind
this whole stack a bit more? Maybe we don't need MemSetTest() and/or
palloc0fast() and/or newNode() at all?
On 14/12/2023 10:32, Peter Eisentraut wrote:
I notice that the existing comments point out that the size argument
should be a compile-time constant, but that is no longer the case for
ExtensibleNode(). Also, newNode() is the only caller of palloc0fast(),
which also points out that the size argument should be a compile-time
constant, and palloc0fast() is the only caller of MemSetTest(). I can
see how an older compiler might have gotten too confused by all that.
But if we think that compilers are now smart enough, maybe we can unwind
this whole stack a bit more? Maybe we don't need MemSetTest() and/or
palloc0fast() and/or newNode() at all?
Good point. Looking closer, modern compilers will actually turn the
MemSetLoop() in MemoryContextAllocZeroAligned() into a call to memset()
anyway! Funny. That is true for recent versions of gcc, clang, and MSVC.
Here's a link to a godbolt snippet to play with this:
https://godbolt.org/z/9b89P3c8x (full link at [0]https://godbolt.org/#z:OYLghAFBqd5QCxAYwPYBMCmBRdBLAF1QCcAaPECAMzwBtMA7AQwFtMQByARg9KtQYEAysib0QXACx8BBAKoBnTAAUAHpwAMvAFYTStJg1DIApACYAQuYukl9ZATwDKjdAGFUtAK4sGe1wAyeAyYAHI%2BAEaYxBIa0gAOqAqETgwe3r56icmOAkEh4SxRMVxxtpj2uQxCBEzEBOk%2Bflzllak1dQT5YZHRsdIKtfWNmS2Dnd2Fxf0AlLaoXsTI7BzmAMzByN5YANQma26D6BGongB0CPvYJhoAguub25h7B0e0eBEXVzf3ZhsMWy8u32hwI6CwVC%2Ba2udweAKeL1BxGCwChMPudwA9AAqHZUYioFg7ZAXHbYzE/B5UHYAfRpAHFQnI3HSXtc1gARHZrSl/CHBZ7vADWFQAnhBVDMbgBOOkRLx0RwMGmYVTxTAOKCSnZgMD7LkaUg7LhS2F8zA0EI7LwMYViiUzWk0%2BWK4IqtUaghax26/U7Q3%2B02/NYVJS8tb8q122jiyUy706vWcwPhyPPG3R2OOhO%2B5MaIPrVx4KiUs0Ri0CnYAWSYqlutFoqGQQjwAC9MPGIC3246NKo1lRB0OqI6ccadsA8MAmBFRQRngBacfk1MVq31xvNtuYACSCgAamI8OgIMke53u5hs2er4i/TW6w2m5eC3dBsQvA5q5gWCRRR5BFVAgOSYWp9isO4CFFdUIR2d9PwIb9f2If9ZCAkDajJKsfz/AD51UAhwJ%2BKCYItODtxpRDLyIyDoMwWD4K/bDkNQwCCI8G152IBQkNwtD2IWQDuJojFbhI%2BiyIAN1QY8dggbFmL/DoCAUZRkUEAAxG1kGzRSULwoDiX4ggjWk2TsXiJgFEGBAP1IUtpRlJyNEclznNcjydiMhhBmJBA6jJcYVJpd8UXs2E3Mijz3JilyvJOTwdnidSCEo1AQrBaJiClNYINE8SGIID8mJw/TjOwggEAwBRKQAdjymUzPQGUvLJeSxE3XTStY/DELQNiTJ2S9yPPXKHLHUQGx2Sq8B4/FMEwGl4h2YJiSs54FEJTABGedY0zADg5uIBaIEdFcIulJr3Pk%2BbFvibMmrJRJgi4nKGpcq64q87F5OOjqmwemT0CemShKNYabzehzPo837MCUAgupYgyCO83qoYumH42xLB6HnGl%2BvRuS9J6wzCaAjGMWlEmUcQ%2BTgEwVLkAQG0hQJ4zAfM56hMpmVqKi%2BnGYJlmGDZhRLOWTngYs0HXpEmUEtoa7sVmlUWHiKCkb4ga0Yp%2BWPqB5WgoULWyp18mCPCqnYuir6SeU1SUq0gEkpSqhtNMoGnqsmy7Icm33K8mnjI4oSeOxIhaloBQrYDqKWp2RXXZetKMqwYhsvlv5i1gqtsCrAB5AAlABNGk3AL0IABVsAADSr8uAAlsDcABpHdQnpaHDYF7FmY1IVTdJ1GLcRrOQwYfAS1hWquWDgaKqq9AarGiKxLowrisQ%2Bfeowpg6ryrzMUxJLgBpBgMEW0CiogGd3yYBxTQ8sdKuiZ46meC%2BdgvrAeNQakZo8XEmSCka8j6gOlKES%2BVcmDAHcuJGittpRjmPIwRwNB4Y7FVA/RCQpgjA3/rrVG50qYvwQPDZ4BAADuqA8R4AqMvHYH8koGGWMDchx1pq0JYMEPAPD2xMPeMABgbBBA7CoVZWoDMQAgIcordys0i4UMIqvZ%2BuIq4vC5F/cWD935PjYeRAEgpJE7GOgjWRF15FRX%2BlQncaRkTCE9KkcCOwxw2KSv9ZAK0GDEgcXgSacEnECAsVTfmHk2AsBpJ40C9FEEv2IA/IUOwIl/kEZuGJwN%2BDEGmggWaRDEIkJlP1XyO8gKL2qmSNglVqpxNxJJPA9QvBiDxNpKo00Zz0BCTKUpqNLLHUEIgmUY4mQBACCtakX8%2BloLkkQeI9BJIVHyWdUBVMemIRoNxAgzM6DNVUWOchTACHUneKLeiOx3i%2BUIds2g6B%2BldJcms12mBJLXN2RBZBuJkrPKcF4HirydiEIUKwZ4UyxGFIed1Wm38gKvNqdCkeuSbkAupECtgHj%2BkFJWUUgQvlmYBWxMwNggyooTWMt/YFcltC/PWSQHYWB5TAEnEYZZDlil9X8tklWWABmqPcqSnWO4uTFiYQwUUlLqV4lpfSrwjKUQsruOAtZbgOoRESWSMxQtkARBXu8scFzEKEI1QQTEuNGbPEmrQVVyAhThyxbPXiZtd6gX3qvUSq5LTPGYkIRmARTjxFPEMQakkxBGnoAwR0JgACsbgHLoFoVGmNF0TD1T2NG/2LlGxGDJLScY9RNFyUzcAMk15A1vVTYm62GaBBFtxBlVAy0/QQELcWuSEA8WcsdBlToexLByRfAWxgMwy0Jv9mmpNLkqGIueBALteaQQ5tmRGsdlaPLYlnYRSw1h80aBcSOpN9rJ10GnfmUsolHrYh%2BGsjcTYABa0RUD1inCEE8jzR7g23CNK8B9u7HgTuqxm%2BtSGYluNZaIXo1l7kPO8E8o8h362QcB0D9QoEEDsW4Pxk1vUOFSG2jmgGZTCogLqa9W52yQaPCeSGr5pReRnCQL0vMLqjwXFcRRyj81UDEGGV1DljqIT9Mxq4VSl4KBY9CTxuGBpGkhvBwjGY8AihjBAPjmi/QjICHB/d70aNMJOPUU68Hk0clPUMzE%2B5bgBHpEXDuHIaR5yrMoAuBcAg0gswEAubhJO9SNHx6T25GNAa9T6v1ynGZGgDDJ11rVkk/gRqFwaEX/P4ZcnxxYPi%2BPy1nj8DgcxaCcEjbwPwHAtCkFQJwGNm7e2bUWMsHtaweCkAIJoHLcwhQgEjYaPLHBJCFea6VzgvAFAgENE14rOXSBwFgEgNA6sj1kAoLh2b9AYjAC4GsMwfBFTRCGxACIfWIjBDqKKTgDWDvMBQgXCI2hPQnd4DN0RBAC62mO2N0g0rgDKobEN7gvAsAsEMMAcQr38DHWwws77JXVQai8POW75BAJdZK%2B8CICSypYD60VPht25hUAMMAA89CqEF3VEVhr/BBAiDEOwKQMhBCKBUOoV7ugWgGCMCgaw1h9AfCG5AOY9aqjfYXAXMwvBUALIzqgnnp1WhBL8BAVwIxmikECAKKYfQWjZBSAIRXWQkha4YJMXoJQZfYYEB0YYngmh6DsLL83XRVdG%2Bt4GnXYxA2G6KOruY1WljU9y/l3rr2yscB2KoAAHAANgXOHyQE5kBeLW2cMwclcCEFpesE0vBRtaCHaQNrHX9CcB66QIrJWg%2BDeG415rOeusi%2BL31svlexs5/F8kZwkggA%3D%3D).
Yeah, +1 on removing all that, including MemoryContextAllocZeroAligned.
It's not doing any good as it is, as it gets compiled to be identical to
MemoryContextAllocZero. (There are small differences depending compiler
and version, but e.g. on gcc 13.2, the code generated for
MemoryContextAllocZero() is actually smaller even though both call memset())
Another approach would be to add more hints to
MemoryContextAllocZeroAligned to dissuade the compiler from turning the
loop into a memset() call. If you add an "if (size > 1024) abort" there,
then gcc 13 doesn't optimize into a memset() call, but clang still does.
Some micro-benchmarks on that would be nice.
But given that the compiler has been doing this optimization for a while
and we haven't noticed, I think memset() should be considered the status
quo, and converting it to a loop again should be considered a new
optimization.
Also, replacing MemoryContextAllocZeroAligned(CurrentMemoryContext,
size) with palloc0(size) has one fewer argument and the assembly code of
the call has one fewer instruction. That's something too.
[0]: https://godbolt.org/#z:OYLghAFBqd5QCxAYwPYBMCmBRdBLAF1QCcAaPECAMzwBtMA7AQwFtMQByARg9KtQYEAysib0QXACx8BBAKoBnTAAUAHpwAMvAFYTStJg1DIApACYAQuYukl9ZATwDKjdAGFUtAK4sGe1wAyeAyYAHI%2BAEaYxBIa0gAOqAqETgwe3r56icmOAkEh4SxRMVxxtpj2uQxCBEzEBOk%2Bflzllak1dQT5YZHRsdIKtfWNmS2Dnd2Fxf0AlLaoXsTI7BzmAMzByN5YANQma26D6BGongB0CPvYJhoAguub25h7B0e0eBEXVzf3ZhsMWy8u32hwI6CwVC%2Ba2udweAKeL1BxGCwChMPudwA9AAqHZUYioFg7ZAXHbYzE/B5UHYAfRpAHFQnI3HSXtc1gARHZrSl/CHBZ7vADWFQAnhBVDMbgBOOkRLx0RwMGmYVTxTAOKCSnZgMD7LkaUg7LhS2F8zA0EI7LwMYViiUzWk0%2BWK4IqtUaghax26/U7Q3%2B02/NYVJS8tb8q122jiyUy706vWcwPhyPPG3R2OOhO%2B5MaIPrVx4KiUs0Ri0CnYAWSYqlutFoqGQQjwAC9MPGIC3246NKo1lRB0OqI6ccadsA8MAmBFRQRngBacfk1MVq31xvNtuYACSCgAamI8OgIMke53u5hs2er4i/TW6w2m5eC3dBsQvA5q5gWCRRR5BFVAgOSYWp9isO4CFFdUIR2d9PwIb9f2If9ZCAkDajJKsfz/AD51UAhwJ%2BKCYItODtxpRDLyIyDoMwWD4K/bDkNQwCCI8G152IBQkNwtD2IWQDuJojFbhI%2BiyIAN1QY8dggbFmL/DoCAUZRkUEAAxG1kGzRSULwoDiX4ggjWk2TsXiJgFEGBAP1IUtpRlJyNEclznNcjydiMhhBmJBA6jJcYVJpd8UXs2E3Mijz3JilyvJOTwdnidSCEo1AQrBaJiClNYINE8SGIID8mJw/TjOwggEAwBRKQAdjymUzPQGUvLJeSxE3XTStY/DELQNiTJ2S9yPPXKHLHUQGx2Sq8B4/FMEwGl4h2YJiSs54FEJTABGedY0zADg5uIBaIEdFcIulJr3Pk%2BbFvibMmrJRJgi4nKGpcq64q87F5OOjqmwemT0CemShKNYabzehzPo837MCUAgupYgyCO83qoYumH42xLB6HnGl%2BvRuS9J6wzCaAjGMWlEmUcQ%2BTgEwVLkAQG0hQJ4zAfM56hMpmVqKi%2BnGYJlmGDZhRLOWTngYs0HXpEmUEtoa7sVmlUWHiKCkb4ga0Yp%2BWPqB5WgoULWyp18mCPCqnYuir6SeU1SUq0gEkpSqhtNMoGnqsmy7Icm33K8mnjI4oSeOxIhaloBQrYDqKWp2RXXZetKMqwYhsvlv5i1gqtsCrAB5AAlABNGk3AL0IABVsAADSr8uAAlsDcABpHdQnpaHDYF7FmY1IVTdJ1GLcRrOQwYfAS1hWquWDgaKqq9AarGiKxLowrisQ%2Bfeowpg6ryrzMUxJLgBpBgMEW0CiogGd3yYBxTQ8sdKuiZ46meC%2BdgvrAeNQakZo8XEmSCka8j6gOlKES%2BVcmDAHcuJGittpRjmPIwRwNB4Y7FVA/RCQpgjA3/rrVG50qYvwQPDZ4BAADuqA8R4AqMvHYH8koGGWMDchx1pq0JYMEPAPD2xMPeMABgbBBA7CoVZWoDMQAgIcordys0i4UMIqvZ%2BuIq4vC5F/cWD935PjYeRAEgpJE7GOgjWRF15FRX%2BlQncaRkTCE9KkcCOwxw2KSv9ZAK0GDEgcXgSacEnECAsVTfmHk2AsBpJ40C9FEEv2IA/IUOwIl/kEZuGJwN%2BDEGmggWaRDEIkJlP1XyO8gKL2qmSNglVqpxNxJJPA9QvBiDxNpKo00Zz0BCTKUpqNLLHUEIgmUY4mQBACCtakX8%2BloLkkQeI9BJIVHyWdUBVMemIRoNxAgzM6DNVUWOchTACHUneKLeiOx3i%2BUIds2g6B%2BldJcms12mBJLXN2RBZBuJkrPKcF4HirydiEIUKwZ4UyxGFIed1Wm38gKvNqdCkeuSbkAupECtgHj%2BkFJWUUgQvlmYBWxMwNggyooTWMt/YFcltC/PWSQHYWB5TAEnEYZZDlil9X8tklWWABmqPcqSnWO4uTFiYQwUUlLqV4lpfSrwjKUQsruOAtZbgOoRESWSMxQtkARBXu8scFzEKEI1QQTEuNGbPEmrQVVyAhThyxbPXiZtd6gX3qvUSq5LTPGYkIRmARTjxFPEMQakkxBGnoAwR0JgACsbgHLoFoVGmNF0TD1T2NG/2LlGxGDJLScY9RNFyUzcAMk15A1vVTYm62GaBBFtxBlVAy0/QQELcWuSEA8WcsdBlToexLByRfAWxgMwy0Jv9mmpNLkqGIueBALteaQQ5tmRGsdlaPLYlnYRSw1h80aBcSOpN9rJ10GnfmUsolHrYh%2BGsjcTYABa0RUD1inCEE8jzR7g23CNK8B9u7HgTuqxm%2BtSGYluNZaIXo1l7kPO8E8o8h362QcB0D9QoEEDsW4Pxk1vUOFSG2jmgGZTCogLqa9W52yQaPCeSGr5pReRnCQL0vMLqjwXFcRRyj81UDEGGV1DljqIT9Mxq4VSl4KBY9CTxuGBpGkhvBwjGY8AihjBAPjmi/QjICHB/d70aNMJOPUU68Hk0clPUMzE%2B5bgBHpEXDuHIaR5yrMoAuBcAg0gswEAubhJO9SNHx6T25GNAa9T6v1ynGZGgDDJ11rVkk/gRqFwaEX/P4ZcnxxYPi%2BPy1nj8DgcxaCcEjbwPwHAtCkFQJwGNm7e2bUWMsHtaweCkAIJoHLcwhQgEjYaPLHBJCFea6VzgvAFAgENE14rOXSBwFgEgNA6sj1kAoLh2b9AYjAC4GsMwfBFTRCGxACIfWIjBDqKKTgDWDvMBQgXCI2hPQnd4DN0RBAC62mO2N0g0rgDKobEN7gvAsAsEMMAcQr38DHWwws77JXVQai8POW75BAJdZK%2B8CICSypYD60VPht25hUAMMAA89CqEF3VEVhr/BBAiDEOwKQMhBCKBUOoV7ugWgGCMCgaw1h9AfCG5AOY9aqjfYXAXMwvBUALIzqgnnp1WhBL8BAVwIxmikECAKKYfQWjZBSAIRXWQkha4YJMXoJQZfYYEB0YYngmh6DsLL83XRVdG%2Bt4GnXYxA2G6KOruY1WljU9y/l3rr2yscB2KoAAHAANgXOHyQE5kBeLW2cMwclcCEFpesE0vBRtaCHaQNrHX9CcB66QIrJWg%2BDeG415rOeusi%2BL31svlexs5/F8kZwkggA%3D%3D
https://godbolt.org/#z:OYLghAFBqd5QCxAYwPYBMCmBRdBLAF1QCcAaPECAMzwBtMA7AQwFtMQByARg9KtQYEAysib0QXACx8BBAKoBnTAAUAHpwAMvAFYTStJg1DIApACYAQuYukl9ZATwDKjdAGFUtAK4sGe1wAyeAyYAHI%2BAEaYxBIa0gAOqAqETgwe3r56icmOAkEh4SxRMVxxtpj2uQxCBEzEBOk%2Bflzllak1dQT5YZHRsdIKtfWNmS2Dnd2Fxf0AlLaoXsTI7BzmAMzByN5YANQma26D6BGongB0CPvYJhoAguub25h7B0e0eBEXVzf3ZhsMWy8u32hwI6CwVC%2Ba2udweAKeL1BxGCwChMPudwA9AAqHZUYioFg7ZAXHbYzE/B5UHYAfRpAHFQnI3HSXtc1gARHZrSl/CHBZ7vADWFQAnhBVDMbgBOOkRLx0RwMGmYVTxTAOKCSnZgMD7LkaUg7LhS2F8zA0EI7LwMYViiUzWk0%2BWK4IqtUaghax26/U7Q3%2B02/NYVJS8tb8q122jiyUy706vWcwPhyPPG3R2OOhO%2B5MaIPrVx4KiUs0Ri0CnYAWSYqlutFoqGQQjwAC9MPGIC3246NKo1lRB0OqI6ccadsA8MAmBFRQRngBacfk1MVq31xvNtuYACSCgAamI8OgIMke53u5hs2er4i/TW6w2m5eC3dBsQvA5q5gWCRRR5BFVAgOSYWp9isO4CFFdUIR2d9PwIb9f2If9ZCAkDajJKsfz/AD51UAhwJ%2BKCYItODtxpRDLyIyDoMwWD4K/bDkNQwCCI8G152IBQkNwtD2IWQDuJojFbhI%2BiyIAN1QY8dggbFmL/DoCAUZRkUEAAxG1kGzRSULwoDiX4ggjWk2TsXiJgFEGBAP1IUtpRlJyNEclznNcjydiMhhBmJBA6jJcYVJpd8UXs2E3Mijz3JilyvJOTwdnidSCEo1AQrBaJiClNYINE8SGIID8mJw/TjOwggEAwBRKQAdjymUzPQGUvLJeSxE3XTStY/DELQNiTJ2S9yPPXKHLHUQGx2Sq8B4/FMEwGl4h2YJiSs54FEJTABGedY0zADg5uIBaIEdFcIulJr3Pk%2BbFvibMmrJRJgi4nKGpcq64q87F5OOjqmwemT0CemShKNYabzehzPo837MCUAgupYgyCO83qoYumH42xLB6HnGl%2BvRuS9J6wzCaAjGMWlEmUcQ%2BTgEwVLkAQG0hQJ4zAfM56hMpmVqKi%2BnGYJlmGDZhRLOWTngYs0HXpEmUEtoa7sVmlUWHiKCkb4ga0Yp%2BWPqB5WgoULWyp18mCPCqnYuir6SeU1SUq0gEkpSqhtNMoGnqsmy7Icm33K8mnjI4oSeOxIhaloBQrYDqKWp2RXXZetKMqwYhsvlv5i1gqtsCrAB5AAlABNGk3AL0IABVsAADSr8uAAlsDcABpHdQnpaHDYF7FmY1IVTdJ1GLcRrOQwYfAS1hWquWDgaKqq9AarGiKxLowrisQ%2Bfeowpg6ryrzMUxJLgBpBgMEW0CiogGd3yYBxTQ8sdKuiZ46meC%2BdgvrAeNQakZo8XEmSCka8j6gOlKES%2BVcmDAHcuJGittpRjmPIwRwNB4Y7FVA/RCQpgjA3/rrVG50qYvwQPDZ4BAADuqA8R4AqMvHYH8koGGWMDchx1pq0JYMEPAPD2xMPeMABgbBBA7CoVZWoDMQAgIcordys0i4UMIqvZ%2BuIq4vC5F/cWD935PjYeRAEgpJE7GOgjWRF15FRX%2BlQncaRkTCE9KkcCOwxw2KSv9ZAK0GDEgcXgSacEnECAsVTfmHk2AsBpJ40C9FEEv2IA/IUOwIl/kEZuGJwN%2BDEGmggWaRDEIkJlP1XyO8gKL2qmSNglVqpxNxJJPA9QvBiDxNpKo00Zz0BCTKUpqNLLHUEIgmUY4mQBACCtakX8%2BloLkkQeI9BJIVHyWdUBVMemIRoNxAgzM6DNVUWOchTACHUneKLeiOx3i%2BUIds2g6B%2BldJcms12mBJLXN2RBZBuJkrPKcF4HirydiEIUKwZ4UyxGFIed1Wm38gKvNqdCkeuSbkAupECtgHj%2BkFJWUUgQvlmYBWxMwNggyooTWMt/YFcltC/PWSQHYWB5TAEnEYZZDlil9X8tklWWABmqPcqSnWO4uTFiYQwUUlLqV4lpfSrwjKUQsruOAtZbgOoRESWSMxQtkARBXu8scFzEKEI1QQTEuNGbPEmrQVVyAhThyxbPXiZtd6gX3qvUSq5LTPGYkIRmARTjxFPEMQakkxBGnoAwR0JgACsbgHLoFoVGmNF0TD1T2NG/2LlGxGDJLScY9RNFyUzcAMk15A1vVTYm62GaBBFtxBlVAy0/QQELcWuSEA8WcsdBlToexLByRfAWxgMwy0Jv9mmpNLkqGIueBALteaQQ5tmRGsdlaPLYlnYRSw1h80aBcSOpN9rJ10GnfmUsolHrYh%2BGsjcTYABa0RUD1inCEE8jzR7g23CNK8B9u7HgTuqxm%2BtSGYluNZaIXo1l7kPO8E8o8h362QcB0D9QoEEDsW4Pxk1vUOFSG2jmgGZTCogLqa9W52yQaPCeSGr5pReRnCQL0vMLqjwXFcRRyj81UDEGGV1DljqIT9Mxq4VSl4KBY9CTxuGBpGkhvBwjGY8AihjBAPjmi/QjICHB/d70aNMJOPUU68Hk0clPUMzE%2B5bgBHpEXDuHIaR5yrMoAuBcAg0gswEAubhJO9SNHx6T25GNAa9T6v1ynGZGgDDJ11rVkk/gRqFwaEX/P4ZcnxxYPi%2BPy1nj8DgcxaCcEjbwPwHAtCkFQJwGNm7e2bUWMsHtaweCkAIJoHLcwhQgEjYaPLHBJCFea6VzgvAFAgENE14rOXSBwFgEgNA6sj1kAoLh2b9AYjAC4GsMwfBFTRCGxACIfWIjBDqKKTgDWDvMBQgXCI2hPQnd4DN0RBAC62mO2N0g0rgDKobEN7gvAsAsEMMAcQr38DHWwws77JXVQai8POW75BAJdZK%2B8CICSypYD60VPht25hUAMMAA89CqEF3VEVhr/BBAiDEOwKQMhBCKBUOoV7ugWgGCMCgaw1h9AfCG5AOY9aqjfYXAXMwvBUALIzqgnnp1WhBL8BAVwIxmikECAKKYfQWjZBSAIRXWQkha4YJMXoJQZfYYEB0YYngmh6DsLL83XRVdG%2Bt4GnXYxA2G6KOruY1WljU9y/l3rr2yscB2KoAAHAANgXOHyQE5kBeLW2cMwclcCEFpesE0vBRtaCHaQNrHX9CcB66QIrJWg%2BDeG415rOeusi%2BL31svlexs5/F8kZwkggA%3D%3D
--
Heikki Linnakangas
Neon (https://neon.tech)
Attachments:
v2-0001-Simplify-newNode-by-removing-now-unnecessary-fast.patchtext/x-patch; charset=UTF-8; name=v2-0001-Simplify-newNode-by-removing-now-unnecessary-fast.patchDownload
From 15e93b1b9fdaf19dd55f943ab4f567b5feed2961 Mon Sep 17 00:00:00 2001
From: Heikki Linnakangas <heikki.linnakangas@iki.fi>
Date: Thu, 14 Dec 2023 14:44:10 +0100
Subject: [PATCH v2 1/1] Simplify newNode() by removing now-unnecessary fast
paths and special cases
- Convert newNode() to a static inline function, to make it simpler. We
didn't require static inline support from compiler when this was
originally written, but now we do.
- Remove MemoryContextAllocZeroAligned(). It was supposed to be a
faster version of MemoryContextAllocZero(), but modern compilers turn
the MemSetLoop() into a call to memset() anyway, making it identical to
MemoryContextAllocZero(). That was the only user of MemSetTest,
MemSetLoop, and palloc0fast(), so remove all those too.
Discussion: XXX
Reviewed-by: Peter Eisentraut
---
src/backend/nodes/Makefile | 1 -
src/backend/nodes/meson.build | 1 -
src/backend/nodes/nodes.c | 31 ---------------------------
src/backend/utils/mmgr/mcxt.c | 38 ---------------------------------
src/include/c.h | 24 ---------------------
src/include/nodes/nodes.h | 40 +++++++++--------------------------
src/include/utils/palloc.h | 14 ------------
7 files changed, 10 insertions(+), 139 deletions(-)
delete mode 100644 src/backend/nodes/nodes.c
diff --git a/src/backend/nodes/Makefile b/src/backend/nodes/Makefile
index ebbe9052cb7..66bbad8e6e0 100644
--- a/src/backend/nodes/Makefile
+++ b/src/backend/nodes/Makefile
@@ -23,7 +23,6 @@ OBJS = \
makefuncs.o \
multibitmapset.o \
nodeFuncs.o \
- nodes.o \
outfuncs.o \
params.o \
print.o \
diff --git a/src/backend/nodes/meson.build b/src/backend/nodes/meson.build
index 31467a12d3b..1efbf2c11ca 100644
--- a/src/backend/nodes/meson.build
+++ b/src/backend/nodes/meson.build
@@ -7,7 +7,6 @@ backend_sources += files(
'makefuncs.c',
'multibitmapset.c',
'nodeFuncs.c',
- 'nodes.c',
'params.c',
'print.c',
'read.c',
diff --git a/src/backend/nodes/nodes.c b/src/backend/nodes/nodes.c
deleted file mode 100644
index 1913a4bdf7d..00000000000
--- a/src/backend/nodes/nodes.c
+++ /dev/null
@@ -1,31 +0,0 @@
-/*-------------------------------------------------------------------------
- *
- * nodes.c
- * support code for nodes (now that we have removed the home-brew
- * inheritance system, our support code for nodes is much simpler)
- *
- * Portions Copyright (c) 1996-2023, PostgreSQL Global Development Group
- * Portions Copyright (c) 1994, Regents of the University of California
- *
- *
- * IDENTIFICATION
- * src/backend/nodes/nodes.c
- *
- * HISTORY
- * Andrew Yu Oct 20, 1994 file creation
- *
- *-------------------------------------------------------------------------
- */
-#include "postgres.h"
-
-#include "nodes/nodes.h"
-
-/*
- * Support for newNode() macro
- *
- * In a GCC build there is no need for the global variable newNodeMacroHolder.
- * However, we create it anyway, to support the case of a non-GCC-built
- * loadable module being loaded into a GCC-built backend.
- */
-
-Node *newNodeMacroHolder;
diff --git a/src/backend/utils/mmgr/mcxt.c b/src/backend/utils/mmgr/mcxt.c
index 9fc83f11f6f..4b30fcaebd0 100644
--- a/src/backend/utils/mmgr/mcxt.c
+++ b/src/backend/utils/mmgr/mcxt.c
@@ -1091,44 +1091,6 @@ MemoryContextAllocZero(MemoryContext context, Size size)
return ret;
}
-/*
- * MemoryContextAllocZeroAligned
- * MemoryContextAllocZero where length is suitable for MemSetLoop
- *
- * This might seem overly specialized, but it's not because newNode()
- * is so often called with compile-time-constant sizes.
- */
-void *
-MemoryContextAllocZeroAligned(MemoryContext context, Size size)
-{
- void *ret;
-
- Assert(MemoryContextIsValid(context));
- AssertNotInCriticalSection(context);
-
- if (!AllocSizeIsValid(size))
- elog(ERROR, "invalid memory alloc request size %zu", size);
-
- context->isReset = false;
-
- ret = context->methods->alloc(context, size);
- if (unlikely(ret == NULL))
- {
- MemoryContextStats(TopMemoryContext);
- ereport(ERROR,
- (errcode(ERRCODE_OUT_OF_MEMORY),
- errmsg("out of memory"),
- errdetail("Failed on request of size %zu in memory context \"%s\".",
- size, context->name)));
- }
-
- VALGRIND_MEMPOOL_ALLOC(context, ret, size);
-
- MemSetLoop(ret, 0, size);
-
- return ret;
-}
-
/*
* MemoryContextAllocExtended
* Allocate space within the specified context using the given flags.
diff --git a/src/include/c.h b/src/include/c.h
index 4b0f5138d83..26bf7ec16e7 100644
--- a/src/include/c.h
+++ b/src/include/c.h
@@ -1060,30 +1060,6 @@ extern void ExceptionalCondition(const char *conditionName,
} while (0)
-/*
- * MemSetTest/MemSetLoop are a variant version that allow all the tests in
- * MemSet to be done at compile time in cases where "val" and "len" are
- * constants *and* we know the "start" pointer must be word-aligned.
- * If MemSetTest succeeds, then it is okay to use MemSetLoop, otherwise use
- * MemSetAligned. Beware of multiple evaluations of the arguments when using
- * this approach.
- */
-#define MemSetTest(val, len) \
- ( ((len) & LONG_ALIGN_MASK) == 0 && \
- (len) <= MEMSET_LOOP_LIMIT && \
- MEMSET_LOOP_LIMIT != 0 && \
- (val) == 0 )
-
-#define MemSetLoop(start, val, len) \
- do \
- { \
- long * _start = (long *) (start); \
- long * _stop = (long *) ((char *) _start + (Size) (len)); \
- \
- while (_start < _stop) \
- *_start++ = 0; \
- } while (0)
-
/*
* Macros for range-checking float values before converting to integer.
* We must be careful here that the boundary values are expressed exactly
diff --git a/src/include/nodes/nodes.h b/src/include/nodes/nodes.h
index 4c32682e4ce..bc1dae98220 100644
--- a/src/include/nodes/nodes.h
+++ b/src/include/nodes/nodes.h
@@ -132,6 +132,7 @@ typedef struct Node
#define nodeTag(nodeptr) (((const Node*)(nodeptr))->type)
+
/*
* newNode -
* create a new node of the specified size and tag the node with the
@@ -139,39 +140,18 @@ typedef struct Node
*
* !WARNING!: Avoid using newNode directly. You should be using the
* macro makeNode. eg. to create a Query node, use makeNode(Query)
- *
- * Note: the size argument should always be a compile-time constant, so the
- * apparent risk of multiple evaluation doesn't matter in practice.
*/
-#ifdef __GNUC__
-
-/* With GCC, we can use a compound statement within an expression */
-#define newNode(size, tag) \
-({ Node *_result; \
- AssertMacro((size) >= sizeof(Node)); /* need the tag, at least */ \
- _result = (Node *) palloc0fast(size); \
- _result->type = (tag); \
- _result; \
-})
-#else
-
-/*
- * There is no way to dereference the palloc'ed pointer to assign the
- * tag, and also return the pointer itself, so we need a holder variable.
- * Fortunately, this macro isn't recursive so we just define
- * a global variable for this purpose.
- */
-extern PGDLLIMPORT Node *newNodeMacroHolder;
+static inline Node *
+newNode(size_t size, NodeTag tag)
+{
+ Node *result;
-#define newNode(size, tag) \
-( \
- AssertMacro((size) >= sizeof(Node)), /* need the tag, at least */ \
- newNodeMacroHolder = (Node *) palloc0fast(size), \
- newNodeMacroHolder->type = (tag), \
- newNodeMacroHolder \
-)
-#endif /* __GNUC__ */
+ Assert(size >= sizeof(Node)); /* need the tag, at least */
+ result = (Node *) palloc0(size);
+ result->type = tag;
+ return result;
+}
#define makeNode(_type_) ((_type_ *) newNode(sizeof(_type_),T_##_type_))
#define NodeSetTag(nodeptr,t) (((Node*)(nodeptr))->type = (t))
diff --git a/src/include/utils/palloc.h b/src/include/utils/palloc.h
index d1146c12351..cf98ddc0ec9 100644
--- a/src/include/utils/palloc.h
+++ b/src/include/utils/palloc.h
@@ -70,7 +70,6 @@ extern PGDLLIMPORT MemoryContext CurrentMemoryContext;
*/
extern void *MemoryContextAlloc(MemoryContext context, Size size);
extern void *MemoryContextAllocZero(MemoryContext context, Size size);
-extern void *MemoryContextAllocZeroAligned(MemoryContext context, Size size);
extern void *MemoryContextAllocExtended(MemoryContext context,
Size size, int flags);
extern void *MemoryContextAllocAligned(MemoryContext context,
@@ -109,19 +108,6 @@ extern void pfree(void *pointer);
#define repalloc_array(pointer, type, count) ((type *) repalloc(pointer, sizeof(type) * (count)))
#define repalloc0_array(pointer, type, oldcount, count) ((type *) repalloc0(pointer, sizeof(type) * (oldcount), sizeof(type) * (count)))
-/*
- * The result of palloc() is always word-aligned, so we can skip testing
- * alignment of the pointer when deciding which MemSet variant to use.
- * Note that this variant does not offer any advantage, and should not be
- * used, unless its "sz" argument is a compile-time constant; therefore, the
- * issue that it evaluates the argument multiple times isn't a problem in
- * practice.
- */
-#define palloc0fast(sz) \
- ( MemSetTest(0, sz) ? \
- MemoryContextAllocZeroAligned(CurrentMemoryContext, sz) : \
- MemoryContextAllocZero(CurrentMemoryContext, sz) )
-
/* Higher-limit allocators. */
extern void *MemoryContextAllocHuge(MemoryContext context, Size size);
extern pg_nodiscard void *repalloc_huge(void *pointer, Size size);
--
2.39.2
Heikki Linnakangas <hlinnaka@iki.fi> writes:
On 14/12/2023 10:32, Peter Eisentraut wrote:
But if we think that compilers are now smart enough, maybe we can unwind
this whole stack a bit more? Maybe we don't need MemSetTest() and/or
palloc0fast() and/or newNode() at all?
Good point. Looking closer, modern compilers will actually turn the
MemSetLoop() in MemoryContextAllocZeroAligned() into a call to memset()
anyway! Funny. That is true for recent versions of gcc, clang, and MSVC.
I experimented with the same planner-intensive test case that I used
in the last discussion back in 2008. I got these results:
HEAD:
tps = 144.974195 (without initial connection time)
v1 patch:
tps = 146.302004 (without initial connection time)
v2 patch:
tps = 144.882208 (without initial connection time)
While there's not much daylight between these numbers, the times are
quite reproducible for me. This is with RHEL8's gcc 8.5.0 on x86_64.
That's probably a bit trailing-edge in terms of what people might be
using with v17, but I don't think it's discountable.
I also looked at the backend's overall code size per size(1):
HEAD:
text data bss dec hex filename
8613007 100192 220176 8933375 884fff testversion.stock/bin/postgres
v1 patch:
text data bss dec hex filename
8615126 100192 220144 8935462 885826 testversion.v1/bin/postgres
v2 patch:
text data bss dec hex filename
8595322 100192 220144 8915658 880aca testversion.v2/bin/postgres
I did check that the v1 patch successfully inlines newNode() and
reduces it to just a MemoryContextAllocZeroAligned call, so it's
correct that modern compilers do that better than whatever I tested
in 2008. But I wonder what is happening in v2 to reduce the code
size so much. MemoryContextAllocZeroAligned is not 20kB by itself.
Good point. Looking closer, modern compilers will actually turn the
MemSetLoop() in MemoryContextAllocZeroAligned() into a call to memset()
anyway! Funny. That is true for recent versions of gcc, clang, and MSVC.
Not here ...
Yeah, +1 on removing all that, including MemoryContextAllocZeroAligned.
It's not doing any good as it is, as it gets compiled to be identical to
MemoryContextAllocZero.
Also not so here. Admittedly, my results don't make much of a case
for keeping the two code paths, even on compilers where it matters.
regards, tom lane
On Fri, Dec 15, 2023 at 11:44 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
Heikki Linnakangas <hlinnaka@iki.fi> writes:
Yeah, +1 on removing all that, including MemoryContextAllocZeroAligned.
It's not doing any good as it is, as it gets compiled to be identical to
MemoryContextAllocZero.Also not so here. Admittedly, my results don't make much of a case
for keeping the two code paths, even on compilers where it matters.
FWIW here is what I figured out once about why it gets compiled the
same these days:
/messages/by-id/CA+hUKGLfa6ANa0vs7Lf0op0XBH05HE8SyX8NFhDyT7k2CHYLXw@mail.gmail.com
On Fri, Dec 15, 2023 at 5:44 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
I did check that the v1 patch successfully inlines newNode() and
reduces it to just a MemoryContextAllocZeroAligned call, so it's
correct that modern compilers do that better than whatever I tested
in 2008. But I wonder what is happening in v2 to reduce the code
size so much. MemoryContextAllocZeroAligned is not 20kB by itself.
I poked at this a bit and it seems to come from what Heikki said
upthread about fewer instructions before the calls: Running objdump on
v1 and v2 copyfuncs.o and diff'ing shows there are fewer MOV
instructions (some extraneous stuff removed):
e9 da 5f 00 00 jmp <_copyReindexStmt>
- 48 8b 05 00 00 00 00 mov rax,QWORD PTR [rip+0x0]
- be 18 00 00 00 mov esi,0x18
- 48 8b 38 mov rdi,QWORD PTR [rax]
- e8 00 00 00 00 call MemoryContextAllocZeroAligned-0x4
+ bf 18 00 00 00 mov edi,0x18
+ e8 00 00 00 00 call palloc0-0x4That's 10 bytes savings.
- 48 8b 05 00 00 00 00 mov rax,QWORD PTR [rip+0x0]
- 48 8b 38 mov rdi,QWORD PTR [rax]
- e8 00 00 00 00 call MemoryContextAllocZeroAligned-0x4
+ e8 00 00 00 00 call palloc0-0x4...another 10 bytes. Over and over again.
Because of the size differences, the compiler is inlining more: e.g.
in v1 _copyFieldStore has 4 call sites, but in v2 it got inlined.
About the patch, I'm wondering if this whitespace is intentional, but
it's otherwise straightforward:
--- a/src/include/nodes/nodes.h
+++ b/src/include/nodes/nodes.h
@@ -132,6 +132,7 @@ typedef struct Node#define nodeTag(nodeptr) (((const Node*)(nodeptr))->type)
+
/*
On 15/12/2023 00:44, Tom Lane wrote:
Good point. Looking closer, modern compilers will actually turn the
MemSetLoop() in MemoryContextAllocZeroAligned() into a call to memset()
anyway! Funny. That is true for recent versions of gcc, clang, and MSVC.Not here ...
Hmm, according to godbolt, the change happened in GCC version 10.1.
Starting with gcc 10.1, it is turned into a memset(). On clang, the same
change happened in version 3.4.1.
I think we have consensus on patch v2. It's simpler and not less
performant than what we have now, at least on modern compilers. Barring
objections, I'll commit that.
I'm not planning to spend more time on this, but there might be some
room for further optimization if someone is interested to do the
micro-benchmarking. The obvious thing would be to persuade modern
compilers to not switch to memset() in MemoryContextAllocZeroAligned
(*), making the old macro logic work the same way it used to on old
compilers.
Also, instead of palloc0, it might be better for newNode() to call
palloc followed by memset. That would allow the compiler to partially
optimize away the memset. Most callers fill at least some of the fields
after calling makeNode(), so the compiler could generate code that
clears only the uninitialized fields and padding bytes.
(*) or rather, a new function like MemoryContextAllocZeroAligned but
without the 'context' argument. We want to keep the savings in the
callers from eliminating the extra argument.
--
Heikki Linnakangas
Neon (https://neon.tech)
On 18/12/2023 16:28, Heikki Linnakangas wrote:
I think we have consensus on patch v2. It's simpler and not less
performant than what we have now, at least on modern compilers. Barring
objections, I'll commit that.
Committed that.
--
Heikki Linnakangas
Neon (https://neon.tech)