Thread: list rewrite committed

I've applied the list rewrite patch to CVS HEAD. I've also sent a 
copy of the patch I applied to the -patches list.

Notes:

- the tree compiles without warnings and passes the regression 
tests. I'm not aware of any bugs, regression failures, or compiler 
warnings caused by the list rewrite patch -- please let me know if 
you encounter anything

- client code that uses the List API is _not_ source compatible with 
the new List API. The most common change will be the need to change 
the foreach() iteration variable to a ListCell*, rather than a 
List*. There are also some subtle changes in behavior: for example, 
lcons() is now destructive (in the sense that you cannot call it on 
a list and expect the list you pass to lcons() to be unmodified; 
this was the case with the old list API)

Remaining work:

- investigate inline functions for non-GCC compilers

- disable the use of the compatibility API throughout the tree and 
change the code over to use the new API function names. This ought 
to be largely a mechanical search and replace operation -- any 
patches are welcome. I plan to start converting the remainder of the 
tree in this fashion tomorrow.

- use the new for_each_cell() and forboth() macros throughout the 
tree, as appropriate.

- remove the FastList API -- there is no need for it anymore. Tom, 
would you like to do this or should I?

- anything else?

Thanks to Tom and Alvaro for their assistance in completing this work.

-Neil

> - anything else?

All compiles and passes regression tests on FreeBSD...

Chris

On May 26, 2004, at 12:47 AM, Neil Conway wrote:

> I've applied the list rewrite patch to CVS HEAD. I've also sent a copy 
> of the patch I applied to the -patches list.

Nifty.

Do we have any numbers as to how much this will help things?

If not, would something like a pg_bench exercise the new code enough to 
see the results?

--
Jeff Trout <jeff@jefftrout.com>
http://www.jefftrout.com/
http://www.stuarthamm.net/

Jeff wrote:
> Do we have any numbers as to how much this will help things?

No, I haven't done any benchmarking yet (I might do some before I 
leave for the summer, but it's not a priority...)

FWIW, the performance improvement from this patch won't be as large 
as it might be, since Tom already replaced some lappend() hot spots 
with the "FastList" code. The new list API makes that optimization 
global, so we'll fix anywhere that fell through the cracks.

> If not, would something like a pg_bench exercise the new code enough to 
> see the results?

Something like TPC-H would be better, I'd think.

-Neil

Neil Conway <neilc@samurai.com> writes:
>> If not, would something like a pg_bench exercise the new code enough to 
>> see the results?

> Something like TPC-H would be better, I'd think.

You'd only be likely to notice a difference in queries with hundreds of
selected columns, CASE expressions with hundreds of alternatives, that
kind of thing.  I doubt that any of the standard benchmarks really
stress this sort of problem.

As Neil mentioned, we'd already tamped down the more common cases with
the FastList kluge.  But there definitely are cases that we'd not
covered with FastList, some because it was notationally impractical
and some because we'd just not noticed a problem.  For instance, here
http://archives.postgresql.org/pgsql-hackers/2004-03/msg00696.php
are some profiles documenting a case where nearly 40% of the runtime
goes into lappend's in 7.4.  I haven't had time to repeat the test case
but I'd think that time is near-zero in CVS tip.
        regards, tom lane

Tom Lane <tgl@sss.pgh.pa.us> writes:

> For instance, here
> http://archives.postgresql.org/pgsql-hackers/2004-03/msg00696.php are some
> profiles documenting a case where nearly 40% of the runtime goes into
> lappend's in 7.4. I haven't had time to repeat the test case but I'd think
> that time is near-zero in CVS tip.

Of course now ExecEvalExpr's share of runtime in that test is probably up to
28-35% in those tests. Or did you get to doing the things you proposed with
making it a macro?

-- 
greg

Greg Stark <gsstark@mit.edu> writes:
> Tom Lane <tgl@sss.pgh.pa.us> writes:
>> For instance, here
>> http://archives.postgresql.org/pgsql-hackers/2004-03/msg00696.php are some
>> profiles documenting a case where nearly 40% of the runtime goes into
>> lappend's in 7.4. I haven't had time to repeat the test case but I'd think
>> that time is near-zero in CVS tip.

> Of course now ExecEvalExpr's share of runtime in that test is probably up to
> 28-35% in those tests. Or did you get to doing the things you proposed with
> making it a macro?

I did.  I'd like to re-run that test, but have no time for it right now
--- too much to do before feature freeze.
        regards, tom lane