From d8c8623f50af8f5324e1679ff95b1a2071c0c61e Mon Sep 17 00:00:00 2001 From: Peter Zhu Date: Mon, 29 Sep 2025 18:33:04 -0400 Subject: [PATCH 1/7] Set context_stack on main thread We allocate the stack of the main thread using malloc, but we never set malloc_stack to true and context_stack. If we fork, the main thread may no longer be the main thread anymore so it reports memory being leaked in RUBY_FREE_AT_EXIT. This commit allows the main thread to free its own VM stack at shutdown. --- thread_none.c | 6 ++++++ thread_pthread.c | 7 +++++++ thread_win32.c | 6 ++++++ vm.c | 7 ++++--- vm_core.h | 1 + 5 files changed, 24 insertions(+), 3 deletions(-) diff --git a/thread_none.c b/thread_none.c index 38686e17c1c339..e6616c05856ff9 100644 --- a/thread_none.c +++ b/thread_none.c @@ -335,4 +335,10 @@ rb_thread_prevent_fork(void *(*func)(void *), void *data) return func(data); } +void +rb_thread_malloc_stack_set(rb_thread_t *th, void *stack) +{ + // no-op +} + #endif /* THREAD_SYSTEM_DEPENDENT_IMPLEMENTATION */ diff --git a/thread_pthread.c b/thread_pthread.c index 730ecb54163f7d..5150a6173e6e6b 100644 --- a/thread_pthread.c +++ b/thread_pthread.c @@ -3492,4 +3492,11 @@ rb_thread_lock_native_thread(void) return is_snt; } +void +rb_thread_malloc_stack_set(rb_thread_t *th, void *stack) +{ + th->sched.malloc_stack = true; + th->sched.context_stack = stack; +} + #endif /* THREAD_SYSTEM_DEPENDENT_IMPLEMENTATION */ diff --git a/thread_win32.c b/thread_win32.c index 576f617e8d5c2a..3fc763924846bd 100644 --- a/thread_win32.c +++ b/thread_win32.c @@ -1020,4 +1020,10 @@ rb_thread_prevent_fork(void *(*func)(void *), void *data) return func(data); } +void +rb_thread_malloc_stack_set(rb_thread_t *th, void *stack) +{ + // no-op +} + #endif /* THREAD_SYSTEM_DEPENDENT_IMPLEMENTATION */ diff --git a/vm.c b/vm.c index 524bde55e6447f..431db0cb0bc88d 100644 --- a/vm.c +++ b/vm.c @@ -3280,7 +3280,7 @@ ruby_vm_destruct(rb_vm_t *vm) if (vm) { rb_thread_t *th = vm->ractor.main_thread; - VALUE *stack = th->ec->vm_stack; + if (rb_free_at_exit) { rb_free_encoded_insn_data(); rb_free_global_enc_table(); @@ -3345,7 +3345,6 @@ ruby_vm_destruct(rb_vm_t *vm) rb_free_default_rand_key(); if (th && vm->fork_gen == 0) { /* If we have forked, main_thread may not be the initial thread */ - xfree(stack); ruby_mimfree(th); } } @@ -3827,7 +3826,9 @@ th_init(rb_thread_t *th, VALUE self, rb_vm_t *vm) if (self == 0) { size_t size = vm->default_params.thread_vm_stack_size / sizeof(VALUE); - rb_ec_initialize_vm_stack(th->ec, ALLOC_N(VALUE, size), size); + VALUE *stack = ALLOC_N(VALUE, size); + rb_ec_initialize_vm_stack(th->ec, stack, size); + rb_thread_malloc_stack_set(th, stack); } else { VM_ASSERT(th->ec->cfp == NULL); diff --git a/vm_core.h b/vm_core.h index 51898f56f9c559..da0249e567977d 100644 --- a/vm_core.h +++ b/vm_core.h @@ -1965,6 +1965,7 @@ VALUE *rb_vm_svar_lep(const rb_execution_context_t *ec, const rb_control_frame_t int rb_vm_get_sourceline(const rb_control_frame_t *); void rb_vm_stack_to_heap(rb_execution_context_t *ec); void ruby_thread_init_stack(rb_thread_t *th, void *local_in_parent_frame); +void rb_thread_malloc_stack_set(rb_thread_t *th, void *stack); rb_thread_t * ruby_thread_from_native(void); int ruby_thread_set_native(rb_thread_t *th); int rb_vm_control_frame_id_and_class(const rb_control_frame_t *cfp, ID *idp, ID *called_idp, VALUE *klassp); From d016595387069677c6b992dffe9322f67dc9bc73 Mon Sep 17 00:00:00 2001 From: Takashi Kokubun Date: Tue, 30 Sep 2025 08:15:06 -0700 Subject: [PATCH 2/7] ZJIT: Unify fallback counters for send-ish insns (#14676) --- zjit.rb | 10 ++-- zjit/src/codegen.rs | 62 +++++++++++++---------- zjit/src/hir.rs | 119 ++++++++++++++++++++++++++++++++------------ zjit/src/state.rs | 15 ++++-- zjit/src/stats.rs | 80 +++++++++++++++++++++++------ 5 files changed, 209 insertions(+), 77 deletions(-) diff --git a/zjit.rb b/zjit.rb index a46802553c274a..4438a10c754a43 100644 --- a/zjit.rb +++ b/zjit.rb @@ -39,12 +39,14 @@ def stats_string buf = +"***ZJIT: Printing ZJIT statistics on exit***\n" stats = self.stats - # Show non-exit counters - print_counters_with_prefix(prefix: 'dynamic_send_type_', prompt: 'dynamic send types', buf:, stats:, limit: 20) - print_counters_with_prefix(prefix: 'unspecialized_def_type_', prompt: 'send fallback unspecialized def_types', buf:, stats:, limit: 20) - print_counters_with_prefix(prefix: 'send_fallback_', prompt: 'dynamic send types', buf:, stats:, limit: 20) + # Show counters independent from exit_* or dynamic_send_* print_counters_with_prefix(prefix: 'not_optimized_cfuncs_', prompt: 'unoptimized sends to C functions', buf:, stats:, limit: 20) + # Show fallback counters, ordered by the typical amount of fallbacks for the prefix at the time + print_counters_with_prefix(prefix: 'unspecialized_def_type_', prompt: 'not optimized method types', buf:, stats:, limit: 20) + print_counters_with_prefix(prefix: 'not_optimized_yarv_insn_', prompt: 'not optimized instructions', buf:, stats:, limit: 20) + print_counters_with_prefix(prefix: 'send_fallback_', prompt: 'send fallback reasons', buf:, stats:, limit: 20) + # Show exit counters, ordered by the typical amount of exits for the prefix at the time print_counters_with_prefix(prefix: 'unhandled_yarv_insn_', prompt: 'unhandled YARV insns', buf:, stats:, limit: 20) print_counters_with_prefix(prefix: 'compile_error_', prompt: 'compile error reasons', buf:, stats:, limit: 20) diff --git a/zjit/src/codegen.rs b/zjit/src/codegen.rs index 7676d7eed46e70..c62fef73dea6be 100644 --- a/zjit/src/codegen.rs +++ b/zjit/src/codegen.rs @@ -12,12 +12,12 @@ use crate::backend::current::{Reg, ALLOC_REGS}; use crate::invariants::{track_bop_assumption, track_cme_assumption, track_no_ep_escape_assumption, track_no_trace_point_assumption, track_single_ractor_assumption, track_stable_constant_names_assumption}; use crate::gc::{append_gc_offsets, get_or_create_iseq_payload, get_or_create_iseq_payload_ptr, IseqCodePtrs, IseqPayload, IseqStatus}; use crate::state::ZJITState; -use crate::stats::{exit_counter_for_compile_error, incr_counter, incr_counter_by, CompileError}; -use crate::stats::{counter_ptr, with_time_stat, Counter, send_fallback_counter, Counter::{compile_time_ns, exit_compile_error}}; +use crate::stats::{send_fallback_counter, exit_counter_for_compile_error, incr_counter, incr_counter_by, send_fallback_counter_for_method_type, send_fallback_counter_ptr_for_opcode, CompileError}; +use crate::stats::{counter_ptr, with_time_stat, Counter, Counter::{compile_time_ns, exit_compile_error}}; use crate::{asm::CodeBlock, cruby::*, options::debug, virtualmem::CodePtr}; use crate::backend::lir::{self, asm_comment, asm_ccall, Assembler, Opnd, Target, CFP, C_ARG_OPNDS, C_RET_OPND, EC, NATIVE_STACK_PTR, NATIVE_BASE_PTR, SCRATCH_OPND, SP}; -use crate::hir::{iseq_to_hir, BlockId, BranchEdge, Invariant, MethodType, RangeType, SideExitReason::{self, *}, SpecialBackrefSymbol, SpecialObjectType}; -use crate::hir::{Const, FrameState, Function, Insn, InsnId}; +use crate::hir::{iseq_to_hir, BlockId, BranchEdge, Invariant, RangeType, SideExitReason::{self, *}, SpecialBackrefSymbol, SpecialObjectType}; +use crate::hir::{Const, FrameState, Function, Insn, InsnId, SendFallbackReason}; use crate::hir_type::{types, Type}; use crate::options::get_option; use crate::cast::IntoUsize; @@ -366,15 +366,15 @@ fn gen_insn(cb: &mut CodeBlock, jit: &mut JITState, asm: &mut Assembler, functio Insn::Jump(branch) => no_output!(gen_jump(jit, asm, branch)), Insn::IfTrue { val, target } => no_output!(gen_if_true(jit, asm, opnd!(val), target)), Insn::IfFalse { val, target } => no_output!(gen_if_false(jit, asm, opnd!(val), target)), - &Insn::Send { cd, blockiseq, state, .. } => gen_send(jit, asm, cd, blockiseq, &function.frame_state(state)), - &Insn::SendForward { cd, blockiseq, state, .. } => gen_send_forward(jit, asm, cd, blockiseq, &function.frame_state(state)), - Insn::SendWithoutBlock { cd, state, def_type, .. } => gen_send_without_block(jit, asm, *cd, *def_type, &function.frame_state(*state)), + &Insn::Send { cd, blockiseq, state, reason, .. } => gen_send(jit, asm, cd, blockiseq, &function.frame_state(state), reason), + &Insn::SendForward { cd, blockiseq, state, reason, .. } => gen_send_forward(jit, asm, cd, blockiseq, &function.frame_state(state), reason), + &Insn::SendWithoutBlock { cd, state, reason, .. } => gen_send_without_block(jit, asm, cd, &function.frame_state(state), reason), // Give up SendWithoutBlockDirect for 6+ args since asm.ccall() doesn't support it. Insn::SendWithoutBlockDirect { cd, state, args, .. } if args.len() + 1 > C_ARG_OPNDS.len() => // +1 for self - gen_send_without_block(jit, asm, *cd, None, &function.frame_state(*state)), + gen_send_without_block(jit, asm, *cd, &function.frame_state(*state), SendFallbackReason::SendWithoutBlockDirectTooManyArgs), Insn::SendWithoutBlockDirect { cme, iseq, recv, args, state, .. } => gen_send_without_block_direct(cb, jit, asm, *cme, *iseq, opnd!(recv), opnds!(args), &function.frame_state(*state)), - &Insn::InvokeSuper { cd, blockiseq, state, .. } => gen_invokesuper(jit, asm, cd, blockiseq, &function.frame_state(state)), - Insn::InvokeBlock { cd, state, .. } => gen_invokeblock(jit, asm, *cd, &function.frame_state(*state)), + &Insn::InvokeSuper { cd, blockiseq, state, reason, .. } => gen_invokesuper(jit, asm, cd, blockiseq, &function.frame_state(state), reason), + &Insn::InvokeBlock { cd, state, reason, .. } => gen_invokeblock(jit, asm, cd, &function.frame_state(state), reason), // Ensure we have enough room fit ec, self, and arguments // TODO remove this check when we have stack args (we can use Time.new to test it) Insn::InvokeBuiltin { bf, state, .. } if bf.argc + 2 > (C_ARG_OPNDS.len() as i32) => return Err(*state), @@ -981,9 +981,9 @@ fn gen_send( cd: *const rb_call_data, blockiseq: IseqPtr, state: &FrameState, + reason: SendFallbackReason, ) -> lir::Opnd { - gen_incr_counter(asm, Counter::dynamic_send_count); - gen_incr_counter(asm, Counter::dynamic_send_type_send); + gen_incr_send_fallback_counter(asm, reason); gen_prepare_non_leaf_call(jit, asm, state); asm_comment!(asm, "call #{} with dynamic dispatch", ruby_call_method_name(cd)); @@ -1003,9 +1003,9 @@ fn gen_send_forward( cd: *const rb_call_data, blockiseq: IseqPtr, state: &FrameState, + reason: SendFallbackReason, ) -> lir::Opnd { - gen_incr_counter(asm, Counter::dynamic_send_count); - gen_incr_counter(asm, Counter::dynamic_send_type_send_forward); + gen_incr_send_fallback_counter(asm, reason); gen_prepare_non_leaf_call(jit, asm, state); @@ -1024,15 +1024,10 @@ fn gen_send_without_block( jit: &mut JITState, asm: &mut Assembler, cd: *const rb_call_data, - def_type: Option, state: &FrameState, + reason: SendFallbackReason, ) -> lir::Opnd { - gen_incr_counter(asm, Counter::dynamic_send_count); - gen_incr_counter(asm, Counter::dynamic_send_type_send_without_block); - - if let Some(def_type) = def_type { - gen_incr_counter(asm, send_fallback_counter(def_type)); - } + gen_incr_send_fallback_counter(asm, reason); gen_prepare_non_leaf_call(jit, asm, state); asm_comment!(asm, "call #{} with dynamic dispatch", ruby_call_method_name(cd)); @@ -1118,9 +1113,9 @@ fn gen_invokeblock( asm: &mut Assembler, cd: *const rb_call_data, state: &FrameState, + reason: SendFallbackReason, ) -> lir::Opnd { - gen_incr_counter(asm, Counter::dynamic_send_count); - gen_incr_counter(asm, Counter::dynamic_send_type_invokeblock); + gen_incr_send_fallback_counter(asm, reason); gen_prepare_non_leaf_call(jit, asm, state); @@ -1141,9 +1136,9 @@ fn gen_invokesuper( cd: *const rb_call_data, blockiseq: IseqPtr, state: &FrameState, + reason: SendFallbackReason, ) -> lir::Opnd { - gen_incr_counter(asm, Counter::dynamic_send_count); - gen_incr_counter(asm, Counter::dynamic_send_type_invokesuper); + gen_incr_send_fallback_counter(asm, reason); gen_prepare_non_leaf_call(jit, asm, state); asm_comment!(asm, "call super with dynamic dispatch"); @@ -1548,6 +1543,23 @@ fn gen_incr_counter(asm: &mut Assembler, counter: Counter) { } } +/// Increment a counter for each DynamicSendReason. If the variant has +/// a counter prefix to break down the details, increment that as well. +fn gen_incr_send_fallback_counter(asm: &mut Assembler, reason: SendFallbackReason) { + gen_incr_counter(asm, send_fallback_counter(reason)); + + use SendFallbackReason::*; + match reason { + NotOptimizedInstruction(opcode) => { + gen_incr_counter_ptr(asm, send_fallback_counter_ptr_for_opcode(opcode)); + } + SendWithoutBlockNotOptimizedMethodType(method_type) => { + gen_incr_counter(asm, send_fallback_counter_for_method_type(method_type)); + } + _ => {} + } +} + /// Save the current PC on the CFP as a preparation for calling a C function /// that may allocate objects and trigger GC. Use gen_prepare_non_leaf_call() /// if it may raise exceptions or call arbitrary methods. diff --git a/zjit/src/hir.rs b/zjit/src/hir.rs index d81231e2820b88..8f6e92d6539dd0 100644 --- a/zjit/src/hir.rs +++ b/zjit/src/hir.rs @@ -15,6 +15,7 @@ use crate::hir_type::{Type, types}; use crate::bitset::BitSet; use crate::profile::{TypeDistributionSummary, ProfiledType}; use crate::stats::Counter; +use SendFallbackReason::*; /// An index of an [`Insn`] in a [`Function`]. This is a popular /// type since this effectively acts as a pointer to an [`Insn`]. @@ -514,6 +515,21 @@ impl std::fmt::Display for SideExitReason { } } +/// Reason why a send-ish instruction cannot be optimized from a fallback instruction +#[derive(Debug, Clone, Copy)] +pub enum SendFallbackReason { + SendWithoutBlockPolymorphic, + SendWithoutBlockNoProfiles, + SendWithoutBlockCfuncNotVariadic, + SendWithoutBlockCfuncArrayVariadic, + SendWithoutBlockNotOptimizedMethodType(MethodType), + SendWithoutBlockDirectTooManyArgs, + ObjToStringNotString, + /// Initial fallback reason for every instruction, which should be mutated to + /// a more actionable reason when an attempt to specialize the instruction fails. + NotOptimizedInstruction(ruby_vminsn_type), +} + /// An instruction in the SSA IR. The output of an instruction is referred to by the index of /// the instruction ([`InsnId`]). SSA form enables this, and [`UnionFind`] ([`Function::find`]) /// helps with editing. @@ -638,13 +654,39 @@ pub enum Insn { recv: InsnId, cd: *const rb_call_data, args: Vec, - def_type: Option, // Assigned in `optimize_direct_sends` if it's not optimized state: InsnId, + reason: SendFallbackReason, + }, + Send { + recv: InsnId, + cd: *const rb_call_data, + blockiseq: IseqPtr, + args: Vec, + state: InsnId, + reason: SendFallbackReason, + }, + SendForward { + recv: InsnId, + cd: *const rb_call_data, + blockiseq: IseqPtr, + args: Vec, + state: InsnId, + reason: SendFallbackReason, + }, + InvokeSuper { + recv: InsnId, + cd: *const rb_call_data, + blockiseq: IseqPtr, + args: Vec, + state: InsnId, + reason: SendFallbackReason, + }, + InvokeBlock { + cd: *const rb_call_data, + args: Vec, + state: InsnId, + reason: SendFallbackReason, }, - Send { recv: InsnId, cd: *const rb_call_data, blockiseq: IseqPtr, args: Vec, state: InsnId }, - SendForward { recv: InsnId, cd: *const rb_call_data, blockiseq: IseqPtr, args: Vec, state: InsnId }, - InvokeSuper { recv: InsnId, cd: *const rb_call_data, blockiseq: IseqPtr, args: Vec, state: InsnId }, - InvokeBlock { cd: *const rb_call_data, args: Vec, state: InsnId }, /// Optimized ISEQ call SendWithoutBlockDirect { @@ -1442,12 +1484,12 @@ impl Function { str: find!(str), state, }, - &SendWithoutBlock { recv, cd, ref args, def_type, state } => SendWithoutBlock { + &SendWithoutBlock { recv, cd, ref args, state, reason } => SendWithoutBlock { recv: find!(recv), cd, args: find_vec!(args), - def_type, state, + reason, }, &SendWithoutBlockDirect { recv, cd, cme, iseq, ref args, state } => SendWithoutBlockDirect { recv: find!(recv), @@ -1457,31 +1499,35 @@ impl Function { args: find_vec!(args), state, }, - &Send { recv, cd, blockiseq, ref args, state } => Send { + &Send { recv, cd, blockiseq, ref args, state, reason } => Send { recv: find!(recv), cd, blockiseq, args: find_vec!(args), state, + reason, }, - &SendForward { recv, cd, blockiseq, ref args, state } => SendForward { + &SendForward { recv, cd, blockiseq, ref args, state, reason } => SendForward { recv: find!(recv), cd, blockiseq, args: find_vec!(args), state, + reason, }, - &InvokeSuper { recv, cd, blockiseq, ref args, state } => InvokeSuper { + &InvokeSuper { recv, cd, blockiseq, ref args, state, reason } => InvokeSuper { recv: find!(recv), cd, blockiseq, args: find_vec!(args), state, + reason, }, - &InvokeBlock { cd, ref args, state } => InvokeBlock { + &InvokeBlock { cd, ref args, state, reason } => InvokeBlock { cd, args: find_vec!(args), state, + reason, }, &InvokeBuiltin { bf, ref args, state, return_type } => InvokeBuiltin { bf, args: find_vec!(args), state, return_type }, &ArrayDup { val, state } => ArrayDup { val: find!(val), state }, @@ -1515,6 +1561,22 @@ impl Function { } } + /// Update DynamicSendReason for the instruction at insn_id + fn set_dynamic_send_reason(&mut self, insn_id: InsnId, dynamic_send_reason: SendFallbackReason) { + use Insn::*; + if get_option!(stats) { + match self.insns.get_mut(insn_id.0).unwrap() { + Send { reason, .. } + | SendForward { reason, .. } + | SendWithoutBlock { reason, .. } + | InvokeSuper { reason, .. } + | InvokeBlock { reason, .. } + => *reason = dynamic_send_reason, + _ => unreachable!("unexpected instruction {} at {insn_id}", self.find(insn_id)) + } + } + } + /// Replace `insn` with the new instruction `replacement`, which will get appended to `insns`. fn make_equal_to(&mut self, insn: InsnId, replacement: InsnId) { // Don't push it to the block @@ -1927,12 +1989,11 @@ impl Function { let Some(recv_type) = self.profiled_type_of_at(recv, frame_state.insn_idx) else { if get_option!(stats) { match self.is_polymorphic_at(recv, frame_state.insn_idx) { - Some(true) => self.push_insn(block, Insn::IncrCounter(Counter::send_fallback_polymorphic)), + Some(true) => self.set_dynamic_send_reason(insn_id, SendWithoutBlockPolymorphic), // If the class isn't known statically, then it should not also be monomorphic Some(false) => panic!("Should not have monomorphic profile at this point in this branch"), - None => self.push_insn(block, Insn::IncrCounter(Counter::send_fallback_no_profiles)), - - }; + None => self.set_dynamic_send_reason(insn_id, SendWithoutBlockNoProfiles), + } } self.push_insn_id(block, insn_id); continue; }; @@ -1943,9 +2004,7 @@ impl Function { // Do method lookup let mut cme = unsafe { rb_callable_method_entry(klass, mid) }; if cme.is_null() { - if let Insn::SendWithoutBlock { def_type: insn_def_type, .. } = &mut self.insns[insn_id.0] { - *insn_def_type = Some(MethodType::Null); - } + self.set_dynamic_send_reason(insn_id, SendWithoutBlockNotOptimizedMethodType(MethodType::Null)); self.push_insn_id(block, insn_id); continue; } // Load an overloaded cme if applicable. See vm_search_cc(). @@ -1958,9 +2017,7 @@ impl Function { // TODO(max): Handle other kinds of parameter passing let iseq = unsafe { get_def_iseq_ptr((*cme).def) }; if !can_direct_send(iseq) { - if let Insn::SendWithoutBlock { def_type: insn_def_type, .. } = &mut self.insns[insn_id.0] { - *insn_def_type = Some(MethodType::from(def_type)); - } + self.set_dynamic_send_reason(insn_id, SendWithoutBlockNotOptimizedMethodType(MethodType::Iseq)); self.push_insn_id(block, insn_id); continue; } self.push_insn(block, Insn::PatchPoint { invariant: Invariant::MethodRedefined { klass, method: mid, cme }, state }); @@ -1987,9 +2044,7 @@ 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 Insn::SendWithoutBlock { def_type: insn_def_type, .. } = &mut self.insns[insn_id.0] { - *insn_def_type = Some(MethodType::from(def_type)); - } + self.set_dynamic_send_reason(insn_id, SendWithoutBlockNotOptimizedMethodType(MethodType::from(def_type))); self.push_insn_id(block, insn_id); continue; } } @@ -2031,7 +2086,7 @@ impl Function { self.make_equal_to(insn_id, guard); } else { self.push_insn(block, Insn::GuardTypeNot { val, guard_type: types::String, state}); - let send_to_s = self.push_insn(block, Insn::SendWithoutBlock { recv: val, cd, args: vec![], def_type: None, state}); + let send_to_s = self.push_insn(block, Insn::SendWithoutBlock { recv: val, cd, args: vec![], state, reason: ObjToStringNotString }); self.make_equal_to(insn_id, send_to_s); } } @@ -2206,6 +2261,7 @@ impl Function { let Some(FnProperties { leaf: true, no_gc: true, return_type, elidable }) = ZJITState::get_method_annotations().get_cfunc_properties(method) else { + fun.set_dynamic_send_reason(send_insn_id, SendWithoutBlockCfuncNotVariadic); return Err(Some(method)); }; @@ -2269,6 +2325,7 @@ impl Function { -2 => { // (self, args_ruby_array) parameter form // Falling through for now + fun.set_dynamic_send_reason(send_insn_id, SendWithoutBlockCfuncArrayVariadic); } _ => unreachable!("unknown cfunc kind: argc={argc}") } @@ -3787,7 +3844,7 @@ pub fn iseq_to_hir(iseq: *const rb_iseq_t) -> Result { let args = state.stack_pop_n(argc as usize)?; let recv = state.stack_pop()?; let exit_id = fun.push_insn(block, Insn::Snapshot { state: exit_state }); - let send = fun.push_insn(block, Insn::SendWithoutBlock { recv, cd, args, def_type: None, state: exit_id }); + let send = fun.push_insn(block, Insn::SendWithoutBlock { recv, cd, args, state: exit_id, reason: NotOptimizedInstruction(opcode) }); state.stack_push(send); } YARVINSN_opt_hash_freeze => { @@ -3895,7 +3952,7 @@ pub fn iseq_to_hir(iseq: *const rb_iseq_t) -> Result { let args = state.stack_pop_n(argc as usize)?; let recv = state.stack_pop()?; let exit_id = fun.push_insn(block, Insn::Snapshot { state: exit_state }); - let send = fun.push_insn(block, Insn::SendWithoutBlock { recv, cd, args, def_type: None, state: exit_id }); + let send = fun.push_insn(block, Insn::SendWithoutBlock { recv, cd, args, state: exit_id, reason: NotOptimizedInstruction(opcode) }); state.stack_push(send); } YARVINSN_send => { @@ -3915,7 +3972,7 @@ pub fn iseq_to_hir(iseq: *const rb_iseq_t) -> Result { let args = state.stack_pop_n(argc as usize + usize::from(block_arg))?; let recv = state.stack_pop()?; let exit_id = fun.push_insn(block, Insn::Snapshot { state: exit_state }); - let send = fun.push_insn(block, Insn::Send { recv, cd, blockiseq, args, state: exit_id }); + let send = fun.push_insn(block, Insn::Send { recv, cd, blockiseq, args, state: exit_id, reason: NotOptimizedInstruction(opcode) }); state.stack_push(send); if !blockiseq.is_null() { @@ -3947,7 +4004,7 @@ pub fn iseq_to_hir(iseq: *const rb_iseq_t) -> Result { let args = state.stack_pop_n(argc as usize + usize::from(forwarding))?; let recv = state.stack_pop()?; let exit_id = fun.push_insn(block, Insn::Snapshot { state: exit_state }); - let send_forward = fun.push_insn(block, Insn::SendForward { recv, cd, blockiseq, args, state: exit_id }); + let send_forward = fun.push_insn(block, Insn::SendForward { recv, cd, blockiseq, args, state: exit_id, reason: NotOptimizedInstruction(opcode) }); state.stack_push(send_forward); if !blockiseq.is_null() { @@ -3976,7 +4033,7 @@ pub fn iseq_to_hir(iseq: *const rb_iseq_t) -> Result { let recv = state.stack_pop()?; let blockiseq: IseqPtr = get_arg(pc, 1).as_ptr(); let exit_id = fun.push_insn(block, Insn::Snapshot { state: exit_state }); - let result = fun.push_insn(block, Insn::InvokeSuper { recv, cd, blockiseq, args, state: exit_id }); + let result = fun.push_insn(block, Insn::InvokeSuper { recv, cd, blockiseq, args, state: exit_id, reason: NotOptimizedInstruction(opcode) }); state.stack_push(result); if !blockiseq.is_null() { @@ -4005,7 +4062,7 @@ pub fn iseq_to_hir(iseq: *const rb_iseq_t) -> Result { let block_arg = (flags & VM_CALL_ARGS_BLOCKARG) != 0; let args = state.stack_pop_n(argc as usize + usize::from(block_arg))?; let exit_id = fun.push_insn(block, Insn::Snapshot { state: exit_state }); - let result = fun.push_insn(block, Insn::InvokeBlock { cd, args, state: exit_id }); + let result = fun.push_insn(block, Insn::InvokeBlock { cd, args, state: exit_id, reason: NotOptimizedInstruction(opcode) }); state.stack_push(result); } YARVINSN_getglobal => { diff --git a/zjit/src/state.rs b/zjit/src/state.rs index fa5d3bc83f506c..81c05f4986ac78 100644 --- a/zjit/src/state.rs +++ b/zjit/src/state.rs @@ -6,7 +6,7 @@ use crate::cruby_methods; use crate::invariants::Invariants; use crate::asm::CodeBlock; use crate::options::get_option; -use crate::stats::{Counters, ExitCounters}; +use crate::stats::{Counters, InsnCounters}; use crate::virtualmem::CodePtr; use std::collections::HashMap; @@ -28,7 +28,10 @@ pub struct ZJITState { counters: Counters, /// Side-exit counters - exit_counters: ExitCounters, + exit_counters: InsnCounters, + + /// Send fallback counters + send_fallback_counters: InsnCounters, /// Assumptions that require invalidation invariants: Invariants, @@ -78,6 +81,7 @@ impl ZJITState { code_block: cb, counters: Counters::default(), exit_counters: [0; VM_INSTRUCTION_SIZE as usize], + send_fallback_counters: [0; VM_INSTRUCTION_SIZE as usize], invariants: Invariants::default(), assert_compiles: false, method_annotations: cruby_methods::init(), @@ -139,10 +143,15 @@ impl ZJITState { } /// Get a mutable reference to side-exit counters - pub fn get_exit_counters() -> &'static mut ExitCounters { + pub fn get_exit_counters() -> &'static mut InsnCounters { &mut ZJITState::get_instance().exit_counters } + /// Get a mutable reference to fallback counters + pub fn get_send_fallback_counters() -> &'static mut InsnCounters { + &mut ZJITState::get_instance().send_fallback_counters + } + /// Get a mutable reference to unoptimized cfunc counter pointers pub fn get_unoptimized_cfunc_counter_pointers() -> &'static mut HashMap> { &mut ZJITState::get_instance().unoptimized_cfunc_counter_pointers diff --git a/zjit/src/stats.rs b/zjit/src/stats.rs index 7329b3442af0df..f9f9fb9e37bd5b 100644 --- a/zjit/src/stats.rs +++ b/zjit/src/stats.rs @@ -17,6 +17,9 @@ macro_rules! make_counters { exit { $($exit_counter_name:ident,)+ } + dynamic_send { + $($dynamic_send_counter_name:ident,)+ + } $($counter_name:ident,)+ ) => { /// Struct containing the counter values @@ -24,6 +27,7 @@ macro_rules! make_counters { pub struct Counters { $(pub $default_counter_name: u64,)+ $(pub $exit_counter_name: u64,)+ + $(pub $dynamic_send_counter_name: u64,)+ $(pub $counter_name: u64,)+ } @@ -33,6 +37,7 @@ macro_rules! make_counters { pub enum Counter { $($default_counter_name,)+ $($exit_counter_name,)+ + $($dynamic_send_counter_name,)+ $($counter_name,)+ } @@ -41,6 +46,7 @@ macro_rules! make_counters { match self { $( Counter::$default_counter_name => stringify!($default_counter_name).to_string(), )+ $( Counter::$exit_counter_name => stringify!($exit_counter_name).to_string(), )+ + $( Counter::$dynamic_send_counter_name => stringify!($dynamic_send_counter_name).to_string(), )+ $( Counter::$counter_name => stringify!($counter_name).to_string(), )+ } } @@ -52,6 +58,7 @@ macro_rules! make_counters { match counter { $( 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::$counter_name => std::ptr::addr_of_mut!(counters.$counter_name), )+ } } @@ -67,6 +74,11 @@ macro_rules! make_counters { $( Counter::$exit_counter_name, )+ ]; + /// List of other counters that are summed as dynamic_send_count. + pub const DYNAMIC_SEND_COUNTERS: &'static [Counter] = &[ + $( Counter::$dynamic_send_counter_name, )+ + ]; + /// List of other counters that are available only for --zjit-stats. pub const OTHER_COUNTERS: &'static [Counter] = &[ $( Counter::$counter_name, )+ @@ -114,6 +126,19 @@ make_counters! { exit_block_param_proxy_not_iseq_or_ifunc, } + // Send fallback counters that are summed as dynamic_send_count + dynamic_send { + // send_fallback_: Fallback reasons for send-ish instructions + send_fallback_send_without_block_polymorphic, + send_fallback_send_without_block_no_profiles, + send_fallback_send_without_block_cfunc_not_variadic, + send_fallback_send_without_block_cfunc_array_variadic, + send_fallback_send_without_block_not_optimized_method_type, + send_fallback_send_without_block_direct_too_many_args, + send_fallback_obj_to_string_not_string, + send_fallback_not_optimized_instruction, + } + // compile_error_: Compile error reasons compile_error_iseq_stack_too_large, compile_error_exception_handler, @@ -134,14 +159,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 dispatch from JIT code - dynamic_send_count, - dynamic_send_type_send_without_block, - dynamic_send_type_send, - dynamic_send_type_send_forward, - dynamic_send_type_invokeblock, - dynamic_send_type_invokesuper, - // The number of times we do a dynamic ivar lookup from JIT code dynamic_getivar_count, dynamic_setivar_count, @@ -161,9 +178,6 @@ make_counters! { unspecialized_def_type_refined, unspecialized_def_type_null, - send_fallback_polymorphic, - send_fallback_no_profiles, - // Writes to the VM frame vm_write_pc_count, vm_write_sp_count, @@ -190,7 +204,7 @@ macro_rules! incr_counter { pub(crate) use incr_counter; /// The number of side exits from each YARV instruction -pub type ExitCounters = [u64; VM_INSTRUCTION_SIZE as usize]; +pub type InsnCounters = [u64; VM_INSTRUCTION_SIZE as usize]; /// Return a raw pointer to the exit counter for a given YARV opcode pub fn exit_counter_ptr_for_opcode(opcode: u32) -> *mut u64 { @@ -198,6 +212,12 @@ pub fn exit_counter_ptr_for_opcode(opcode: u32) -> *mut u64 { unsafe { exit_counters.get_unchecked_mut(opcode as usize) } } +/// Return a raw pointer to the fallback counter for a given YARV opcode +pub fn send_fallback_counter_ptr_for_opcode(opcode: u32) -> *mut u64 { + let fallback_counters = ZJITState::get_send_fallback_counters(); + unsafe { fallback_counters.get_unchecked_mut(opcode as usize) } +} + /// Reason why ZJIT failed to produce any JIT code #[derive(Clone, Debug, PartialEq)] pub enum CompileError { @@ -268,11 +288,26 @@ pub fn exit_counter_ptr(reason: crate::hir::SideExitReason) -> *mut u64 { counter_ptr(counter) } -pub fn send_fallback_counter(def_type: crate::hir::MethodType) -> Counter { +pub fn send_fallback_counter(reason: crate::hir::SendFallbackReason) -> Counter { + use crate::hir::SendFallbackReason::*; + use crate::stats::Counter::*; + match reason { + SendWithoutBlockPolymorphic => send_fallback_send_without_block_polymorphic, + SendWithoutBlockNoProfiles => send_fallback_send_without_block_no_profiles, + SendWithoutBlockCfuncNotVariadic => send_fallback_send_without_block_cfunc_not_variadic, + SendWithoutBlockCfuncArrayVariadic => send_fallback_send_without_block_cfunc_array_variadic, + SendWithoutBlockNotOptimizedMethodType(_) => send_fallback_send_without_block_not_optimized_method_type, + SendWithoutBlockDirectTooManyArgs => send_fallback_send_without_block_direct_too_many_args, + ObjToStringNotString => send_fallback_obj_to_string_not_string, + NotOptimizedInstruction(_) => send_fallback_not_optimized_instruction, + } +} + +pub fn send_fallback_counter_for_method_type(method_type: crate::hir::MethodType) -> Counter { use crate::hir::MethodType::*; use crate::stats::Counter::*; - match def_type { + match method_type { Iseq => unspecialized_def_type_iseq, Cfunc => unspecialized_def_type_cfunc, Attrset => unspecialized_def_type_attrset, @@ -376,6 +411,23 @@ pub extern "C" fn rb_zjit_stats(_ec: EcPtr, _self: VALUE, target_key: VALUE) -> set_stat_usize!(hash, &key_string, *count); } + // Set send fallback counters for each DynamicSendReason + let mut dynamic_send_count = 0; + for &counter in DYNAMIC_SEND_COUNTERS { + let count = unsafe { *counter_ptr(counter) }; + dynamic_send_count += count; + set_stat_usize!(hash, &counter.name(), count); + } + set_stat_usize!(hash, "dynamic_send_count", dynamic_send_count); + + // Set send fallback counters for NotOptimizedInstruction + 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) { + let op_name = insn_name(op_idx); + let key_string = "not_optimized_yarv_insn_".to_owned() + &op_name; + set_stat_usize!(hash, &key_string, *count); + } + // Only ZJIT_STATS builds support rb_vm_insn_count if unsafe { rb_vm_insn_count } > 0 { let vm_insn_count = unsafe { rb_vm_insn_count }; From 2f1c30cd50e464880e44da670d3ad8ebe00fc899 Mon Sep 17 00:00:00 2001 From: Aiden Fox Ivey Date: Tue, 30 Sep 2025 11:55:33 -0400 Subject: [PATCH 3/7] ZJIT: Add --zjit-trace-exits (#14640) Add side exit tracing functionality for ZJIT --- doc/zjit.md | 14 +++ gc.c | 8 ++ yjit/src/stats.rs | 4 +- zjit.c | 91 ++++++++++++++++ zjit.rb | 114 +++++++++++++++++++- zjit/bindgen/src/main.rs | 3 + zjit/src/backend/lir.rs | 12 ++- zjit/src/cruby_bindings.inc.rs | 5 + zjit/src/gc.rs | 7 ++ zjit/src/options.rs | 10 ++ zjit/src/state.rs | 185 ++++++++++++++++++++++++++++++++- zjit/src/stats.rs | 49 +++++++++ 12 files changed, 496 insertions(+), 6 deletions(-) diff --git a/doc/zjit.md b/doc/zjit.md index 4eedcca3ba7ac9..57a95457d304e3 100644 --- a/doc/zjit.md +++ b/doc/zjit.md @@ -153,6 +153,20 @@ To build with stats support: 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`. + +```bash +./miniruby --zjit-stats --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: + +```bash +stackprof path/to/zjit_exit_locations.dump +``` + ## ZJIT Glossary This glossary contains terms that are helpful for understanding ZJIT. diff --git a/gc.c b/gc.c index 8c8887c46b7d78..1961670c54d062 100644 --- a/gc.c +++ b/gc.c @@ -3070,6 +3070,14 @@ rb_gc_mark_roots(void *objspace, const char **categoryp) } #endif +#if USE_ZJIT + void rb_zjit_root_mark(void); + if (rb_zjit_enabled_p) { + MARK_CHECKPOINT("ZJIT"); + rb_zjit_root_mark(); + } +#endif + MARK_CHECKPOINT("machine_context"); mark_current_machine_context(ec); diff --git a/yjit/src/stats.rs b/yjit/src/stats.rs index 09971c5b3afb48..b63e1c3272e356 100644 --- a/yjit/src/stats.rs +++ b/yjit/src/stats.rs @@ -893,7 +893,7 @@ fn rb_yjit_gen_stats_dict(key: VALUE) -> VALUE { /// and line samples. Their length should be the same, however the data stored in /// them is different. #[no_mangle] -pub extern "C" fn rb_yjit_record_exit_stack(_exit_pc: *const VALUE) +pub extern "C" fn rb_yjit_record_exit_stack(exit_pc: *const VALUE) { // Return if YJIT is not enabled if !yjit_enabled_p() { @@ -920,7 +920,7 @@ pub extern "C" fn rb_yjit_record_exit_stack(_exit_pc: *const VALUE) #[cfg(not(test))] { // Get the opcode from the encoded insn handler at this PC - let insn = unsafe { rb_vm_insn_addr2opcode((*_exit_pc).as_ptr()) }; + let insn = unsafe { rb_vm_insn_addr2opcode((*exit_pc).as_ptr()) }; // Use the same buffer size as Stackprof. const BUFF_LEN: usize = 2048; diff --git a/zjit.c b/zjit.c index 37619fd7296494..4bc27d9fe2dd15 100644 --- a/zjit.c +++ b/zjit.c @@ -31,6 +31,95 @@ #include +#define PTR2NUM(x) (rb_int2inum((intptr_t)(void *)(x))) + +// For a given raw_sample (frame), set the hash with the caller's +// name, file, and line number. Return the hash with collected frame_info. +static void +rb_zjit_add_frame(VALUE hash, VALUE frame) +{ + VALUE frame_id = PTR2NUM(frame); + + if (RTEST(rb_hash_aref(hash, frame_id))) { + return; + } + else { + VALUE frame_info = rb_hash_new(); + // Full label for the frame + VALUE name = rb_profile_frame_full_label(frame); + // Absolute path of the frame from rb_iseq_realpath + VALUE file = rb_profile_frame_absolute_path(frame); + // Line number of the frame + VALUE line = rb_profile_frame_first_lineno(frame); + + // If absolute path isn't available use the rb_iseq_path + if (NIL_P(file)) { + file = rb_profile_frame_path(frame); + } + + rb_hash_aset(frame_info, ID2SYM(rb_intern("name")), name); + rb_hash_aset(frame_info, ID2SYM(rb_intern("file")), file); + + if (line != INT2FIX(0)) { + rb_hash_aset(frame_info, ID2SYM(rb_intern("line")), line); + } + + rb_hash_aset(hash, frame_id, frame_info); + } +} + +// Parses the ZjitExitLocations raw_samples and line_samples collected by +// rb_zjit_record_exit_stack and turns them into 3 hashes (raw, lines, and frames) to +// be used by RubyVM::ZJIT.exit_locations. zjit_raw_samples represents the raw frames information +// (without name, file, and line), and zjit_line_samples represents the line information +// of the iseq caller. +VALUE +rb_zjit_exit_locations_dict(VALUE *zjit_raw_samples, int *zjit_line_samples, int samples_len) +{ + VALUE result = rb_hash_new(); + VALUE raw_samples = rb_ary_new_capa(samples_len); + VALUE line_samples = rb_ary_new_capa(samples_len); + VALUE frames = rb_hash_new(); + int idx = 0; + + // While the index is less than samples_len, parse zjit_raw_samples and + // zjit_line_samples, then add casted values to raw_samples and line_samples array. + while (idx < samples_len) { + int num = (int)zjit_raw_samples[idx]; + int line_num = (int)zjit_line_samples[idx]; + idx++; + + rb_ary_push(raw_samples, SIZET2NUM(num)); + rb_ary_push(line_samples, INT2NUM(line_num)); + + // Loop through the length of samples_len and add data to the + // frames hash. Also push the current value onto the raw_samples + // and line_samples arrary respectively. + for (int o = 0; o < num; o++) { + rb_zjit_add_frame(frames, zjit_raw_samples[idx]); + rb_ary_push(raw_samples, SIZET2NUM(zjit_raw_samples[idx])); + rb_ary_push(line_samples, INT2NUM(zjit_line_samples[idx])); + idx++; + } + + rb_ary_push(raw_samples, SIZET2NUM(zjit_raw_samples[idx])); + rb_ary_push(line_samples, INT2NUM(zjit_line_samples[idx])); + idx++; + + rb_ary_push(raw_samples, SIZET2NUM(zjit_raw_samples[idx])); + rb_ary_push(line_samples, INT2NUM(zjit_line_samples[idx])); + idx++; + } + + // Set add the raw_samples, line_samples, and frames to the results + // hash. + rb_hash_aset(result, ID2SYM(rb_intern("raw")), raw_samples); + rb_hash_aset(result, ID2SYM(rb_intern("lines")), line_samples); + rb_hash_aset(result, ID2SYM(rb_intern("frames")), frames); + + return result; +} + void rb_zjit_profile_disable(const rb_iseq_t *iseq); void @@ -217,6 +306,8 @@ VALUE rb_zjit_stats(rb_execution_context_t *ec, VALUE self, VALUE target_key); VALUE rb_zjit_reset_stats_bang(rb_execution_context_t *ec, VALUE self); VALUE rb_zjit_stats_enabled_p(rb_execution_context_t *ec, VALUE self); VALUE rb_zjit_print_stats_p(rb_execution_context_t *ec, VALUE self); +VALUE rb_zjit_trace_exit_locations_enabled_p(rb_execution_context_t *ec, VALUE self); +VALUE rb_zjit_get_exit_locations(rb_execution_context_t *ec, VALUE self); // Preprocessed zjit.rb generated during build #include "zjit.rbinc" diff --git a/zjit.rb b/zjit.rb index 4438a10c754a43..8289846c03b403 100644 --- a/zjit.rb +++ b/zjit.rb @@ -9,7 +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 } + at_exit { + print_stats + dump_locations + } end end @@ -19,6 +22,106 @@ def enabled? Primitive.cexpr! 'RBOOL(rb_zjit_enabled_p)' end + # Check if `--zjit-trace-exits` is used + def trace_exit_locations_enabled? + Primitive.rb_zjit_trace_exit_locations_enabled_p + end + + # If --zjit-trace-exits is enabled parse the hashes from + # Primitive.rb_zjit_get_exit_locations into a format readable + # by Stackprof. This will allow us to find the exact location of a + # side exit in ZJIT based on the instruction that is exiting. + def exit_locations + return unless trace_exit_locations_enabled? + + results = Primitive.rb_zjit_get_exit_locations + raw_samples = results[:raw].dup + line_samples = results[:lines].dup + 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 } + results[:frames][frame_id] = frame_hash + frames[frame_id] = frame_hash + end + + # 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) + prev_frame_id = nil + + 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 + end + + frame_info = frames[frame_id] + frame_info[:total_samples] ||= 0 + frame_info[:total_samples] += 1 + + frame_info[:lines] ||= {} + frame_info[:lines][lines[idx]] ||= [0, 0] + frame_info[:lines][lines[idx]][0] += 1 + + prev_frame_id = frame_id + end + + top_frame_id = stack_trace.last + top_frame_line = 1 + + frames[top_frame_id][:samples] += 1 + 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 + + samples_count += raw_samples.shift + line_samples.shift + end + + results[:samples] = samples_count + # Set missed_samples and gc_samples to 0 as their values + # don't matter to us in this context. + results[:missed_samples] = 0 + results[:gc_samples] = 0 + results + end + + # Marshal dumps exit locations to the given filename. + # + # Usage: + # + # In a script call: + # + # RubyVM::ZJIT.dump_exit_locations("my_file.dump") + # + # Then run the file with the following options: + # + # ruby --zjit --zjit-stats --zjit-trace-exits test.rb + # + # Once the code is done running, use Stackprof to read the dump file. + # See Stackprof documentation for options. + def dump_exit_locations(filename) + unless trace_exit_locations_enabled? + raise ArgumentError, "--zjit-trace-exits must be enabled to use dump_exit_locations." + end + + File.write(filename, Marshal.dump(RubyVM::ZJIT.exit_locations)) + end + # Check if `--zjit-stats` is used def stats_enabled? Primitive.rb_zjit_stats_enabled_p @@ -148,4 +251,13 @@ def number_with_delimiter(number) def print_stats $stderr.write stats_string end + + def dump_locations # :nodoc: + return unless trace_exit_locations_enabled? + + filename = "zjit_exit_locations.dump" + dump_exit_locations(filename) + + $stderr.puts("ZJIT exit locations dumped to `#{filename}`.") + end end diff --git a/zjit/bindgen/src/main.rs b/zjit/bindgen/src/main.rs index c4233521cce7fc..e1d19f9442c62b 100644 --- a/zjit/bindgen/src/main.rs +++ b/zjit/bindgen/src/main.rs @@ -281,6 +281,7 @@ fn main() { .allowlist_function("rb_RSTRING_PTR") .allowlist_function("rb_RSTRING_LEN") .allowlist_function("rb_ENCODING_GET") + .allowlist_function("rb_zjit_exit_locations_dict") .allowlist_function("rb_optimized_call") .allowlist_function("rb_jit_icache_invalidate") .allowlist_function("rb_zjit_print_exception") @@ -327,6 +328,8 @@ fn main() { .allowlist_function("rb_class_new_instance_pass_kw") .allowlist_function("rb_obj_alloc") .allowlist_function("rb_obj_info") + // From include/ruby/debug.h + .allowlist_function("rb_profile_frames") .allowlist_function("ruby_xfree") .allowlist_function("rb_profile_frames") diff --git a/zjit/src/backend/lir.rs b/zjit/src/backend/lir.rs index 21adc42cd1c753..76a53c66d6b652 100644 --- a/zjit/src/backend/lir.rs +++ b/zjit/src/backend/lir.rs @@ -9,6 +9,7 @@ use crate::cruby::VALUE; use crate::stats::{exit_counter_ptr, exit_counter_ptr_for_opcode, CompileError}; use crate::virtualmem::CodePtr; use crate::asm::{CodeBlock, Label}; +use crate::state::rb_zjit_record_exit_stack; pub use crate::backend::current::{ Reg, @@ -1629,6 +1630,16 @@ impl Assembler } } + if get_option!(trace_side_exits) { + // Use `load_into` with `C_ARG_OPNDS` instead of `opnds` argument for ccall, since `compile_side_exits` + // is after the split pass, which would allow use of `opnds`. + self.load_into(C_ARG_OPNDS[0], Opnd::const_ptr(pc as *const u8)); + self.ccall( + rb_zjit_record_exit_stack as *const u8, + vec![] + ); + } + asm_comment!(self, "exit to the interpreter"); self.frame_teardown(&[]); // matching the setup in :bb0-prologue: self.mov(C_RET_OPND, Opnd::UImm(Qundef.as_u64())); @@ -2080,4 +2091,3 @@ mod tests { asm.load_into(mem, mem); } } - diff --git a/zjit/src/cruby_bindings.inc.rs b/zjit/src/cruby_bindings.inc.rs index 17a2d5a63d6e6a..2d8a8eb11e7036 100644 --- a/zjit/src/cruby_bindings.inc.rs +++ b/zjit/src/cruby_bindings.inc.rs @@ -921,6 +921,11 @@ unsafe extern "C" { lines: *mut ::std::os::raw::c_int, ) -> ::std::os::raw::c_int; pub fn rb_jit_cont_each_iseq(callback: rb_iseq_callback, data: *mut ::std::os::raw::c_void); + pub fn rb_zjit_exit_locations_dict( + zjit_raw_samples: *mut VALUE, + zjit_line_samples: *mut ::std::os::raw::c_int, + samples_len: ::std::os::raw::c_int, + ) -> VALUE; pub fn rb_zjit_profile_disable(iseq: *const rb_iseq_t); pub fn rb_vm_base_ptr(cfp: *mut rb_control_frame_struct) -> *mut VALUE; pub fn rb_zjit_constcache_shareable(ice: *const iseq_inline_constant_cache_entry) -> bool; diff --git a/zjit/src/gc.rs b/zjit/src/gc.rs index cc08b8fc9ebb07..0974c5bfce18f2 100644 --- a/zjit/src/gc.rs +++ b/zjit/src/gc.rs @@ -5,6 +5,7 @@ use crate::codegen::IseqCallRef; use crate::stats::CompileError; use crate::{cruby::*, profile::IseqProfile, state::ZJITState, stats::with_time_stat, virtualmem::CodePtr}; use crate::stats::Counter::gc_time_ns; +use crate::state::gc_mark_raw_samples; /// This is all the data ZJIT stores on an ISEQ. We mark objects in this struct on GC. #[derive(Debug)] @@ -250,3 +251,9 @@ pub fn remove_gc_offsets(payload_ptr: *mut IseqPayload, removed_range: &Range(left: &Range, right: &Range) -> bool where T: PartialOrd { left.start < right.end && right.start < left.end } + +/// Callback for marking GC objects inside [Invariants]. +#[unsafe(no_mangle)] +pub extern "C" fn rb_zjit_root_mark() { + gc_mark_raw_samples(); +} diff --git a/zjit/src/options.rs b/zjit/src/options.rs index b33d18efffd4ab..ab9d1960ebaa20 100644 --- a/zjit/src/options.rs +++ b/zjit/src/options.rs @@ -69,6 +69,9 @@ pub struct Options { /// Dump all compiled machine code. pub dump_disasm: bool, + /// Trace and write side exit source maps to /tmp for stackprof. + pub trace_side_exits: bool, + /// Dump code map to /tmp for performance profilers. pub perf: bool, @@ -94,6 +97,7 @@ impl Default for Options { dump_hir_graphviz: None, dump_lir: false, dump_disasm: false, + trace_side_exits: false, perf: false, allowed_iseqs: None, log_compiled_iseqs: None, @@ -115,6 +119,8 @@ pub const ZJIT_OPTIONS: &[(&str, &str)] = &[ ("--zjit-perf", "Dump ISEQ symbols into /tmp/perf-{}.map for Linux perf."), ("--zjit-log-compiled-iseqs=path", "Log compiled ISEQs to the file. The file will be truncated."), + ("--zjit-trace-exits", + "Record Ruby source location when side-exiting.") ]; #[derive(Clone, Copy, Debug)] @@ -235,6 +241,10 @@ fn parse_option(str_ptr: *const std::os::raw::c_char) -> Option<()> { options.print_stats = false; } + ("trace-exits", "") => { + options.trace_side_exits = true; + } + ("debug", "") => options.debug = true, ("disable-hir-opt", "") => options.disable_hir_opt = true, diff --git a/zjit/src/state.rs b/zjit/src/state.rs index 81c05f4986ac78..50c3f4b1c18fa2 100644 --- a/zjit/src/state.rs +++ b/zjit/src/state.rs @@ -1,12 +1,12 @@ //! Runtime state of ZJIT. use crate::codegen::{gen_exit_trampoline, gen_exit_trampoline_with_counter, gen_function_stub_hit_trampoline}; -use crate::cruby::{self, rb_bug_panic_hook, rb_vm_insn_count, EcPtr, Qnil, VALUE, VM_INSTRUCTION_SIZE}; +use crate::cruby::{self, rb_bug_panic_hook, rb_vm_insn_count, EcPtr, Qnil, rb_vm_insn_addr2opcode, rb_profile_frames, VALUE, VM_INSTRUCTION_SIZE, size_t, rb_gc_mark}; use crate::cruby_methods; use crate::invariants::Invariants; use crate::asm::CodeBlock; use crate::options::get_option; -use crate::stats::{Counters, InsnCounters}; +use crate::stats::{Counters, InsnCounters, SideExitLocations}; use crate::virtualmem::CodePtr; use std::collections::HashMap; @@ -53,6 +53,9 @@ pub struct ZJITState { /// Counter pointers for unoptimized C functions unoptimized_cfunc_counter_pointers: HashMap>, + + /// Locations of side exists within generated code + exit_locations: Option, } /// Private singleton instance of the codegen globals @@ -76,6 +79,12 @@ impl ZJITState { let exit_trampoline = gen_exit_trampoline(&mut cb).unwrap(); let function_stub_hit_trampoline = gen_function_stub_hit_trampoline(&mut cb).unwrap(); + let exit_locations = if get_option!(trace_side_exits) { + Some(SideExitLocations::default()) + } else { + None + }; + // Initialize the codegen globals instance let zjit_state = ZJITState { code_block: cb, @@ -89,6 +98,7 @@ impl ZJITState { function_stub_hit_trampoline, exit_trampoline_with_counter: exit_trampoline, unoptimized_cfunc_counter_pointers: HashMap::new(), + exit_locations, }; unsafe { ZJIT_STATE = Some(zjit_state); } @@ -203,6 +213,16 @@ impl ZJITState { pub fn get_function_stub_hit_trampoline() -> CodePtr { ZJITState::get_instance().function_stub_hit_trampoline } + + /// Get a mutable reference to the ZJIT raw samples Vec + pub fn get_raw_samples() -> Option<&'static mut Vec> { + ZJITState::get_instance().exit_locations.as_mut().map(|el| &mut el.raw_samples) + } + + /// Get a mutable reference to the ZJIT line samples Vec. + pub fn get_line_samples() -> Option<&'static mut Vec> { + ZJITState::get_instance().exit_locations.as_mut().map(|el| &mut el.line_samples) + } } /// Initialize ZJIT @@ -238,3 +258,164 @@ pub extern "C" fn rb_zjit_assert_compiles(_ec: EcPtr, _self: VALUE) -> VALUE { ZJITState::enable_assert_compiles(); Qnil } + +/// Call `rb_profile_frames` and write the result into buffers to be consumed by `rb_zjit_record_exit_stack`. +fn record_profiling_frames() -> (i32, Vec, Vec) { + // Stackprof uses a buffer of length 2048 when collating the frames into statistics. + // Since eventually the collected information will be used by Stackprof, collect only + // 2048 frames at a time. + // https://github.com/tmm1/stackprof/blob/5d832832e4afcb88521292d6dfad4a9af760ef7c/ext/stackprof/stackprof.c#L21 + const BUFF_LEN: usize = 2048; + + let mut frames_buffer = vec![VALUE(0_usize); BUFF_LEN]; + let mut lines_buffer = vec![0; BUFF_LEN]; + + let stack_length = unsafe { + rb_profile_frames( + 0, + BUFF_LEN as i32, + frames_buffer.as_mut_ptr(), + lines_buffer.as_mut_ptr(), + ) + }; + + // Trim at `stack_length` since anything past it is redundant + frames_buffer.truncate(stack_length as usize); + lines_buffer.truncate(stack_length as usize); + + (stack_length, frames_buffer, lines_buffer) +} + +/// Write samples in `frames_buffer` and `lines_buffer` from profiling into +/// `raw_samples` and `line_samples`. Also write opcode, number of frames, +/// and stack size to be consumed by Stackprof. +fn write_exit_stack_samples( + raw_samples: &'static mut Vec, + line_samples: &'static mut Vec, + frames_buffer: &[VALUE], + lines_buffer: &[i32], + stack_length: i32, + exit_pc: *const VALUE, +) { + raw_samples.push(VALUE(stack_length as usize)); + line_samples.push(stack_length); + + // Push frames and their lines in reverse order. + for i in (0..stack_length as usize).rev() { + raw_samples.push(frames_buffer[i]); + line_samples.push(lines_buffer[i]); + } + + // Get the opcode from instruction handler at exit PC. + let exit_opcode = unsafe { rb_vm_insn_addr2opcode((*exit_pc).as_ptr()) }; + raw_samples.push(VALUE(exit_opcode as usize)); + // Push a dummy line number since we don't know where this insn is from. + line_samples.push(0); + + // Push number of times seen onto the stack. + raw_samples.push(VALUE(1usize)); + line_samples.push(1); +} + +fn try_increment_existing_stack( + raw_samples: &mut [VALUE], + line_samples: &mut [i32], + frames_buffer: &[VALUE], + stack_length: i32, + samples_length: usize, +) -> bool { + let prev_stack_len_index = raw_samples.len() - samples_length; + let prev_stack_len = i64::from(raw_samples[prev_stack_len_index]); + + if prev_stack_len == stack_length as i64 { + // Check if all stack lengths match and all frames are identical + let frames_match = (0..stack_length).all(|i| { + let current_frame = frames_buffer[stack_length as usize - 1 - i as usize]; + let prev_frame = raw_samples[prev_stack_len_index + i as usize + 1]; + current_frame == prev_frame + }); + + if frames_match { + let counter_idx = raw_samples.len() - 1; + let new_count = i64::from(raw_samples[counter_idx]) + 1; + + raw_samples[counter_idx] = VALUE(new_count as usize); + line_samples[counter_idx] = new_count as i32; + return true; + } + } + false +} + +/// Record a backtrace with ZJIT side exits +#[unsafe(no_mangle)] +pub extern "C" fn rb_zjit_record_exit_stack(exit_pc: *const VALUE) { + if !zjit_enabled_p() || !get_option!(trace_side_exits) { + return; + } + + let (stack_length, frames_buffer, lines_buffer) = record_profiling_frames(); + + // Can safely unwrap since `trace_side_exits` must be true at this point + let zjit_raw_samples = ZJITState::get_raw_samples().unwrap(); + let zjit_line_samples = ZJITState::get_line_samples().unwrap(); + assert_eq!(zjit_raw_samples.len(), zjit_line_samples.len()); + + // Represents pushing the stack length, the instruction opcode, and the sample count. + const SAMPLE_METADATA_SIZE: usize = 3; + let samples_length = (stack_length as usize) + SAMPLE_METADATA_SIZE; + + // If zjit_raw_samples is greater than or equal to the current length of the samples + // we might have seen this stack trace previously. + if zjit_raw_samples.len() >= samples_length + && try_increment_existing_stack( + zjit_raw_samples, + zjit_line_samples, + &frames_buffer, + stack_length, + samples_length, + ) + { + return; + } + + write_exit_stack_samples( + zjit_raw_samples, + zjit_line_samples, + &frames_buffer, + &lines_buffer, + stack_length, + exit_pc, + ); +} + +/// 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) { + return; + } + + let mut idx: size_t = 0; + let zjit_raw_samples = ZJITState::get_raw_samples().unwrap(); + + while idx < zjit_raw_samples.len() as size_t { + let num = zjit_raw_samples[idx as usize]; + let mut i = 0; + idx += 1; + + // Mark the zjit_raw_samples at the given index. These represent + // the data that needs to be GC'd which are the current frames. + while i < i32::from(num) { + unsafe { rb_gc_mark(zjit_raw_samples[idx as usize]); } + i += 1; + idx += 1; + } + + // Increase index for exit instruction. + idx += 1; + // Increase index for bookeeping value (number of times we've seen this + // row in a stack). + idx += 1; + } +} diff --git a/zjit/src/stats.rs b/zjit/src/stats.rs index f9f9fb9e37bd5b..05ae231dad067a 100644 --- a/zjit/src/stats.rs +++ b/zjit/src/stats.rs @@ -463,3 +463,52 @@ pub fn with_time_stat(counter: Counter, func: F) -> R where F: FnOnce() -> pub fn zjit_alloc_size() -> usize { jit::GLOBAL_ALLOCATOR.alloc_size.load(Ordering::SeqCst) } + +/// Struct of arrays for --zjit-trace-exits. +#[derive(Default)] +pub struct SideExitLocations { + /// Control frames of method entries. + pub raw_samples: Vec, + /// Line numbers of the iseq caller. + pub line_samples: Vec, +} + +/// Primitive called in zjit.rb +/// +/// Check if trace_exits generation is enabled. Requires the stats feature +/// to be 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) { + Qtrue + } else { + Qfalse + } +} + +/// Call the C function to parse the raw_samples and line_samples +/// 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) { + return Qnil; + } + + // Can safely unwrap since `trace_side_exits` must be true at this point + let zjit_raw_samples = ZJITState::get_raw_samples().unwrap(); + let zjit_line_samples = ZJITState::get_line_samples().unwrap(); + + assert_eq!(zjit_raw_samples.len(), zjit_line_samples.len()); + + // zjit_raw_samples and zjit_line_samples are the same length so + // pass only one of the lengths in the C function. + let samples_len = zjit_raw_samples.len() as i32; + + unsafe { + rb_zjit_exit_locations_dict( + zjit_raw_samples.as_mut_ptr(), + zjit_line_samples.as_mut_ptr(), + samples_len + ) + } +} From 8ce886b2d8bef0d0bc0edf8c1cd2c1348c1cbfef Mon Sep 17 00:00:00 2001 From: Burdette Lamar Date: Tue, 30 Sep 2025 11:02:27 -0500 Subject: [PATCH 4/7] [DOC] Tweaks for String#partition --- doc/string/partition.rdoc | 52 +++++++++++++++++++++++++++------------ string.c | 2 +- 2 files changed, 37 insertions(+), 17 deletions(-) diff --git a/doc/string/partition.rdoc b/doc/string/partition.rdoc index ebe575e8eb3c51..ece034ee66225e 100644 --- a/doc/string/partition.rdoc +++ b/doc/string/partition.rdoc @@ -1,24 +1,44 @@ Returns a 3-element array of substrings of +self+. -Matches a pattern against +self+, scanning from the beginning. -The pattern is: +If +pattern+ is matched, returns the array: -- +string_or_regexp+ itself, if it is a Regexp. -- Regexp.quote(string_or_regexp), if +string_or_regexp+ is a string. + [pre_match, first_match, post_match] -If the pattern is matched, returns pre-match, first-match, post-match: +where: - 'hello'.partition('l') # => ["he", "l", "lo"] - 'hello'.partition('ll') # => ["he", "ll", "o"] - 'hello'.partition('h') # => ["", "h", "ello"] - 'hello'.partition('o') # => ["hell", "o", ""] - 'hello'.partition(/l+/) #=> ["he", "ll", "o"] - 'hello'.partition('') # => ["", "", "hello"] - 'тест'.partition('т') # => ["", "т", "ест"] - 'こんにちは'.partition('に') # => ["こん", "に", "ちは"] +- +first_match+ is the first-found matching substring. +- +pre_match+ and +post_match+ are the preceding and following substrings. -If the pattern is not matched, returns a copy of +self+ and two empty strings: +If +pattern+ is not matched, returns the array: - 'hello'.partition('x') # => ["hello", "", ""] + [self.dup, "", ""] -Related: String#rpartition, String#split. +Note that in the examples below, a returned string 'hello' +is a copy of +self+, not +self+. + +If +pattern+ is a Regexp, performs the equivalent of self.match(pattern) +(also setting {pattern-matching global variables}[rdoc-ref:globals.md@Pattern+Matching]): + + 'hello'.partition(/h/) # => ["", "h", "ello"] + 'hello'.partition(/l/) # => ["he", "l", "lo"] + 'hello'.partition(/l+/) # => ["he", "ll", "o"] + 'hello'.partition(/o/) # => ["hell", "o", ""] + 'hello'.partition(/^/) # => ["", "", "hello"] + 'hello'.partition(//) # => ["", "", "hello"] + 'hello'.partition(/$/) # => ["hello", "", ""] + 'hello'.partition(/x/) # => ["hello", "", ""] + +If +pattern+ is not a Regexp, converts it to a string (if it is not already one), +then performs the equivalet of self.index(pattern) +(and does _not_ set {pattern-matching global variables}[rdoc-ref:globals.md@Pattern+Matching]): + + 'hello'.partition('h') # => ["", "h", "ello"] + 'hello'.partition('l') # => ["he", "l", "lo"] + 'hello'.partition('ll') # => ["he", "ll", "o"] + 'hello'.partition('o') # => ["hell", "o", ""] + 'hello'.partition('') # => ["", "", "hello"] + 'hello'.partition('x') # => ["hello", "", ""] + 'тест'.partition('т') # => ["", "т", "ест"] + 'こんにちは'.partition('に') # => ["こん", "に", "ちは"] + +Related: see {Converting to Non-String}[rdoc-ref:String@Converting+to+Non--5CString]. diff --git a/string.c b/string.c index 81353556c956c0..9d11936de7b700 100644 --- a/string.c +++ b/string.c @@ -11217,7 +11217,7 @@ rb_str_center(int argc, VALUE *argv, VALUE str) /* * call-seq: - * partition(string_or_regexp) -> [head, match, tail] + * partition(pattern) -> [pre_match, first_match, post_match] * * :include: doc/string/partition.rdoc * From 4ae5d69d1fca828ec1a50fab0126d88fc0a3f361 Mon Sep 17 00:00:00 2001 From: BurdetteLamar Date: Tue, 30 Sep 2025 18:10:21 +0100 Subject: [PATCH 5/7] [DOC] Tweaks for String#reverse --- string.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/string.c b/string.c index 9d11936de7b700..53bbeb419150fb 100644 --- a/string.c +++ b/string.c @@ -6983,12 +6983,16 @@ rb_str_bytesplice(int argc, VALUE *argv, VALUE str) /* * call-seq: - * reverse -> string + * reverse -> new_string * * Returns a new string with the characters from +self+ in reverse order. * - * 'stressed'.reverse # => "desserts" + * 'drawer'.reverse # => "reward" + * 'reviled'.reverse # => "deliver" + * 'stressed'.reverse # => "desserts" + * 'semordnilaps'.reverse # => "spalindromes" * + * Related: see {Converting to New String}[rdoc-ref:String@Converting+to+New+String]. */ static VALUE From 83f1082e638e6f9161c4ddd0acad7a658ce4648b Mon Sep 17 00:00:00 2001 From: Takashi Kokubun Date: Tue, 30 Sep 2025 10:50:33 -0700 Subject: [PATCH 6/7] ZJIT: Fix "malformed format string" on stats (#14681) --- zjit.rb | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/zjit.rb b/zjit.rb index 8289846c03b403..83f8adb866c663 100644 --- a/zjit.rb +++ b/zjit.rb @@ -221,8 +221,8 @@ def print_counters_with_prefix(buf:, stats:, prefix:, prompt:, limit: nil) return if stats.empty? counters.transform_keys! { |key| key.to_s.delete_prefix(prefix) } - left_pad = counters.keys.map(&:size).max - right_pad = counters.values.map { |value| number_with_delimiter(value).size }.max + key_pad = counters.keys.map(&:size).max + value_pad = counters.values.map { |value| number_with_delimiter(value).size }.max total = counters.values.sum counters = counters.to_a @@ -234,9 +234,7 @@ def print_counters_with_prefix(buf:, stats:, prefix:, prompt:, limit: nil) buf << " (%.1f%% of total #{number_with_delimiter(total)})" % (100.0 * counters.map(&:last).sum / total) if limit buf << ":\n" counters.each do |key, value| - padded_key = key.rjust(left_pad, ' ') - padded_value = number_with_delimiter(value).rjust(right_pad, ' ') - buf << " #{padded_key}: #{padded_value} (%4.1f%%)\n" % (100.0 * value / total) + buf << " %*s: %*s (%4.1f%%)\n" % [key_pad, key, value_pad, number_with_delimiter(value), (100.0 * value / total)] end end From 14fce2746acc4ff5963d8c296af8b952746e1241 Mon Sep 17 00:00:00 2001 From: Alan Wu Date: Mon, 29 Sep 2025 21:23:10 -0400 Subject: [PATCH 7/7] CI: Fail the dump crash log step for visual reminder I forgot that this step existed and thought crash reporting wasn't working when they were simply moved to a different step. Failing these should give a nice visual hint. --- .github/workflows/yjit-macos.yml | 7 +++++-- .github/workflows/yjit-ubuntu.yml | 4 +++- .github/workflows/zjit-macos.yml | 4 +++- .github/workflows/zjit-ubuntu.yml | 4 +++- 4 files changed, 14 insertions(+), 5 deletions(-) diff --git a/.github/workflows/yjit-macos.yml b/.github/workflows/yjit-macos.yml index fc538fe51be876..6299500ab34984 100644 --- a/.github/workflows/yjit-macos.yml +++ b/.github/workflows/yjit-macos.yml @@ -169,9 +169,12 @@ jobs: if: ${{ matrix.test_task == 'check' && matrix.skipped_tests }} continue-on-error: ${{ matrix.continue-on-skipped_tests || false }} - - if: ${{ failure() }} + - name: Dump crash logs + if: ${{ failure() }} continue-on-error: true - run: tail --verbose --lines=+1 rb_crash_*.txt + run: | + tail --verbose --lines=+1 rb_crash_*.txt + exit 1 - uses: ./.github/actions/slack with: diff --git a/.github/workflows/yjit-ubuntu.yml b/.github/workflows/yjit-ubuntu.yml index e086582e2430b3..bf12f80c0eae1a 100644 --- a/.github/workflows/yjit-ubuntu.yml +++ b/.github/workflows/yjit-ubuntu.yml @@ -214,7 +214,9 @@ jobs: - name: Dump crash logs if: ${{ failure() }} continue-on-error: true - run: tail --verbose --lines=+1 rb_crash_*.txt + run: | + tail --verbose --lines=+1 rb_crash_*.txt + exit 1 - uses: ./.github/actions/slack with: diff --git a/.github/workflows/zjit-macos.yml b/.github/workflows/zjit-macos.yml index bc4fef7ce2427f..a638a3b1b353dd 100644 --- a/.github/workflows/zjit-macos.yml +++ b/.github/workflows/zjit-macos.yml @@ -136,7 +136,9 @@ jobs: - name: Dump crash logs if: ${{ failure() }} continue-on-error: true - run: tail --verbose --lines=+1 rb_crash_*.txt + run: | + tail --verbose --lines=+1 rb_crash_*.txt + exit 1 - uses: ./.github/actions/slack with: diff --git a/.github/workflows/zjit-ubuntu.yml b/.github/workflows/zjit-ubuntu.yml index 813f88c53b6713..99d5dbfdedc390 100644 --- a/.github/workflows/zjit-ubuntu.yml +++ b/.github/workflows/zjit-ubuntu.yml @@ -178,7 +178,9 @@ jobs: - name: Dump crash logs if: ${{ failure() }} continue-on-error: true - run: tail --verbose --lines=+1 rb_crash_*.txt + run: | + tail --verbose --lines=+1 rb_crash_*.txt + exit 1 - uses: ./.github/actions/slack with: