Project

General

Profile

Backport #5335

[RFC/PATCH] test_old_thread_select: timing tweaks

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

Status:
Closed
Priority:
Normal
[ruby-core:39618]

Description

I was getting timing errors and short select() timeouts on CentOS 5.6
2.6.18-238.9.1.el5xen most likely due to CONFIG_HZ=250 and the lack of dynticks,
but maybe being a Xen VM has this effect, too.

select() itself appears to just be inaccurate on this system:

strace -e select -T ./ruby -e 'select([STDIN],nil,nil,0.001)'
select(1, [0], NULL, NULL, {0, 1000})   = 0 (Timeout) <0.000046>

I'm not happy with timing tests, but I can't think of another way to test
functionality like this.


Files

test_old_thread_select-timing-tweaks.patch (2.42 KB) test_old_thread_select-timing-tweaks.patch normalperson (Eric Wong), 09/19/2011 04:14 PM
old_thread_select-skip-ancient-linux.patch (2.64 KB) old_thread_select-skip-ancient-linux.patch normalperson (Eric Wong), 09/19/2011 06:10 PM

Associated revisions

Revision d4db53a0
Added by kosaki (Motohiro KOSAKI) almost 8 years ago

  • test/-ext-/old_thread_select/test_old_thread_select.rb: select() with timeout may return early in old Linux kernels with 250 Hz tickrate and no dynticks, so skip everything older than 2.6.32 (which has long term support). And, Make the timing assertions consistently use assert_operator with timing difference in error message Patch by Eric Wong. [Bug #5335] [ruby-core:39618]

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

Revision 33296
Added by kosaki (Motohiro KOSAKI) almost 8 years ago

  • test/-ext-/old_thread_select/test_old_thread_select.rb: select() with timeout may return early in old Linux kernels with 250 Hz tickrate and no dynticks, so skip everything older than 2.6.32 (which has long term support). And, Make the timing assertions consistently use assert_operator with timing difference in error message Patch by Eric Wong. [Bug #5335] [ruby-core:39618]

Revision 33296
Added by kosaki (Motohiro KOSAKI) almost 8 years ago

  • test/-ext-/old_thread_select/test_old_thread_select.rb: select() with timeout may return early in old Linux kernels with 250 Hz tickrate and no dynticks, so skip everything older than 2.6.32 (which has long term support). And, Make the timing assertions consistently use assert_operator with timing difference in error message Patch by Eric Wong. [Bug #5335] [ruby-core:39618]

Revision 33296
Added by kosaki (Motohiro KOSAKI) almost 8 years ago

  • test/-ext-/old_thread_select/test_old_thread_select.rb: select() with timeout may return early in old Linux kernels with 250 Hz tickrate and no dynticks, so skip everything older than 2.6.32 (which has long term support). And, Make the timing assertions consistently use assert_operator with timing difference in error message Patch by Eric Wong. [Bug #5335] [ruby-core:39618]

Revision 33296
Added by kosaki (Motohiro KOSAKI) almost 8 years ago

  • test/-ext-/old_thread_select/test_old_thread_select.rb: select() with timeout may return early in old Linux kernels with 250 Hz tickrate and no dynticks, so skip everything older than 2.6.32 (which has long term support). And, Make the timing assertions consistently use assert_operator with timing difference in error message Patch by Eric Wong. [Bug #5335] [ruby-core:39618]

Revision 33296
Added by kosaki (Motohiro KOSAKI) almost 8 years ago

  • test/-ext-/old_thread_select/test_old_thread_select.rb: select() with timeout may return early in old Linux kernels with 250 Hz tickrate and no dynticks, so skip everything older than 2.6.32 (which has long term support). And, Make the timing assertions consistently use assert_operator with timing difference in error message Patch by Eric Wong. [Bug #5335] [ruby-core:39618]

Revision 33296
Added by kosaki (Motohiro KOSAKI) almost 8 years ago

  • test/-ext-/old_thread_select/test_old_thread_select.rb: select() with timeout may return early in old Linux kernels with 250 Hz tickrate and no dynticks, so skip everything older than 2.6.32 (which has long term support). And, Make the timing assertions consistently use assert_operator with timing difference in error message Patch by Eric Wong. [Bug #5335] [ruby-core:39618]

Revision 62b19328
Added by kosaki (Motohiro KOSAKI) almost 8 years ago

merge revision(s) 33296:

    * test/-ext-/old_thread_select/test_old_thread_select.rb:
      select() with timeout may return early in old Linux kernels
      with 250 Hz tickrate and no dynticks, so skip everything older
      than 2.6.32 (which has long term support).
      And, Make the timing assertions consistently use assert_operator with
      timing difference in error message
      Patch by Eric Wong. [Bug #5335] [ruby-core:39618]

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

Revision 33456
Added by kosaki (Motohiro KOSAKI) almost 8 years ago

merge revision(s) 33296:

* test/-ext-/old_thread_select/test_old_thread_select.rb:
  select() with timeout may return early in old Linux kernels
  with 250 Hz tickrate and no dynticks, so skip everything older
  than 2.6.32 (which has long term support).
  And, Make the timing assertions consistently use assert_operator with
  timing difference in error message
  Patch by Eric Wong. [Bug #5335] [ruby-core:39618]

History

Updated by kosaki (Motohiro KOSAKI) almost 8 years ago

  • Assignee changed from kosaki (Motohiro KOSAKI) to normalperson (Eric Wong)

diff --git a/test/-ext-/old_thread_select/test_old_thread_select.rb b/test/-ext-/old_thread_select/test_old_thread_select.rb
index 1ccdb34..a8a0ce2 100644
--- a/test/-ext-/old_thread_select/test_old_thread_select.rb
+++ b/test/-ext-/old_thread_select/test_old_thread_select.rb
@@ -16,10 +16,10 @@ class TestOldThreadSelect < Test::Unit::TestCase
def test_old_select_read_timeout
with_pipe do |r, w|
t0 = Time.now

  • rc = IO.old_thread_select([r.fileno], nil, nil, 0.001)
  • rc = IO.old_thread_select([r.fileno], nil, nil, 0.01) diff = Time.now - t0 assert_equal 0, rc
  • assert diff >= 0.001, "returned too early"
  • assert_operator diff, :>=, 0.004, "returned too early: diff=#{diff}"

No, I really dislike this. both 0.01 and 0.004 make no sense and no good reason we choose them.
I guess 0.004 mean 250Hz. But in fact, only several linux systems use 250Hz. No platform independent meanings.

Instead, I just recommend to skip this test if platform is too old linux.
(I guess our chkbuild caught the same issue, http://www.rubyist.net/~akr/chkbuild/debian/ruby-trunk/log/20110919T060500Z.log.html.gz)

In addition, the correct way is, to detect inaccurate select timeout by configure script and do_select() retry to call select(2)
automatically if we really need to care.

Updated by normalperson (Eric Wong) almost 8 years ago

While dynticks appeared in 2.6.21, I'm defining "ancient" as pre-2.6.32 since
2.6.32 is still supported and widely deployed.

(having SSL troubles with redmine, apologies if you see this twice/thrice)

Updated by normalperson (Eric Wong) almost 8 years ago

Motohiro KOSAKI kosaki.motohiro@gmail.com wrote:

diff --git a/test/-ext-/old_thread_select/test_old_thread_select.rb b/test/-ext-/old_thread_select/test_old_thread_select.rb
index 1ccdb34..a8a0ce2 100644
--- a/test/-ext-/old_thread_select/test_old_thread_select.rb
+++ b/test/-ext-/old_thread_select/test_old_thread_select.rb
@@ -16,10 +16,10 @@ class TestOldThreadSelect < Test::Unit::TestCase
def test_old_select_read_timeout
with_pipe do |r, w|
t0 = Time.now

  • rc = IO.old_thread_select([r.fileno], nil, nil, 0.001)
  • rc = IO.old_thread_select([r.fileno], nil, nil, 0.01) diff = Time.now - t0 assert_equal 0, rc
  • assert diff >= 0.001, "returned too early"
  • assert_operator diff, :>=, 0.004, "returned too early: diff=#{diff}"

No, I really dislike this. both 0.01 and 0.004 make no sense and no
good reason we choose them. I guess 0.004 mean 250Hz. But in fact,
only several linux systems use 250Hz. No platform independent
meanings.

Yes I used 0.004 to mean 250Hz. 250Hz is/was the default for a long
time, but dynticks got introduced.

Instead, I just recommend to skip this test if platform is too old linux.
(I guess our chkbuild caught the same issue, http://www.rubyist.net/~akr/chkbuild/debian/ruby-trunk/log/20110919T060500Z.log.html.gz)

OK, new patch on the way.

In addition, the correct way is, to detect inaccurate select timeout
by configure script and do_select() retry to call select(2)
automatically if we really need to care.

I don't care enough for this, too much effort for an old platform.

#4

Updated by kosaki (Motohiro KOSAKI) almost 8 years ago

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

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


  • test/-ext-/old_thread_select/test_old_thread_select.rb: select() with timeout may return early in old Linux kernels with 250 Hz tickrate and no dynticks, so skip everything older than 2.6.32 (which has long term support). And, Make the timing assertions consistently use assert_operator with timing difference in error message Patch by Eric Wong. [Bug #5335] [ruby-core:39618]
#5

Updated by kosaki (Motohiro KOSAKI) almost 8 years ago

  • Tracker changed from Bug to Backport
  • Project changed from Ruby master to Backport193
  • Category deleted (core)
  • Status changed from Closed to Assigned
  • Assignee changed from normalperson (Eric Wong) to kosaki (Motohiro KOSAKI)
  • Target version deleted (1.9.3)

I've commited this one to trunk only. I'm watching what's happen boron chkbuild.

Thank you.

#6

Updated by kosaki (Motohiro KOSAKI) almost 8 years ago

  • Status changed from Assigned to Closed

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


merge revision(s) 33296:

* test/-ext-/old_thread_select/test_old_thread_select.rb:
  select() with timeout may return early in old Linux kernels
  with 250 Hz tickrate and no dynticks, so skip everything older
  than 2.6.32 (which has long term support).
  And, Make the timing assertions consistently use assert_operator with
  timing difference in error message
  Patch by Eric Wong. [Bug #5335] [ruby-core:39618]

Also available in: Atom PDF