Re: inline newNode()
От | Bruce Momjian |
---|---|
Тема | Re: inline newNode() |
Дата | |
Msg-id | 200210090007.g9907U105087@candle.pha.pa.us обсуждение исходный текст |
Ответ на | Re: inline newNode() (Peter Eisentraut <peter_e@gmx.net>) |
Ответы |
Re: inline newNode()
(Tom Lane <tgl@sss.pgh.pa.us>)
|
Список | pgsql-patches |
Peter Eisentraut wrote: > Tom Lane writes: > > > A brute-force approach is to say "we know _start is word-aligned because > > we just got it from palloc, which guarantees MAXALIGNment". We could > > make a variant version of MemSet that omits the alignment check, and use > > it here and anywhere else we're sure it's safe. > > Or make a version of palloc that zeroes the memory. OK, here is a version of newNode that is a macro. However, there are problems: First, I can't get the code to assign the tag (dereference the pointer) _and_ return the pointer, in a macro. I can convert it to take a third pointer argument, and it is only called <10 times, so that is easy, but it is used by makeNode, which is called ~1000 times, so that seems like a bad idea. My solution was to declare a global variable in the header file to be used by the macro. Because the macro isn't called recursively, that should be fine. Now, the other problem is MemSet. MemSet isn't designed to be called inside a macro. In fact, the 'if' tests are easy to do in a macro with "? :", but the "while" loop seems to be impossible in a macro. I tried seeing if 'goto' would work in a macro, but of course it doesn't, and even if it did, I doubt it would be portable. The patch merely calls memset() rather than MemSet. Not sure if this is a win or not. It makes makeNode a macro, but removes the use of the MemSet macro in favor of memset(). Does anyone have additional suggestions? The only thing I can suggest is to make a clear-memory version of palloc because palloc always calls MemoryContextAlloc() so I can put it in there. How does that sound? However, now that I look at it, MemoryContextAlloc() could be made a macro easier than newNode() and that would save function call in more cases than newNode. In fact, the comment at the top of MemoryContextAlloc() says: * This could be turned into a macro, but we'd have to import * nodes/memnodes.h into postgres.h which seems a bad idea. Ideas? The regression tests do pass with this patch, so functionally it works fine. -- 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 8 Oct 2002 23:52:21 -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/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 8 Oct 2002 23:52:25 -0000 *************** *** 261,266 **** --- 261,285 ---- #define nodeTag(nodeptr) (((Node*)(nodeptr))->type) + /* + * There is no way to deferernce 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 *) palloc(size), \ + memset((char *)newNodeMacroHolder, 0, (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} --- 300,305 ----
В списке pgsql-patches по дате отправления: