Project

General

Profile

Bug #6629

[PATCH] io.c: avoid rb_thread_wait_fd() if we may call rb_io_wait_readable()

Added by normalperson (Eric Wong) about 7 years ago. Updated almost 7 years ago.

Status:
Closed
Priority:
Normal
Target version:
-
ruby -v:
ruby 2.0.0dev (2012-06-23 trunk 36192) [x86_64-linux]
Backport:
[ruby-core:45789]

Description

Blindly calling rb_thread_wait_fd() is an extra, unnecessary
system call in some cases. Since we already call
rb_io_wait_readable() when encountering EAGAIN, there is no
user-visible change in behavior and a small potential for
speedup by avoiding a system call (especially on regular
filesystem that should never return EAGAIN/EINTR).

This also helps avoid triggering a bugs on some buggy (ancient)
Linux kernels. I encountered this issue[1] on a CentOS 5.7
machine running the 2.6.18-274.7.1.el5 kernel. Upgrading to
the 2.6.18-308.8.2.el5 kernel fixes the issue.

This Linux kernel regression I encountered was introduced with
commit 786d7e1612f0b0adb6046f19b906609e4fe8b1ba (Jul 15 2007)
and fixed in
commit dd23aae4f5edf4e1dbd8f7f8013a754ba3253f48 (Sep 11 2007)
I've verified the dd23aae4f5edf4e1dbd8f7f8013a754ba3253f48
change is present in the 2.6.18-308.8.2.el5 (working) kernel
but not in the 2.6.18-274.7.1.el5 (broken) kernel.

I realize this is a 5 year-old (fixed) bug in Linux, but I also
believe Ruby is is wrong to call select()/ppoll() before
encountering EAGAIN.

[1] http://mid.gmane.org/20120621004402.GA7450@dcvr.yhbt.net

Note: I left IO#sysread unchanged (with just a comment) since
removing rb_thread_wait_fd() there would change semantics
(as would handling EAGAIN explicitly).


Files

Associated revisions

Revision 3df2fc2d
Added by kosaki (Motohiro KOSAKI) almost 7 years ago

  • io.c (io_bufread): removed unnecessary rb_thread_wait_fd(). Patch by Eric Wong. [Bug #6629] [ruby-core:45789]
  • io.c (rb_io_sysread): ditto.
  • io.c (copy_stream_fallback_body): ditto.

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

Revision 36931
Added by kosaki (Motohiro KOSAKI) almost 7 years ago

  • io.c (io_bufread): removed unnecessary rb_thread_wait_fd(). Patch by Eric Wong. [Bug #6629] [ruby-core:45789]
  • io.c (rb_io_sysread): ditto.
  • io.c (copy_stream_fallback_body): ditto.

Revision 36931
Added by kosaki (Motohiro KOSAKI) almost 7 years ago

  • io.c (io_bufread): removed unnecessary rb_thread_wait_fd(). Patch by Eric Wong. [Bug #6629] [ruby-core:45789]
  • io.c (rb_io_sysread): ditto.
  • io.c (copy_stream_fallback_body): ditto.

Revision 36931
Added by kosaki (Motohiro KOSAKI) almost 7 years ago

  • io.c (io_bufread): removed unnecessary rb_thread_wait_fd(). Patch by Eric Wong. [Bug #6629] [ruby-core:45789]
  • io.c (rb_io_sysread): ditto.
  • io.c (copy_stream_fallback_body): ditto.

Revision 36931
Added by kosaki (Motohiro KOSAKI) almost 7 years ago

  • io.c (io_bufread): removed unnecessary rb_thread_wait_fd(). Patch by Eric Wong. [Bug #6629] [ruby-core:45789]
  • io.c (rb_io_sysread): ditto.
  • io.c (copy_stream_fallback_body): ditto.

Revision 36931
Added by kosaki (Motohiro KOSAKI) almost 7 years ago

  • io.c (io_bufread): removed unnecessary rb_thread_wait_fd(). Patch by Eric Wong. [Bug #6629] [ruby-core:45789]
  • io.c (rb_io_sysread): ditto.
  • io.c (copy_stream_fallback_body): ditto.

Revision 36931
Added by kosaki (Motohiro KOSAKI) almost 7 years ago

  • io.c (io_bufread): removed unnecessary rb_thread_wait_fd(). Patch by Eric Wong. [Bug #6629] [ruby-core:45789]
  • io.c (rb_io_sysread): ditto.
  • io.c (copy_stream_fallback_body): ditto.

History

Updated by mame (Yusuke Endoh) about 7 years ago

  • Status changed from Open to Assigned
  • Assignee set to kosaki (Motohiro KOSAKI)
#2

Updated by kosaki (Motohiro KOSAKI) almost 7 years ago

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

This issue was solved with changeset r36931.
Eric, thank you for reporting this issue.
Your contribution to Ruby is greatly appreciated.
May Ruby be with you.


  • io.c (io_bufread): removed unnecessary rb_thread_wait_fd(). Patch by Eric Wong. [Bug #6629] [ruby-core:45789]
  • io.c (rb_io_sysread): ditto.
  • io.c (copy_stream_fallback_body): ditto.

Also available in: Atom PDF