Re: MemSet inline for newNode

Tom Lane wrote:
> Bruce Momjian <pgman(at)candle(dot)pha(dot)pa(dot)us> writes:
> >> But your patch as committed does NOT inline newNode, in fact it does the
> >> exact opposite: the MemSet macro is removed from the callsite.
>
> > Yes, there were actually two patches; the first inlined newNode by
> > calling palloc0. Without palloc, there was no way to inline newNode.
>
> But you have *not* inlined newNode: the memset is still done at a location
> that cannot know the size at compile time.

I am sorry. I inlined newNode, but by pushing MemSet into palloc0, the
MemSet conditional tests can not be optimized away. Is that clearer?

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

In this case, I needed palloc0 to make newNode inlined, and the old
function version of newNode couldn't optimize away the conditional test
either because it was called with various sizes.

> > The second was a more general one to merge palloc/MemSet into a single
> > palloc0 call. That has been backed out while I research it.
>
> That part could be sold just on the basis of making the code easier
> to read and less error-prone; particularly for call sites where the
> length is a runtime computation anyway. I think that newNode() is the

Yes, that was my thought.

> principal case where the length is knowable at compile time. So my
> feeling is that the general change to invent a palloc0() call is fine
> ... but if you want performance then newNode() should *not* be using the
> generic palloc0() call.

OK. I am still struggling about how to do that.

> > My new idea is to add a third boolean argument to
> > MemoryContextAllocZero() which will control whether the MemSet
> > assignment loop is used, or memset().
>
> But we are *trying to eliminate any runtime test whatever*. Short
> of that, you aren't going to get the speedup. Certainly passing a third
> argument to the alloc subroutine will eat enough cycles to negate any
> savings from simplifying the runtime test slightly.

My idea is that the conditional tests will be optimized away before the
palloc0 call is made.

Attached is a patch that implements the MemSet split and allows the size
tests to be optimized away _before_ at the call location. It isn't done
yet, but you can get the idea.

--
Bruce Momjian | http://candle.pha.pa.us
pgman(at)candle(dot)pha(dot)pa(dot)us | (610) 359-1001
+ If your life is a hard drive, | 13 Roberts Road
+ Christ can be your backup. | Newtown Square, Pennsylvania 19073