Re: [DEMO][PATCH v2 6/5] compat: add a qsort_s() implementation based on GNU's qsort_r(1)

From: Jeff King <peff@peff.net>
Date: Jan 24, 2017, 20:39 UTC
Message-ID: <20170124203949.46lbmiyj26xx4hrk@sigill.intra.peff.net>
In-Reply-To: <4e416167-2a33-0943-5738-79b4da5f2c11@web.de>

On Tue, Jan 24, 2017 at 07:00:03PM +0100, René Scharfe wrote:

Show 9 quoted lines

> > I do worry about having to support more implementations in the
> > future that have different function signature for the comparison
> > callbacks, which will make things ugly, but this addition alone
> > doesn't look too bad to me.
> 
> It is unfair of me to show a 5% speedup and then recommend to not
> include it. ;-)  That difference won't be measurable in real use cases
> and the patch is not necessary.  This patch is simple, but the overall
> complexity (incl. #ifdefs etc.) will be lower without it.

I care less about squeezing out the last few percent performance and more that somebody libc qsort_r() might offer some other improvement. For instance, it could sort in-place to lower memory use for some cases, or do some clever thing that gives more than a few percent in the real world (something like TimSort).

I don't know to what degree we should care about that.

Show 7 quoted lines

> But here's another one, with even higher performance and with an even
> bigger recommendation to not include it! :)  It veers off into another
> direction: Parallel execution.  It requires thread-safe comparison
> functions, which might surprise callers.  The value 1000 for the minimum
> number of items before threading kicks in is just a guess, not based on
> measurements.  So it's quite raw -- and I'm not sure why it's still a
> bit slower than sort(1).

Fun, but probably insane for our not-very-threadsafe code base. :)

-Peff