diff options
| author | KJ Tsanaktsidis <kj@kjtsanaktsidis.id.au> | 2023-11-27 16:50:05 +1100 |
|---|---|---|
| committer | Peter Zhu <peter@peterzhu.ca> | 2023-12-07 10:19:35 -0500 |
| commit | cbc0e0bef08f9389f5bbe76de016795f01c3bc76 (patch) | |
| tree | 415d9399dc5db15e641e4f0c52dfe02f8b27389b /test/ruby | |
| parent | 5d832d16d9878766dfe527934ef1ad64698b9bf8 (diff) | |
Fix GC.verify_compaction_references not moving every object
The intention of GC.verify_compaction_references is, I believe, to force
every single movable object to be moved, so that it's possible to debug
native extensions which not correctly updating their references to
objects they mark as movable.
To do this, it doubles the number of allocated pages for each size pool,
and sorts the heap pages so that the free ones are swept first; thus,
every object in an old page should be moved into a free slot in one of
the new pages.
This worked fine until movement of objects _between_ size pools during
compaction was implemented. That causes some problems for
verify_compaction_references:
* We were doubling the number of pages in each size pool, but actually
if some objects need to move into a _different_ pool, there's no
guarantee that they'll be enough room in that one.
* It's possible for the sweep & compact cursors to meet in one size pool
before all the objects that want to move into that size pool from
another are processed by the compaction.
You can see these problems by changing some of the movement tests in
test_gc_compact.rb to try and move e.g. 50,000 objects instead of
500; the test is not able to actually move all of the objects in a
single compaction run.
To fix this, we do two things in verify_compaction_references:
* Firstly, we add enough pages to every size pool to make them the same
size. This ensures that their compact cursors will all have space to
move during compaction (even if that means empty pages are
pointlessly compacted)
* Then, we examine every object and determine where it _wants_ to be
compacted into. We use this information to add additional pages to
each size pool to handle all objects which should live there.
With these two changes, we can move arbitrary amounts of objects into
the correct size pool in a single call to verify_compaction_references.
My _motivation_ for performing this work was to try and fix some test
stability issues in test_gc_compact.rb. I now no longer think that we
actually see this particular bug in rubyci.org, but I also think
verify_compaction_references should do what it says on the tin, so it's
worth keeping.
[Bug #20022]
Diffstat (limited to 'test/ruby')
| -rw-r--r-- | test/ruby/test_gc_compact.rb | 18 |
1 files changed, 9 insertions, 9 deletions
diff --git a/test/ruby/test_gc_compact.rb b/test/ruby/test_gc_compact.rb index 6d26811cbb..a20e2d67d9 100644 --- a/test/ruby/test_gc_compact.rb +++ b/test/ruby/test_gc_compact.rb @@ -389,14 +389,14 @@ class TestGCCompact < Test::Unit::TestCase def test_moving_strings_up_size_pools omit if GC::INTERNAL_CONSTANTS[:SIZE_POOL_COUNT] == 1 - assert_separately(%w[-robjspace], "#{<<~"begin;"}\n#{<<~"end;"}", timeout: 10, signal: :SEGV) + assert_separately(%w[-robjspace], "#{<<~"begin;"}\n#{<<~"end;"}", timeout: 30, signal: :SEGV) begin; - STR_COUNT = 500 + STR_COUNT = 50000 GC.verify_compaction_references(expand_heap: true, toward: :empty) Fiber.new { - str = "a" * GC::INTERNAL_CONSTANTS[:BASE_SLOT_SIZE] + str = "a" * GC::INTERNAL_CONSTANTS[:BASE_SLOT_SIZE] * 4 $ary = STR_COUNT.times.map { "" << str } }.resume @@ -410,14 +410,14 @@ class TestGCCompact < Test::Unit::TestCase def test_moving_strings_down_size_pools omit if GC::INTERNAL_CONSTANTS[:SIZE_POOL_COUNT] == 1 - assert_separately(%w[-robjspace], "#{<<~"begin;"}\n#{<<~"end;"}", timeout: 10, signal: :SEGV) + assert_separately(%w[-robjspace], "#{<<~"begin;"}\n#{<<~"end;"}", timeout: 30, signal: :SEGV) begin; - STR_COUNT = 500 + STR_COUNT = 50000 GC.verify_compaction_references(expand_heap: true, toward: :empty) Fiber.new { - $ary = STR_COUNT.times.map { ("a" * GC::INTERNAL_CONSTANTS[:BASE_SLOT_SIZE]).squeeze! } + $ary = STR_COUNT.times.map { ("a" * GC::INTERNAL_CONSTANTS[:BASE_SLOT_SIZE] * 4).squeeze! } }.resume stats = GC.verify_compaction_references(expand_heap: true, toward: :empty) @@ -432,9 +432,9 @@ class TestGCCompact < Test::Unit::TestCase # AR and ST hashes are in the same size pool on 32 bit omit unless RbConfig::SIZEOF["uint64_t"] <= RbConfig::SIZEOF["void*"] - assert_separately(%w[-robjspace], "#{<<~"begin;"}\n#{<<~"end;"}", timeout: 10, signal: :SEGV) + assert_separately(%w[-robjspace], "#{<<~"begin;"}\n#{<<~"end;"}", timeout: 30, signal: :SEGV) begin; - HASH_COUNT = 500 + HASH_COUNT = 50000 GC.verify_compaction_references(expand_heap: true, toward: :empty) @@ -446,7 +446,7 @@ class TestGCCompact < Test::Unit::TestCase stats = GC.verify_compaction_references(expand_heap: true, toward: :empty) - assert_operator(stats[:moved_down][:T_HASH], :>=, 500) + assert_operator(stats[:moved_down][:T_HASH], :>=, HASH_COUNT) end; end |
