Re: some aspects of our qsort might not be ideal

From: John Naylor <john(dot)naylor(at)enterprisedb(dot)com>
To: Matthias van de Meent <boekewurm+postgres(at)gmail(dot)com>
Cc: Peter Geoghegan <pg(at)bowt(dot)ie>, Robert Haas <robertmhaas(at)gmail(dot)com>, pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: some aspects of our qsort might not be ideal
Date: 2022-07-22 03:01:50
Message-ID: CAFBsxsFhq8VUSkUL5YO17cFXbCPwtbbxBu+d9MFrrsssfDXm3Q@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Tue, Jul 5, 2022 at 12:14 PM John Naylor <john(dot)naylor(at)enterprisedb(dot)com>
wrote:

> I suspect the branches in the sorttuple comparators for null handling
> are negating the benefits from better d-cache and TLB behavior. In
> that case, a useful thing to try would be to pre-partition the
> sorttuple array between null and non-null values as it's being
> populated, then sort each of those in turn. (The null portion would
> only need sorting if there was more than one sort key.) That would
> automatically take care of nulls-first vs nulls-last upfront, save a
> bunch of binary space used for branches (worth doing on its own),

To see if this idea had any legs, I simply ripped out the null handling
from the specialized sorttuple comparators. This works for my tests since
all the values are not null. Doing this much seems to be more effective
than dual pivot in every case but purely-descending input. The combination
of the two seems a bit faster still on bigint, but not on text. From this
test, it seems the conclusions for tuple sorts with dual-pivot haven't
changed: impressive speedup on descending input, not so much elsewhere.
(spreadsheet attached, note the insertion sort thresholds are reused from
previous testing, namely 10 and 18 for single / dual pivot)

> and
> the "isnull" member would not be needed anymore. That wouldn't save
> data space because of alignment, but the compiler wouldn't need to
> generate a load/store instruction for it.

This piece might not be workable, or would make external merges more
difficult. That said, doing the null-handling up front when forming sort
tuples seems worth doing, and I intend to come back to it at some point.

Regarding binary size, removing the null handling from the comparators
shrinks the binary by about 4.3kB. Adding dual pivot on top of that expands
it to about 3.5kB larger than HEAD, but I'm pretty sure I could get that
down to net zero by replacing the presorted check with a partial insertion
sort (as discussed before), and reusing that for the pivot selection as
well. Being able to add partial insertion sort with net ~zero binary
increase could be another small point in favor of dual-pivot.

I've attached the patches for this experiment for completeness, but
withdrawing the CF entry for now.

--
John Naylor
EDB: http://www.enterprisedb.com

Attachment Content-Type Size
v3-0005-Set-insertion-sort-threshold-to-18.patch text/x-patch 764 bytes
v3-0006-Revert-Remove-null-comparisons-from-sort-comparat.patch text/x-patch 3.4 KB
v3-0004-Implement-dual-pivot-quicksort.patch text/x-patch 68.0 KB
remove-null-cmp-sp10-dp18.ods application/vnd.oasis.opendocument.spreadsheet 47.4 KB
v3-0003-Create-internal-workhorse-for-ST_SORT.patch text/x-patch 2.9 KB
v3-0002-Remove-null-comparisons-from-sort-comparators.patch text/x-patch 3.4 KB
v3-0001-Set-insertion-sort-threshold-to-10.patch text/x-patch 733 bytes

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Tom Lane 2022-07-22 03:03:43 Re: question about `static inline` functions in header files
Previous Message wangw.fnst@fujitsu.com 2022-07-22 02:56:39 RE: Perform streaming logical transactions by background workers and parallel apply