diff options
author | KJ Tsanaktsidis <kj@kjtsanaktsidis.id.au> | 2023-11-27 17:33:08 +1100 |
---|---|---|
committer | Peter Zhu <peter@peterzhu.ca> | 2023-11-27 11:02:11 -0500 |
commit | 8427a8a655e2a04bfdc6a645ec967405d3617137 (patch) | |
tree | 0904e2f93209223f2aad33823fcf515bfb8f7ec0 | |
parent | 196c4aeb766a66b3557ddab61086db58c7a08226 (diff) | |
download | ruby-8427a8a655e2a04bfdc6a645ec967405d3617137.tar.gz |
Fix flaky "Expected 499 to be >= 500" assertion in test_gc_compact.rb
There have been some sproradically flaky tests related to GC compaction,
which fail with:
1) Failure:
TestGCCompact#test_moving_hashes_down_size_pools [/test/ruby/test_gc_compact.rb:442]:
Expected 499 to be >= 500.
What's happening here, is that, _sometimes_, depending on very unlucky
combinations of machine things, one of the expected-to-be-moved hashes
might be found on the machine stack during GC, and thus pinned.
One factor which seems to make this _more_ likely is that GCC 11 on
Ubuntu 22.04 seems to want to allocate 440 bytes of stack space for
`gc_start`, which is much more than it actually uses on the common code
path. The result is that there are some 50-odd VALUE-sized cells "live"
on the stack which may well contain valid heap pointers from previous
function calls, and will need to be pinned.
This is, of course, totally normal and expected; Ruby's GC is
conservative and if there is the possibility that a VALUE might be live
on the machine stack, it can't be moved. However, it does make these
tests flaky.
This commit "fixes" the tests by performing the work in a fiber; the
fiber goes out of scope and should be collected by the call to
verify_compaction_references, so there should be no references to the
to-be-moved objects floating around on the machine stack.
Fixes [#20021]
-rw-r--r-- | test/ruby/test_gc_compact.rb | 67 |
1 files changed, 37 insertions, 30 deletions
diff --git a/test/ruby/test_gc_compact.rb b/test/ruby/test_gc_compact.rb index 8a3f1f145d..6d26811cbb 100644 --- a/test/ruby/test_gc_compact.rb +++ b/test/ruby/test_gc_compact.rb @@ -317,14 +317,16 @@ class TestGCCompact < Test::Unit::TestCase GC.verify_compaction_references(expand_heap: true, toward: :empty) - arys = ARY_COUNT.times.map do - ary = "abbbbbbbbbb".chars - ary.uniq! - end + Fiber.new { + $arys = ARY_COUNT.times.map do + ary = "abbbbbbbbbb".chars + ary.uniq! + end + }.resume stats = GC.verify_compaction_references(expand_heap: true, toward: :empty) assert_operator(stats.dig(:moved_down, :T_ARRAY) || 0, :>=, ARY_COUNT) - refute_empty(arys.keep_if { |o| ObjectSpace.dump(o).include?('"embedded":true') }) + refute_empty($arys.keep_if { |o| ObjectSpace.dump(o).include?('"embedded":true') }) end; end @@ -337,22 +339,23 @@ class TestGCCompact < Test::Unit::TestCase GC.verify_compaction_references(expand_heap: true, toward: :empty) - ary = "hello".chars - arys = ARY_COUNT.times.map do - x = [] - ary.each { |e| x << e } - x - end + Fiber.new { + ary = "hello".chars + $arys = ARY_COUNT.times.map do + x = [] + ary.each { |e| x << e } + x + end + }.resume stats = GC.verify_compaction_references(expand_heap: true, toward: :empty) assert_operator(stats.dig(:moved_up, :T_ARRAY) || 0, :>=, ARY_COUNT) - refute_empty(arys.keep_if { |o| ObjectSpace.dump(o).include?('"embedded":true') }) + refute_empty($arys.keep_if { |o| ObjectSpace.dump(o).include?('"embedded":true') }) end; end def test_moving_objects_between_size_pools omit if GC::INTERNAL_CONSTANTS[:SIZE_POOL_COUNT] == 1 - omit "Flaky on Solaris" if /solaris/i =~ RUBY_PLATFORM assert_separately(%w[-robjspace], "#{<<~"begin;"}\n#{<<~"end;"}", timeout: 10, signal: :SEGV) begin; @@ -368,16 +371,18 @@ class TestGCCompact < Test::Unit::TestCase GC.verify_compaction_references(expand_heap: true, toward: :empty) - ary = OBJ_COUNT.times.map { Foo.new } - ary.each(&:add_ivars) + Fiber.new { + $ary = OBJ_COUNT.times.map { Foo.new } + $ary.each(&:add_ivars) - GC.start - Foo.new.add_ivars + GC.start + Foo.new.add_ivars + }.resume stats = GC.verify_compaction_references(expand_heap: true, toward: :empty) assert_operator(stats.dig(:moved_up, :T_OBJECT) || 0, :>=, OBJ_COUNT) - refute_empty(ary.keep_if { |o| ObjectSpace.dump(o).include?('"embedded":true') }) + refute_empty($ary.keep_if { |o| ObjectSpace.dump(o).include?('"embedded":true') }) end; end @@ -390,13 +395,15 @@ class TestGCCompact < Test::Unit::TestCase GC.verify_compaction_references(expand_heap: true, toward: :empty) - str = "a" * GC::INTERNAL_CONSTANTS[:BASE_SLOT_SIZE] - ary = STR_COUNT.times.map { "" << str } + Fiber.new { + str = "a" * GC::INTERNAL_CONSTANTS[:BASE_SLOT_SIZE] + $ary = STR_COUNT.times.map { "" << str } + }.resume stats = GC.verify_compaction_references(expand_heap: true, toward: :empty) assert_operator(stats[:moved_up][:T_STRING], :>=, STR_COUNT) - refute_empty(ary.keep_if { |o| ObjectSpace.dump(o).include?('"embedded":true') }) + refute_empty($ary.keep_if { |o| ObjectSpace.dump(o).include?('"embedded":true') }) end; end @@ -409,12 +416,14 @@ class TestGCCompact < Test::Unit::TestCase GC.verify_compaction_references(expand_heap: true, toward: :empty) - ary = STR_COUNT.times.map { ("a" * GC::INTERNAL_CONSTANTS[:BASE_SLOT_SIZE]).squeeze! } + Fiber.new { + $ary = STR_COUNT.times.map { ("a" * GC::INTERNAL_CONSTANTS[:BASE_SLOT_SIZE]).squeeze! } + }.resume stats = GC.verify_compaction_references(expand_heap: true, toward: :empty) assert_operator(stats[:moved_down][:T_STRING], :>=, STR_COUNT) - refute_empty(ary.keep_if { |o| ObjectSpace.dump(o).include?('"embedded":true') }) + refute_empty($ary.keep_if { |o| ObjectSpace.dump(o).include?('"embedded":true') }) end; end @@ -422,10 +431,6 @@ class TestGCCompact < Test::Unit::TestCase omit if GC::INTERNAL_CONSTANTS[:SIZE_POOL_COUNT] == 1 # AR and ST hashes are in the same size pool on 32 bit omit unless RbConfig::SIZEOF["uint64_t"] <= RbConfig::SIZEOF["void*"] - # This test fails on Solaris SPARC with the following error and I can't figure out why: - # TestGCCompact#test_moving_hashes_down_size_pools - # Expected 499 to be >= 500. - omit if /sparc-solaris/ =~ RUBY_PLATFORM assert_separately(%w[-robjspace], "#{<<~"begin;"}\n#{<<~"end;"}", timeout: 10, signal: :SEGV) begin; @@ -433,9 +438,11 @@ class TestGCCompact < Test::Unit::TestCase GC.verify_compaction_references(expand_heap: true, toward: :empty) - base_hash = { a: 1, b: 2, c: 3, d: 4, e: 5, f: 6, g: 7, h: 8 } - ary = HASH_COUNT.times.map { base_hash.dup } - ary.each { |h| h[:i] = 9 } + Fiber.new { + base_hash = { a: 1, b: 2, c: 3, d: 4, e: 5, f: 6, g: 7, h: 8 } + $ary = HASH_COUNT.times.map { base_hash.dup } + $ary.each_with_index { |h, i| h[:i] = 9 } + }.resume stats = GC.verify_compaction_references(expand_heap: true, toward: :empty) |