summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorNARUSE, Yui <naruse@airemix.jp>2023-01-19 21:52:47 +0900
committerNARUSE, Yui <naruse@airemix.jp>2023-01-19 21:52:47 +0900
commit6a8fcb50210f8414d76968298576e39b9fa82562 (patch)
tree654944cb9e09b7876805aa6a4976752325bad1c8
parent98abe4a0be09e0571104f4ca1b5655e353910aa0 (diff)
downloadruby-6a8fcb50210f8414d76968298576e39b9fa82562.tar.gz
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(-)
-rw-r--r--gc.c16
-rw-r--r--string.c12
-rw-r--r--test/ruby/test_gc_compact.rb8
-rw-r--r--version.h2
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"