Re: embedded list v2

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>
Cc: Andres Freund <andres(at)2ndquadrant(dot)com>, Pg Hackers <pgsql-hackers(at)postgresql(dot)org>, Robert Haas <robertmhaas(at)gmail(dot)com>, Peter Geoghegan <peter(at)2ndquadrant(dot)com>
Subject: Re: embedded list v2
Date: 2012-09-14 20:57:54
Message-ID: 21666.1347656274@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Alvaro Herrera <alvherre(at)2ndquadrant(dot)com> writes:
> One thing I would like more input in, is whether people think it's
> worthwhile to split dlists and slists into separate files. Thus far
> this has been mentioned by three people independently.

They're small enough and similar enough that one header and one .c file
seem like plenty; but I don't really have a strong opinion about it.

> Another question is whether ilist_container() should actually be a more
> general macro "containerof" defined in c.h. (ISTM it would be necessary
> to have this macro if we want to split into two files; that way we don't
> need to have two macros dlist_container and slist_container with
> identical definition, or alternatively a third file that defines just
> ilist_container)

I'd vote for not having that at all, but rather two separate macros
dlist_container and slist_container. If we had a bunch of operations
that could work interchangeably on dlists and slists, it might be worth
having a concept of "ilist" --- but if we only have this, it would be
better to remove the concept from the API altogether.

> Third question is about the INLINE_IF_POSSIBLE business as commented by
> Peter. It seems to me that the simple technique used here to avoid
> having two copies of the source could be used by memcxt.c, list.c,
> sortsupport.c as well (maybe clean up fastgetattr too).

Yeah, looks reasonable ... material for a different patch of course.
But that would mean INLINE_IF_POSSIBLE should be defined someplace else,
perhaps c.h. Also, I'm not that thrilled with having the header file
define ILIST_USE_DEFINITION. I suggest that it might be better to do

#if defined(USE_INLINE) || defined(DEFINE_ILIST_FUNCTIONS)
... function decls here ...
#else
... extern decls here ...
#endif

where ilist.c defines DEFINE_ILIST_FUNCTIONS before including the
header.

regards, tom lane

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Tom Lane 2012-09-14 21:49:09 Re: Identity projection
Previous Message Peter Eisentraut 2012-09-14 20:49:55 Re: contrib translations