Backport #2553

Fix pthreads slowness by eliminating unnecessary sigprocmask calls

Added by Dan Peterson over 4 years ago. Updated almost 3 years ago.

[ruby-core:27380]
Status:Open
Priority:Low
Assignee:Shyouhei Urabe

Description

=begin
This is a bug report for what's described here:

http://timetobleed.com/fix-a-bug-in-rubys-configurein-and-get-a-30-performance-boost/

Matz says here that this should already be fixed in 1.8.7:

http://blade.nagaokaut.ac.jp/cgi-bin/scat.rb/ruby/ruby-core/23583

But it does not appear to be. Example with 1.8.7-p248:

# apply patch from above link
% make distclean
% ./configure --enable-pthread
% make
% time ./ruby -e '1000000.times { x = 2 ** 256 }'
./ruby -e '1000000.times { x = 2 ** 256 }' 1.45s user 0.34s system 99% cpu 1.809 total
% strace ./ruby -e '1000.times { x = 2 ** 256 }' 2>&1 | grep -c sigproc
1009
% make distclean
% ./configure --disable-ucontext --enable-pthread
% make
% time ./ruby -e '1
000000.times { x = 2 ** 256 }'
./ruby -e '1
000000.times { x = 2 ** 256 }' 1.16s user 0.00s system 99% cpu 1.171 total
% strace ./ruby -e '1
000.times { x = 2 ** 256 }' 2>&1 | grep -c sigproc
3

% ./ruby -v
ruby 1.8.7 (2009-12-24 patchlevel 248) [i686-linux]
=end

Associated revisions

Revision 29854
Added by Shyouhei Urabe over 3 years ago

Wed, 23 Jun 2010 12:48:31 +0000 kosaki kosaki@b2dd03c8-39d4-4d8f-98ff-823fe69b080e
merge revision(s) 28404,28595,28597:
?\012
* configure.in: avoid getcontext() overhead if possible.
[Bug #2553]
Thanks, Joe Damato, Dan Peterson and Patrick Mohr.

git-svn-id: svn+ssh://ci.ruby-lang.org/ruby/branches/ruby18@28404 b2dd03c8-39d4-4d8f-98ff-823fe69b080e
Signed-off-by: URABE, Shyouhei shyouhei@ruby-lang.org
?\012
* configure.in: fix use_context condition inversion.

git-svn-id: svn+ssh://ci.ruby-lang.org/ruby/branches/ruby18@28595 b2dd03c8-39d4-4d8f-98ff-823fe69b080e
Signed-off-by: URABE, Shyouhei shyouhei@ruby-lang.org
?\012
Fix changelog of revision 28595

git-svn-id: svn+ssh://ci.ruby-lang.org/ruby/branches/ruby18@28597 b2dd03c8-39d4-4d8f-98ff-823fe69b080e
Signed-off-by: URABE, Shyouhei shyouhei@ruby-lang.org

Revision 30895
Added by Shyouhei Urabe about 3 years ago

  • configure.in: revert revision r29854. This revision introduced binary incompatibilities on some circumstances. The bug that revision was fixing gets reopened by this reversion. cf. [Bug #2553]

History

#1 Updated by Dan Peterson over 4 years ago

=begin
It's entirely possible this is a duplicate but after 10 minutes searching via both the Redmine interface and Google focused on this site I couldn't find it already reported.
=end

#2 Updated by Yui NARUSE over 4 years ago

  • Status changed from Open to Assigned
  • Assignee set to Yukihiro Matsumoto

=begin

=end

#3 Updated by Dan Peterson almost 4 years ago

=begin
Anything else I can do to help this along?
=end

#4 Updated by Roger Pack almost 4 years ago

=begin
I thought this was fixed in 1.8.7
Could you verify which branches (1.8.8, 1.8.7, 1.8.6) still show this problem?
That might help.

=end

#5 Updated by Patrick Mohr almost 4 years ago

=begin

I thought this was fixed in 1.8.7
Could you verify which branches (1.8.8, 1.8.7, 1.8.6) still show this problem?

I just ran the same tests on 1.8.6 and 1.8.7. It's still not fixed. I'm not sure where the source code for 1.8.8 is so I didn't test it.

The build information and test results are below. I didn't test 1.8.7 with --disable-ucontext because that configure flag doesn't seem to exist anymore.

Admittedly, ruby 1.8.7 is much faster than 1.8.6, but 1.8.7 still has this bug.

I would be happy to test 1.8.8 if you would tell me where to get the source code.


1.8.6 with pthreads test results:
# svn co http://svn.ruby-lang.org/repos/ruby/branches/ruby_1_8_6
# autoconf && ./configure --enable-pthread && make
# ./ruby -v
ruby 1.8.6 (2010-06-12 patchlevel 415) [i686-linux]
# time ./ruby -e '1000000.times { x = 2 ** 256 }'
real 0m6.462s
user 0m6.128s
sys 0m0.308s


1.8.6 with pthreads enabled and ucontext disabled
# svn co http://svn.ruby-lang.org/repos/ruby/branches/ruby_1_8_6
# autoconf && ./configure --disable-ucontext --enable-pthread && make
# ./ruby -v
ruby 1.8.6 (2010-06-12 patchlevel 415) [i686-linux]
# time ./ruby -e '1000000.times { x = 2 ** 256 }'
real 0m5.582s
user 0m5.548s
sys 0m0.000s


1.8.7 with pthreads
# svn co http://svn.ruby-lang.org/repos/ruby/branches/ruby_1_8_7
# autoconf && ./configure --enable-pthread && make
# ./ruby -v
ruby 1.8.7 (2010-06-16 patchlevel 296) [i686-linux]
# time ./ruby -e '1000000.times { x = 2 ** 256 }'
real 0m3.241s
user 0m2.960s
sys 0m0.260s


1.8.7 with --disable-pthread
# svn co http://svn.ruby-lang.org/repos/ruby/branches/ruby_1_8_7
# autoconf && ./configure --disable-pthread && make
# ./ruby -v
ruby 1.8.7 (2010-06-16 patchlevel 296) [i686-linux]
# time ./ruby -e '1000000.times { x = 2 ** 256 }'
real 0m2.535s
user 0m2.512s
sys 0m0.004s

=end

#6 Updated by Motohiro KOSAKI almost 4 years ago

=begin
ruby 1.8.8 have the same issue.

$ autoconf && ./configure --disable-pthread && make

$ time ./ruby -ve '1000000.times { x = 2 ** 256 }'
ruby 1.8.8dev (2010-06-15) [x86_64-linux]

real 0m1.176s
user 0m1.171s
sys 0m0.005s

$ autoconf && ./configure --enable-pthread && make

[kosaki@kosaopt ruby18]$ time ./ruby -ve '1000000.times { x = 2 ** 256 }'
ruby 1.8.8dev (2010-06-15) [x86_64-linux]

real 0m1.513s
user 0m1.340s
sys 0m0.173s

$ strace -c ./ruby -e '1000000.times { x = 2 ** 256 }'
% time seconds usecs/call calls errors syscall


99.98 0.160160 0 1000011 rtsigprocmask
0.02 0.000025 2 14 mprotect
0.00 0.000000 0 8 read
0.00 0.000000 0 19 10 open
0.00 0.000000 0 9 close
0.00 0.000000 0 4 3 stat
0.00 0.000000 0 9 fstat
0.00 0.000000 0 26 mmap
0.00 0.000000 0 1 munmap
0.00 0.000000 0 7 brk
0.00 0.000000 0 14 rt
sigaction
0.00 0.000000 0 1 1 access
0.00 0.000000 0 1 execve
0.00 0.000000 0 6 getrlimit
0.00 0.000000 0 1 getuid
0.00 0.000000 0 1 getgid
0.00 0.000000 0 2 geteuid
0.00 0.000000 0 2 getegid
0.00 0.000000 0 1 archprctl
0.00 0.000000 0 2 1 futex
0.00 0.000000 0 1 set
tidaddress
0.00 0.000000 0 1 set
robust_list


100.00 0.160185 1000141 15 total

summary of ruby18 code

node.h


#if defined(HAVEGETCONTEXT) && defined(HAVESETCONTEXT)
#include
#define USE_CONTEXT
#endif
#include
#include "st.h"

The syscam can use getcontext(), USE_CONTEXT is always set.

eval.c


#ifdef USECONTEXT
(snip)
# define ruby
setjmp(justbeforesetjmp, j) ((j)->status = 0, \
(justbeforesetjmp), \
PREGETCONTEXT, \
getcontext(&(j)->context), \
POST
GETCONTEXT, \
(j)->status)

if USECONTEXT is defined, rubysetjmp() mean to call getcontext().
and linux getcontext() implementation call rt_sigprocmask() internally.

=end

#7 Updated by Motohiro KOSAKI almost 4 years ago

=begin
note: 1.9.x tree don't have this issue beucause ruby_setjmp() don't use getcontext().

=end

#8 Updated by Motohiro KOSAKI almost 4 years ago

=begin
This seems to be introduced following commit.

commit 9fdbc41973971f4ef032d57c4e02ed0430227a0a
Author: nobu nobu@b2dd03c8-39d4-4d8f-98ff-823fe69b080e
Date: Sat Dec 13 00:01:28 2003 +0000

 * configure.in: check ucontext.h.

 * eval.c: use getcontext/setcontext() instead of setjmp/longjmp()
   on ia64 or with native thread enabled.  


 git-svn-id: svn+ssh://ci.ruby-lang.org/ruby/trunk@5180 b2dd03c8-39d4-4d8f-98ff-823fe69b080e

but unfortunatelly, has alomost zero information ;)

http://blade.nagaokaut.ac.jp/cgi-bin/scat.rb/ruby/ruby-core/1932

=end

#9 Updated by Roger Pack almost 4 years ago

=begin
Thanks for looking into this one.
-r
=end

#10 Updated by Motohiro KOSAKI almost 4 years ago

=begin
I and nobu digged previous discussion awhile. Now, we have concluded getcontext() is only necessary when following cases
1) Using linuxthreads (it has broken signal model)
2) cpu arch has register stack

=end

#11 Updated by Motohiro KOSAKI almost 4 years ago

  • Category set to core
  • Assignee changed from Yukihiro Matsumoto to Shyouhei Urabe
  • Priority changed from Normal to Low

=begin
This issue has been fixed by commit r28404 (ruby18 branch).

=end

#12 Updated by Andre Nathan almost 4 years ago

=begin
Any chance of backporting the fix to the ruby18_7 branch?

Thanks,
Andre
=end

#13 Updated by Motohiro KOSAKI almost 4 years ago

=begin
There is.
But unfortunately 1.8.7-p299 was released very recently. So you need to wait about half year ;-)
Also, If anyone will find a bug in this patch, our chance will disappear....

=end

#14 Updated by Shyouhei Urabe almost 4 years ago

=begin
Oh, sorry for the inconvenience... I was not aware of this. It should be backported.
=end

#15 Updated by Andre Nathan almost 4 years ago

=begin
Hello

I was trying to update the ubuntu ruby 1.8.7 package with this patch, but I'm still seeing the bad performance. The patch has the following line:

if test -n "(/lib/libc.so.6 2>/dev/null | fgrep 'Native POSIX Threads') 2> /dev/null"; then
use_context=yes
fi

Isn't the use of setcontext necessary only for linuxthreads, i.e. if the system doesn't have native posix threads? In that case, shouldn't the test be "test -z", so that use_context is set to yes if "Native POSIX Threads" does not appear in that output?

Apologies if I have misunderstood the patch.

Best regards,
Andre
=end

#16 Updated by Motohiro KOSAKI almost 4 years ago

=begin
2010/7/10 Andre Nathan redmine@ruby-lang.org:

Issue #2553 has been updated by Andre Nathan.

Hello

I was trying to update the ubuntu ruby 1.8.7 package with this patch, but I'm still seeing the bad performance. The patch has the following line:

if test -n "(/lib/libc.so.6 2>/dev/null | fgrep 'Native POSIX Threads') 2> /dev/null"; then
   use_context=yes
fi

Isn't the use of setcontext necessary only for linuxthreads, i.e. if the system doesn't have native posix threads? In that case, shouldn't the test be "test -z", so that use_context is set to yes if "Native POSIX Threads" does not appear in that output?

Apologies if I have misunderstood the patch.

Oops, this patch only works on 32bit machine, because some 64bit
distro don't have /lib/libc.so.6 (instead they use /lib64), and this
condition select use_context=yes.

I'll update this soon.
And, if you are using 32bit distro, please show your /lib/libc.so.6
execution result.

I'm very sorry this.

=end

#17 Updated by Motohiro KOSAKI almost 4 years ago

=begin
2010/7/10 KOSAKI Motohiro kosaki.motohiro@gmail.com:

2010/7/10 Andre Nathan redmine@ruby-lang.org:

Issue #2553 has been updated by Andre Nathan.

Hello

I was trying to update the ubuntu ruby 1.8.7 package with this patch, but I'm still seeing the bad performance. The patch has the following line:

if test -n "(/lib/libc.so.6 2>/dev/null | fgrep 'Native POSIX Threads') 2> /dev/null"; then
use_context=yes
fi

Isn't the use of setcontext necessary only for linuxthreads, i.e. if the system doesn't have native posix threads? In that case, shouldn't the test be "test -z", so that use_context is set to yes if "Native POSIX Threads" does not appear in that output?

Apologies if I have misunderstood the patch.

Oops, this patch only works on 32bit machine, because some 64bit
distro don't have /lib/libc.so.6 (instead they use /lib64), and this
condition select use_context=yes.

I'll update this soon.
And, if you are using 32bit distro, please show your /lib/libc.so.6
execution result.

I'm very sorry this.

following patch works both 32bit and 64bit on my environment.

$ svn diff
Index: configure.in
===================================================================
--- configure.in (リビジョン 28594)
+++ configure.in (作業コピー)
@@ -1151,7 +1151,7 @@
if test x"$rbwithpthread" = xyes; then
ASCASE("$targetcpu:$targetos:$crosscompiling",
[:linux:no], [
- if test -n "(/lib/libc.so.6 2>/dev/null | fgrep 'Native
POSIX Threads') 2> /dev/null
"; then
+ if test -n "(/lib/libc.so.6 2>/dev/null | fgrep
'linuxthreads') 2> /dev/null
"; then
use_context=yes
fi
],

=end

#18 Updated by Motohiro KOSAKI almost 4 years ago

=begin

following patch works both 32bit and 64bit on my environment.

Committed.
Revision 28595 and 28597.

Thanks Andre, you are great.

=end

#19 Updated by Andre Nathan almost 4 years ago

=begin
The new patch fixed this issue for me. Thanks a lot!

Here's hoping that the patch will be merged into ruby18_7. I believe it'll make it easier for distributions who package this version to have this performance bug fixed.

Thanks again!
Andre
=end

#20 Updated by Shyouhei Urabe over 3 years ago

  • Status changed from Assigned to Closed
  • % Done changed from 0 to 100

=begin
This issue was solved with changeset r29854.
Dan, thank you for reporting this issue.
Your contribution to Ruby is greatly appreciated.
May Ruby be with you.

=end

#21 Updated by Shyouhei Urabe about 3 years ago

  • Tracker changed from Bug to Backport
  • Status changed from Closed to Open
  • % Done changed from 100 to 0

=begin
As of 1.8.7-334 this issue is reopened because the proposed fix broke binary compatibility. cf
=end

Also available in: Atom PDF