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
