On 1 Feb 2009, at 21:35, Robert Haas wrote:
On Sun, Feb 1, 2009 at 3:25 PM, Grzegorz Jaskiewicz <g...@pointblue.com.pl
> wrote:
I don't like the fact that you hardcoded that here. I know that you
are
trying to pass on few calls in one go here, but still... ugly.
Well, I think you'll find that using a dynamically sized data
structure destroys the possibility of squeezing any additional
performance out of this part of the code. The nice thing about
fixed-size data structures is that they cost essentially nothing to
stack-allocate; you just move the stack pointer and away you go. We
should in fact be looking for MORE places where we can avoid the use
of constructs like List, since the second-highest CPU hog in my tests
was AllocSetAlloc(), beaten out only by add_path(). With this patch
applied, AllocSetAlloc() moves up to first.
well, true - but also, statically allocated table, without any
predefined size (with #DEFINE) , and no boundary check - is bad as well.
I suppose , this code is easy enough to let it be with your changes,
but I would still call it not pretty.
Hmm, well I didn't either, but there's this handy tool called gprof
that you might want to try out. I wouldn't be wasting my time
patching this part of the code if it didn't make a difference, and in
fact if you do 10% of the amount of benchmarking that I did in the
process of creating this patch, you will find that it in fact does
make a difference.
To be honest, I really didn't had a time to run it down with your
patch and gprof. I believe that you did that already, hence your
suggestion, right ?
Actually - if you did profile postgresql with bunch of queries, I
wouldn't mind to see results of it - I don't know whether it makes
sense to send that to the list (I would think it does), but if it is
too big, or something - you could send it to me in private.
It's already static to that .c file, so the compiler likely will
inline it. In fact, I suspect you will find that removing the static
keyword from the implementation of that function in CVS HEAD is itself
sufficient to produce a small but measurable slowdown in planning of
large join trees, exactly because it will defeat inlining.
that depends on many things, including whether optimizations are on or
not.
Because that function basically consists of two ifs essentially - it
could easily be turned into two separate inlines/macros - that would
remove any function's specific overhead (stack alloc, etc, etc).
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
