> 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
refactoringof 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.
