Re: Last Commitfest patches waiting review

On Thu, Oct 9, 2014 at 12:51 PM, Heikki Linnakangas
<hlinnakangas(at)vmware(dot)com> wrote:
> Oh, I didn't realize we don't do that already! I'm surprised, I would've
> expected index build to have been the first thing we'd use the SortSupport
> stuff in.

The thing is that the most compelling numbers for sortsupport (plus
the related improvements to tuplesort itself) were from the "onlyKey"
optimization - the numbers are only so-so when you look at
multi-attribute sorts, because we specialize qsort() for both of those
two cases (i.e. one specialization, qsort_ssup(), only looks at
datum1, while qsort_tuple() looks at everything else, through which
comparetup_heap() and comparetup_datum() are called). But with the
B-Tree comparator, we're only ever going to be able to use
qsort_tuple() which is roughly equivalent to the so-so multi-attribute
case for heap tuples (because we need to detect duplicate violations,
and a few other things - no choice there).

It kind of makes sense that we didn't push ourselves to get B-Tree
support until now. But with sortsupport for text accelerating sorts by
perhaps as much as 10 times in sympathetic (though realistic) cases,
it would be crazy to have that without B-Tree support (abbreviated
keys always force us to use the qsort_tuple() specialization, because
the comparator tie-breaker logic must live in places like
comparetup_heap()).

> Yeah, that seems worth doing, independently of the this patch.

As I mentioned, that might be less true than you'd think.

> Can you write
> a separate patch to use SortSupport for B-tree index builds, please?
> Eliminating the FunctionCallInfoData overhead should shave off some some
> cycles from every index build.

I'll look into it. Hopefully an effort to actually implement it will
show that I was right, and there isn't much to it.

--
Peter Geoghegan