From df7d9812cc504f3361792f3dd4843d1ffa3c5ead Mon Sep 17 00:00:00 2001 From: Stan Lo Date: Wed, 13 Aug 2025 01:39:46 +0100 Subject: [PATCH 1/8] ZJIT: Prepare non-leaf calls for SetGlobal (#14197) When trace_var is used, setting a global variable can cause exceptions to be raised. We need to prepare for that. --- test/ruby/test_zjit.rb | 24 ++++++++++++++++++++++++ zjit/src/codegen.rs | 8 ++++++-- 2 files changed, 30 insertions(+), 2 deletions(-) diff --git a/test/ruby/test_zjit.rb b/test/ruby/test_zjit.rb index 4dc0919b6b..98146b80db 100644 --- a/test/ruby/test_zjit.rb +++ b/test/ruby/test_zjit.rb @@ -61,6 +61,30 @@ class TestZJIT < Test::Unit::TestCase } end + def test_setglobal + assert_compiles '1', %q{ + def test + $a = 1 + $a + end + + test + }, insns: [:setglobal] + end + + def test_setglobal_with_trace_var_exception + assert_compiles '"rescued"', %q{ + def test + $a = 1 + rescue + "rescued" + end + + trace_var(:$a) { raise } + test + }, insns: [:setglobal] + end + def test_setlocal assert_compiles '3', %q{ def test(n) diff --git a/zjit/src/codegen.rs b/zjit/src/codegen.rs index 01ed0f0590..e7bd3285dd 100644 --- a/zjit/src/codegen.rs +++ b/zjit/src/codegen.rs @@ -367,7 +367,7 @@ fn gen_insn(cb: &mut CodeBlock, jit: &mut JITState, asm: &mut Assembler, functio Insn::PatchPoint { invariant, state } => return gen_patch_point(jit, asm, invariant, &function.frame_state(*state)), Insn::CCall { cfun, args, name: _, return_type: _, elidable: _ } => gen_ccall(asm, *cfun, opnds!(args))?, Insn::GetIvar { self_val, id, state: _ } => gen_getivar(asm, opnd!(self_val), *id), - Insn::SetGlobal { id, val, state: _ } => return Some(gen_setglobal(asm, *id, opnd!(val))), + Insn::SetGlobal { id, val, state } => return gen_setglobal(jit, asm, *id, opnd!(val), &function.frame_state(*state)), Insn::GetGlobal { id, state: _ } => gen_getglobal(asm, *id), &Insn::GetLocal { ep_offset, level } => gen_getlocal_with_ep(asm, ep_offset, level)?, Insn::SetLocal { val, ep_offset, level } => return gen_setlocal_with_ep(asm, opnd!(val), *ep_offset, *level), @@ -592,8 +592,12 @@ fn gen_getglobal(asm: &mut Assembler, id: ID) -> Opnd { } /// Set global variables -fn gen_setglobal(asm: &mut Assembler, id: ID, val: Opnd) { +fn gen_setglobal(jit: &mut JITState, asm: &mut Assembler, id: ID, val: Opnd, state: &FrameState) -> Option<()> { + // When trace_var is used, setting a global variable can cause exceptions + gen_prepare_non_leaf_call(jit, asm, state)?; + asm_ccall!(asm, rb_gvar_set, id.0.into(), val); + Some(()) } /// Side-exit into the interpreter From 40d07f268e63aa2cdbaf3b31b227cecc5ba7e9e0 Mon Sep 17 00:00:00 2001 From: Kazuhiro NISHIYAMA Date: Thu, 17 Jul 2025 14:35:01 +0900 Subject: [PATCH 2/8] [DOC] Move Therad#join under Thread in NEWS-3.0.0.md --- doc/NEWS/NEWS-3.0.0.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/doc/NEWS/NEWS-3.0.0.md b/doc/NEWS/NEWS-3.0.0.md index 004fa4bf67..9fbaf504b4 100644 --- a/doc/NEWS/NEWS-3.0.0.md +++ b/doc/NEWS/NEWS-3.0.0.md @@ -367,11 +367,11 @@ Outstanding ones only. * Fiber.blocking? tells whether the current execution context is blocking. [[Feature #16786]] +* Thread + * Thread#join invokes the scheduler hooks `block`/`unblock` in a non-blocking execution context. [[Feature #16786]] -* Thread - * Thread.ignore_deadlock accessor has been added for disabling the default deadlock detection, allowing the use of signal handlers to break deadlock. [[Bug #13768]] From 7595ac9a9e8dc460ced50a49e3facaa55a259c35 Mon Sep 17 00:00:00 2001 From: Burdette Lamar Date: Wed, 13 Aug 2025 09:53:22 -0500 Subject: [PATCH 3/8] [DOC] Tweaks for GC.count --- gc.rb | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/gc.rb b/gc.rb index 1d45b48b34..22d3c5398f 100644 --- a/gc.rb +++ b/gc.rb @@ -104,9 +104,14 @@ module GC end # call-seq: - # GC.count -> Integer + # self.count -> integer + # + # Returns the total number of times garbage collection has occurred: + # + # GC.count # => 385 + # GC.start + # GC.count # => 386 # - # Returns the number of times \GC has occurred since the process started. def self.count Primitive.gc_count end From 31ff07ed1eb05d01f7da3c017d542137a3db1e94 Mon Sep 17 00:00:00 2001 From: Peter Zhu Date: Thu, 7 Aug 2025 14:23:07 -0400 Subject: [PATCH 4/8] Add link to Ruby options doc in help text Adds link to https://docs.ruby-lang.org/en/master/ruby/options_md.html in Ruby help text (-h and --help). --- depend | 2 ++ doc/ruby/options.md | 7 +++++++ ruby.c | 6 +++++- test/ruby/test_rubyoptions.rb | 12 +++++++++--- 4 files changed, 23 insertions(+), 4 deletions(-) diff --git a/depend b/depend index ea2486e9e8..735afd8cc9 100644 --- a/depend +++ b/depend @@ -14598,6 +14598,7 @@ ruby.$(OBJEXT): $(top_srcdir)/prism/util/pm_newline_list.h ruby.$(OBJEXT): $(top_srcdir)/prism/util/pm_string.h ruby.$(OBJEXT): $(top_srcdir)/prism/util/pm_strncasecmp.h ruby.$(OBJEXT): $(top_srcdir)/prism/util/pm_strpbrk.h +ruby.$(OBJEXT): $(top_srcdir)/version.h ruby.$(OBJEXT): {$(VPATH)}assert.h ruby.$(OBJEXT): {$(VPATH)}atomic.h ruby.$(OBJEXT): {$(VPATH)}backward/2/assume.h @@ -14781,6 +14782,7 @@ ruby.$(OBJEXT): {$(VPATH)}prism/ast.h ruby.$(OBJEXT): {$(VPATH)}prism/diagnostic.h ruby.$(OBJEXT): {$(VPATH)}prism/version.h ruby.$(OBJEXT): {$(VPATH)}prism_compile.h +ruby.$(OBJEXT): {$(VPATH)}revision.h ruby.$(OBJEXT): {$(VPATH)}ruby.c ruby.$(OBJEXT): {$(VPATH)}ruby_assert.h ruby.$(OBJEXT): {$(VPATH)}ruby_atomic.h diff --git a/doc/ruby/options.md b/doc/ruby/options.md index 95f8cf453c..3ed36bf7ea 100644 --- a/doc/ruby/options.md +++ b/doc/ruby/options.md @@ -1,3 +1,10 @@ + + # Ruby Command-Line Options ## About the Examples diff --git a/ruby.c b/ruby.c index 6d2b5833b6..134ac5ae3b 100644 --- a/ruby.c +++ b/ruby.c @@ -61,6 +61,7 @@ #include "ruby/util.h" #include "ruby/version.h" #include "ruby/internal/error.h" +#include "version.h" #define singlebit_only_p(x) !((x) & ((x)-1)) STATIC_ASSERT(Qnil_1bit_from_Qfalse, singlebit_only_p(Qnil^Qfalse)); @@ -403,7 +404,10 @@ usage(const char *name, int help, int highlight, int columns) unsigned int w = (columns > 80 ? (columns - 79) / 2 : 0) + 16; #define SHOW(m) show_usage_line(&(m), help, highlight, w, columns) - printf("%sUsage:%s %s [options] [--] [filepath] [arguments]\n", sb, se, name); + printf("%sUsage:%s %s [options] [--] [filepath] [arguments]\n\n", sb, se, name); + printf("Details and examples at https://docs.ruby-lang.org/en/%s/ruby/options_md.html\n", + RUBY_PATCHLEVEL == -1 ? "master" : STRINGIZE(RUBY_VERSION_MAJOR) "." STRINGIZE(RUBY_VERSION_MINOR)); + for (i = 0; i < num; ++i) SHOW(usage_msg[i]); diff --git a/test/ruby/test_rubyoptions.rb b/test/ruby/test_rubyoptions.rb index 631b188677..4144191ec7 100644 --- a/test/ruby/test_rubyoptions.rb +++ b/test/ruby/test_rubyoptions.rb @@ -49,18 +49,24 @@ class TestRubyOptions < Test::Unit::TestCase def test_usage assert_in_out_err(%w(-h)) do |r, e| - assert_operator(r.size, :<=, 25) - longer = r[1..-1].select {|x| x.size >= 80} + assert_operator(r.size, :<=, 26) + longer = r[3..-1].select {|x| x.size >= 80} assert_equal([], longer) assert_equal([], e) + + version = RUBY_PATCHLEVEL == -1 ? "master" : "#{RUBY_VERSION_MAJOR}.#{RUBY_VERSION_MINOR}" + assert_include(r, "Details and examples at https://docs.ruby-lang.org/en/#{version}/ruby/options_md.html") end end def test_usage_long assert_in_out_err(%w(--help)) do |r, e| - longer = r[1..-1].select {|x| x.size > 80} + longer = r[3..-1].select {|x| x.size > 80} assert_equal([], longer) assert_equal([], e) + + version = RUBY_PATCHLEVEL == -1 ? "master" : "#{RUBY_VERSION_MAJOR}.#{RUBY_VERSION_MINOR}" + assert_include(r, "Details and examples at https://docs.ruby-lang.org/en/#{version}/ruby/options_md.html") end end From ad12db4b3d3853125b50244190c7ac404fb3a4a5 Mon Sep 17 00:00:00 2001 From: Max Bernstein Date: Tue, 12 Aug 2025 12:13:05 -0400 Subject: [PATCH 5/8] ZJIT: Only validate HIR in debug mode --- zjit/src/codegen.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/zjit/src/codegen.rs b/zjit/src/codegen.rs index e7bd3285dd..8a04d04993 100644 --- a/zjit/src/codegen.rs +++ b/zjit/src/codegen.rs @@ -1288,6 +1288,7 @@ fn compile_iseq(iseq: IseqPtr) -> Option { if !get_option!(disable_hir_opt) { function.optimize(); } + #[cfg(debug_assertions)] if let Err(err) = function.validate() { debug!("ZJIT: compile_iseq: {err:?}"); return None; From 10aa4134d408cab7b5754f3dcac2e75a52962b7c Mon Sep 17 00:00:00 2001 From: Jean Boussier Date: Fri, 8 Aug 2025 11:41:02 +0200 Subject: [PATCH 6/8] imemo_fields: store owner object in RBasic.klass It is much more convenient than storing the klass, especially when dealing with `object_id` as it allows to update the id2ref table without having to dereference the owner, which may be garbage at that point. --- gc.c | 16 +++++++++++----- imemo.c | 26 +++++++++++++------------- internal/class.h | 2 +- internal/imemo.h | 12 +++++++++--- shape.c | 13 +++++++++++-- test/ruby/test_shapes.rb | 7 +++++-- variable.c | 24 ++++++++++++------------ 7 files changed, 62 insertions(+), 38 deletions(-) diff --git a/gc.c b/gc.c index 5cfef6ff2c..fd4d8954fb 100644 --- a/gc.c +++ b/gc.c @@ -1921,7 +1921,7 @@ object_id(VALUE obj) // in fields. return class_object_id(obj); case T_IMEMO: - rb_bug("T_IMEMO can't have an object_id"); + RUBY_ASSERT(IMEMO_TYPE_P(obj, imemo_fields)); break; default: break; @@ -1945,20 +1945,26 @@ build_id2ref_i(VALUE obj, void *data) switch (BUILTIN_TYPE(obj)) { case T_CLASS: case T_MODULE: + RUBY_ASSERT(!rb_objspace_garbage_object_p(obj)); if (RCLASS(obj)->object_id) { - RUBY_ASSERT(!rb_objspace_garbage_object_p(obj)); st_insert(id2ref_tbl, RCLASS(obj)->object_id, obj); } break; case T_IMEMO: - case T_NONE: + RUBY_ASSERT(!rb_objspace_garbage_object_p(obj)); + if (IMEMO_TYPE_P(obj, imemo_fields) && rb_shape_obj_has_id(obj)) { + st_insert(id2ref_tbl, rb_obj_id(obj), rb_imemo_fields_owner(obj)); + } break; - default: + case T_OBJECT: + RUBY_ASSERT(!rb_objspace_garbage_object_p(obj)); if (rb_shape_obj_has_id(obj)) { - RUBY_ASSERT(!rb_objspace_garbage_object_p(obj)); st_insert(id2ref_tbl, rb_obj_id(obj), obj); } break; + default: + // For generic_fields, the T_IMEMO/fields is responsible for populating the entry. + break; } } diff --git a/imemo.c b/imemo.c index 30dae8d583..fde5b15ad6 100644 --- a/imemo.c +++ b/imemo.c @@ -109,16 +109,16 @@ rb_imemo_tmpbuf_parser_heap(void *buf, rb_imemo_tmpbuf_t *old_heap, size_t cnt) } static VALUE -imemo_fields_new(VALUE klass, size_t capa) +imemo_fields_new(VALUE owner, size_t capa) { size_t embedded_size = offsetof(struct rb_fields, as.embed) + capa * sizeof(VALUE); if (rb_gc_size_allocatable_p(embedded_size)) { - VALUE fields = rb_imemo_new(imemo_fields, klass, embedded_size); + VALUE fields = rb_imemo_new(imemo_fields, owner, embedded_size); RUBY_ASSERT(IMEMO_TYPE_P(fields, imemo_fields)); return fields; } else { - VALUE fields = rb_imemo_new(imemo_fields, klass, sizeof(struct rb_fields)); + VALUE fields = rb_imemo_new(imemo_fields, owner, sizeof(struct rb_fields)); FL_SET_RAW(fields, OBJ_FIELD_EXTERNAL); IMEMO_OBJ_FIELDS(fields)->as.external.ptr = ALLOC_N(VALUE, capa); return fields; @@ -126,23 +126,23 @@ imemo_fields_new(VALUE klass, size_t capa) } VALUE -rb_imemo_fields_new(VALUE klass, size_t capa) +rb_imemo_fields_new(VALUE owner, size_t capa) { - return imemo_fields_new(klass, capa); + return imemo_fields_new(owner, capa); } static VALUE -imemo_fields_new_complex(VALUE klass, size_t capa) +imemo_fields_new_complex(VALUE owner, size_t capa) { - VALUE fields = imemo_fields_new(klass, sizeof(struct rb_fields)); + VALUE fields = imemo_fields_new(owner, sizeof(struct rb_fields)); IMEMO_OBJ_FIELDS(fields)->as.complex.table = st_init_numtable_with_size(capa); return fields; } VALUE -rb_imemo_fields_new_complex(VALUE klass, size_t capa) +rb_imemo_fields_new_complex(VALUE owner, size_t capa) { - return imemo_fields_new_complex(klass, capa); + return imemo_fields_new_complex(owner, capa); } static int @@ -161,9 +161,9 @@ imemo_fields_complex_wb_i(st_data_t key, st_data_t value, st_data_t arg) } VALUE -rb_imemo_fields_new_complex_tbl(VALUE klass, st_table *tbl) +rb_imemo_fields_new_complex_tbl(VALUE owner, st_table *tbl) { - VALUE fields = imemo_fields_new(klass, sizeof(struct rb_fields)); + VALUE fields = imemo_fields_new(owner, sizeof(struct rb_fields)); IMEMO_OBJ_FIELDS(fields)->as.complex.table = tbl; st_foreach(tbl, imemo_fields_trigger_wb_i, (st_data_t)fields); return fields; @@ -176,7 +176,7 @@ rb_imemo_fields_clone(VALUE fields_obj) VALUE clone; if (rb_shape_too_complex_p(shape_id)) { - clone = rb_imemo_fields_new_complex(CLASS_OF(fields_obj), 0); + clone = rb_imemo_fields_new_complex(rb_imemo_fields_owner(fields_obj), 0); RBASIC_SET_SHAPE_ID(clone, shape_id); st_table *src_table = rb_imemo_fields_complex_tbl(fields_obj); st_table *dest_table = rb_imemo_fields_complex_tbl(clone); @@ -184,7 +184,7 @@ rb_imemo_fields_clone(VALUE fields_obj) st_foreach(dest_table, imemo_fields_complex_wb_i, (st_data_t)clone); } else { - clone = imemo_fields_new(CLASS_OF(fields_obj), RSHAPE_CAPACITY(shape_id)); + clone = imemo_fields_new(rb_imemo_fields_owner(fields_obj), RSHAPE_CAPACITY(shape_id)); RBASIC_SET_SHAPE_ID(clone, shape_id); VALUE *fields = rb_imemo_fields_ptr(clone); attr_index_t fields_count = RSHAPE_LEN(shape_id); diff --git a/internal/class.h b/internal/class.h index 328d650e8b..db4ae2ada1 100644 --- a/internal/class.h +++ b/internal/class.h @@ -546,7 +546,7 @@ RCLASS_WRITABLE_ENSURE_FIELDS_OBJ(VALUE obj) RUBY_ASSERT(RB_TYPE_P(obj, RUBY_T_CLASS) || RB_TYPE_P(obj, RUBY_T_MODULE)); rb_classext_t *ext = RCLASS_EXT_WRITABLE(obj); if (!ext->fields_obj) { - RB_OBJ_WRITE(obj, &ext->fields_obj, rb_imemo_fields_new(rb_singleton_class(obj), 1)); + RB_OBJ_WRITE(obj, &ext->fields_obj, rb_imemo_fields_new(obj, 1)); } return ext->fields_obj; } diff --git a/internal/imemo.h b/internal/imemo.h index 5dd2d04fa4..8d1e42353d 100644 --- a/internal/imemo.h +++ b/internal/imemo.h @@ -273,12 +273,18 @@ struct rb_fields { #define OBJ_FIELD_EXTERNAL IMEMO_FL_USER0 #define IMEMO_OBJ_FIELDS(fields) ((struct rb_fields *)fields) -VALUE rb_imemo_fields_new(VALUE klass, size_t capa); -VALUE rb_imemo_fields_new_complex(VALUE klass, size_t capa); -VALUE rb_imemo_fields_new_complex_tbl(VALUE klass, st_table *tbl); +VALUE rb_imemo_fields_new(VALUE owner, size_t capa); +VALUE rb_imemo_fields_new_complex(VALUE owner, size_t capa); +VALUE rb_imemo_fields_new_complex_tbl(VALUE owner, st_table *tbl); VALUE rb_imemo_fields_clone(VALUE fields_obj); void rb_imemo_fields_clear(VALUE fields_obj); +static inline VALUE +rb_imemo_fields_owner(VALUE fields_obj) +{ + return CLASS_OF(fields_obj); +} + static inline VALUE * rb_imemo_fields_ptr(VALUE obj_fields) { diff --git a/shape.c b/shape.c index d6d05e12d5..df4faf960c 100644 --- a/shape.c +++ b/shape.c @@ -877,8 +877,17 @@ shape_get_next(rb_shape_t *shape, VALUE obj, ID id, bool emit_warnings) #endif VALUE klass; - if (IMEMO_TYPE_P(obj, imemo_fields)) { // HACK - klass = CLASS_OF(obj); + if (IMEMO_TYPE_P(obj, imemo_fields)) { + VALUE owner = rb_imemo_fields_owner(obj); + switch (BUILTIN_TYPE(owner)) { + case T_CLASS: + case T_MODULE: + klass = rb_singleton_class(owner); + break; + default: + klass = rb_obj_class(owner); + break; + } } else { klass = rb_obj_class(obj); diff --git a/test/ruby/test_shapes.rb b/test/ruby/test_shapes.rb index ed55b95c3e..77bba6421b 100644 --- a/test/ruby/test_shapes.rb +++ b/test/ruby/test_shapes.rb @@ -149,11 +149,14 @@ class TestShapes < Test::Unit::TestCase def test_too_many_ivs_on_class obj = Class.new - (MANY_IVS + 1).times do + obj.instance_variable_set(:@test_too_many_ivs_on_class, 1) + refute_predicate RubyVM::Shape.of(obj), :too_complex? + + MANY_IVS.times do obj.instance_variable_set(:"@a#{_1}", 1) end - assert_false RubyVM::Shape.of(obj).too_complex? + refute_predicate RubyVM::Shape.of(obj), :too_complex? end def test_removing_when_too_many_ivs_on_class diff --git a/variable.c b/variable.c index 7e5875519d..0c596d5c9e 100644 --- a/variable.c +++ b/variable.c @@ -1687,10 +1687,10 @@ imemo_fields_complex_from_obj_i(ID key, VALUE val, st_data_t arg) } static VALUE -imemo_fields_complex_from_obj(VALUE klass, VALUE source_fields_obj, shape_id_t shape_id) +imemo_fields_complex_from_obj(VALUE owner, VALUE source_fields_obj, shape_id_t shape_id) { attr_index_t len = source_fields_obj ? RSHAPE_LEN(RBASIC_SHAPE_ID(source_fields_obj)) : 0; - VALUE fields_obj = rb_imemo_fields_new_complex(klass, len + 1); + VALUE fields_obj = rb_imemo_fields_new_complex(owner, len + 1); rb_field_foreach(source_fields_obj, imemo_fields_complex_from_obj_i, (st_data_t)fields_obj, false); RBASIC_SET_SHAPE_ID(fields_obj, shape_id); @@ -1699,9 +1699,9 @@ imemo_fields_complex_from_obj(VALUE klass, VALUE source_fields_obj, shape_id_t s } static VALUE -imemo_fields_copy_capa(VALUE klass, VALUE source_fields_obj, attr_index_t new_size) +imemo_fields_copy_capa(VALUE owner, VALUE source_fields_obj, attr_index_t new_size) { - VALUE fields_obj = rb_imemo_fields_new(klass, new_size); + VALUE fields_obj = rb_imemo_fields_new(owner, new_size); if (source_fields_obj) { attr_index_t fields_count = RSHAPE_LEN(RBASIC_SHAPE_ID(source_fields_obj)); VALUE *fields = rb_imemo_fields_ptr(fields_obj); @@ -1853,7 +1853,7 @@ general_field_set(VALUE obj, shape_id_t target_shape_id, VALUE val, void *data, } static VALUE -imemo_fields_set(VALUE klass, VALUE fields_obj, shape_id_t target_shape_id, ID field_name, VALUE val, bool concurrent) +imemo_fields_set(VALUE owner, VALUE fields_obj, shape_id_t target_shape_id, ID field_name, VALUE val, bool concurrent) { const VALUE original_fields_obj = fields_obj; shape_id_t current_shape_id = fields_obj ? RBASIC_SHAPE_ID(fields_obj) : ROOT_SHAPE_ID; @@ -1868,7 +1868,7 @@ imemo_fields_set(VALUE klass, VALUE fields_obj, shape_id_t target_shape_id, ID f } } else { - fields_obj = imemo_fields_complex_from_obj(klass, original_fields_obj, target_shape_id); + fields_obj = imemo_fields_complex_from_obj(owner, original_fields_obj, target_shape_id); current_shape_id = target_shape_id; } @@ -1882,7 +1882,7 @@ imemo_fields_set(VALUE klass, VALUE fields_obj, shape_id_t target_shape_id, ID f else { attr_index_t index = RSHAPE_INDEX(target_shape_id); if (concurrent || index >= RSHAPE_CAPACITY(current_shape_id)) { - fields_obj = imemo_fields_copy_capa(klass, original_fields_obj, RSHAPE_CAPACITY(target_shape_id)); + fields_obj = imemo_fields_copy_capa(owner, original_fields_obj, RSHAPE_CAPACITY(target_shape_id)); } VALUE *table = rb_imemo_fields_ptr(fields_obj); @@ -1905,7 +1905,7 @@ generic_field_set(VALUE obj, shape_id_t target_shape_id, ID field_name, VALUE va } const VALUE original_fields_obj = rb_obj_fields(obj, field_name); - VALUE fields_obj = imemo_fields_set(rb_obj_class(obj), original_fields_obj, target_shape_id, field_name, val, false); + VALUE fields_obj = imemo_fields_set(obj, original_fields_obj, target_shape_id, field_name, val, false); rb_obj_set_fields(obj, fields_obj, field_name, original_fields_obj); } @@ -2340,7 +2340,7 @@ rb_copy_generic_ivar(VALUE dest, VALUE obj) return; } - new_fields_obj = rb_imemo_fields_new(rb_obj_class(dest), RSHAPE_CAPACITY(dest_shape_id)); + new_fields_obj = rb_imemo_fields_new(dest, RSHAPE_CAPACITY(dest_shape_id)); VALUE *src_buf = rb_imemo_fields_ptr(fields_obj); VALUE *dest_buf = rb_imemo_fields_ptr(new_fields_obj); rb_shape_copy_fields(new_fields_obj, dest_buf, dest_shape_id, src_buf, src_shape_id); @@ -4640,7 +4640,7 @@ class_fields_ivar_set(VALUE klass, VALUE fields_obj, ID id, VALUE val, bool conc { bool existing = true; const VALUE original_fields_obj = fields_obj; - fields_obj = original_fields_obj ? original_fields_obj : rb_imemo_fields_new(rb_singleton_class(klass), 1); + fields_obj = original_fields_obj ? original_fields_obj : rb_imemo_fields_new(klass, 1); shape_id_t current_shape_id = RBASIC_SHAPE_ID(fields_obj); shape_id_t next_shape_id = current_shape_id; @@ -4660,7 +4660,7 @@ class_fields_ivar_set(VALUE klass, VALUE fields_obj, ID id, VALUE val, bool conc next_shape_id = rb_shape_transition_add_ivar(fields_obj, id); if (UNLIKELY(rb_shape_too_complex_p(next_shape_id))) { - fields_obj = imemo_fields_complex_from_obj(rb_singleton_class(klass), fields_obj, next_shape_id); + fields_obj = imemo_fields_complex_from_obj(klass, fields_obj, next_shape_id); goto too_complex; } @@ -4670,7 +4670,7 @@ class_fields_ivar_set(VALUE klass, VALUE fields_obj, ID id, VALUE val, bool conc if (next_capacity > current_capacity) { // We allocate a new fields_obj even when concurrency isn't a concern // so that we're embedded as long as possible. - fields_obj = imemo_fields_copy_capa(rb_singleton_class(klass), fields_obj, next_capacity); + fields_obj = imemo_fields_copy_capa(klass, fields_obj, next_capacity); } RUBY_ASSERT(RSHAPE(next_shape_id)->type == SHAPE_IVAR); From 2083fa89fc29005035c1a098185c4b707686a437 Mon Sep 17 00:00:00 2001 From: Jean Boussier Date: Wed, 6 Aug 2025 17:39:24 +0200 Subject: [PATCH 7/8] Implement `gen_fields_tbl` cache There is a high likelyhood that `rb_obj_fields` is called consecutively for the same object. If we keep a cache of the last IMEMO/fields we interacted with, we can save having to lookup the `gen_fields_tbl`, synchronize the VM lock, etc. On yjit-bench's, I instrumented the hit rate of this cache at: - `shipit`: 38%, with 111k hits. - `lobsters`: 59%, with 367k hits. - `rubocop`: 100% with only 300 hits. I also ran a micro-benchmark which shows that ivar access is: - 1.25x faster when the cache is hit in single ractor mode. - 2x faster when the cache is hit in multi ractor mode. - 1.06x slower when the cache miss in single ractor mode. - 1.01x slower when the cache miss in multi ractor mode. ```yml prelude: | class GenIvar < Array def initialize(...) super @iv = 1 end attr_reader :iv end a = GenIvar.new b = GenIvar.new benchmark: hit: a.iv; a.iv; a.iv; a.iv; a.iv; a.iv; a.iv; a.iv; a.iv; a.iv; a.iv; a.iv; a.iv; a.iv; a.iv; a.iv; a.iv; a.iv; a.iv; a.iv; miss: a.iv; b.iv; a.iv; b.iv; a.iv; b.iv; a.iv; b.iv; a.iv; b.iv; a.iv; b.iv; a.iv; b.iv; a.iv; b.iv; a.iv; b.iv; a.iv; b.iv; ``` Single ractor: ``` compare-ruby: ruby 3.5.0dev (2025-08-12T02:14:57Z master 428937a536) +YJIT +PRISM [arm64-darwin24] built-ruby: ruby 3.5.0dev (2025-08-12T09:25:35Z gen-fields-cache 9456c35893) +YJIT +PRISM [arm64-darwin24] warming up.. | |compare-ruby|built-ruby| |:-----|-----------:|---------:| |hit | 4.090M| 5.121M| | | -| 1.25x| |miss | 3.756M| 3.534M| | | 1.06x| -| ``` Multi-ractor: ``` compare-ruby: ruby 3.5.0dev (2025-08-12T02:14:57Z master 428937a536) +YJIT +PRISM [arm64-darwin24] built-ruby: ruby 3.5.0dev (2025-08-12T09:25:35Z gen-fields-cache 9456c35893) +YJIT +PRISM [arm64-darwin24] warming up.. | |compare-ruby|built-ruby| |:-----|-----------:|---------:| |hit | 2.205M| 4.460M| | | -| 2.02x| |miss | 2.117M| 2.094M| | | 1.01x| -| ``` --- variable.c | 41 +++++++++++++++++++++++++++++++++-------- vm.c | 6 ++++++ vm_core.h | 5 +++++ 3 files changed, 44 insertions(+), 8 deletions(-) diff --git a/variable.c b/variable.c index 0c596d5c9e..4f0f83d203 100644 --- a/variable.c +++ b/variable.c @@ -1245,9 +1245,19 @@ rb_obj_fields(VALUE obj, ID field_name) goto generic_fields; default: generic_fields: - RB_VM_LOCKING() { - if (!st_lookup(generic_fields_tbl_, (st_data_t)obj, (st_data_t *)&fields_obj)) { - rb_bug("Object is missing entry in generic_fields_tbl"); + { + rb_execution_context_t *ec = GET_EC(); + if (ec->gen_fields_cache.obj == obj) { + fields_obj = ec->gen_fields_cache.fields_obj; + } + else { + RB_VM_LOCKING() { + if (!st_lookup(generic_fields_tbl_, (st_data_t)obj, (st_data_t *)&fields_obj)) { + rb_bug("Object is missing entry in generic_fields_tbl"); + } + } + ec->gen_fields_cache.fields_obj = fields_obj; + ec->gen_fields_cache.obj = obj; } } } @@ -1275,8 +1285,15 @@ rb_free_generic_ivar(VALUE obj) goto generic_fields; default: generic_fields: - RB_VM_LOCKING() { - st_delete(generic_fields_tbl_no_ractor_check(), &key, &value); + { + rb_execution_context_t *ec = GET_EC(); + if (ec->gen_fields_cache.obj == obj) { + ec->gen_fields_cache.obj = Qundef; + ec->gen_fields_cache.fields_obj = Qundef; + } + RB_VM_LOCKING() { + st_delete(generic_fields_tbl_no_ractor_check(), &key, &value); + } } } RBASIC_SET_SHAPE_ID(obj, ROOT_SHAPE_ID); @@ -1307,10 +1324,18 @@ rb_obj_set_fields(VALUE obj, VALUE fields_obj, ID field_name, VALUE original_fie goto generic_fields; default: generic_fields: - RB_VM_LOCKING() { - st_insert(generic_fields_tbl_, (st_data_t)obj, (st_data_t)fields_obj); + { + RB_VM_LOCKING() { + st_insert(generic_fields_tbl_, (st_data_t)obj, (st_data_t)fields_obj); + } + RB_OBJ_WRITTEN(obj, original_fields_obj, fields_obj); + + rb_execution_context_t *ec = GET_EC(); + if (ec->gen_fields_cache.fields_obj != fields_obj) { + ec->gen_fields_cache.obj = obj; + ec->gen_fields_cache.fields_obj = fields_obj; + } } - RB_OBJ_WRITTEN(obj, original_fields_obj, fields_obj); } if (original_fields_obj) { diff --git a/vm.c b/vm.c index 4223c2d2ac..479b3be94f 100644 --- a/vm.c +++ b/vm.c @@ -3441,6 +3441,9 @@ rb_execution_context_update(rb_execution_context_t *ec) } ec->storage = rb_gc_location(ec->storage); + + ec->gen_fields_cache.obj = rb_gc_location(ec->gen_fields_cache.obj); + ec->gen_fields_cache.fields_obj = rb_gc_location(ec->gen_fields_cache.fields_obj); } static enum rb_id_table_iterator_result @@ -3505,6 +3508,9 @@ rb_execution_context_mark(const rb_execution_context_t *ec) rb_gc_mark(ec->private_const_reference); rb_gc_mark_movable(ec->storage); + + rb_gc_mark_weak((VALUE *)&ec->gen_fields_cache.obj); + rb_gc_mark_weak((VALUE *)&ec->gen_fields_cache.fields_obj); } void rb_fiber_mark_self(rb_fiber_t *fib); diff --git a/vm_core.h b/vm_core.h index 569aebaba4..b5a04101ab 100644 --- a/vm_core.h +++ b/vm_core.h @@ -1070,6 +1070,11 @@ struct rb_execution_context_struct { VALUE private_const_reference; + struct { + VALUE obj; + VALUE fields_obj; + } gen_fields_cache; + /* for GC */ struct { VALUE *stack_start; From 943d9f828df365e0b650ee2d67faab702229877c Mon Sep 17 00:00:00 2001 From: Max Bernstein Date: Wed, 13 Aug 2025 12:18:59 -0400 Subject: [PATCH 8/8] ZJIT: Don't eliminate NewHash with operands Hashing and checking operands for equality is re-entrant. We could later optimize this to check for hash/eq methods on operands and eliminate if they don't have side effects, but this is fine for now. --- zjit/src/hir.rs | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/zjit/src/hir.rs b/zjit/src/hir.rs index f3d39281ad..8f681afc18 100644 --- a/zjit/src/hir.rs +++ b/zjit/src/hir.rs @@ -604,7 +604,9 @@ impl Insn { Insn::Param { .. } => false, Insn::StringCopy { .. } => false, Insn::NewArray { .. } => false, - Insn::NewHash { .. } => false, + // NewHash's operands may be hashed and compared for equality, which could have + // side-effects. + Insn::NewHash { elements, .. } => elements.len() > 0, Insn::NewRange { .. } => false, Insn::ArrayDup { .. } => false, Insn::HashDup { .. } => false, @@ -6089,7 +6091,7 @@ mod opt_tests { } #[test] - fn test_eliminate_new_hash_with_elements() { + fn test_no_eliminate_new_hash_with_elements() { eval(" def test(aval, bval) c = {a: aval, b: bval} @@ -6099,6 +6101,10 @@ mod opt_tests { assert_optimized_method_hir("test", expect![[r#" fn test@:3: bb0(v0:BasicObject, v1:BasicObject, v2:BasicObject): + v3:NilClass = Const Value(nil) + v5:StaticSymbol[:a] = Const Value(VALUE(0x1000)) + v6:StaticSymbol[:b] = Const Value(VALUE(0x1008)) + v8:HashExact = NewHash v5: v1, v6: v2 v9:Fixnum[5] = Const Value(5) Return v9 "#]]);