Euler Taveira de Oliveira wrote:
> Tom Lane escreveu:
>
> > Hmm ... I'd not looked at that patch before, but now that I have I think
> > it's gone pretty seriously off on the overdesigned-and-inefficient end
> > of the spectrum. Turning RelationGetFillFactor and friends from simple
> > macros into functions that are probably *at least* a thousand times slower
> > than the macros doesn't seem like a good idea at all. Deconstructing a
> > reloptions datum is supposed to be done once by the relcache, not every
> > time one of the values is needed. Frankly I'd throw the entire thing
> > away and go back to a hardwired set of options feeding into a predefined
> > struct that's held by the relcache and examined by callers.
> >
> I think about that but I don't do any benchmarks after I finished the
> patch. We can do an exception as the current code do for "oids" and
> don't rip out the StdRdOptions just to maitain the
> RelationGetFillFactor() and others like a macro. Honestly, I don't like
> to bring RelationGet*() to reloptions.c but we can always refactor that
> before committing it.
I think the point is not only not regressing in fillfactor's
performance, but also to get good performance for the other options.
The problem with this design is that to find any option you have to
trawl the arrays and do strcmp() on each to find whether it's the one
we're looking for, which seems a loser. I thought about keeping the
array sorted and then doing bsearch, but Tom's right that this is still
a lot slower than a predefined struct.
> Alvaro, let me know if you want me to send another patch; or will you do
> it? I have some small corrections too.
I've already modified your patch a bit ... please send your updated
patch so I can merge it into mine. However, my changes were also
relatively minor. Since Tom wants it to be entirely rewritten then
maybe merging minor fixes to it is a waste of time ...
--
Alvaro Herrera http://www.CommandPrompt.com/
PostgreSQL Replication, Consulting, Custom Development, 24x7 support
