From 231407c251d82573f578caf569a934c0ebb344e5 Mon Sep 17 00:00:00 2001 From: Takashi Kokubun Date: Tue, 12 Aug 2025 13:40:42 -0700 Subject: [PATCH] ZJIT: Avoid compiling failed ISEQs repeatedly (#14195) --- zjit/src/codegen.rs | 53 ++++++++++++++++++++++++++++++--------------- zjit/src/gc.rs | 16 ++++++++++---- 2 files changed, 48 insertions(+), 21 deletions(-) diff --git a/zjit/src/codegen.rs b/zjit/src/codegen.rs index 9fc3b643b7..01ed0f0590 100644 --- a/zjit/src/codegen.rs +++ b/zjit/src/codegen.rs @@ -5,7 +5,7 @@ use std::ffi::{c_int, c_void}; use crate::asm::Label; use crate::backend::current::{Reg, ALLOC_REGS}; use crate::invariants::{track_bop_assumption, track_cme_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}; +use crate::gc::{append_gc_offsets, get_or_create_iseq_payload, get_or_create_iseq_payload_ptr, IseqStatus}; use crate::state::ZJITState; use crate::stats::{counter_ptr, with_time_stat, Counter, Counter::compile_time_ns}; use crate::{asm::CodeBlock, cruby::*, options::debug, virtualmem::CodePtr}; @@ -136,7 +136,7 @@ fn gen_iseq_entry_point_body(cb: &mut CodeBlock, iseq: IseqPtr) -> Option Option<(CodePtr, Vec<(Rc, IseqPtr)>)> { // Return an existing pointer if it's already compiled let payload = get_or_create_iseq_payload(iseq); - if let Some(start_ptr) = payload.start_ptr { - return Some((start_ptr, vec![])); + match payload.status { + IseqStatus::Compiled(start_ptr) => return Some((start_ptr, vec![])), + IseqStatus::CantCompile => return None, + IseqStatus::NotCompiled => {}, } // Convert ISEQ into High-level IR and optimize HIR let function = match compile_iseq(iseq) { Some(function) => function, - None => return None, + None => { + payload.status = IseqStatus::CantCompile; + return None; + } }; // Compile the High-level IR let result = gen_function(cb, iseq, &function); if let Some((start_ptr, gc_offsets, jit)) = result { - payload.start_ptr = Some(start_ptr); + payload.status = IseqStatus::Compiled(start_ptr); append_gc_offsets(iseq, &gc_offsets); Some((start_ptr, jit.branch_iseqs)) } else { + payload.status = IseqStatus::CantCompile; None } } @@ -1356,27 +1362,40 @@ macro_rules! c_callable { pub(crate) use c_callable; c_callable! { - /// Generated code calls this function with the SysV calling convention. - /// See [gen_function_stub]. + /// Generated code calls this function with the SysV calling convention. See [gen_function_stub]. + /// This function is expected to be called repeatedly when ZJIT fails to compile the stub. + /// We should be able to compile most (if not all) function stubs by side-exiting at unsupported + /// instructions, so this should be used primarily for cb.has_dropped_bytes() situations. fn function_stub_hit(iseq: IseqPtr, branch_ptr: *const c_void, ec: EcPtr, sp: *mut VALUE) -> *const u8 { with_vm_lock(src_loc!(), || { - // Get a pointer to compiled code or the side-exit trampoline - let cb = ZJITState::get_code_block(); - let code_ptr = with_time_stat(compile_time_ns, || function_stub_hit_body(cb, iseq, branch_ptr)); - let code_ptr = if let Some(code_ptr) = code_ptr { - code_ptr - } else { - // gen_push_frame() doesn't set PC and SP, so we need to set them for side-exit - // TODO: We could generate code that sets PC/SP. Note that we'd still need to handle OOM. + /// gen_push_frame() doesn't set PC and SP, so we need to set them before exit + fn set_pc_and_sp(iseq: IseqPtr, ec: EcPtr, sp: *mut VALUE) { let cfp = unsafe { get_ec_cfp(ec) }; let pc = unsafe { rb_iseq_pc_at_idx(iseq, 0) }; // TODO: handle opt_pc once supported unsafe { rb_set_cfp_pc(cfp, pc) }; unsafe { rb_set_cfp_sp(cfp, sp) }; + } + // If we already know we can't compile the ISEQ, fail early without cb.mark_all_executable(). + // TODO: Alan thinks the payload status part of this check can happen without the VM lock, since the whole + // code path can be made read-only. But you still need the check as is while holding the VM lock in any case. + let cb = ZJITState::get_code_block(); + let payload = get_or_create_iseq_payload(iseq); + if cb.has_dropped_bytes() || payload.status == IseqStatus::CantCompile { // Exit to the interpreter + set_pc_and_sp(iseq, ec, sp); + return ZJITState::get_stub_exit().raw_ptr(cb); + } + + // Otherwise, attempt to compile the ISEQ. We have to mark_all_executable() beyond this point. + let code_ptr = with_time_stat(compile_time_ns, || function_stub_hit_body(cb, iseq, branch_ptr)); + let code_ptr = if let Some(code_ptr) = code_ptr { + code_ptr + } else { + // Exit to the interpreter + set_pc_and_sp(iseq, ec, sp); ZJITState::get_stub_exit() }; - cb.mark_all_executable(); code_ptr.raw_ptr(cb) }) diff --git a/zjit/src/gc.rs b/zjit/src/gc.rs index ea1b0ed2ea..52a036d49e 100644 --- a/zjit/src/gc.rs +++ b/zjit/src/gc.rs @@ -7,12 +7,12 @@ use crate::stats::Counter::gc_time_ns; /// This is all the data ZJIT stores on an ISEQ. We mark objects in this struct on GC. #[derive(Debug)] pub struct IseqPayload { + /// Compilation status of the ISEQ. It has the JIT code address of the first block if Compiled. + pub status: IseqStatus, + /// Type information of YARV instruction operands pub profile: IseqProfile, - /// JIT code address of the first block - pub start_ptr: Option, - /// GC offsets of the JIT code. These are the addresses of objects that need to be marked. pub gc_offsets: Vec, } @@ -20,13 +20,21 @@ pub struct IseqPayload { impl IseqPayload { fn new(iseq_size: u32) -> Self { Self { + status: IseqStatus::NotCompiled, profile: IseqProfile::new(iseq_size), - start_ptr: None, gc_offsets: vec![], } } } +#[derive(Debug, PartialEq)] +pub enum IseqStatus { + /// CodePtr has the JIT code address of the first block + Compiled(CodePtr), + CantCompile, + NotCompiled, +} + /// Get a pointer to the payload object associated with an ISEQ. Create one if none exists. pub fn get_or_create_iseq_payload_ptr(iseq: IseqPtr) -> *mut IseqPayload { type VoidPtr = *mut c_void;