Re: Inlining comparators as a performance optimisation

Peter Geoghegan <peter@2ndquadrant.com>

From: Peter Geoghegan <peter@2ndquadrant.com>
To: Robert Haas <robertmhaas@gmail.com>
Cc: Tom Lane <tgl@sss.pgh.pa.us>, Greg Stark <stark@mit.edu>, Andrew Dunstan <andrew@dunslane.net>, Heikki Linnakangas <heikki.linnakangas@enterprisedb.com>, PG Hackers <pgsql-hackers@postgresql.org>
Date: 2011-11-21T02:02:06Z
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

Part of my problem with not producing specialisations that I really
neglected to complain about until now is the inconsistency between
types, and the need to deal with that.

We benefit from assuming in our specialisation that we're only dealing
with POD types, simply not considering things that we would otherwise
have had to for the benefit of types that have a Datum representation
that is pass-by-reference, or have collations, or have multiple
scankeys. Leaving aside the question of straight compiler-optimisation
benefits for the moment, the remaining benefit of what I've done comes
not so much from avoiding the usual function call machinery per se, as
from doing so *as well as* cutting down on what currently happens in
comparetup_heap to handle every single compound and scalar type.
Compare the function comparetup_heap with my meta-function
TYPE##AppSort to see what I mean. The function comparetup_heap is the
comparator directly used by qsort_arg when sorting heap tuples, and
qsort_arg outsources to comparetup_heap some things that you might not
expect it to (very little has changed about qsort_arg since we lifted
it from NetBSD back in 2006). So while you might imagine that that
loop in comparetup_heap and things like its use of the heap_getattr
macros won't expand to that many instructions, you really don't want
them in the inner loop of a long operation like qsorting, paid for up
to O(n ^ 2) times. There's also the obvious implications for compiler
optimisations, particularly relating to effective usage of CPU cache.

I'm trying to get you what you asked for: A straight choice between
what Tom suggested and what I suggested, with perhaps some compromises
between the two positions. That's sort of tricky though, especially
considering the issues raised above.

-- 
Peter Geoghegan       http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training and Services