Re: inline newNode()

From: Bruce Momjian <pgman(at)candle(dot)pha(dot)pa(dot)us>
To: Peter Eisentraut <peter_e(at)gmx(dot)net>
Cc: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Neil Conway <neilc(at)samurai(dot)com>, PostgreSQL Patches <pgsql-patches(at)postgresql(dot)org>
Subject: Re: inline newNode()
Date: 2002-10-09 00:07:30
Message-ID: 200210090007.g9907U105087@candle.pha.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers 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(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

Attachment Content-Type Size
unknown_filename text/plain 2.2 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Bruce Momjian 2002-10-09 00:26:57 Re: [HACKERS] Hot Backup
Previous Message Josh Berkus 2002-10-08 23:36:40 Re: CHAR, VARCHAR, TEXT (Was Large Databases)

Browse pgsql-patches by date

  From Date Subject
Next Message Tom Lane 2002-10-09 04:12:12 Re: inline newNode()
Previous Message Neil Conway 2002-10-08 22:59:44 Re: inline newNode()