Re: A qsort template
David Rowley <dgrowleyml@gmail.com>
From: David Rowley <dgrowleyml@gmail.com>
To: Thomas Munro <thomas.munro@gmail.com>
Cc: Andres Freund <andres@anarazel.de>, 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>
Date: 2022-04-12T00:58:09Z
Lists: pgsql-hackers
Attachments
- skip_calling_sort_tiebreak_when_not_needed.patch (text/plain) patch
- 1m_tup_mod_100_v2.png (image/png)
- v14_vs_v15_sorting_v2 .ods (application/vnd.oasis.opendocument.spreadsheet)
On Mon, 11 Apr 2022 at 09:44, Thomas Munro <thomas.munro@gmail.com> wrote: > 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. Thanks for putting the patch together. I had a look at the patch and I wondered if we really need to add an entire dimension of sort functions for just this case. My thought process here is that when I look at a function such as ApplySignedSortComparator(), I think that it might be better to save adding another dimension for a sort case such as a column that does not contain any NULLs. There's quite a bit more branching saved from getting rid of NULL tests there than what we could save by checking if we need to call the tiebreaker function in a function like qsort_tuple_signed_compare(). I didn't really know what the performance implications would be of checking an extra flag would be, so I very quickly put a patch together and ran the benchmarks. The 4GB work_mem 1 million tuple test with values MOD 100 comes out as: Thomas' patch: 10.13% faster than v14 My patch: 9.48% faster than v14 master: 15.62% *slower* than v14 So it does seem like we can fix the regression in a more simple way. We could then maybe do some more meaningful performance tests during the v16 cycle to explore the most useful dimension to add that gains the most performance. Perhaps that's NULLs, or maybe it's something else. I've attached the patch I tested. It was thrown together very quickly just to try out the performance. If it's interesting I can polish it up a bit. If not, I didn't waste too much time. David
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