Re: Is it really such a good thing for newNode() to be a macro?

Tom Lane wrote:
>I happened to be looking at nodes.h and started wondering just how
>sane this coding really is:

>extern PGDLLIMPORT Node *newNodeMacroHolder;

>#define newNode(size, tag) \
>( \
> AssertMacro((size) >= sizeof(Node)), /* need the tag, at least */ \
> newNodeMacroHolder = (Node *) palloc0fast(size), \
> newNodeMacroHolder->type = (tag), \
> newNodeMacroHolder \
>)

>Given that we're calling palloc, it's not clear that saving one level of
>function call is really buying much; and what it's costing us is a store
>to a global variable that the compiler has no way to optimize away.
>On a lot of platforms, accessing global variables isn't especially
>cheap. Also, considering that palloc0fast is a nontrivial macro, and
>that there are a LOT of uses of newNode(), we're paying rather a lot of
>code space for a pretty dubious savings.

Correct analysis, I'd say.

>So I'm tempted to get rid of this and just make newNode() an out-of-line
>function.

Getting rid of the global variable is imperative.
However, for the rest you'd have two alternate options (besides making
it a normal function):

a. Use macros like:

#define makeNode(_type_,_variable_) \
newNode(sizeof(_type_), T_##_type_, _variable_)
#define newNode(size, tag, variable) \
do { \
Node * newNodeMacroHolder; \
AssertMacro((size) >= sizeof(Node)); /* need the tag, at least */ \
newNodeMacroHolder = (Node *) palloc0fast(size); \
newNodeMacroHolder->type = (tag); \
_variable_ = newNodeMacroHolder; \
} while(0)

b. Create a function newNode() which is declared as inline, which
basically gives you the same code as under (a).
--
Sincerely,
Stephen R. van den Berg.

"Good moaning!"