mailing list of musl libc
 help / color / mirror / code / Atom feed
From: Alexander Monakov <amonakov@ispras.ru>
To: musl@lists.openwall.com
Cc: "Érico Nogueira" <ericonr@disroot.org>
Subject: Re: [musl] [PATCH v2] add qsort_r.
Date: Tue, 9 Mar 2021 17:13:39 +0300 (MSK)	[thread overview]
Message-ID: <alpine.LNX.2.20.13.2103091649540.16269@monopod.intra.ispras.ru> (raw)
In-Reply-To: <20210309134242.GS32655@brightrain.aerifal.cx>

[-- Attachment #1: Type: text/plain, Size: 2092 bytes --]

> On Tue, Mar 09, 2021 at 12:11:37PM +0300, Alexander Monakov wrote:
> > On Tue, 9 Mar 2021, Érico Nogueira wrote:
> > 
> > > since most discussion around the addition of this function has centered
> > > around the possible code duplication it requires or that qsort would
> > > become much slower if implemented as a wrapper around qsort_r
> > 
> > How much is "much slower", did anyone provide figures to support this claim?
> > The extra cost that a wrapper brings is either one indirect jump instruction,
> > or one trivially-predictable conditional branch per one comparator invocation.
> 
> Quite a bit I'd expect. Each call to cmp would involve an extra level
> of call wrapper. With full IPA/inlining it could be optimized out, but
> only by making a non-_r copy of all the qsort code in the process at
> optimize time.
> 
> > Constant factor in musl qsort is quite high, I'd be surprised if the extra
> > overhead from one additional branch is even possible to measure.
> 
> I don't think it's just a branch. It's a call layer. qsort_r internals
> with cmp=wrapper_cmp, ctx=real_cmp -> wrapper_cmp(x, y, real_cmp) ->
> real_cmp(x, y). But I'm not opposed to looking at some numbers if you
> think it might not matter. Maybe because it's a tail call it does
> collapse to essentially just a branch in terms of cost..

First of all it's not necessarily a "call layer".

You could change cmp call site such that NULL comparator implies that
non-_r version was called and the original comparator address is in ctx:

static inline int call_cmp(void *v1, void *v2, void *ctx, cmpfun cmp)
{
	if (cmp)
		return cmp(v1, v2, ctx);
	return ((cmpfun)ctx)(v1, v2);
}

This is just a conditional branch at call site after trivial inlining.

Second, if you make a "conventional" wrapper, then on popular architectures
it is a single instruction (powerpc64 ABI demonstrates its insanity here):

static int wrapper_cmp(void *v1, void *v2, void *ctx)
{
	return ((cmpfun)ctx)(v1, v2);
}

Some examples:

amd64:	jmp %rdx
i386:	jmp *12(%esp)
arm:	bx r2
aarch64:br x2

How is this not obvious?

Alexander

  reply	other threads:[~2021-03-09 14:13 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-03-09  3:56 Érico Nogueira
2021-03-09  9:11 ` Alexander Monakov
2021-03-09 13:42   ` Rich Felker
2021-03-09 14:13     ` Alexander Monakov [this message]
2021-03-09 15:03       ` Rich Felker
2021-03-09 16:54         ` Markus Wichmann
2021-03-09 18:04           ` Rich Felker
2021-03-15 23:49             ` Alexander Monakov
2021-03-16  2:18               ` Rich Felker

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=alpine.LNX.2.20.13.2103091649540.16269@monopod.intra.ispras.ru \
    --to=amonakov@ispras.ru \
    --cc=ericonr@disroot.org \
    --cc=musl@lists.openwall.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://git.vuxu.org/mirror/musl/

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).