Horton

Re: Inlining comparators as a performance optimisation

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

View thread
From: Tom Lane <tgl@sss.pgh.pa.us>
To: Robert Haas <robertmhaas@gmail.com>
Cc: Bruce Momjian <bruce@momjian.us>, Peter Geoghegan <peter@2ndquadrant.com>, PG Hackers <pgsql-hackers@postgresql.org>
Date: 2011-12-07T15:09:32Z
Lists: pgsql-hackers

Commits

Same data as JSON: GET /api/v1/messages/:b64id/commits the thread's linked commits as JSON, with link sources. API reference →

  1. Speed up conversion of signed integers to C strings.

  2. Remove some unnecessary tests of pgstat_track_counts.

  3. Remove cvs keywords from all files.

  4. Code cleanup for function prototypes: change two K&R-style prototypes

  5. Use Min() instead of min() in qsort, for consistency and to avoid

  6. pgindent run for 8.2.

  7. Switch over to using our own qsort() all the time, as has been proposed

Robert Haas <robertmhaas@gmail.com> writes:
> On Tue, Dec 6, 2011 at 8:46 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> 1. Adding sortsupport infrastructure for more datatypes.
>> 2. Revising nbtree and related code to use this infrastructure.
>> 3. Integrating Peter's work into this framework.
>> 
>> I'll try to take care of #1 for at least a few key datatypes before
>> I commit, but I think #2 is best done as a separate patch, so I'll
>> postpone that till later.

> I see you've committed a chunk of this now.  Does it make sense to do
> #1 for every data type we support, or should we be more selective than
> that?

Basically, I tried to do #1 for every datatype for which the comparator
was cheap enough that reducing the call overhead seemed likely to make a
useful difference.  I'm not in favor of adding sortsupport functions
where this is not true, as I think it'll be useless code and catalog
bloat.  I don't want to add 'em for cruft like abstime either.

There's some stuff that's debatable according to this criterion --- in
particular, I wondered whether it'd be worth having a fast path for
bttextcmp, especially if we pre-tested the collate_is_c condition and
had a separate version that just hardwired the memcmp code path.  (The
idea of doing that was one reason I insisted on collation being known at
the setup step.)  But it would still have to be prepared for detoasting,
so in the end I was unenthused.  Anyone who feels like testing could try
to prove me wrong about it though.

> Are you planning to do anything about #2 or #3?

I am willing to do #2, but not right now; I feel what I need to do next
is go review SPGist.  I don't believe that #2 blocks progress on #3
anyway.  I think #3 is in Peter's court, or yours if you want to do it.

(BTW, I agree with your comments yesterday about trying to break down
the different aspects of what Peter did, and put as many of them as we
can into the non-inlined code paths.)

			regards, tom lane