From 6a8fcb50210f8414d76968298576e39b9fa82562 Mon Sep 17 00:00:00 2001 From: "NARUSE, Yui" Date: Thu, 19 Jan 2023 21:52:47 +0900 Subject: merge revision(s) 3be2acfafd3b3c6168e2266c7c6561d143d7ae5c: [Backport #19327] Fix re-embedding of strings during compaction 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. --- gc.c | 16 +++++++++------- string.c | 12 ++++++++---- test/ruby/test_gc_compact.rb | 8 ++++---- 3 files changed, 21 insertions(+), 15 deletions(-) --- gc.c | 16 +++++++++------- string.c | 12 ++++++++---- test/ruby/test_gc_compact.rb | 8 ++++---- version.h | 2 +- 4 files changed, 22 insertions(+), 16 deletions(-) diff --git a/gc.c b/gc.c index 9866ed579d..b30c022356 100644 --- a/gc.c +++ b/gc.c @@ -10611,16 +10611,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 (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); - } +#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 } +#endif break; } diff --git a/string.c b/string.c index fc5629638c..9fa482dbc7 100644 --- a/string.c +++ b/string.c @@ -312,14 +312,18 @@ 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); - ruby_xfree(buf); + if (len > 0) { + memcpy(RSTRING_PTR(str), buf, len); + ruby_xfree(buf); + } + + TERM_FILL(RSTRING(str)->as.embed.ary + len, TERM_LEN(str)); } void diff --git a/test/ruby/test_gc_compact.rb b/test/ruby/test_gc_compact.rb index d92c979cf0..4d7bb8f7ae 100644 --- a/test/ruby/test_gc_compact.rb +++ b/test/ruby/test_gc_compact.rb @@ -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 diff --git a/version.h b/version.h index 41ec61e156..9a73bc2e57 100644 --- a/version.h +++ b/version.h @@ -11,7 +11,7 @@ # define RUBY_VERSION_MINOR RUBY_API_VERSION_MINOR #define RUBY_VERSION_TEENY 0 #define RUBY_RELEASE_DATE RUBY_RELEASE_YEAR_STR"-"RUBY_RELEASE_MONTH_STR"-"RUBY_RELEASE_DAY_STR -#define RUBY_PATCHLEVEL 14 +#define RUBY_PATCHLEVEL 15 #include "ruby/version.h" #include "ruby/internal/abi.h" -- cgit v1.2.1