Backport #5335: [RFC/PATCH] test_old_thread_select: timing tweaks

• Status: Closed
• Assignee: kosaki (Motohiro KOSAKI)

[normalperson (Eric Wong)]

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.

[kosaki (Motohiro KOSAKI)]

• 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.

[normalperson (Eric Wong)]

• File old_thread_select-skip-ancient-linux.patch old_thread_select-skip-ancient-linux.patch added

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)

[normalperson (Eric Wong)]

Motohiro KOSAKI [e-mail] 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.

[kosaki (Motohiro KOSAKI)]

• 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]

[kosaki (Motohiro KOSAKI)]

• 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.

[kosaki (Motohiro KOSAKI)]

• 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]

[kosaki (Motohiro KOSAKI)]

Done.
r33454 and r33456.