Re: inline newNode()

Bruce Momjian wrote:
> Tom Lane wrote:
> > Bruce Momjian <pgman@candle.pha.pa.us> writes:
> > > Right, palloc shouldn't.  I was thinking of having another version of
> > > palloc that _does_ clear out memory, and calling that from a newNode()
> > > macro.  We already know palloc is going to call MemoryContextAlloc, so
> > > we could have a pallocC() that calls a new MemoryContextAllocC() that
> > > would call the underlying memory allocation function, then do the loop
> > > like MemSet to clear it.
> >
> > But if the MemSet is inside the called function then it cannot reduce
> > the if-tests to a compile-time decision to invoke the word-zeroing loop.
> > We want the MemSet to be expanded at the newNode call site, where the
> > size of the allocated memory is a compile-time constant.
>
> I can easily do the tests in the MemSet macro, but I can't do a loop in
> a macro that has to return a value;  I need while().  Though a loop in a
> new fuction will not be as fast as a MemSet macro, I think it will be
> better than what we have now with newNode only because newNode will be a
> macro and not a function anymore, i.e. the MemSet will happen in the
> function called by pallocC and not in newNode anymore, and there will be
> zero code bloat.  I wish I saw another way.

OK, here's a patch for testing.  It needs cleanup because the final
version would remove the nodes/nodes.c file.  The net effect of the
patch is to make newNode a macro with little code bloat.

--
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 359-1001
  +  If your life is a hard drive,     |  13 Roberts Road
  +  Christ can be your backup.        |  Newtown Square, Pennsylvania 19073
Index: src/backend/nodes/nodes.c
===================================================================
RCS file: /cvsroot/pgsql-server/src/backend/nodes/nodes.c,v
retrieving revision 1.15
diff -c -c -r1.15 nodes.c
*** src/backend/nodes/nodes.c    20 Jun 2002 20:29:29 -0000    1.15
--- src/backend/nodes/nodes.c    9 Oct 2002 05:17:13 -0000
***************
*** 28,42 ****
   *      macro makeNode. eg. to create a Resdom node, use makeNode(Resdom)
   *
   */
! Node *
! newNode(Size size, NodeTag tag)
! {
!     Node       *newNode;

-     Assert(size >= sizeof(Node));        /* need the tag, at least */
-
-     newNode = (Node *) palloc(size);
-     MemSet((char *) newNode, 0, size);
-     newNode->type = tag;
-     return newNode;
- }
--- 28,32 ----
   *      macro makeNode. eg. to create a Resdom node, use makeNode(Resdom)
   *
   */
! Node *newNodeMacroHolder;

Index: src/backend/utils/mmgr/mcxt.c
===================================================================
RCS file: /cvsroot/pgsql-server/src/backend/utils/mmgr/mcxt.c,v
retrieving revision 1.32
diff -c -c -r1.32 mcxt.c
*** src/backend/utils/mmgr/mcxt.c    12 Aug 2002 00:36:12 -0000    1.32
--- src/backend/utils/mmgr/mcxt.c    9 Oct 2002 05:17:17 -0000
***************
*** 453,458 ****
--- 453,481 ----
  }

  /*
+  * MemoryContextAllocC
+  *        Like MemoryContextAlloc, but clears allocated memory
+  *
+  *    We could just call MemoryContextAlloc then clear the memory, but this
+  *    function is called too many times, so we have a separate version.
+  */
+ void *
+ MemoryContextAllocC(MemoryContext context, Size size)
+ {
+     void *ret;
+
+     AssertArg(MemoryContextIsValid(context));
+
+     if (!AllocSizeIsValid(size))
+         elog(ERROR, "MemoryContextAlloc: invalid request size %lu",
+              (unsigned long) size);
+
+     ret = (*context->methods->alloc) (context, size);
+     MemSet(ret, 0, size);
+     return ret;
+ }
+
+ /*
   * pfree
   *        Release an allocated chunk.
   */
Index: src/include/nodes/nodes.h
===================================================================
RCS file: /cvsroot/pgsql-server/src/include/nodes/nodes.h,v
retrieving revision 1.118
diff -c -c -r1.118 nodes.h
*** src/include/nodes/nodes.h    31 Aug 2002 22:10:47 -0000    1.118
--- src/include/nodes/nodes.h    9 Oct 2002 05:17:20 -0000
***************
*** 261,266 ****
--- 261,284 ----

  #define nodeTag(nodeptr)        (((Node*)(nodeptr))->type)

+ /*
+  *    There is no way to dereference the palloc'ed pointer to assign the
+  *    tag, and return the pointer itself, so we need a holder variable.
+  *    Fortunately, this function isn't recursive so we just define
+  *    a global variable for this purpose.
+  */
+ extern Node *newNodeMacroHolder;
+
+ #define newNode(size, tag) \
+ ( \
+     AssertMacro((size) >= sizeof(Node)),        /* need the tag, at least */ \
+ \
+     newNodeMacroHolder = (Node *) pallocC(size), \
+     newNodeMacroHolder->type = (tag), \
+     newNodeMacroHolder \
+ )
+
+
  #define makeNode(_type_)        ((_type_ *) newNode(sizeof(_type_),T_##_type_))
  #define NodeSetTag(nodeptr,t)    (((Node*)(nodeptr))->type = (t))

***************
*** 281,291 ****
   *                      extern declarations follow
   * ----------------------------------------------------------------
   */
-
- /*
-  * nodes/nodes.c
-  */
- extern Node *newNode(Size size, NodeTag tag);

  /*
   * nodes/{outfuncs.c,print.c}
--- 299,304 ----
Index: src/include/utils/palloc.h
===================================================================
RCS file: /cvsroot/pgsql-server/src/include/utils/palloc.h,v
retrieving revision 1.19
diff -c -c -r1.19 palloc.h
*** src/include/utils/palloc.h    20 Jun 2002 20:29:53 -0000    1.19
--- src/include/utils/palloc.h    9 Oct 2002 05:17:21 -0000
***************
*** 46,53 ****
--- 46,56 ----
   * Fundamental memory-allocation operations (more are in utils/memutils.h)
   */
  extern void *MemoryContextAlloc(MemoryContext context, Size size);
+ extern void *MemoryContextAllocC(MemoryContext context, Size size);

  #define palloc(sz)    MemoryContextAlloc(CurrentMemoryContext, (sz))
+
+ #define pallocC(sz)    MemoryContextAllocC(CurrentMemoryContext, (sz))

  extern void pfree(void *pointer);