Re: Sort functions with specialized comparators
x4mmm@yandex-team.ru
From: "Andrey M. Borodin" <x4mmm@yandex-team.ru>
To: John Naylor <johncnaylorls@gmail.com>
Cc: David Rowley <dgrowleyml@gmail.com>, Антуан Виолин <violin.antuan@gmail.com>, PostgreSQL Hackers <pgsql-hackers@postgresql.org>
Date: 2025-01-06T15:50:37Z
Lists: pgsql-hackers
> On 6 Jan 2025, at 15:54, John Naylor <johncnaylorls@gmail.com> wrote: >> >> I thought about it, but decided to rename the routine. >> Here's a version 7 with compASC(). > > I had the right idea, but the wrong function -- HEAD already had a > suitable comp function, and one that works well with inlined > specialized sorts: isort_cmp(). We just need to remove the extra > argument. Like some other patches in this series, this does have the > side effect of removing the ability to skip quinique(), so that should > be benchmarked (I can do that this week unless you beat me to it). With the same setup as in the first message of this thread we can do: postgres=# SELECT _int_contains(arr,ARRAY[1]) FROM arrays_to_sort; before patch patch Time: 567.928 ms after patch Time: 890.297 ms timing of this function is dominated by PREPAREARR(a); What bothers me is that PREPAREARR(a); is returning new array in case of empty input. That's why I considered little refactoring of resize_intArrayType(): reorder cases so that if (num == ARRNELEMS(a)) was first. > We > can specialize quinique too, as mentioned upthread, but I'm not sure > we need to. > >> And, just in case, if we already have ASC, why not keep DESC too instead of newly invented cmp function :) PFA v8. > > Those functions from common/int.h are probably not good when inlined > (see comment there). If we really want to keep the branch for asc/desc > out of the comparison, it makes more sense to add a function to > reverse the elements after sorting. That allows using the same cmp > function for everything, thus removing the apparent need for a global > wrapper around the static sort function. > > I've done both ideas in v9, which also tries to improve patch > legibility by keeping things in the same place they were before. It > also removes the internal "n > 1" checks that you mentioned earlier -- > after thinking about it that seems better than adding braces to one > macro to keep it functional. LGTM. Thanks! Best regards, Andrey Borodin.
Commits
-
Specialize intarray sorting
- 53d3daa491be 18.0 landed
-
Replace insertion sort in contrib/intarray with qsort().
- 8d1f239003d0 9.5.0 cited