Re: Inlining comparators as a performance optimisation

Robert Haas <robertmhaas@gmail.com>

From: Robert Haas <robertmhaas@gmail.com>
To: Tom Lane <tgl@sss.pgh.pa.us>
Cc: Bruce Momjian <bruce@momjian.us>, Peter Geoghegan <peter@2ndquadrant.com>, PG Hackers <pgsql-hackers@postgresql.org>
Date: 2011-12-07T00:18:24Z
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

On Tue, Dec 6, 2011 at 4:23 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Robert Haas <robertmhaas@gmail.com> writes:
>> On Tue, Dec 6, 2011 at 1:07 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>>> I'll take another crack at it.  I'm not entirely sold yet on merging
>>> the two structs; I think first we'd better look and see what the needs
>>> are in the other potential callers I mentioned.  If we'd end up
>>> cluttering the struct with half a dozen weird fields, it'd be better to
>>> stick to a minimal interface struct with various wrapper structs, IMO.
>
>> OK.  I'll defer to whatever you come up with after looking at it.
>
> OK, it looks like nodeMergeAppend.c could use something exactly like the
> draft SortKey struct, while nodeMergejoin.c could embed such a struct in
> MergeJoinClauseData.  The btree stuff needs something more nearly
> equivalent to a ScanKey, including a datum-to-compare-to and a flags
> field.  I'm inclined to think the latter would be too specialized to put
> in the generic struct.  On the other hand, including the reverse and
> nulls_first flags in the generic struct is clearly a win since it allows
> ApplyComparator() to be defined as a generic function.  So the only
> thing that's really debatable is the attno field, and I'm not anal
> enough to insist on a separate level of struct just for that.
>
> I am however inclined to stick with the shortened struct name SortSupport
> rather than using SortKey.  The presence of the function pointer fields
> (especially the inlined-qsort pointers, assuming we adopt some form of
> Peter's patch) changes the struct's nature in my view; it's not really
> describing just a sort key (ie an ORDER BY column specification).

Works for me.  I think we should go ahead and get this part committed
first, and then we can look at the inlining stuff as a further
optimization for certain cases...

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company