Re: A qsort template
Thomas Munro <thomas.munro@gmail.com>
From: Thomas Munro <thomas.munro@gmail.com>
To: Andres Freund <andres@anarazel.de>
Cc: Justin Pryzby <pryzby@telsasoft.com>,
John Naylor <john.naylor@enterprisedb.com>, Peter Geoghegan <pg@bowt.ie>, pgsql-hackers <pgsql-hackers@postgresql.org>, Robert Haas <rhaas@postgresql.org>, David Rowley <dgrowleyml@gmail.com>
Date: 2022-04-10T21:44:02Z
Lists: pgsql-hackers
Attachments
- 0001-Specialize-tuplesort-for-keys-without-tiebreaker.patch (text/x-patch) patch 0001
Hi,
David Rowley privately reported a performance regression when sorting
single ints with a lot of duplicates, in a case that previously hit
qsort_ssup() but now hits qsort_tuple_int32() and then has to call the
tiebreaker comparator. Note that this comes up only for sorts in a
query, not for eg index builds which always have to tiebreak on item
ptr. I don't have data right now but that'd likely be due to:
+ * XXX: For now, there is no specialization for cases where datum1 is
+ * authoritative and we don't even need to fall back to a callback at all (that
+ * would be true for types like int4/int8/timestamp/date, but not true for
+ * abbreviations of text or multi-key sorts. There could be! Is it worth it?
Upthread we were discussing which variations it'd be worth investing
extra text segment space on to gain speedup and we put those hard
decisions off for future work, but on reflection, we probably should
tackle this particular point to avoid a regression. I think something
like the attached achieves that (draft, not tested much yet, could
perhaps find a tidier way to code the decision tree). In short:
variants qsort_tuple_{int32,signed,unsigned}() no longer fall back,
but new variants qsort_tuple_{int32,signed,unsigned}_tiebreak() do.
We should perhaps also reconsider the other XXX comment about finding
a way to skip the retest of column 1 in the tiebreak comparator.
Perhaps you'd just install a different comparetup function, eg
comparetup_index_btree_tail (which would sharing code), so no need to
multiply specialisations for that.
Planning to look at this more closely after I've sorted out some other
problems, but thought I'd post this draft/problem report early in case
John or others have thoughts or would like to run some experiments.
Commits
-
Remove debug messages from tuplesort_sort_memtuples()
- 6e647ef0e750 15.0 landed
-
Fix performance regression in tuplesort specializations
- 99c754129d78 15.0 landed
-
Fix tuplesort optimization for CLUSTER-on-expression.
- cc58eecc5d75 15.0 landed
-
Initial pgindent and pgperltidy run for v14.
- def5b065ff22 14.0 cited
-
Specialize checkpointer sort functions.
- 1b88b8908e75 14.0 landed
-
Use sort_template.h for qsort() and qsort_arg().
- f374f4d66425 14.0 landed
-
Use sort_template.h for qsort_tuple() and qsort_ssup().
- 8eda3eba3063 14.0 landed
-
Add sort_template.h for making sort functions.
- 0a1f1d3cac6b 14.0 landed
-
Use abbreviated keys for faster sorting of text datums.
- 4ea51cdfe85c 9.5.0 cited