[RFC] pthread_once: Use unified variant instead of custom x86_64/i386

  On Mon, 2014-04-07 at 15:47 +0200, Torvald Riegel wrote:
> On Fri, 2013-10-11 at 23:28 +0300, Torvald Riegel wrote:
> > Assuming the pthread_once unification I sent recently is applied, we
> > still have custom x86_64 and i386 variants of pthread_once.  The
> > algorithm they use is the same as the unified variant, so we would be
> > able to remove the custom variants if this doesn't affect performance.
> > 
> > The common case when pthread_once is executed is that the initialization
> > has already been performed; thus, this is the fast path that we can
> > focus on.  (I haven't looked specifically at the generated code for the
> > slow path, but the algorithm is the same and I assume that the overhead
> > of the synchronizing instructions and futex syscalls determines the
> > performance of it, not any differences between compiler-generated code
> > and the custom code.)
> > 
> > The fast path of the custom assembler version:
> > 	testl	$2, (%rdi)
> > 	jz	1f
> > 	xorl	%eax, %eax
> > 	retq
> > 
> > The fast path of the generic pthread_once C code, as it is after the
> > pthread_once unification patch:
> >   20:   48 89 5c 24 e8          mov    %rbx,-0x18(%rsp)
> >   25:   48 89 6c 24 f0          mov    %rbp,-0x10(%rsp)
> >   2a:   48 89 fb                mov    %rdi,%rbx
> >   2d:   4c 89 64 24 f8          mov    %r12,-0x8(%rsp)
> >   32:   48 89 f5                mov    %rsi,%rbp
> >   35:   48 83 ec 38             sub    $0x38,%rsp
> >   39:   41 b8 ca 00 00 00       mov    $0xca,%r8d
> >   3f:   8b 13                   mov    (%rbx),%edx
> >   41:   f6 c2 02                test   $0x2,%dl
> >   44:   74 16                   je     5c <__pthread_once+0x3c>
> >   46:   31 c0                   xor    %eax,%eax
> >   48:   48 8b 5c 24 20          mov    0x20(%rsp),%rbx
> >   4d:   48 8b 6c 24 28          mov    0x28(%rsp),%rbp
> >   52:   4c 8b 64 24 30          mov    0x30(%rsp),%r12
> >   57:   48 83 c4 38             add    $0x38,%rsp
> >   5b:   c3                      retq   
> > 
> > The only difference is more stack save/restore.  However, a quick run of
> > benchtests/pthread_once (see the patch I sent for review) on my laptop
> > doesn't show any noticeable differences between both (averages of 8 runs
> > of the microbenchmark differ by 0.2%).
> > 
> > When splitting out the slow path like this:
> > 
> > static int
> > __attribute__((noinline))
> > __pthread_once_slow (once_control, init_routine)
> > /* ... */
> > 
> > int
> > __pthread_once (once_control, init_routine)
> >      pthread_once_t *once_control;
> >      void (*init_routine) (void);
> > {
> >   int val;
> >   val = *once_control;
> >   atomic_read_barrier();
> >   if (__builtin_expect ((val & __PTHREAD_ONCE_DONE) != 0, 1))
> >     return 0;
> >   else
> >     return __pthread_once_slow(once_control, init_routine);
> > }
> > 
> > we get this for the C variants fast path:
> > 
> > 00000000000000e0 <__pthread_once>:
> >   e0:   8b 07                   mov    (%rdi),%eax
> >   e2:   a8 02                   test   $0x2,%al
> >   e4:   74 03                   je     e9 <__pthread_once+0x9>
> >   e6:   31 c0                   xor    %eax,%eax
> >   e8:   c3                      retq   
> >   e9:   31 c0                   xor    %eax,%eax
> >   eb:   e9 30 ff ff ff          jmpq   20 <__pthread_once_slow>
> > 
> > This is very close to the fast path of the custom assembler code.
> > 
> > I haven't looked further at i386, but the custom code is pretty similar
> > to the x86_64 variant.
> > 
> > 
> > What do you all prefer?:
> > 1) Keep the x86-specific assembler versions?
> > 2) Remove the x86-specific assembler versions and split out the slow
> > path?
> > 2) Just remove the x86-specific assembler versions?
> > 
> 
> Here is an updated patch for 2).
> 
> Without the fast path, I get the following without and with assembly (on
> my x86_64 laptop):
> 
> "pthread_once": {
> "": {
> "duration": 2.42853e+10, "iterations": 2.16569e+09, "max": 2217.22,
> "min": 11.024, "mean": 11.2137
> }
> }
> "pthread_once": {
> "": {
> "duration": 2.40695e+10, "iterations": 2.65473e+09, "max": 2185.57,
> "min": 9.016, "mean": 9.06665
> }
> }
> 
> With the fast path as split out, I get:
> "pthread_once": {
> "": {
> "duration": 2.40632e+10, "iterations": 2.6526e+09, "max": 2336.96,
> "min": 9.016, "mean": 9.07154
> }
> }
> 
> Okay to commit the fast path (after the pthread_once unification has
> been committed) and remove the x86 assembler variants?

This slipped through and this was never committed.  I took Richard's
advice into account, and the x86_64 code generated for the generic
version is now:
00000000000000d0 <__pthread_once>:
  d0:   8b 07                   mov    (%rdi),%eax
  d2:   a8 02                   test   $0x2,%al
  d4:   74 03                   je     d9 <__pthread_once+0x9>
  d6:   31 c0                   xor    %eax,%eax
  d8:   c3                      retq   
  d9:   e9 42 ff ff ff          jmpq   20 <__pthread_once_slow>

Attached is an updated patch, which I'll commit after two days if nobody
objects.

2014-10-20  Torvald Riegel  <triegel@redhat.com>

	[BZ #15215]
	* nptl/pthread_once.c (__pthread_once): Split out fast path to ...
	(__pthread_once_slow): ... here.
	* sysdeps/unix/sysv/linux/i386/pthread_once.S: Remove file.
	* sysdeps/unix/sysv/linux/x86_64/pthread_once.S: Remove file.

[RFC] pthread_once: Use unified variant instead of custom x86_64/i386

Commit Message

Comments

Patch