From c42c6c27c37c8336b219678f858dabd17442737d Mon Sep 17 00:00:00 2001 From: Stan Lo Date: Wed, 26 Nov 2025 18:55:31 +0000 Subject: [PATCH 1/9] ZJIT: Remove dead unnecessary_transmutes allow ``` warning: unknown lint: `unnecessary_transmutes` --> zjit/src/cruby.rs:107:9 | 107 | #[allow(unnecessary_transmutes)] // https://github.com/rust-lang/rust-bindgen/issues/2807 | ^^^^^^^^^^^^^^^^^^^^^^ | = note: `#[warn(unknown_lints)]` on by default ``` --- zjit/src/cruby.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/zjit/src/cruby.rs b/zjit/src/cruby.rs index 1a60e2a7fe2a06..9070cb38be5f42 100644 --- a/zjit/src/cruby.rs +++ b/zjit/src/cruby.rs @@ -104,7 +104,6 @@ pub type RedefinitionFlag = u32; #[allow(unsafe_op_in_unsafe_fn)] #[allow(dead_code)] -#[allow(unnecessary_transmutes)] // https://github.com/rust-lang/rust-bindgen/issues/2807 #[allow(clippy::all)] // warning meant to help with reading; not useful for generated code mod autogened { use super::*; From 5f55c9c8fb8f401537e7121171747196e66c3ba0 Mon Sep 17 00:00:00 2001 From: Randy Stauner Date: Tue, 25 Nov 2025 19:29:02 -0700 Subject: [PATCH 2/9] YJIT: Abort expandarray optimization if method_missing is defined Fixes: [Bug #21707] [AW: rewrote comments] Co-authored-by: Alan Wu --- bootstraptest/test_yjit.rb | 16 ++++++++++++++++ yjit/src/codegen.rs | 10 +++++++++- yjit/src/cruby.rs | 1 + yjit/src/stats.rs | 1 + 4 files changed, 27 insertions(+), 1 deletion(-) diff --git a/bootstraptest/test_yjit.rb b/bootstraptest/test_yjit.rb index f9cdca6f28c76a..bd44f3ac3ece0b 100644 --- a/bootstraptest/test_yjit.rb +++ b/bootstraptest/test_yjit.rb @@ -2680,6 +2680,22 @@ def expandarray_redefined_nilclass expandarray_redefined_nilclass } +assert_equal 'not_array', %q{ + def expandarray_not_array(obj) + a, = obj + a + end + + obj = Object.new + def obj.method_missing(m, *args, &block) + return [:not_array] if m == :to_ary + super + end + + expandarray_not_array(obj) + expandarray_not_array(obj) +} + assert_equal '[1, 2, nil]', %q{ def expandarray_rhs_too_small a, b, c = [1, 2] diff --git a/yjit/src/codegen.rs b/yjit/src/codegen.rs index 9eeccddf6ce490..a426ad07737496 100644 --- a/yjit/src/codegen.rs +++ b/yjit/src/codegen.rs @@ -2258,7 +2258,8 @@ fn gen_expandarray( let comptime_recv = jit.peek_at_stack(&asm.ctx, 0); - // If the comptime receiver is not an array + // If the comptime receiver is not an array, speculate for when the `rb_check_array_type()` + // conversion returns nil and without side-effects (e.g. arbitrary method calls). if !unsafe { RB_TYPE_P(comptime_recv, RUBY_T_ARRAY) } { // at compile time, ensure to_ary is not defined let target_cme = unsafe { rb_callable_method_entry_or_negative(comptime_recv.class_of(), ID!(to_ary)) }; @@ -2270,6 +2271,13 @@ fn gen_expandarray( return None; } + // Bail when method_missing is defined to avoid generating code to call it. + // Also, for simplicity, bail when BasicObject#method_missing has been removed. + if !assume_method_basic_definition(jit, asm, comptime_recv.class_of(), ID!(method_missing)) { + gen_counter_incr(jit, asm, Counter::expandarray_method_missing); + return None; + } + // invalidate compile block if to_ary is later defined jit.assume_method_lookup_stable(asm, target_cme); diff --git a/yjit/src/cruby.rs b/yjit/src/cruby.rs index 0d9e3b74dad874..cfaf48c3f0b68e 100644 --- a/yjit/src/cruby.rs +++ b/yjit/src/cruby.rs @@ -816,6 +816,7 @@ pub(crate) mod ids { def_ids! { name: NULL content: b"" name: respond_to_missing content: b"respond_to_missing?" + name: method_missing content: b"method_missing" name: to_ary content: b"to_ary" name: to_s content: b"to_s" name: eq content: b"==" diff --git a/yjit/src/stats.rs b/yjit/src/stats.rs index 817c842cf4144d..e8ce7b8b353d63 100644 --- a/yjit/src/stats.rs +++ b/yjit/src/stats.rs @@ -496,6 +496,7 @@ make_counters! { expandarray_postarg, expandarray_not_array, expandarray_to_ary, + expandarray_method_missing, expandarray_chain_max_depth, // getblockparam From 1660b8145c30f53771671dec343fa7025a953fb5 Mon Sep 17 00:00:00 2001 From: Luke Gruber Date: Wed, 26 Nov 2025 16:23:34 -0500 Subject: [PATCH 3/9] Eliminate redundant work and branching when marking T_OBJECT (#15274) --- gc.c | 6 ++++-- ractor.c | 2 +- shape.h | 20 ++++++++++++++++---- 3 files changed, 21 insertions(+), 7 deletions(-) diff --git a/gc.c b/gc.c index 26afb4e71817bc..4f6751316f07da 100644 --- a/gc.c +++ b/gc.c @@ -3227,19 +3227,21 @@ rb_gc_mark_children(void *objspace, VALUE obj) } case T_OBJECT: { + uint32_t len; if (rb_shape_obj_too_complex_p(obj)) { gc_mark_tbl_no_pin(ROBJECT_FIELDS_HASH(obj)); + len = ROBJECT_FIELDS_COUNT_COMPLEX(obj); } else { const VALUE * const ptr = ROBJECT_FIELDS(obj); - uint32_t len = ROBJECT_FIELDS_COUNT(obj); + len = ROBJECT_FIELDS_COUNT_NOT_COMPLEX(obj); for (uint32_t i = 0; i < len; i++) { gc_mark_internal(ptr[i]); } } - attr_index_t fields_count = ROBJECT_FIELDS_COUNT(obj); + attr_index_t fields_count = (attr_index_t)len; if (fields_count) { VALUE klass = RBASIC_CLASS(obj); diff --git a/ractor.c b/ractor.c index 3fc507128c1939..41c32b79def68c 100644 --- a/ractor.c +++ b/ractor.c @@ -1796,7 +1796,7 @@ obj_traverse_replace_i(VALUE obj, struct obj_traverse_replace_data *data) if (d.stop) return 1; } else { - uint32_t len = ROBJECT_FIELDS_COUNT(obj); + uint32_t len = ROBJECT_FIELDS_COUNT_NOT_COMPLEX(obj); VALUE *ptr = ROBJECT_FIELDS(obj); for (uint32_t i = 0; i < len; i++) { diff --git a/shape.h b/shape.h index 6e4a1f079b44f3..ec5c25b32f16f5 100644 --- a/shape.h +++ b/shape.h @@ -367,16 +367,28 @@ ROBJECT_SET_FIELDS_HASH(VALUE obj, const st_table *tbl) ROBJECT(obj)->as.heap.fields = (VALUE *)tbl; } +static inline uint32_t +ROBJECT_FIELDS_COUNT_COMPLEX(VALUE obj) +{ + return (uint32_t)rb_st_table_size(ROBJECT_FIELDS_HASH(obj)); +} + +static inline uint32_t +ROBJECT_FIELDS_COUNT_NOT_COMPLEX(VALUE obj) +{ + RBIMPL_ASSERT_TYPE(obj, RUBY_T_OBJECT); + RUBY_ASSERT(!rb_shape_obj_too_complex_p(obj)); + return RSHAPE(RBASIC_SHAPE_ID(obj))->next_field_index; +} + static inline uint32_t ROBJECT_FIELDS_COUNT(VALUE obj) { if (rb_shape_obj_too_complex_p(obj)) { - return (uint32_t)rb_st_table_size(ROBJECT_FIELDS_HASH(obj)); + return ROBJECT_FIELDS_COUNT_COMPLEX(obj); } else { - RBIMPL_ASSERT_TYPE(obj, RUBY_T_OBJECT); - RUBY_ASSERT(!rb_shape_obj_too_complex_p(obj)); - return RSHAPE(RBASIC_SHAPE_ID(obj))->next_field_index; + return ROBJECT_FIELDS_COUNT_NOT_COMPLEX(obj); } } From bee02c41bc15780a45f47986ef638e17ca323ec3 Mon Sep 17 00:00:00 2001 From: Luke Gruber Date: Wed, 26 Nov 2025 17:14:55 -0500 Subject: [PATCH 4/9] Fix a ractor barrier issue during VM cleanup. (#15091) --- ractor.c | 21 +++++++++++++++------ 1 file changed, 15 insertions(+), 6 deletions(-) diff --git a/ractor.c b/ractor.c index 41c32b79def68c..8238d9a456e233 100644 --- a/ractor.c +++ b/ractor.c @@ -834,13 +834,11 @@ rb_ractor_terminate_all(void) VM_ASSERT(cr == GET_RACTOR()); // only main-ractor's main-thread should kick it. - if (vm->ractor.cnt > 1) { - RB_VM_LOCK(); - { - ractor_terminal_interrupt_all(vm); // kill all ractors - } - RB_VM_UNLOCK(); + RB_VM_LOCK(); + { + ractor_terminal_interrupt_all(vm); // kill all ractors } + RB_VM_UNLOCK(); rb_thread_terminate_all(GET_THREAD()); // kill other threads in main-ractor and wait RB_VM_LOCK(); @@ -853,6 +851,17 @@ rb_ractor_terminate_all(void) rb_vm_ractor_blocking_cnt_inc(vm, cr, __FILE__, __LINE__); rb_del_running_thread(rb_ec_thread_ptr(cr->threads.running_ec)); rb_vm_cond_timedwait(vm, &vm->ractor.sync.terminate_cond, 1000 /* ms */); +#ifdef RUBY_THREAD_PTHREAD_H + while (vm->ractor.sched.barrier_waiting) { + // A barrier is waiting. Threads relinquish the VM lock before joining the barrier and + // since we just acquired the VM lock back, we're blocking other threads from joining it. + // We loop until the barrier is over. We can't join this barrier because our thread isn't added to + // running_threads until the call below to `rb_add_running_thread`. + RB_VM_UNLOCK(); + unsigned int lev; + RB_VM_LOCK_ENTER_LEV_NB(&lev); + } +#endif rb_add_running_thread(rb_ec_thread_ptr(cr->threads.running_ec)); rb_vm_ractor_blocking_cnt_dec(vm, cr, __FILE__, __LINE__); From 52426a22de8ce5e2f02ce4169a1b6944e2165b46 Mon Sep 17 00:00:00 2001 From: Nobuyoshi Nakada Date: Thu, 27 Nov 2025 08:10:56 +0900 Subject: [PATCH 5/9] Remove an excess colon [ci skip] --- defs/gmake.mk | 2 +- template/Makefile.in | 2 +- win32/Makefile.sub | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/defs/gmake.mk b/defs/gmake.mk index da98f70d9a7cb7..36d65c0ea0b4c8 100644 --- a/defs/gmake.mk +++ b/defs/gmake.mk @@ -275,7 +275,7 @@ pull-github: fetch-github $(call pull-github,$(PR)) define pull-github - $(eval GITHUB_MERGE_BASE := $(shell $(GIT_LOG_FORMAT):%H -1) + $(eval GITHUB_MERGE_BASE := $(shell $(GIT_LOG_FORMAT)%H -1) $(eval GITHUB_MERGE_BRANCH := $(shell $(GIT_IN_SRC) symbolic-ref --short HEAD)) $(eval GITHUB_MERGE_WORKTREE := $(shell mktemp -d "$(srcdir)/gh-$(1)-XXXXXX")) $(GIT_IN_SRC) worktree prune diff --git a/template/Makefile.in b/template/Makefile.in index 2987c86488cf2f..7bc40b9d019604 100644 --- a/template/Makefile.in +++ b/template/Makefile.in @@ -748,4 +748,4 @@ yes-test-syntax-suggest: $(PREPARE_SYNTAX_SUGGEST) no-test-syntax-suggest: yesterday: - $(GIT_IN_SRC) reset --hard `TZ=UTC-9 $(GIT_LOG_FORMAT):%H -1 --before=00:00` + $(GIT_IN_SRC) reset --hard `TZ=UTC-9 $(GIT_LOG_FORMAT)%H -1 --before=00:00` diff --git a/win32/Makefile.sub b/win32/Makefile.sub index 04ec2873275dd5..edbe2a340209d3 100644 --- a/win32/Makefile.sub +++ b/win32/Makefile.sub @@ -1393,5 +1393,5 @@ exts: rubyspec-capiext yesterday: (set TZ=UTC-9) && \ for /f "usebackq" %H in \ - (`$(GIT_LOG_FORMAT):%H -1 "--before=00:00"`) do \ + (`$(GIT_LOG_FORMAT)%H -1 "--before=00:00"`) do \ $(GIT_IN_SRC) reset --hard %%H From db94a79da432bcdb9d48517733f11ccf03c7cd5d Mon Sep 17 00:00:00 2001 From: Max Bernstein Date: Wed, 26 Nov 2025 15:36:00 -0800 Subject: [PATCH 6/9] ZJIT: Count fallback reasons for set/get/definedivar (#15324) lobsters: ``` Top-4 setivar fallback reasons (100.0% of total 7,789,008): shape_transition: 6,074,085 (78.0%) not_monomorphic: 1,484,013 (19.1%) not_t_object: 172,629 ( 2.2%) too_complex: 58,281 ( 0.7%) Top-3 getivar fallback reasons (100.0% of total 9,348,832): not_t_object: 4,658,833 (49.8%) not_monomorphic: 4,542,316 (48.6%) too_complex: 147,683 ( 1.6%) Top-3 definedivar fallback reasons (100.0% of total 366,383): not_monomorphic: 361,389 (98.6%) too_complex: 3,062 ( 0.8%) not_t_object: 1,932 ( 0.5%) ``` railsbench: ``` Top-3 setivar fallback reasons (100.0% of total 15,119,057): shape_transition: 13,760,763 (91.0%) not_monomorphic: 982,368 ( 6.5%) not_t_object: 375,926 ( 2.5%) Top-2 getivar fallback reasons (100.0% of total 14,438,747): not_t_object: 7,643,870 (52.9%) not_monomorphic: 6,794,877 (47.1%) Top-2 definedivar fallback reasons (100.0% of total 209,613): not_monomorphic: 209,526 (100.0%) not_t_object: 87 ( 0.0%) ``` shipit: ``` Top-3 setivar fallback reasons (100.0% of total 14,516,254): shape_transition: 8,613,512 (59.3%) not_monomorphic: 5,761,398 (39.7%) not_t_object: 141,344 ( 1.0%) Top-2 getivar fallback reasons (100.0% of total 21,016,444): not_monomorphic: 11,313,482 (53.8%) not_t_object: 9,702,962 (46.2%) Top-2 definedivar fallback reasons (100.0% of total 290,382): not_monomorphic: 287,755 (99.1%) not_t_object: 2,627 ( 0.9%) ``` --- zjit.rb | 9 ++-- zjit/src/codegen.rs | 2 - zjit/src/hir.rs | 14 ++++++ zjit/src/hir/opt_tests.rs | 15 +++++- zjit/src/stats.rs | 99 +++++++++++++++++++++++++++++++++++++-- 5 files changed, 128 insertions(+), 11 deletions(-) diff --git a/zjit.rb b/zjit.rb index fc306c19a47fba..d128adead6fe39 100644 --- a/zjit.rb +++ b/zjit.rb @@ -183,6 +183,9 @@ def stats_string print_counters_with_prefix(prefix: 'unspecialized_send_without_block_def_type_', prompt: 'not optimized method types for send_without_block', buf:, stats:, limit: 20) print_counters_with_prefix(prefix: 'uncategorized_fallback_yarv_insn_', prompt: 'instructions with uncategorized fallback reason', buf:, stats:, limit: 20) print_counters_with_prefix(prefix: 'send_fallback_', prompt: 'send fallback reasons', buf:, stats:, limit: 20) + print_counters_with_prefix(prefix: 'setivar_fallback_', prompt: 'setivar fallback reasons', buf:, stats:, limit: 5) + print_counters_with_prefix(prefix: 'getivar_fallback_', prompt: 'getivar fallback reasons', buf:, stats:, limit: 5) + print_counters_with_prefix(prefix: 'definedivar_fallback_', prompt: 'definedivar fallback reasons', buf:, stats:, limit: 5) print_counters_with_prefix(prefix: 'invokeblock_handler_', prompt: 'invokeblock handler', buf:, stats:, limit: 10) # Show most popular unsupported call features. Because each call can @@ -201,6 +204,9 @@ def stats_string :send_count, :dynamic_send_count, :optimized_send_count, + :dynamic_setivar_count, + :dynamic_getivar_count, + :dynamic_definedivar_count, :iseq_optimized_send_count, :inline_cfunc_optimized_send_count, :inline_iseq_optimized_send_count, @@ -208,9 +214,6 @@ def stats_string :variadic_cfunc_optimized_send_count, ], buf:, stats:, right_align: true, base: :send_count) print_counters([ - :dynamic_getivar_count, - :dynamic_setivar_count, - :compiled_iseq_count, :failed_iseq_count, diff --git a/zjit/src/codegen.rs b/zjit/src/codegen.rs index 3300219ccda07a..66436b2374b4ff 100644 --- a/zjit/src/codegen.rs +++ b/zjit/src/codegen.rs @@ -892,7 +892,6 @@ fn gen_ccall_variadic( /// Emit an uncached instance variable lookup fn gen_getivar(jit: &mut JITState, asm: &mut Assembler, recv: Opnd, id: ID, ic: *const iseq_inline_iv_cache_entry) -> Opnd { - gen_incr_counter(asm, Counter::dynamic_getivar_count); if ic.is_null() { asm_ccall!(asm, rb_ivar_get, recv, id.0.into()) } else { @@ -903,7 +902,6 @@ fn gen_getivar(jit: &mut JITState, asm: &mut Assembler, recv: Opnd, id: ID, ic: /// Emit an uncached instance variable store fn gen_setivar(jit: &mut JITState, asm: &mut Assembler, recv: Opnd, id: ID, ic: *const iseq_inline_iv_cache_entry, val: Opnd, state: &FrameState) { - gen_incr_counter(asm, Counter::dynamic_setivar_count); // Setting an ivar can raise FrozenError, so we need proper frame state for exception handling. gen_prepare_non_leaf_call(jit, asm, state); if ic.is_null() { diff --git a/zjit/src/hir.rs b/zjit/src/hir.rs index b7149ad14c66fe..fbc9d80700dcaf 100644 --- a/zjit/src/hir.rs +++ b/zjit/src/hir.rs @@ -2806,19 +2806,23 @@ impl Function { let frame_state = self.frame_state(state); let Some(recv_type) = self.profiled_type_of_at(self_val, frame_state.insn_idx) else { // No (monomorphic/skewed polymorphic) profile info + self.push_insn(block, Insn::IncrCounter(Counter::getivar_fallback_not_monomorphic)); self.push_insn_id(block, insn_id); continue; }; if recv_type.flags().is_immediate() { // Instance variable lookups on immediate values are always nil + self.push_insn(block, Insn::IncrCounter(Counter::getivar_fallback_immediate)); self.push_insn_id(block, insn_id); continue; } assert!(recv_type.shape().is_valid()); if !recv_type.flags().is_t_object() { // Check if the receiver is a T_OBJECT + self.push_insn(block, Insn::IncrCounter(Counter::getivar_fallback_not_t_object)); self.push_insn_id(block, insn_id); continue; } if recv_type.shape().is_too_complex() { // too-complex shapes can't use index access + self.push_insn(block, Insn::IncrCounter(Counter::getivar_fallback_too_complex)); self.push_insn_id(block, insn_id); continue; } let self_val = self.push_insn(block, Insn::GuardType { val: self_val, guard_type: types::HeapBasicObject, state }); @@ -2845,19 +2849,23 @@ impl Function { let frame_state = self.frame_state(state); let Some(recv_type) = self.profiled_type_of_at(self_val, frame_state.insn_idx) else { // No (monomorphic/skewed polymorphic) profile info + self.push_insn(block, Insn::IncrCounter(Counter::definedivar_fallback_not_monomorphic)); self.push_insn_id(block, insn_id); continue; }; if recv_type.flags().is_immediate() { // Instance variable lookups on immediate values are always nil + self.push_insn(block, Insn::IncrCounter(Counter::definedivar_fallback_immediate)); self.push_insn_id(block, insn_id); continue; } assert!(recv_type.shape().is_valid()); if !recv_type.flags().is_t_object() { // Check if the receiver is a T_OBJECT + self.push_insn(block, Insn::IncrCounter(Counter::definedivar_fallback_not_t_object)); self.push_insn_id(block, insn_id); continue; } if recv_type.shape().is_too_complex() { // too-complex shapes can't use index access + self.push_insn(block, Insn::IncrCounter(Counter::definedivar_fallback_too_complex)); self.push_insn_id(block, insn_id); continue; } let self_val = self.push_insn(block, Insn::GuardType { val: self_val, guard_type: types::HeapBasicObject, state }); @@ -2877,28 +2885,34 @@ impl Function { let frame_state = self.frame_state(state); let Some(recv_type) = self.profiled_type_of_at(self_val, frame_state.insn_idx) else { // No (monomorphic/skewed polymorphic) profile info + self.push_insn(block, Insn::IncrCounter(Counter::setivar_fallback_not_monomorphic)); self.push_insn_id(block, insn_id); continue; }; if recv_type.flags().is_immediate() { // Instance variable lookups on immediate values are always nil + self.push_insn(block, Insn::IncrCounter(Counter::setivar_fallback_immediate)); self.push_insn_id(block, insn_id); continue; } assert!(recv_type.shape().is_valid()); if !recv_type.flags().is_t_object() { // Check if the receiver is a T_OBJECT + self.push_insn(block, Insn::IncrCounter(Counter::setivar_fallback_not_t_object)); self.push_insn_id(block, insn_id); continue; } if recv_type.shape().is_too_complex() { // too-complex shapes can't use index access + self.push_insn(block, Insn::IncrCounter(Counter::setivar_fallback_too_complex)); self.push_insn_id(block, insn_id); continue; } if recv_type.shape().is_frozen() { // Can't set ivars on frozen objects + self.push_insn(block, Insn::IncrCounter(Counter::setivar_fallback_frozen)); self.push_insn_id(block, insn_id); continue; } let mut ivar_index: u16 = 0; if !unsafe { rb_shape_get_iv_index(recv_type.shape().0, id, &mut ivar_index) } { // TODO(max): Shape transition + self.push_insn(block, Insn::IncrCounter(Counter::setivar_fallback_shape_transition)); self.push_insn_id(block, insn_id); continue; } let self_val = self.push_insn(block, Insn::GuardType { val: self_val, guard_type: types::HeapBasicObject, state }); diff --git a/zjit/src/hir/opt_tests.rs b/zjit/src/hir/opt_tests.rs index c460942fc13e42..b32da5a9ebb8e5 100644 --- a/zjit/src/hir/opt_tests.rs +++ b/zjit/src/hir/opt_tests.rs @@ -3343,6 +3343,7 @@ mod hir_opt_tests { Jump bb2(v4) bb2(v6:BasicObject): PatchPoint SingleRactorMode + IncrCounter getivar_fallback_not_monomorphic v11:BasicObject = GetIvar v6, :@foo CheckInterrupts Return v11 @@ -3366,6 +3367,7 @@ mod hir_opt_tests { bb2(v6:BasicObject): v10:Fixnum[1] = Const Value(1) PatchPoint SingleRactorMode + IncrCounter setivar_fallback_not_monomorphic SetIvar v6, :@foo, v10 CheckInterrupts Return v10 @@ -3442,6 +3444,7 @@ mod hir_opt_tests { EntryPoint JIT(0) Jump bb2(v4) bb2(v6:BasicObject): + IncrCounter definedivar_fallback_not_t_object v10:StringExact|NilClass = DefinedIvar v6, :@a CheckInterrupts Return v10 @@ -3473,6 +3476,7 @@ mod hir_opt_tests { EntryPoint JIT(0) Jump bb2(v4) bb2(v6:BasicObject): + IncrCounter definedivar_fallback_not_monomorphic v10:StringExact|NilClass = DefinedIvar v6, :@a CheckInterrupts Return v10 @@ -3525,6 +3529,7 @@ mod hir_opt_tests { bb2(v6:BasicObject): v10:Fixnum[5] = Const Value(5) PatchPoint SingleRactorMode + IncrCounter setivar_fallback_shape_transition SetIvar v6, :@foo, v10 CheckInterrupts Return v10 @@ -3554,6 +3559,7 @@ mod hir_opt_tests { bb2(v6:BasicObject): v10:Fixnum[5] = Const Value(5) PatchPoint SingleRactorMode + IncrCounter setivar_fallback_not_t_object SetIvar v6, :@a, v10 CheckInterrupts Return v10 @@ -3587,6 +3593,7 @@ mod hir_opt_tests { bb2(v6:BasicObject): v10:Fixnum[5] = Const Value(5) PatchPoint SingleRactorMode + IncrCounter setivar_fallback_not_monomorphic SetIvar v6, :@a, v10 CheckInterrupts Return v10 @@ -3618,6 +3625,7 @@ mod hir_opt_tests { bb2(v6:BasicObject): v10:Fixnum[5] = Const Value(5) PatchPoint SingleRactorMode + IncrCounter setivar_fallback_shape_transition SetIvar v6, :@a, v10 CheckInterrupts Return v10 @@ -5527,6 +5535,7 @@ mod hir_opt_tests { v16:Fixnum[5] = Const Value(5) PatchPoint MethodRedefined(C@0x1000, foo=@0x1008, cme:0x1010) v26:HeapObject[class_exact:C] = GuardType v9, HeapObject[class_exact:C] + IncrCounter setivar_fallback_shape_transition SetIvar v26, :@foo, v16 CheckInterrupts Return v16 @@ -5558,6 +5567,7 @@ mod hir_opt_tests { v16:Fixnum[5] = Const Value(5) PatchPoint MethodRedefined(C@0x1000, foo=@0x1008, cme:0x1010) v26:HeapObject[class_exact:C] = GuardType v9, HeapObject[class_exact:C] + IncrCounter setivar_fallback_shape_transition SetIvar v26, :@foo, v16 CheckInterrupts Return v16 @@ -8609,17 +8619,18 @@ mod hir_opt_tests { SetLocal l0, EP@3, v27 v39:BasicObject = GetLocal l0, EP@3 PatchPoint SingleRactorMode + IncrCounter setivar_fallback_shape_transition SetIvar v14, :@formatted, v39 v45:Class[VMFrozenCore] = Const Value(VALUE(0x1000)) PatchPoint MethodRedefined(Class@0x1008, lambda@0x1010, cme:0x1018) PatchPoint NoSingletonClass(Class@0x1008) - v59:BasicObject = CCallWithFrame RubyVM::FrozenCore.lambda@0x1040, v45, block=0x1048 + v60:BasicObject = CCallWithFrame RubyVM::FrozenCore.lambda@0x1040, v45, block=0x1048 v48:BasicObject = GetLocal l0, EP@6 v49:BasicObject = GetLocal l0, EP@5 v50:BasicObject = GetLocal l0, EP@4 v51:BasicObject = GetLocal l0, EP@3 CheckInterrupts - Return v59 + Return v60 "); } } diff --git a/zjit/src/stats.rs b/zjit/src/stats.rs index 1277db5b7e9e7d..890d92bc569a28 100644 --- a/zjit/src/stats.rs +++ b/zjit/src/stats.rs @@ -24,6 +24,15 @@ macro_rules! make_counters { optimized_send { $($optimized_send_counter_name:ident,)+ } + dynamic_setivar { + $($dynamic_setivar_counter_name:ident,)+ + } + dynamic_getivar { + $($dynamic_getivar_counter_name:ident,)+ + } + dynamic_definedivar { + $($dynamic_definedivar_counter_name:ident,)+ + } $($counter_name:ident,)+ ) => { /// Struct containing the counter values @@ -33,6 +42,9 @@ macro_rules! make_counters { $(pub $exit_counter_name: u64,)+ $(pub $dynamic_send_counter_name: u64,)+ $(pub $optimized_send_counter_name: u64,)+ + $(pub $dynamic_setivar_counter_name: u64,)+ + $(pub $dynamic_getivar_counter_name: u64,)+ + $(pub $dynamic_definedivar_counter_name: u64,)+ $(pub $counter_name: u64,)+ } @@ -44,6 +56,9 @@ macro_rules! make_counters { $($exit_counter_name,)+ $($dynamic_send_counter_name,)+ $($optimized_send_counter_name,)+ + $($dynamic_setivar_counter_name,)+ + $($dynamic_getivar_counter_name,)+ + $($dynamic_definedivar_counter_name,)+ $($counter_name,)+ } @@ -54,6 +69,9 @@ macro_rules! make_counters { $( Counter::$exit_counter_name => stringify!($exit_counter_name), )+ $( Counter::$dynamic_send_counter_name => stringify!($dynamic_send_counter_name), )+ $( Counter::$optimized_send_counter_name => stringify!($optimized_send_counter_name), )+ + $( Counter::$dynamic_setivar_counter_name => stringify!($dynamic_setivar_counter_name), )+ + $( Counter::$dynamic_getivar_counter_name => stringify!($dynamic_getivar_counter_name), )+ + $( Counter::$dynamic_definedivar_counter_name => stringify!($dynamic_definedivar_counter_name), )+ $( Counter::$counter_name => stringify!($counter_name), )+ } } @@ -64,6 +82,9 @@ macro_rules! make_counters { $( stringify!($exit_counter_name) => Some(Counter::$exit_counter_name), )+ $( stringify!($dynamic_send_counter_name) => Some(Counter::$dynamic_send_counter_name), )+ $( stringify!($optimized_send_counter_name) => Some(Counter::$optimized_send_counter_name), )+ + $( stringify!($dynamic_setivar_counter_name) => Some(Counter::$dynamic_setivar_counter_name), )+ + $( stringify!($dynamic_getivar_counter_name) => Some(Counter::$dynamic_getivar_counter_name), )+ + $( stringify!($dynamic_definedivar_counter_name) => Some(Counter::$dynamic_definedivar_counter_name), )+ $( stringify!($counter_name) => Some(Counter::$counter_name), )+ _ => None, } @@ -77,6 +98,9 @@ macro_rules! make_counters { $( Counter::$default_counter_name => std::ptr::addr_of_mut!(counters.$default_counter_name), )+ $( Counter::$exit_counter_name => std::ptr::addr_of_mut!(counters.$exit_counter_name), )+ $( Counter::$dynamic_send_counter_name => std::ptr::addr_of_mut!(counters.$dynamic_send_counter_name), )+ + $( Counter::$dynamic_setivar_counter_name => std::ptr::addr_of_mut!(counters.$dynamic_setivar_counter_name), )+ + $( Counter::$dynamic_getivar_counter_name => std::ptr::addr_of_mut!(counters.$dynamic_getivar_counter_name), )+ + $( Counter::$dynamic_definedivar_counter_name => std::ptr::addr_of_mut!(counters.$dynamic_definedivar_counter_name), )+ $( Counter::$optimized_send_counter_name => std::ptr::addr_of_mut!(counters.$optimized_send_counter_name), )+ $( Counter::$counter_name => std::ptr::addr_of_mut!(counters.$counter_name), )+ } @@ -103,6 +127,21 @@ macro_rules! make_counters { $( Counter::$optimized_send_counter_name, )+ ]; + /// List of other counters that are summed as dynamic_setivar_count. + pub const DYNAMIC_SETIVAR_COUNTERS: &'static [Counter] = &[ + $( Counter::$dynamic_setivar_counter_name, )+ + ]; + + /// List of other counters that are summed as dynamic_getivar_count. + pub const DYNAMIC_GETIVAR_COUNTERS: &'static [Counter] = &[ + $( Counter::$dynamic_getivar_counter_name, )+ + ]; + + /// List of other counters that are summed as dynamic_definedivar_count. + pub const DYNAMIC_DEFINEDIVAR_COUNTERS: &'static [Counter] = &[ + $( Counter::$dynamic_definedivar_counter_name, )+ + ]; + /// List of other counters that are available only for --zjit-stats. pub const OTHER_COUNTERS: &'static [Counter] = &[ $( Counter::$counter_name, )+ @@ -207,6 +246,35 @@ make_counters! { variadic_cfunc_optimized_send_count, } + // Ivar fallback counters that are summed as dynamic_setivar_count + dynamic_setivar { + // setivar_fallback_: Fallback reasons for dynamic setivar instructions + setivar_fallback_not_monomorphic, + setivar_fallback_immediate, + setivar_fallback_not_t_object, + setivar_fallback_too_complex, + setivar_fallback_frozen, + setivar_fallback_shape_transition, + } + + // Ivar fallback counters that are summed as dynamic_getivar_count + dynamic_getivar { + // getivar_fallback_: Fallback reasons for dynamic getivar instructions + getivar_fallback_not_monomorphic, + getivar_fallback_immediate, + getivar_fallback_not_t_object, + getivar_fallback_too_complex, + } + + // Ivar fallback counters that are summed as dynamic_definedivar_count + dynamic_definedivar { + // definedivar_fallback_: Fallback reasons for dynamic definedivar instructions + definedivar_fallback_not_monomorphic, + definedivar_fallback_immediate, + definedivar_fallback_not_t_object, + definedivar_fallback_too_complex, + } + // compile_error_: Compile error reasons compile_error_iseq_stack_too_large, compile_error_exception_handler, @@ -236,10 +304,6 @@ make_counters! { // The number of times YARV instructions are executed on JIT code zjit_insn_count, - // The number of times we do a dynamic ivar lookup from JIT code - dynamic_getivar_count, - dynamic_setivar_count, - // Method call def_type related to send without block fallback to dynamic dispatch unspecialized_send_without_block_def_type_iseq, unspecialized_send_without_block_def_type_cfunc, @@ -657,6 +721,33 @@ pub extern "C" fn rb_zjit_stats(_ec: EcPtr, _self: VALUE, target_key: VALUE) -> set_stat_usize!(hash, "optimized_send_count", optimized_send_count); set_stat_usize!(hash, "send_count", dynamic_send_count + optimized_send_count); + // Set send fallback counters for each setivar fallback reason + let mut dynamic_setivar_count = 0; + for &counter in DYNAMIC_SETIVAR_COUNTERS { + let count = unsafe { *counter_ptr(counter) }; + dynamic_setivar_count += count; + set_stat_usize!(hash, &counter.name(), count); + } + set_stat_usize!(hash, "dynamic_setivar_count", dynamic_setivar_count); + + // Set send fallback counters for each getivar fallback reason + let mut dynamic_getivar_count = 0; + for &counter in DYNAMIC_GETIVAR_COUNTERS { + let count = unsafe { *counter_ptr(counter) }; + dynamic_getivar_count += count; + set_stat_usize!(hash, &counter.name(), count); + } + set_stat_usize!(hash, "dynamic_getivar_count", dynamic_getivar_count); + + // Set send fallback counters for each definedivar fallback reason + let mut dynamic_definedivar_count = 0; + for &counter in DYNAMIC_DEFINEDIVAR_COUNTERS { + let count = unsafe { *counter_ptr(counter) }; + dynamic_definedivar_count += count; + set_stat_usize!(hash, &counter.name(), count); + } + set_stat_usize!(hash, "dynamic_definedivar_count", dynamic_definedivar_count); + // Set send fallback counters for Uncategorized let send_fallback_counters = ZJITState::get_send_fallback_counters(); for (op_idx, count) in send_fallback_counters.iter().enumerate().take(VM_INSTRUCTION_SIZE as usize) { From 970b18e9a94ff3e6496fb7324cff0798ffec6f24 Mon Sep 17 00:00:00 2001 From: John Hawthorn Date: Wed, 26 Nov 2025 13:33:10 -0800 Subject: [PATCH 7/9] Clear fields obj when removing This fixes a bug where the gen_fields_cache could become invalid when the last ivar was removed. Also adds more assertions. --- test/ruby/test_variable.rb | 14 ++++++++++++++ variable.c | 21 ++++++++++++++++----- 2 files changed, 30 insertions(+), 5 deletions(-) diff --git a/test/ruby/test_variable.rb b/test/ruby/test_variable.rb index 68434e0b6c479b..d4ba0f9fc74b6c 100644 --- a/test/ruby/test_variable.rb +++ b/test/ruby/test_variable.rb @@ -515,6 +515,20 @@ def test_genivar_cache assert_equal 4, instance.instance_variable_get(:@a4), bug21547 end + def test_genivar_cache_free + str = +"hello" + str.instance_variable_set(:@x, :old_value) + + str.instance_variable_get(:@x) # populate cache + + Fiber.new { + str.remove_instance_variable(:@x) + str.instance_variable_set(:@x, :new_value) + }.resume + + assert_equal :new_value, str.instance_variable_get(:@x) + end + private def with_kwargs_11(v1:, v2:, v3:, v4:, v5:, v6:, v7:, v8:, v9:, v10:, v11:) local_variables diff --git a/variable.c b/variable.c index d7e04265c4276b..ae122136eeb7ad 100644 --- a/variable.c +++ b/variable.c @@ -1236,6 +1236,18 @@ rb_mark_generic_ivar(VALUE obj) } } +VALUE +rb_obj_fields_generic_uncached(VALUE obj) +{ + VALUE fields_obj = 0; + 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"); + } + } + return fields_obj; +} + VALUE rb_obj_fields(VALUE obj, ID field_name) { @@ -1263,13 +1275,10 @@ rb_obj_fields(VALUE obj, ID field_name) rb_execution_context_t *ec = GET_EC(); if (ec->gen_fields_cache.obj == obj && rb_imemo_fields_owner(ec->gen_fields_cache.fields_obj) == obj) { fields_obj = ec->gen_fields_cache.fields_obj; + RUBY_ASSERT(fields_obj == rb_obj_fields_generic_uncached(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"); - } - } + fields_obj = rb_obj_fields_generic_uncached(obj); ec->gen_fields_cache.fields_obj = fields_obj; ec->gen_fields_cache.obj = obj; } @@ -1320,7 +1329,9 @@ rb_obj_set_fields(VALUE obj, VALUE fields_obj, ID field_name, VALUE original_fie ivar_ractor_check(obj, field_name); if (!fields_obj) { + RUBY_ASSERT(original_fields_obj); rb_free_generic_ivar(obj); + rb_imemo_fields_clear(original_fields_obj); return; } From 5bef7577b3b336a709935dd39e69abcf397c4684 Mon Sep 17 00:00:00 2001 From: John Hawthorn Date: Wed, 26 Nov 2025 14:02:17 -0800 Subject: [PATCH 8/9] Ensure we don't dereference fields_obj as Qundef We rely on the GC to clear this when the GC is run on another EC than the cache. --- test/ruby/test_variable.rb | 16 ++++++++++++++++ variable.c | 4 +++- 2 files changed, 19 insertions(+), 1 deletion(-) diff --git a/test/ruby/test_variable.rb b/test/ruby/test_variable.rb index d4ba0f9fc74b6c..13b8a7905f2b46 100644 --- a/test/ruby/test_variable.rb +++ b/test/ruby/test_variable.rb @@ -529,6 +529,22 @@ def test_genivar_cache_free assert_equal :new_value, str.instance_variable_get(:@x) end + def test_genivar_cache_invalidated_by_gc + str = +"hello" + str.instance_variable_set(:@x, :old_value) + + str.instance_variable_get(:@x) # populate cache + + Fiber.new { + str.remove_instance_variable(:@x) + str.instance_variable_set(:@x, :new_value) + }.resume + + GC.start + + assert_equal :new_value, str.instance_variable_get(:@x) + end + private def with_kwargs_11(v1:, v2:, v3:, v4:, v5:, v6:, v7:, v8:, v9:, v10:, v11:) local_variables diff --git a/variable.c b/variable.c index ae122136eeb7ad..f7ee56122024b7 100644 --- a/variable.c +++ b/variable.c @@ -1273,7 +1273,7 @@ rb_obj_fields(VALUE obj, ID field_name) generic_fields: { rb_execution_context_t *ec = GET_EC(); - if (ec->gen_fields_cache.obj == obj && rb_imemo_fields_owner(ec->gen_fields_cache.fields_obj) == obj) { + if (ec->gen_fields_cache.obj == obj && !UNDEF_P(ec->gen_fields_cache.fields_obj) && rb_imemo_fields_owner(ec->gen_fields_cache.fields_obj) == obj) { fields_obj = ec->gen_fields_cache.fields_obj; RUBY_ASSERT(fields_obj == rb_obj_fields_generic_uncached(obj)); } @@ -1309,6 +1309,8 @@ rb_free_generic_ivar(VALUE obj) default: generic_fields: { + // Other EC may have stale caches, so fields_obj should be + // invalidated and the GC will replace with Qundef rb_execution_context_t *ec = GET_EC(); if (ec->gen_fields_cache.obj == obj) { ec->gen_fields_cache.obj = Qundef; From a60cd8787cb65a6077f609652f6042143d81b227 Mon Sep 17 00:00:00 2001 From: John Hawthorn Date: Wed, 26 Nov 2025 14:37:30 -0800 Subject: [PATCH 9/9] Error if deleting a nonexistent obj from geniv I don't think this ever happened, but we should raise the same error we'd raise on lookup --- variable.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/variable.c b/variable.c index f7ee56122024b7..d4c5d91e25d6dc 100644 --- a/variable.c +++ b/variable.c @@ -1317,7 +1317,9 @@ rb_free_generic_ivar(VALUE obj) ec->gen_fields_cache.fields_obj = Qundef; } RB_VM_LOCKING() { - st_delete(generic_fields_tbl_no_ractor_check(), &key, &value); + if (!st_delete(generic_fields_tbl_no_ractor_check(), &key, &value)) { + rb_bug("Object is missing entry in generic_fields_tbl"); + } } } }