1berry/ruby
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Fix re-embedding of strings during compaction

Browse Source 
The reference updating code for strings is not re-embedding strings
because the code is incorrectly wrapped inside of a
`if (STR_SHARED_P(obj))` clause. Shared strings can't be re-embedded
so this ends up being a no-op. This means that strings can be moved to a
large size pool during compaction, but won't be re-embedded, which would
waste the space.
This commit is contained in:
Peter Zhu 2023-01-06 09:09:18 -05:00
parent 29dc9378d9
commit 3be2acfafd
Notes: git 2023-01-09 13:49:57 +00:00 
Merged: https://github.com/ruby/ruby/pull/7071
3 changed files with 22 additions and 16 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

8
gc.c
View File

@ -10609,16 +10609,18 @@ gc_update_object_references(rb_objspace_t *objspace, VALUE obj)
#if USE_RVARGC
VALUE new_root = any->as.string.as.heap.aux.shared;
rb_str_update_shared_ary(obj, old_root, new_root);
#endif
}
// if, after move the string is not embedded, and can fit in the
// slot it's been placed in, then re-embed it
#if USE_RVARGC
/* If, after move the string is not embedded, and can fit in the
* slot it's been placed in, then re-embed it. */
if (rb_gc_obj_slot_size(obj) >= rb_str_size_as_embedded(obj)) {
if (!STR_EMBED_P(obj) && rb_str_reembeddable_p(obj)) {
rb_str_make_embedded(obj);
}
}
#endif
}
break;
}

10
string.c
View File

@ -312,16 +312,20 @@ rb_str_make_embedded(VALUE str)
RUBY_ASSERT(rb_str_reembeddable_p(str));
RUBY_ASSERT(!STR_EMBED_P(str));
char *buf = RSTRING_PTR(str);
long len = RSTRING_LEN(str);
char *buf = RSTRING(str)->as.heap.ptr;
long len = RSTRING(str)->as.heap.len;
STR_SET_EMBED(str);
STR_SET_EMBED_LEN(str, len);
memmove(RSTRING_PTR(str), buf, len);
if (len > 0) {
memcpy(RSTRING_PTR(str), buf, len);
ruby_xfree(buf);
}
TERM_FILL(RSTRING(str)->as.embed.ary + len, TERM_LEN(str));
}
void
rb_str_update_shared_ary(VALUE str, VALUE old_root, VALUE new_root)
{

8
test/ruby/test_gc_compact.rb
View File

@ -382,7 +382,7 @@ class TestGCCompact < Test::Unit::TestCase
def test_moving_strings_up_size_pools
omit if GC::INTERNAL_CONSTANTS[:SIZE_POOL_COUNT] == 1
assert_separately([], "#{<<~"begin;"}\n#{<<~"end;"}", timeout: 10, signal: :SEGV)
assert_separately(%w[-robjspace], "#{<<~"begin;"}\n#{<<~"end;"}", timeout: 10, signal: :SEGV)
begin;
STR_COUNT = 500
@ -394,14 +394,14 @@ class TestGCCompact < Test::Unit::TestCase
stats = GC.verify_compaction_references(expand_heap: true, toward: :empty)
assert_operator(stats[:moved_up][:T_STRING], :>=, STR_COUNT)
assert(ary) # warning: assigned but unused variable - ary
assert_include(ObjectSpace.dump(ary[0]), '"embedded":true')
end;
end
def test_moving_strings_down_size_pools
omit if GC::INTERNAL_CONSTANTS[:SIZE_POOL_COUNT] == 1
assert_separately([], "#{<<~"begin;"}\n#{<<~"end;"}", timeout: 10, signal: :SEGV)
assert_separately(%w[-robjspace], "#{<<~"begin;"}\n#{<<~"end;"}", timeout: 10, signal: :SEGV)
begin;
STR_COUNT = 500
@ -412,7 +412,7 @@ class TestGCCompact < Test::Unit::TestCase
stats = GC.verify_compaction_references(expand_heap: true, toward: :empty)
assert_operator(stats[:moved_down][:T_STRING], :>=, STR_COUNT)
assert(ary) # warning: assigned but unused variable - ary
assert_include(ObjectSpace.dump(ary[0]), '"embedded":true')
end;
end
end