From 0f3d3c78530260ef1b44f7b7808a3e0e009d54f0 Mon Sep 17 00:00:00 2001 From: Aiden Fox Ivey Date: Tue, 30 Sep 2025 12:42:53 -0400 Subject: [PATCH 1/8] ZJIT: Add extra info to rb_zjit_add_frame --- zjit.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/zjit.c b/zjit.c index 4bc27d9fe2dd15..1affda0caf9bbe 100644 --- a/zjit.c +++ b/zjit.c @@ -59,6 +59,10 @@ rb_zjit_add_frame(VALUE hash, VALUE frame) rb_hash_aset(frame_info, ID2SYM(rb_intern("name")), name); rb_hash_aset(frame_info, ID2SYM(rb_intern("file")), file); + rb_hash_aset(frame_info, ID2SYM(rb_intern("samples")), INT2NUM(0)); + rb_hash_aset(frame_info, ID2SYM(rb_intern("total_samples")), INT2NUM(0)); + rb_hash_aset(frame_info, ID2SYM(rb_intern("edges")), rb_hash_new()); + rb_hash_aset(frame_info, ID2SYM(rb_intern("lines")), rb_hash_new()); if (line != INT2FIX(0)) { rb_hash_aset(frame_info, ID2SYM(rb_intern("line")), line); From 0a4bfb6499041535bebe7a6f5f27cc083716427e Mon Sep 17 00:00:00 2001 From: Aiden Fox Ivey Date: Tue, 30 Sep 2025 12:43:22 -0400 Subject: [PATCH 2/8] ZJIT: Add correction rb_zjit_exit_locations_dict --- zjit.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/zjit.c b/zjit.c index 1affda0caf9bbe..d877c0bacbd507 100644 --- a/zjit.c +++ b/zjit.c @@ -93,8 +93,9 @@ rb_zjit_exit_locations_dict(VALUE *zjit_raw_samples, int *zjit_line_samples, int int line_num = (int)zjit_line_samples[idx]; idx++; - rb_ary_push(raw_samples, SIZET2NUM(num)); - rb_ary_push(line_samples, INT2NUM(line_num)); + // + 1 as we append an additional sample for the insn + rb_ary_push(raw_samples, SIZET2NUM(num + 1)); + rb_ary_push(line_samples, INT2NUM(line_num + 1)); // Loop through the length of samples_len and add data to the // frames hash. Also push the current value onto the raw_samples From 55d363bd8dc8a5eddb63fee19af90edc98529a64 Mon Sep 17 00:00:00 2001 From: Aiden Fox Ivey Date: Tue, 30 Sep 2025 12:44:07 -0400 Subject: [PATCH 3/8] ZJIT: Use binwrite in zjit.rb --- zjit.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/zjit.rb b/zjit.rb index 83f8adb866c663..7ecf278de4d7ad 100644 --- a/zjit.rb +++ b/zjit.rb @@ -119,7 +119,7 @@ def dump_exit_locations(filename) raise ArgumentError, "--zjit-trace-exits must be enabled to use dump_exit_locations." end - File.write(filename, Marshal.dump(RubyVM::ZJIT.exit_locations)) + File.binwrite(filename, Marshal.dump(RubyVM::ZJIT.exit_locations)) end # Check if `--zjit-stats` is used From a0a4068e1be52d01533496237ced56fe2e8f743c Mon Sep 17 00:00:00 2001 From: Aiden Fox Ivey Date: Tue, 30 Sep 2025 12:53:33 -0400 Subject: [PATCH 4/8] ZJIT: Use optimized exit_locations implementation --- zjit.rb | 49 ++++++++++++++++++++++++++++--------------------- 1 file changed, 28 insertions(+), 21 deletions(-) diff --git a/zjit.rb b/zjit.rb index 7ecf278de4d7ad..155f16a7131d9c 100644 --- a/zjit.rb +++ b/zjit.rb @@ -40,16 +40,11 @@ def exit_locations frames = results[:frames].dup samples_count = 0 - frames.each do |frame_id, frame| - frame[:samples] = 0 - frame[:edges] = {} - end - # Loop through the instructions and set the frame hash with the data. # We use nonexistent.def for the file name, otherwise insns.def will be displayed # and that information isn't useful in this context. RubyVM::INSTRUCTION_NAMES.each_with_index do |name, frame_id| - frame_hash = { samples: 0, total_samples: 0, edges: {}, name: name, file: "nonexistent.def", line: nil } + frame_hash = { samples: 0, total_samples: 0, edges: {}, name: name, file: "nonexistent.def", line: nil, lines: {} } results[:frames][frame_id] = frame_hash frames[frame_id] = frame_hash end @@ -57,39 +52,51 @@ def exit_locations # Loop through the raw_samples and build the hashes for StackProf. # The loop is based off an example in the StackProf documentation and therefore # this functionality can only work with that library. - while raw_samples.length > 0 - stack_trace = raw_samples.shift(raw_samples.shift + 1) - lines = line_samples.shift(line_samples.shift + 1) + # + # Raw Samples: + # [ length, frame1, frame2, frameN, ..., instruction, count + # + # Line Samples + # [ length, line_1, line_2, line_n, ..., dummy value, count + i = 0 + while i < raw_samples.length + stack_length = raw_samples[i] + i += 1 # consume the stack length + + sample_count = raw_samples[i + stack_length] + prev_frame_id = nil + stack_length.times do |idx| + idx += i + frame_id = raw_samples[idx] - stack_trace.each_with_index do |frame_id, idx| if prev_frame_id prev_frame = frames[prev_frame_id] prev_frame[:edges][frame_id] ||= 0 - prev_frame[:edges][frame_id] += 1 + prev_frame[:edges][frame_id] += sample_count end frame_info = frames[frame_id] - frame_info[:total_samples] ||= 0 - frame_info[:total_samples] += 1 + frame_info[:total_samples] += sample_count - frame_info[:lines] ||= {} - frame_info[:lines][lines[idx]] ||= [0, 0] - frame_info[:lines][lines[idx]][0] += 1 + frame_info[:lines][line_samples[idx]] ||= [0, 0] + frame_info[:lines][line_samples[idx]][0] += sample_count prev_frame_id = frame_id end - top_frame_id = stack_trace.last + i += stack_length # consume the stack + + top_frame_id = prev_frame_id top_frame_line = 1 - frames[top_frame_id][:samples] += 1 + frames[top_frame_id][:samples] += sample_count frames[top_frame_id][:lines] ||= {} frames[top_frame_id][:lines][top_frame_line] ||= [0, 0] - frames[top_frame_id][:lines][top_frame_line][1] += 1 + frames[top_frame_id][:lines][top_frame_line][1] += sample_count - samples_count += raw_samples.shift - line_samples.shift + samples_count += sample_count + i += 1 end results[:samples] = samples_count From e90729aa6c34d00743e2de9095293d3189587333 Mon Sep 17 00:00:00 2001 From: Stan Lo Date: Tue, 30 Sep 2025 20:20:55 +0100 Subject: [PATCH 5/8] ZJIT: Rust code refactors (#14687) --- zjit/src/codegen.rs | 2 +- zjit/src/hir.rs | 4 ++-- zjit/src/hir_type/mod.rs | 3 +-- zjit/src/options.rs | 2 +- 4 files changed, 5 insertions(+), 6 deletions(-) diff --git a/zjit/src/codegen.rs b/zjit/src/codegen.rs index c62fef73dea6be..b8d527fb8d6494 100644 --- a/zjit/src/codegen.rs +++ b/zjit/src/codegen.rs @@ -573,7 +573,7 @@ fn gen_setlocal(asm: &mut Assembler, val: Opnd, val_type: Type, local_ep_offset: } else { // We're potentially writing a reference to an IMEMO/env object, // so take care of the write barrier with a function. - let local_index = local_ep_offset * -1; + let local_index = -local_ep_offset; asm_ccall!(asm, rb_vm_env_write, ep, local_index.into(), val); } } diff --git a/zjit/src/hir.rs b/zjit/src/hir.rs index 8f6e92d6539dd0..7d6167d490bb74 100644 --- a/zjit/src/hir.rs +++ b/zjit/src/hir.rs @@ -3263,7 +3263,7 @@ struct BytecodeInfo { has_blockiseq: bool, } -fn compute_bytecode_info(iseq: *const rb_iseq_t, opt_table: &Vec) -> BytecodeInfo { +fn compute_bytecode_info(iseq: *const rb_iseq_t, opt_table: &[u32]) -> BytecodeInfo { let iseq_size = unsafe { get_iseq_encoded_size(iseq) }; let mut insn_idx = 0; let mut jump_targets: HashSet = opt_table.iter().copied().collect(); @@ -4251,7 +4251,7 @@ pub fn iseq_to_hir(iseq: *const rb_iseq_t) -> Result { } /// Compile an entry_block for the interpreter -fn compile_entry_block(fun: &mut Function, jit_entry_insns: &Vec) { +fn compile_entry_block(fun: &mut Function, jit_entry_insns: &[u32]) { let entry_block = fun.entry_block; fun.push_insn(entry_block, Insn::EntryPoint { jit_entry_idx: None }); diff --git a/zjit/src/hir_type/mod.rs b/zjit/src/hir_type/mod.rs index 3e690c79d0c04f..ffde7e458d3b28 100644 --- a/zjit/src/hir_type/mod.rs +++ b/zjit/src/hir_type/mod.rs @@ -355,8 +355,7 @@ impl Type { fn is_builtin(class: VALUE) -> bool { types::ExactBitsAndClass .iter() - .find(|&(_, class_object)| unsafe { **class_object } == class) - .is_some() + .any(|&(_, class_object)| unsafe { *class_object } == class) } /// Union both types together, preserving specialization if possible. diff --git a/zjit/src/options.rs b/zjit/src/options.rs index ab9d1960ebaa20..4040c85907de4c 100644 --- a/zjit/src/options.rs +++ b/zjit/src/options.rs @@ -314,7 +314,7 @@ fn update_profile_threshold() { /// Update --zjit-call-threshold for testing #[cfg(test)] pub fn set_call_threshold(call_threshold: u64) { - unsafe { rb_zjit_call_threshold = call_threshold as u64; } + unsafe { rb_zjit_call_threshold = call_threshold; } rb_zjit_prepare_options(); update_profile_threshold(); } From df2d1d5ad386c51ad9750282917ecacf2b343598 Mon Sep 17 00:00:00 2001 From: Aiden Fox Ivey Date: Tue, 30 Sep 2025 16:51:56 -0400 Subject: [PATCH 6/8] ZJIT: Decouple stats and side exit tracing (#14688) --- doc/zjit.md | 4 ++-- zjit.rb | 8 ++++---- zjit/src/state.rs | 2 +- zjit/src/stats.rs | 9 +++++---- 4 files changed, 12 insertions(+), 11 deletions(-) diff --git a/doc/zjit.md b/doc/zjit.md index 57a95457d304e3..ba65739e3ff697 100644 --- a/doc/zjit.md +++ b/doc/zjit.md @@ -155,10 +155,10 @@ make -j ### Tracing side exits -Through [Stackprof](https://github.com/tmm1/stackprof), detailed information about the methods that the JIT side-exits from can be displayed after some execution of a program. Note that the use of `--zjit-trace-exits` must be used alongside `--zjit-stats`. +Through [Stackprof](https://github.com/tmm1/stackprof), detailed information about the methods that the JIT side-exits from can be displayed after some execution of a program. ```bash -./miniruby --zjit-stats --zjit-trace-exits script.rb +./miniruby --zjit-trace-exits script.rb ``` A file called `zjit_exit_locations.dump` will be created in the same directory as `script.rb`. Viewing the side exited methods can be done with Stackprof: diff --git a/zjit.rb b/zjit.rb index 155f16a7131d9c..2ff4cf2a5b2a6f 100644 --- a/zjit.rb +++ b/zjit.rb @@ -9,10 +9,10 @@ module RubyVM::ZJIT # Avoid calling a Ruby method here to avoid interfering with compilation tests if Primitive.rb_zjit_print_stats_p - at_exit { - print_stats - dump_locations - } + at_exit { print_stats } + end + if Primitive.rb_zjit_trace_exit_locations_enabled_p + at_exit { dump_locations } end end diff --git a/zjit/src/state.rs b/zjit/src/state.rs index 50c3f4b1c18fa2..206a7b3b618105 100644 --- a/zjit/src/state.rs +++ b/zjit/src/state.rs @@ -392,7 +392,7 @@ pub extern "C" fn rb_zjit_record_exit_stack(exit_pc: *const VALUE) { /// Mark `raw_samples` so they can be used by rb_zjit_add_frame. pub fn gc_mark_raw_samples() { // Return if ZJIT is not enabled - if !zjit_enabled_p() || !get_option!(stats) || !get_option!(trace_side_exits) { + if !zjit_enabled_p() || !get_option!(trace_side_exits) { return; } diff --git a/zjit/src/stats.rs b/zjit/src/stats.rs index 05ae231dad067a..a9cf1bde7cd131 100644 --- a/zjit/src/stats.rs +++ b/zjit/src/stats.rs @@ -2,6 +2,7 @@ use std::time::Instant; use std::sync::atomic::Ordering; +use crate::options::OPTIONS; #[cfg(feature = "stats_allocator")] #[path = "../../jit/src/lib.rs"] @@ -475,11 +476,11 @@ pub struct SideExitLocations { /// Primitive called in zjit.rb /// -/// Check if trace_exits generation is enabled. Requires the stats feature -/// to be enabled. +/// Check if trace_exits generation is enabled. #[unsafe(no_mangle)] pub extern "C" fn rb_zjit_trace_exit_locations_enabled_p(_ec: EcPtr, _ruby_self: VALUE) -> VALUE { - if get_option!(stats) && get_option!(trace_side_exits) { + // Builtin zjit.rb calls this even if ZJIT is disabled, so OPTIONS may not be set. + if unsafe { OPTIONS.as_ref() }.is_some_and(|opts| opts.trace_side_exits) { Qtrue } else { Qfalse @@ -490,7 +491,7 @@ pub extern "C" fn rb_zjit_trace_exit_locations_enabled_p(_ec: EcPtr, _ruby_self: /// into raw, lines, and frames hash for RubyVM::YJIT.exit_locations. #[unsafe(no_mangle)] pub extern "C" fn rb_zjit_get_exit_locations(_ec: EcPtr, _ruby_self: VALUE) -> VALUE { - if !zjit_enabled_p() || !get_option!(stats) || !get_option!(trace_side_exits) { + if !zjit_enabled_p() || !get_option!(trace_side_exits) { return Qnil; } From 8cefb70e210348f509648df943eebe61ef708c3d Mon Sep 17 00:00:00 2001 From: Max Bernstein Date: Tue, 30 Sep 2025 23:39:06 +0200 Subject: [PATCH 7/8] ZJIT: Re-apply attr_writer inlining (#14678) This re-applies https://github.com/ruby/ruby/pull/14629 / 40bb47665d3ff57e0f2eb5a9fd9e0109617015c9 by reverting https://github.com/ruby/ruby/pull/14673 / d4393772b89dab4f33c118a284d92dc80cd63c39. Co-authored-by: Alan Wu --- test/ruby/test_zjit.rb | 43 ++++++++++++++++++++++- zjit/src/hir.rs | 79 ++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 121 insertions(+), 1 deletion(-) diff --git a/test/ruby/test_zjit.rb b/test/ruby/test_zjit.rb index 683d53f339f8c2..937cf44e190a56 100644 --- a/test/ruby/test_zjit.rb +++ b/test/ruby/test_zjit.rb @@ -1642,7 +1642,7 @@ def test(c) = c.foo }, call_threshold: 2, insns: [:opt_send_without_block] end - def test_attr_accessor + def test_attr_accessor_getivar assert_compiles '[4, 4]', %q{ class C attr_accessor :foo @@ -1658,6 +1658,47 @@ def test(c) = c.foo }, call_threshold: 2, insns: [:opt_send_without_block] end + def test_attr_accessor_setivar + assert_compiles '[5, 5]', %q{ + class C + attr_accessor :foo + + def initialize + @foo = 4 + end + end + + def test(c) + c.foo = 5 + c.foo + end + + c = C.new + [test(c), test(c)] + }, call_threshold: 2, insns: [:opt_send_without_block] + end + + def test_attr_writer + assert_compiles '[5, 5]', %q{ + class C + attr_writer :foo + + def initialize + @foo = 4 + end + + def get_foo = @foo + end + + def test(c) + c.foo = 5 + c.get_foo + end + c = C.new + [test(c), test(c)] + }, call_threshold: 2, insns: [:opt_send_without_block] + end + def test_uncached_getconstant_path assert_compiles RUBY_COPYRIGHT.dump, %q{ def test = RUBY_COPYRIGHT diff --git a/zjit/src/hir.rs b/zjit/src/hir.rs index 7d6167d490bb74..5588544b4f5a66 100644 --- a/zjit/src/hir.rs +++ b/zjit/src/hir.rs @@ -2043,6 +2043,23 @@ impl Function { } let getivar = self.push_insn(block, Insn::GetIvar { self_val: recv, id, state }); self.make_equal_to(insn_id, getivar); + } else if let (VM_METHOD_TYPE_ATTRSET, &[val]) = (def_type, args.as_slice()) { + self.push_insn(block, Insn::PatchPoint { invariant: Invariant::MethodRedefined { klass, method: mid, cme }, state }); + if let Some(profiled_type) = profiled_type { + recv = self.push_insn(block, Insn::GuardType { val: recv, guard_type: Type::from_profiled_type(profiled_type), state }); + } + let id = unsafe { get_cme_def_body_attr_id(cme) }; + + // Check if we're accessing ivars of a Class or Module object as they require single-ractor mode. + // We omit gen_prepare_non_leaf_call on gen_setivar, so it's unsafe to raise for multi-ractor mode. + if unsafe { rb_zjit_singleton_class_p(klass) } { + let attached = unsafe { rb_class_attached_object(klass) }; + if unsafe { RB_TYPE_P(attached, RUBY_T_CLASS) || RB_TYPE_P(attached, RUBY_T_MODULE) } { + self.push_insn(block, Insn::PatchPoint { invariant: Invariant::SingleRactorMode, state }); + } + } + self.push_insn(block, Insn::SetIvar { self_val: recv, id, val, state }); + self.make_equal_to(insn_id, val); } else { self.set_dynamic_send_reason(insn_id, SendWithoutBlockNotOptimizedMethodType(MethodType::from(def_type))); self.push_insn_id(block, insn_id); continue; @@ -12190,4 +12207,66 @@ mod opt_tests { Return v25 "); } + + #[test] + fn test_inline_attr_accessor_set() { + eval(" + class C + attr_accessor :foo + end + + def test(o) = o.foo = 5 + test C.new + test C.new + "); + assert_snapshot!(hir_string("test"), @r" + fn test@:6: + bb0(): + EntryPoint interpreter + v1:BasicObject = LoadSelf + v2:BasicObject = GetLocal l0, SP@4 + Jump bb2(v1, v2) + bb1(v5:BasicObject, v6:BasicObject): + EntryPoint JIT(0) + Jump bb2(v5, v6) + bb2(v8:BasicObject, v9:BasicObject): + v14:Fixnum[5] = Const Value(5) + PatchPoint MethodRedefined(C@0x1000, foo=@0x1008, cme:0x1010) + v23:HeapObject[class_exact:C] = GuardType v9, HeapObject[class_exact:C] + SetIvar v23, :@foo, v14 + CheckInterrupts + Return v14 + "); + } + + #[test] + fn test_inline_attr_writer_set() { + eval(" + class C + attr_writer :foo + end + + def test(o) = o.foo = 5 + test C.new + test C.new + "); + assert_snapshot!(hir_string("test"), @r" + fn test@:6: + bb0(): + EntryPoint interpreter + v1:BasicObject = LoadSelf + v2:BasicObject = GetLocal l0, SP@4 + Jump bb2(v1, v2) + bb1(v5:BasicObject, v6:BasicObject): + EntryPoint JIT(0) + Jump bb2(v5, v6) + bb2(v8:BasicObject, v9:BasicObject): + v14:Fixnum[5] = Const Value(5) + PatchPoint MethodRedefined(C@0x1000, foo=@0x1008, cme:0x1010) + v23:HeapObject[class_exact:C] = GuardType v9, HeapObject[class_exact:C] + SetIvar v23, :@foo, v14 + CheckInterrupts + Return v14 + "); + } } From 17252958c9ce094c583616257452cfb1695dc7ef Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Mon, 29 Sep 2025 16:12:44 -0700 Subject: [PATCH 8/8] [ruby/prism] Add a "LAST" enum field to all flags enums This allows us to use the "last" of the enums in order to make masks, etc. This particular commit uses the call flag's last enum field as an offset so that we can define "private" flags but not accidentally clobber any newly added call node flags. https://github.com/ruby/prism/commit/e71aa980d8 --- prism/prism.c | 9 +++++---- prism/templates/include/prism/ast.h.erb | 2 ++ 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/prism/prism.c b/prism/prism.c index 6dd0b2ae738588..875968f06be820 100644 --- a/prism/prism.c +++ b/prism/prism.c @@ -2622,10 +2622,11 @@ pm_break_node_create(pm_parser_t *parser, const pm_token_t *keyword, pm_argument // There are certain flags that we want to use internally but don't want to // expose because they are not relevant beyond parsing. Therefore we'll define // them here and not define them in config.yml/a header file. -static const pm_node_flags_t PM_WRITE_NODE_FLAGS_IMPLICIT_ARRAY = 0x4; -static const pm_node_flags_t PM_CALL_NODE_FLAGS_IMPLICIT_ARRAY = 0x40; -static const pm_node_flags_t PM_CALL_NODE_FLAGS_COMPARISON = 0x80; -static const pm_node_flags_t PM_CALL_NODE_FLAGS_INDEX = 0x100; +static const pm_node_flags_t PM_WRITE_NODE_FLAGS_IMPLICIT_ARRAY = (1 << 2); + +static const pm_node_flags_t PM_CALL_NODE_FLAGS_IMPLICIT_ARRAY = ((PM_CALL_NODE_FLAGS_LAST - 1) << 1); +static const pm_node_flags_t PM_CALL_NODE_FLAGS_COMPARISON = ((PM_CALL_NODE_FLAGS_LAST - 1) << 2); +static const pm_node_flags_t PM_CALL_NODE_FLAGS_INDEX = ((PM_CALL_NODE_FLAGS_LAST - 1) << 3); /** * Allocate and initialize a new CallNode node. This sets everything to NULL or diff --git a/prism/templates/include/prism/ast.h.erb b/prism/templates/include/prism/ast.h.erb index 087eb81890eb45..e82cb78fe3106f 100644 --- a/prism/templates/include/prism/ast.h.erb +++ b/prism/templates/include/prism/ast.h.erb @@ -212,6 +212,8 @@ typedef enum pm_<%= flag.human %> { /** <%= value.comment %> */ PM_<%= flag.human.upcase %>_<%= value.name %> = <%= 1 << (index + Prism::Template::COMMON_FLAGS_COUNT) %>, <%- end -%> + + PM_<%= flag.human.upcase %>_LAST, } pm_<%= flag.human %>_t; <%- end -%>