From 3640cfe5e0f3ed30f90ae622ef92e6c154213a09 Mon Sep 17 00:00:00 2001 From: Alex Rocha <9896751+alexcrocha@users.noreply.github.com> Date: Wed, 10 Dec 2025 12:30:04 -0800 Subject: [PATCH 01/12] ZJIT: Use inline format args (#15482) --- zjit/src/asm/arm64/arg/sf.rs | 2 +- zjit/src/asm/arm64/inst/atomic.rs | 2 +- zjit/src/asm/arm64/inst/load_literal.rs | 2 +- zjit/src/asm/arm64/inst/load_register.rs | 2 +- zjit/src/asm/arm64/inst/load_store.rs | 2 +- .../asm/arm64/inst/load_store_exclusive.rs | 2 +- zjit/src/asm/arm64/inst/mov.rs | 2 +- zjit/src/asm/arm64/inst/reg_pair.rs | 2 +- zjit/src/asm/arm64/inst/test_bit.rs | 2 +- zjit/src/asm/arm64/mod.rs | 2 +- zjit/src/asm/mod.rs | 2 +- zjit/src/backend/arm64/mod.rs | 4 ++-- zjit/src/codegen.rs | 4 ++-- zjit/src/cruby.rs | 2 +- zjit/src/hir.rs | 18 ++++++++--------- zjit/src/hir_type/mod.rs | 2 +- zjit/src/json.rs | 20 +++++++++---------- zjit/src/options.rs | 10 +++++----- zjit/src/state.rs | 2 +- zjit/src/stats.rs | 6 +++--- 20 files changed, 45 insertions(+), 45 deletions(-) diff --git a/zjit/src/asm/arm64/arg/sf.rs b/zjit/src/asm/arm64/arg/sf.rs index c2fd33302c1ef8..b6091821e906fb 100644 --- a/zjit/src/asm/arm64/arg/sf.rs +++ b/zjit/src/asm/arm64/arg/sf.rs @@ -13,7 +13,7 @@ impl From for Sf { match num_bits { 64 => Sf::Sf64, 32 => Sf::Sf32, - _ => panic!("Invalid number of bits: {}", num_bits) + _ => panic!("Invalid number of bits: {num_bits}"), } } } diff --git a/zjit/src/asm/arm64/inst/atomic.rs b/zjit/src/asm/arm64/inst/atomic.rs index dce9affedf8838..0917a4fd1c17ea 100644 --- a/zjit/src/asm/arm64/inst/atomic.rs +++ b/zjit/src/asm/arm64/inst/atomic.rs @@ -14,7 +14,7 @@ impl From for Size { match num_bits { 64 => Size::Size64, 32 => Size::Size32, - _ => panic!("Invalid number of bits: {}", num_bits) + _ => panic!("Invalid number of bits: {num_bits}"), } } } diff --git a/zjit/src/asm/arm64/inst/load_literal.rs b/zjit/src/asm/arm64/inst/load_literal.rs index a429c0fb53d082..37b5f3c7a7546b 100644 --- a/zjit/src/asm/arm64/inst/load_literal.rs +++ b/zjit/src/asm/arm64/inst/load_literal.rs @@ -15,7 +15,7 @@ impl From for Opc { match num_bits { 64 => Opc::Size64, 32 => Opc::Size32, - _ => panic!("Invalid number of bits: {}", num_bits) + _ => panic!("Invalid number of bits: {num_bits}"), } } } diff --git a/zjit/src/asm/arm64/inst/load_register.rs b/zjit/src/asm/arm64/inst/load_register.rs index 3d94e8da1f8f26..80813ffc870a42 100644 --- a/zjit/src/asm/arm64/inst/load_register.rs +++ b/zjit/src/asm/arm64/inst/load_register.rs @@ -25,7 +25,7 @@ impl From for Size { match num_bits { 64 => Size::Size64, 32 => Size::Size32, - _ => panic!("Invalid number of bits: {}", num_bits) + _ => panic!("Invalid number of bits: {num_bits}"), } } } diff --git a/zjit/src/asm/arm64/inst/load_store.rs b/zjit/src/asm/arm64/inst/load_store.rs index e78edd56752aae..d38e851ed73e30 100644 --- a/zjit/src/asm/arm64/inst/load_store.rs +++ b/zjit/src/asm/arm64/inst/load_store.rs @@ -15,7 +15,7 @@ impl From for Size { match num_bits { 64 => Size::Size64, 32 => Size::Size32, - _ => panic!("Invalid number of bits: {}", num_bits) + _ => panic!("Invalid number of bits: {num_bits}"), } } } diff --git a/zjit/src/asm/arm64/inst/load_store_exclusive.rs b/zjit/src/asm/arm64/inst/load_store_exclusive.rs index 1106b4cb372eb1..30cb663bdb9c67 100644 --- a/zjit/src/asm/arm64/inst/load_store_exclusive.rs +++ b/zjit/src/asm/arm64/inst/load_store_exclusive.rs @@ -17,7 +17,7 @@ impl From for Size { match num_bits { 64 => Size::Size64, 32 => Size::Size32, - _ => panic!("Invalid number of bits: {}", num_bits) + _ => panic!("Invalid number of bits: {num_bits}"), } } } diff --git a/zjit/src/asm/arm64/inst/mov.rs b/zjit/src/asm/arm64/inst/mov.rs index 70814938056f15..e9f9091713107b 100644 --- a/zjit/src/asm/arm64/inst/mov.rs +++ b/zjit/src/asm/arm64/inst/mov.rs @@ -27,7 +27,7 @@ impl From for Hw { 16 => Hw::LSL16, 32 => Hw::LSL32, 48 => Hw::LSL48, - _ => panic!("Invalid value for shift: {}", shift) + _ => panic!("Invalid value for shift: {shift}"), } } } diff --git a/zjit/src/asm/arm64/inst/reg_pair.rs b/zjit/src/asm/arm64/inst/reg_pair.rs index 9bffcd847925c1..39a44c24167370 100644 --- a/zjit/src/asm/arm64/inst/reg_pair.rs +++ b/zjit/src/asm/arm64/inst/reg_pair.rs @@ -26,7 +26,7 @@ impl From for Opc { match num_bits { 64 => Opc::Opc64, 32 => Opc::Opc32, - _ => panic!("Invalid number of bits: {}", num_bits) + _ => panic!("Invalid number of bits: {num_bits}"), } } } diff --git a/zjit/src/asm/arm64/inst/test_bit.rs b/zjit/src/asm/arm64/inst/test_bit.rs index f7aeca70fda8af..45f0c2317ec974 100644 --- a/zjit/src/asm/arm64/inst/test_bit.rs +++ b/zjit/src/asm/arm64/inst/test_bit.rs @@ -17,7 +17,7 @@ impl From for B5 { match bit_num { 0..=31 => B5::B532, 32..=63 => B5::B564, - _ => panic!("Invalid bit number: {}", bit_num) + _ => panic!("Invalid bit number: {bit_num}"), } } } diff --git a/zjit/src/asm/arm64/mod.rs b/zjit/src/asm/arm64/mod.rs index 4094d101fbc32c..c30520cd693f14 100644 --- a/zjit/src/asm/arm64/mod.rs +++ b/zjit/src/asm/arm64/mod.rs @@ -241,7 +241,7 @@ pub fn asr(cb: &mut CodeBlock, rd: A64Opnd, rn: A64Opnd, shift: A64Opnd) { SBFM::asr(rd.reg_no, rn.reg_no, shift.try_into().unwrap(), rd.num_bits).into() }, - _ => panic!("Invalid operand combination to asr instruction: asr {:?}, {:?}, {:?}", rd, rn, shift), + _ => panic!("Invalid operand combination to asr instruction: asr {rd:?}, {rn:?}, {shift:?}"), }; cb.write_bytes(&bytes); diff --git a/zjit/src/asm/mod.rs b/zjit/src/asm/mod.rs index 9b792f5f376325..90496245295f88 100644 --- a/zjit/src/asm/mod.rs +++ b/zjit/src/asm/mod.rs @@ -354,7 +354,7 @@ impl fmt::LowerHex for CodeBlock { for pos in 0..self.write_pos { let mem_block = &*self.mem_block.borrow(); let byte = unsafe { mem_block.start_ptr().raw_ptr(mem_block).add(pos).read() }; - fmtr.write_fmt(format_args!("{:02x}", byte))?; + fmtr.write_fmt(format_args!("{byte:02x}"))?; } Ok(()) } diff --git a/zjit/src/backend/arm64/mod.rs b/zjit/src/backend/arm64/mod.rs index 78fb69b0b04d5b..f927f261cf39b8 100644 --- a/zjit/src/backend/arm64/mod.rs +++ b/zjit/src/backend/arm64/mod.rs @@ -1311,7 +1311,7 @@ impl Assembler { 64 | 32 => stur(cb, src, dest.into()), 16 => sturh(cb, src, dest.into()), 8 => sturb(cb, src, dest.into()), - num_bits => panic!("unexpected dest num_bits: {} (src: {:?}, dest: {:?})", num_bits, src, dest), + num_bits => panic!("unexpected dest num_bits: {num_bits} (src: {src:?}, dest: {dest:?})"), } }, Insn::Load { opnd, out } | @@ -1331,7 +1331,7 @@ impl Assembler { 64 | 32 => ldur(cb, out.into(), opnd.into()), 16 => ldurh(cb, out.into(), opnd.into()), 8 => ldurb(cb, out.into(), opnd.into()), - num_bits => panic!("unexpected num_bits: {}", num_bits) + num_bits => panic!("unexpected num_bits: {num_bits}"), }; }, Opnd::Value(value) => { diff --git a/zjit/src/codegen.rs b/zjit/src/codegen.rs index 05b704d66a4bb4..11b60c546c9716 100644 --- a/zjit/src/codegen.rs +++ b/zjit/src/codegen.rs @@ -160,7 +160,7 @@ fn register_with_perf(iseq_name: String, start_ptr: usize, code_size: usize) { return; }; let mut file = std::io::BufWriter::new(file); - let Ok(_) = writeln!(file, "{:#x} {:#x} zjit::{}", start_ptr, code_size, iseq_name) else { + let Ok(_) = writeln!(file, "{start_ptr:#x} {code_size:#x} zjit::{iseq_name}") else { debug!("Failed to write {iseq_name} to perf map file: {perf_map}"); return; }; @@ -2019,7 +2019,7 @@ fn gen_guard_bit_equals(jit: &mut JITState, asm: &mut Assembler, val: lir::Opnd, let expected_opnd: Opnd = match expected { crate::hir::Const::Value(v) => { Opnd::Value(v) } crate::hir::Const::CInt64(v) => { v.into() } - _ => panic!("gen_guard_bit_equals: unexpected hir::Const {:?}", expected), + _ => panic!("gen_guard_bit_equals: unexpected hir::Const {expected:?}"), }; asm.cmp(val, expected_opnd); asm.jnz(side_exit(jit, state, GuardBitEquals(expected))); diff --git a/zjit/src/cruby.rs b/zjit/src/cruby.rs index e653602874df77..2bd44d21443b8d 100644 --- a/zjit/src/cruby.rs +++ b/zjit/src/cruby.rs @@ -986,7 +986,7 @@ pub fn rb_bug_panic_hook() { // You may also use ZJIT_RB_BUG=1 to trigger this on dev builds. if release_build || env::var("ZJIT_RB_BUG").is_ok() { // Abort with rb_bug(). It has a length limit on the message. - let panic_message = &format!("{}", panic_info)[..]; + let panic_message = &format!("{panic_info}")[..]; let len = std::cmp::min(0x100, panic_message.len()) as c_int; unsafe { rb_bug(b"ZJIT: %*s\0".as_ref().as_ptr() as *const c_char, len, panic_message.as_ptr()); } } else { diff --git a/zjit/src/hir.rs b/zjit/src/hir.rs index 1771f960c3f952..1731667442792b 100644 --- a/zjit/src/hir.rs +++ b/zjit/src/hir.rs @@ -176,7 +176,7 @@ impl From for SpecialObjectType { VM_SPECIAL_OBJECT_VMCORE => SpecialObjectType::VMCore, VM_SPECIAL_OBJECT_CBASE => SpecialObjectType::CBase, VM_SPECIAL_OBJECT_CONST_BASE => SpecialObjectType::ConstBase, - _ => panic!("Invalid special object type: {}", value), + _ => panic!("Invalid special object type: {value}"), } } } @@ -337,7 +337,7 @@ impl std::fmt::Display for RangeType { impl std::fmt::Debug for RangeType { fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result { - write!(f, "{}", self) + write!(f, "{self}") } } @@ -346,7 +346,7 @@ impl From for RangeType { match flag { 0 => RangeType::Inclusive, 1 => RangeType::Exclusive, - _ => panic!("Invalid range flag: {}", flag), + _ => panic!("Invalid range flag: {flag}"), } } } @@ -369,7 +369,7 @@ impl TryFrom for SpecialBackrefSymbol { '`' => Ok(SpecialBackrefSymbol::PreMatch), '\'' => Ok(SpecialBackrefSymbol::PostMatch), '+' => Ok(SpecialBackrefSymbol::LastGroup), - c => Err(format!("invalid backref symbol: '{}'", c)), + c => Err(format!("invalid backref symbol: '{c}'")), } } } @@ -574,7 +574,7 @@ impl std::fmt::Display for SideExitReason { SideExitReason::UnhandledNewarraySend(VM_OPT_NEWARRAY_SEND_PACK) => write!(f, "UnhandledNewarraySend(PACK)"), SideExitReason::UnhandledNewarraySend(VM_OPT_NEWARRAY_SEND_PACK_BUFFER) => write!(f, "UnhandledNewarraySend(PACK_BUFFER)"), SideExitReason::UnhandledNewarraySend(VM_OPT_NEWARRAY_SEND_INCLUDE_P) => write!(f, "UnhandledNewarraySend(INCLUDE_P)"), - SideExitReason::UnhandledDuparraySend(method_id) => write!(f, "UnhandledDuparraySend({})", method_id), + SideExitReason::UnhandledDuparraySend(method_id) => write!(f, "UnhandledDuparraySend({method_id})"), SideExitReason::GuardType(guard_type) => write!(f, "GuardType({guard_type})"), SideExitReason::GuardTypeNot(guard_type) => write!(f, "GuardTypeNot({guard_type})"), SideExitReason::GuardBitEquals(value) => write!(f, "GuardBitEquals({})", value.print(&PtrPrintMap::identity())), @@ -3572,10 +3572,10 @@ impl Function { // rb_zjit_singleton_class_p also checks if it's a class if unsafe { rb_zjit_singleton_class_p(class) } { let class_name = get_class_name(unsafe { rb_class_attached_object(class) }); - format!("{}.{}", class_name, method_name) + format!("{class_name}.{method_name}") } else { let class_name = get_class_name(class); - format!("{}#{}", class_name, method_name) + format!("{class_name}#{method_name}") } } @@ -4372,7 +4372,7 @@ impl Function { .insert("name", function_name) .insert("passes", passes) .build(); - writeln!(file, "{}", json).unwrap(); + writeln!(file, "{json}").unwrap(); } /// Validates the following: @@ -4500,7 +4500,7 @@ impl Function { fn assert_subtype(&self, user: InsnId, operand: InsnId, expected: Type) -> Result<(), ValidationError> { let actual = self.type_of(operand); if !actual.is_subtype(expected) { - return Err(ValidationError::MismatchedOperandType(user, operand, format!("{}", expected), format!("{}", actual))); + return Err(ValidationError::MismatchedOperandType(user, operand, format!("{expected}"), format!("{actual}"))); } Ok(()) } diff --git a/zjit/src/hir_type/mod.rs b/zjit/src/hir_type/mod.rs index a7ffcd1b77e882..206d94f42bdb64 100644 --- a/zjit/src/hir_type/mod.rs +++ b/zjit/src/hir_type/mod.rs @@ -96,7 +96,7 @@ fn write_spec(f: &mut std::fmt::Formatter, printer: &TypePrinter) -> std::fmt::R Specialization::Int(val) if ty.is_subtype(types::CUInt8) => write!(f, "[{}]", val & u8::MAX as u64), Specialization::Int(val) if ty.is_subtype(types::CUInt16) => write!(f, "[{}]", val & u16::MAX as u64), Specialization::Int(val) if ty.is_subtype(types::CUInt32) => write!(f, "[{}]", val & u32::MAX as u64), - Specialization::Int(val) if ty.is_subtype(types::CUInt64) => write!(f, "[{}]", val), + Specialization::Int(val) if ty.is_subtype(types::CUInt64) => write!(f, "[{val}]"), Specialization::Int(val) if ty.is_subtype(types::CPtr) => write!(f, "[{}]", Const::CPtr(val as *const u8).print(printer.ptr_map)), Specialization::Int(val) => write!(f, "[{val}]"), Specialization::Double(val) => write!(f, "[{val}]"), diff --git a/zjit/src/json.rs b/zjit/src/json.rs index 1e63ba7b425902..fa4b2168217868 100644 --- a/zjit/src/json.rs +++ b/zjit/src/json.rs @@ -43,9 +43,9 @@ impl Json { match self { Json::Null => writer.write_all(b"null"), Json::Bool(b) => writer.write_all(if *b { b"true" } else { b"false" }), - Json::Integer(i) => write!(writer, "{}", i), - Json::UnsignedInteger(u) => write!(writer, "{}", u), - Json::Floating(f) => write!(writer, "{}", f), + Json::Integer(i) => write!(writer, "{i}"), + Json::UnsignedInteger(u) => write!(writer, "{u}"), + Json::Floating(f) => write!(writer, "{f}"), Json::String(s) => return Self::write_str(writer, s), Json::Array(jsons) => return Self::write_array(writer, jsons), Json::Object(map) => return Self::write_object(writer, map), @@ -69,9 +69,9 @@ impl Json { '\x0C' => write!(writer, "\\f")?, ch if ch.is_control() => { let code_point = ch as u32; - write!(writer, "\\u{:04X}", code_point)? + write!(writer, "\\u{code_point:04X}")? } - _ => write!(writer, "{}", ch)?, + _ => write!(writer, "{ch}")?, }; } @@ -83,7 +83,7 @@ impl Json { writer.write_all(b"[")?; let mut prefix = ""; for item in jsons { - write!(writer, "{}", prefix)?; + write!(writer, "{prefix}")?; item.marshal(writer)?; prefix = ", "; } @@ -96,7 +96,7 @@ impl Json { let mut prefix = ""; for (k, v) in pairs { // Escape the keys, despite not being `Json::String` objects. - write!(writer, "{}", prefix)?; + write!(writer, "{prefix}")?; Self::write_str(writer, k)?; writer.write_all(b":")?; v.marshal(writer)?; @@ -112,7 +112,7 @@ impl std::fmt::Display for Json { let mut buf = Vec::new(); self.marshal(&mut buf).map_err(|_| std::fmt::Error)?; let s = String::from_utf8(buf).map_err(|_| std::fmt::Error)?; - write!(f, "{}", s) + write!(f, "{s}") } } @@ -225,8 +225,8 @@ impl From for JsonError { impl fmt::Display for JsonError { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { match self { - JsonError::FloatError(v) => write!(f, "Cannot serialize float {}", v), - JsonError::IoError(e) => write!(f, "{}", e), + JsonError::FloatError(v) => write!(f, "Cannot serialize float {v}"), + JsonError::IoError(e) => write!(f, "{e}"), } } } diff --git a/zjit/src/options.rs b/zjit/src/options.rs index ea9dc3249b82ff..40b49146b726a3 100644 --- a/zjit/src/options.rs +++ b/zjit/src/options.rs @@ -247,11 +247,11 @@ fn parse_jit_list(path_like: &str) -> HashSet { } } } else { - eprintln!("Failed to read JIT list from '{}'", path_like); + eprintln!("Failed to read JIT list from '{path_like}'"); } eprintln!("JIT list:"); for item in &result { - eprintln!(" {}", item); + eprintln!(" {item}"); } result } @@ -373,7 +373,7 @@ fn parse_option(str_ptr: *const std::os::raw::c_char) -> Option<()> { .write(true) .truncate(true) .open(opt_val) - .map_err(|e| eprintln!("Failed to open file '{}': {}", opt_val, e)) + .map_err(|e| eprintln!("Failed to open file '{opt_val}': {e}")) .ok(); let opt_val = std::fs::canonicalize(opt_val).unwrap_or_else(|_| opt_val.into()); options.dump_hir_graphviz = Some(opt_val); @@ -400,7 +400,7 @@ fn parse_option(str_ptr: *const std::os::raw::c_char) -> Option<()> { _ => { let valid_options = DUMP_LIR_ALL.iter().map(|opt| format!("{opt:?}")).collect::>().join(", "); eprintln!("invalid --zjit-dump-lir option: '{filter}'"); - eprintln!("valid --zjit-dump-lir options: all, {}", valid_options); + eprintln!("valid --zjit-dump-lir options: all, {valid_options}"); return None; } }; @@ -421,7 +421,7 @@ fn parse_option(str_ptr: *const std::os::raw::c_char) -> Option<()> { .write(true) .truncate(true) .open(opt_val) - .map_err(|e| eprintln!("Failed to open file '{}': {}", opt_val, e)) + .map_err(|e| eprintln!("Failed to open file '{opt_val}': {e}")) .ok(); let opt_val = std::fs::canonicalize(opt_val).unwrap_or_else(|_| opt_val.into()); options.log_compiled_iseqs = Some(opt_val); diff --git a/zjit/src/state.rs b/zjit/src/state.rs index fd59161812a7ee..a807be3f12d4c9 100644 --- a/zjit/src/state.rs +++ b/zjit/src/state.rs @@ -241,7 +241,7 @@ impl ZJITState { return; } }; - if let Err(e) = writeln!(file, "{}", iseq_name) { + if let Err(e) = writeln!(file, "{iseq_name}") { eprintln!("ZJIT: Failed to write to file '{}': {}", filename.display(), e); } } diff --git a/zjit/src/stats.rs b/zjit/src/stats.rs index 7d54f40c8d5dbd..37345f93435185 100644 --- a/zjit/src/stats.rs +++ b/zjit/src/stats.rs @@ -786,21 +786,21 @@ pub extern "C" fn rb_zjit_stats(_ec: EcPtr, _self: VALUE, target_key: VALUE) -> // Set not inlined cfunc counters let not_inlined_cfuncs = ZJITState::get_not_inlined_cfunc_counter_pointers(); for (signature, counter) in not_inlined_cfuncs.iter() { - let key_string = format!("not_inlined_cfuncs_{}", signature); + let key_string = format!("not_inlined_cfuncs_{signature}"); set_stat_usize!(hash, &key_string, **counter); } // Set not annotated cfunc counters let not_annotated_cfuncs = ZJITState::get_not_annotated_cfunc_counter_pointers(); for (signature, counter) in not_annotated_cfuncs.iter() { - let key_string = format!("not_annotated_cfuncs_{}", signature); + let key_string = format!("not_annotated_cfuncs_{signature}"); set_stat_usize!(hash, &key_string, **counter); } // Set ccall counters let ccall = ZJITState::get_ccall_counter_pointers(); for (signature, counter) in ccall.iter() { - let key_string = format!("ccall_{}", signature); + let key_string = format!("ccall_{signature}"); set_stat_usize!(hash, &key_string, **counter); } From ccfd31162a8455ac2502ebaf90873b966a866216 Mon Sep 17 00:00:00 2001 From: Jean Boussier Date: Wed, 10 Dec 2025 21:23:08 +0100 Subject: [PATCH 02/12] Remove object_id in NEWOBJ tracepoint test Generating an object_id for any type other than T_OBJECT (and T_CLASS) will inevitably allocate an IMEMO/fields objects, which isn't supported in a NEWOBJ tracepoint. See: https://bugs.ruby-lang.org/issues/21710#note-23 --- ext/-test-/tracepoint/tracepoint.c | 20 -------------------- test/-ext-/tracepoint/test_tracepoint.rb | 18 ------------------ 2 files changed, 38 deletions(-) diff --git a/ext/-test-/tracepoint/tracepoint.c b/ext/-test-/tracepoint/tracepoint.c index 7f7aa246628858..001d9513b29fb2 100644 --- a/ext/-test-/tracepoint/tracepoint.c +++ b/ext/-test-/tracepoint/tracepoint.c @@ -86,25 +86,6 @@ tracepoint_specify_normal_and_internal_events(VALUE self) return Qnil; /* should not be reached */ } -int rb_objspace_internal_object_p(VALUE obj); - -static void -on_newobj_event(VALUE tpval, void *data) -{ - VALUE obj = rb_tracearg_object(rb_tracearg_from_tracepoint(tpval)); - if (!rb_objspace_internal_object_p(obj)) rb_obj_id(obj); -} - -static VALUE -add_object_id(RB_UNUSED_VAR(VALUE _)) -{ - VALUE tp = rb_tracepoint_new(0, RUBY_INTERNAL_EVENT_NEWOBJ, on_newobj_event, NULL); - rb_tracepoint_enable(tp); - rb_yield(Qnil); - rb_tracepoint_disable(tp); - return Qnil; -} - void Init_gc_hook(VALUE); void @@ -114,5 +95,4 @@ Init_tracepoint(void) Init_gc_hook(tp_mBug); rb_define_module_function(tp_mBug, "tracepoint_track_objspace_events", tracepoint_track_objspace_events, 0); rb_define_module_function(tp_mBug, "tracepoint_specify_normal_and_internal_events", tracepoint_specify_normal_and_internal_events, 0); - rb_define_singleton_method(tp_mBug, "tracepoint_add_object_id", add_object_id, 0); } diff --git a/test/-ext-/tracepoint/test_tracepoint.rb b/test/-ext-/tracepoint/test_tracepoint.rb index 2256f58bc710b0..debddd83d043fe 100644 --- a/test/-ext-/tracepoint/test_tracepoint.rb +++ b/test/-ext-/tracepoint/test_tracepoint.rb @@ -82,24 +82,6 @@ def run(hook) end end - def test_tracepoint_add_object_id - Bug.tracepoint_add_object_id do - klass = Struct.new - 2.times { klass.new } - - klass = Struct.new(:a) - 2.times { klass.new } - - klass = Struct.new(:a, :b, :c) - 2.times { klass.new } - - 2.times { Set.new } # To test T_DATA / TypedData RUBY_TYPED_EMBEDDABLE - 2.times { Proc.new { } } # To test T_DATA / TypedData non embeddable - - 2.times { Object.new } - end - end - def test_teardown_with_active_GC_end_hook assert_separately([], 'require("-test-/tracepoint"); Bug.after_gc_exit_hook = proc {}; GC.start') end From c8909030974772b4f19742bac55875e32674d27f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tobias=20L=C3=BCtke?= Date: Wed, 10 Dec 2025 16:18:08 -0500 Subject: [PATCH 03/12] ZJIT: Fold LoadField on frozen objects to constants (#15483) * ZJIT: Fold LoadField on frozen objects to constants When accessing instance variables from frozen objects via attr_reader/ attr_accessor, fold the LoadField instruction to a constant at compile time. This enables further optimizations like constant propagation. - Add fold_getinstancevariable_frozen optimization in Function::optimize - Check if receiver type has a known ruby_object() that is frozen - Read the field value at compile time and replace with Const instruction - Add 10 unit tests covering various value types (fixnum, string, symbol, nil, true/false) and negative cases (unfrozen, dynamic receiver) * Run zjit-test-update * Add a test that we don't fold non-BasicObject * Small cleanups --------- Co-authored-by: Max Bernstein Co-authored-by: Aaron Patterson --- zjit/src/hir.rs | 13 ++ zjit/src/hir/opt_tests.rs | 431 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 444 insertions(+) diff --git a/zjit/src/hir.rs b/zjit/src/hir.rs index 1731667442792b..deae6c3b653681 100644 --- a/zjit/src/hir.rs +++ b/zjit/src/hir.rs @@ -3682,6 +3682,19 @@ impl Function { // Don't bother re-inferring the type of val; we already know it. continue; } + Insn::LoadField { recv, offset, return_type, .. } if return_type.is_subtype(types::BasicObject) => { + let recv_type = self.type_of(recv); + match recv_type.ruby_object() { + Some(recv_obj) if recv_obj.is_frozen() => { + let recv_ptr = recv_obj.as_ptr() as *const VALUE; + // Rust pointer .add() scales by size_of::() and offset is + // already scaled by SIZEOF_VALUE, so undo that. + let val = unsafe { *recv_ptr.add(offset as usize / SIZEOF_VALUE) }; + self.new_insn(Insn::Const { val: Const::Value(val) }) + } + _ => insn_id, + } + } Insn::AnyToString { str, .. } if self.is_a(str, types::String) => { self.make_equal_to(insn_id, str); // Don't bother re-inferring the type of str; we already know it. diff --git a/zjit/src/hir/opt_tests.rs b/zjit/src/hir/opt_tests.rs index 2e20c8fe8a73e2..62ea7c11b0c672 100644 --- a/zjit/src/hir/opt_tests.rs +++ b/zjit/src/hir/opt_tests.rs @@ -9501,4 +9501,435 @@ mod hir_opt_tests { Return v65 "); } + + #[test] + fn test_fold_load_field_frozen_constant_object() { + // Basic case: frozen constant object with attr_accessor + eval(" + class TestFrozen + attr_accessor :a + def initialize + @a = 1 + end + end + + FROZEN_OBJ = TestFrozen.new.freeze + + def test = FROZEN_OBJ.a + test + test + "); + assert_snapshot!(hir_string("test"), @r" + fn test@:11: + bb0(): + EntryPoint interpreter + v1:BasicObject = LoadSelf + Jump bb2(v1) + bb1(v4:BasicObject): + EntryPoint JIT(0) + Jump bb2(v4) + bb2(v6:BasicObject): + PatchPoint SingleRactorMode + PatchPoint StableConstantNames(0x1000, FROZEN_OBJ) + v20:HeapObject[VALUE(0x1008)] = Const Value(VALUE(0x1008)) + PatchPoint MethodRedefined(TestFrozen@0x1010, a@0x1018, cme:0x1020) + PatchPoint NoSingletonClass(TestFrozen@0x1010) + v25:HeapObject[VALUE(0x1008)] = GuardShape v20, 0x1048 + v27:Fixnum[1] = Const Value(1) + CheckInterrupts + Return v27 + "); + } + + #[test] + fn test_fold_load_field_frozen_multiple_ivars() { + // Frozen object with multiple instance variables + eval(" + class TestMultiIvars + attr_accessor :a, :b, :c + def initialize + @a = 10 + @b = 20 + @c = 30 + end + end + + MULTI_FROZEN = TestMultiIvars.new.freeze + + def test = MULTI_FROZEN.b + test + test + "); + assert_snapshot!(hir_string("test"), @r" + fn test@:13: + bb0(): + EntryPoint interpreter + v1:BasicObject = LoadSelf + Jump bb2(v1) + bb1(v4:BasicObject): + EntryPoint JIT(0) + Jump bb2(v4) + bb2(v6:BasicObject): + PatchPoint SingleRactorMode + PatchPoint StableConstantNames(0x1000, MULTI_FROZEN) + v20:HeapObject[VALUE(0x1008)] = Const Value(VALUE(0x1008)) + PatchPoint MethodRedefined(TestMultiIvars@0x1010, b@0x1018, cme:0x1020) + PatchPoint NoSingletonClass(TestMultiIvars@0x1010) + v25:HeapObject[VALUE(0x1008)] = GuardShape v20, 0x1048 + v27:Fixnum[20] = Const Value(20) + CheckInterrupts + Return v27 + "); + } + + #[test] + fn test_fold_load_field_frozen_string_value() { + // Frozen object with a string ivar + eval(r#" + class TestFrozenStr + attr_accessor :name + def initialize + @name = "hello" + end + end + + FROZEN_STR = TestFrozenStr.new.freeze + + def test = FROZEN_STR.name + test + test + "#); + assert_snapshot!(hir_string("test"), @r" + fn test@:11: + bb0(): + EntryPoint interpreter + v1:BasicObject = LoadSelf + Jump bb2(v1) + bb1(v4:BasicObject): + EntryPoint JIT(0) + Jump bb2(v4) + bb2(v6:BasicObject): + PatchPoint SingleRactorMode + PatchPoint StableConstantNames(0x1000, FROZEN_STR) + v20:HeapObject[VALUE(0x1008)] = Const Value(VALUE(0x1008)) + PatchPoint MethodRedefined(TestFrozenStr@0x1010, name@0x1018, cme:0x1020) + PatchPoint NoSingletonClass(TestFrozenStr@0x1010) + v25:HeapObject[VALUE(0x1008)] = GuardShape v20, 0x1048 + v27:StringExact[VALUE(0x1050)] = Const Value(VALUE(0x1050)) + CheckInterrupts + Return v27 + "); + } + + #[test] + fn test_fold_load_field_frozen_nil_value() { + // Frozen object with nil ivar + eval(" + class TestFrozenNil + attr_accessor :value + def initialize + @value = nil + end + end + + FROZEN_NIL = TestFrozenNil.new.freeze + + def test = FROZEN_NIL.value + test + test + "); + assert_snapshot!(hir_string("test"), @r" + fn test@:11: + bb0(): + EntryPoint interpreter + v1:BasicObject = LoadSelf + Jump bb2(v1) + bb1(v4:BasicObject): + EntryPoint JIT(0) + Jump bb2(v4) + bb2(v6:BasicObject): + PatchPoint SingleRactorMode + PatchPoint StableConstantNames(0x1000, FROZEN_NIL) + v20:HeapObject[VALUE(0x1008)] = Const Value(VALUE(0x1008)) + PatchPoint MethodRedefined(TestFrozenNil@0x1010, value@0x1018, cme:0x1020) + PatchPoint NoSingletonClass(TestFrozenNil@0x1010) + v25:HeapObject[VALUE(0x1008)] = GuardShape v20, 0x1048 + v27:NilClass = Const Value(nil) + CheckInterrupts + Return v27 + "); + } + + #[test] + fn test_no_fold_load_field_unfrozen_object() { + // Non-frozen object should NOT be folded + eval(" + class TestUnfrozen + attr_accessor :a + def initialize + @a = 1 + end + end + + UNFROZEN_OBJ = TestUnfrozen.new + + def test = UNFROZEN_OBJ.a + test + test + "); + assert_snapshot!(hir_string("test"), @r" + fn test@:11: + bb0(): + EntryPoint interpreter + v1:BasicObject = LoadSelf + Jump bb2(v1) + bb1(v4:BasicObject): + EntryPoint JIT(0) + Jump bb2(v4) + bb2(v6:BasicObject): + PatchPoint SingleRactorMode + PatchPoint StableConstantNames(0x1000, UNFROZEN_OBJ) + v20:HeapObject[VALUE(0x1008)] = Const Value(VALUE(0x1008)) + PatchPoint MethodRedefined(TestUnfrozen@0x1010, a@0x1018, cme:0x1020) + PatchPoint NoSingletonClass(TestUnfrozen@0x1010) + v25:HeapObject[VALUE(0x1008)] = GuardShape v20, 0x1048 + v26:BasicObject = LoadField v25, :@a@0x1049 + CheckInterrupts + Return v26 + "); + } + + #[test] + fn test_fold_load_field_frozen_with_attr_reader() { + // Using attr_reader instead of attr_accessor + eval(" + class TestAttrReader + attr_reader :value + def initialize(v) + @value = v + end + end + + FROZEN_READER = TestAttrReader.new(42).freeze + + def test = FROZEN_READER.value + test + test + "); + assert_snapshot!(hir_string("test"), @r" + fn test@:11: + bb0(): + EntryPoint interpreter + v1:BasicObject = LoadSelf + Jump bb2(v1) + bb1(v4:BasicObject): + EntryPoint JIT(0) + Jump bb2(v4) + bb2(v6:BasicObject): + PatchPoint SingleRactorMode + PatchPoint StableConstantNames(0x1000, FROZEN_READER) + v20:HeapObject[VALUE(0x1008)] = Const Value(VALUE(0x1008)) + PatchPoint MethodRedefined(TestAttrReader@0x1010, value@0x1018, cme:0x1020) + PatchPoint NoSingletonClass(TestAttrReader@0x1010) + v25:HeapObject[VALUE(0x1008)] = GuardShape v20, 0x1048 + v27:Fixnum[42] = Const Value(42) + CheckInterrupts + Return v27 + "); + } + + #[test] + fn test_fold_load_field_frozen_symbol_value() { + // Frozen object with a symbol ivar + eval(" + class TestFrozenSym + attr_accessor :sym + def initialize + @sym = :hello + end + end + + FROZEN_SYM = TestFrozenSym.new.freeze + + def test = FROZEN_SYM.sym + test + test + "); + assert_snapshot!(hir_string("test"), @r" + fn test@:11: + bb0(): + EntryPoint interpreter + v1:BasicObject = LoadSelf + Jump bb2(v1) + bb1(v4:BasicObject): + EntryPoint JIT(0) + Jump bb2(v4) + bb2(v6:BasicObject): + PatchPoint SingleRactorMode + PatchPoint StableConstantNames(0x1000, FROZEN_SYM) + v20:HeapObject[VALUE(0x1008)] = Const Value(VALUE(0x1008)) + PatchPoint MethodRedefined(TestFrozenSym@0x1010, sym@0x1018, cme:0x1020) + PatchPoint NoSingletonClass(TestFrozenSym@0x1010) + v25:HeapObject[VALUE(0x1008)] = GuardShape v20, 0x1048 + v27:StaticSymbol[:hello] = Const Value(VALUE(0x1050)) + CheckInterrupts + Return v27 + "); + } + + #[test] + fn test_fold_load_field_frozen_true_false() { + // Frozen object with boolean ivars + eval(" + class TestFrozenBool + attr_accessor :flag + def initialize + @flag = true + end + end + + FROZEN_TRUE = TestFrozenBool.new.freeze + + def test = FROZEN_TRUE.flag + test + test + "); + assert_snapshot!(hir_string("test"), @r" + fn test@:11: + bb0(): + EntryPoint interpreter + v1:BasicObject = LoadSelf + Jump bb2(v1) + bb1(v4:BasicObject): + EntryPoint JIT(0) + Jump bb2(v4) + bb2(v6:BasicObject): + PatchPoint SingleRactorMode + PatchPoint StableConstantNames(0x1000, FROZEN_TRUE) + v20:HeapObject[VALUE(0x1008)] = Const Value(VALUE(0x1008)) + PatchPoint MethodRedefined(TestFrozenBool@0x1010, flag@0x1018, cme:0x1020) + PatchPoint NoSingletonClass(TestFrozenBool@0x1010) + v25:HeapObject[VALUE(0x1008)] = GuardShape v20, 0x1048 + v27:TrueClass = Const Value(true) + CheckInterrupts + Return v27 + "); + } + + #[test] + fn test_no_fold_load_field_dynamic_receiver() { + // Dynamic receiver (not a constant) should NOT be folded even if object is frozen + eval(" + class TestDynamic + attr_accessor :val + def initialize + @val = 99 + end + end + + def test(obj) = obj.val + o = TestDynamic.new.freeze + test o + test o + "); + assert_snapshot!(hir_string("test"), @r" + fn test@:9: + bb0(): + EntryPoint interpreter + v1:BasicObject = LoadSelf + v2:BasicObject = GetLocal :obj, l0, SP@4 + Jump bb2(v1, v2) + bb1(v5:BasicObject, v6:BasicObject): + EntryPoint JIT(0) + Jump bb2(v5, v6) + bb2(v8:BasicObject, v9:BasicObject): + PatchPoint MethodRedefined(TestDynamic@0x1000, val@0x1008, cme:0x1010) + PatchPoint NoSingletonClass(TestDynamic@0x1000) + v21:HeapObject[class_exact:TestDynamic] = GuardType v9, HeapObject[class_exact:TestDynamic] + v24:HeapObject[class_exact:TestDynamic] = GuardShape v21, 0x1038 + v25:BasicObject = LoadField v24, :@val@0x1039 + CheckInterrupts + Return v25 + "); + } + + #[test] + fn test_fold_load_field_frozen_nested_access() { + // Accessing multiple fields from frozen constant in sequence + eval(" + class TestNestedAccess + attr_accessor :x, :y + def initialize + @x = 100 + @y = 200 + end + end + + NESTED_FROZEN = TestNestedAccess.new.freeze + + def test = NESTED_FROZEN.x + NESTED_FROZEN.y + test + test + "); + assert_snapshot!(hir_string("test"), @r" + fn test@:12: + bb0(): + EntryPoint interpreter + v1:BasicObject = LoadSelf + Jump bb2(v1) + bb1(v4:BasicObject): + EntryPoint JIT(0) + Jump bb2(v4) + bb2(v6:BasicObject): + PatchPoint SingleRactorMode + PatchPoint StableConstantNames(0x1000, NESTED_FROZEN) + v28:HeapObject[VALUE(0x1008)] = Const Value(VALUE(0x1008)) + PatchPoint MethodRedefined(TestNestedAccess@0x1010, x@0x1018, cme:0x1020) + PatchPoint NoSingletonClass(TestNestedAccess@0x1010) + v39:HeapObject[VALUE(0x1008)] = GuardShape v28, 0x1048 + v50:Fixnum[100] = Const Value(100) + PatchPoint SingleRactorMode + PatchPoint StableConstantNames(0x1050, NESTED_FROZEN) + v34:HeapObject[VALUE(0x1008)] = Const Value(VALUE(0x1008)) + PatchPoint MethodRedefined(TestNestedAccess@0x1010, y@0x1058, cme:0x1060) + PatchPoint NoSingletonClass(TestNestedAccess@0x1010) + v42:HeapObject[VALUE(0x1008)] = GuardShape v34, 0x1048 + v51:Fixnum[200] = Const Value(200) + PatchPoint MethodRedefined(Integer@0x1088, +@0x1090, cme:0x1098) + v52:Fixnum[300] = Const Value(300) + IncrCounter inline_cfunc_optimized_send_count + CheckInterrupts + Return v52 + "); + } + + #[test] + fn test_dont_fold_load_field_with_primitive_return_type() { + eval(r#" + S = "abc".freeze + def test = S.bytesize + test + "#); + assert_snapshot!(hir_string("test"), @r" + fn test@:3: + bb0(): + EntryPoint interpreter + v1:BasicObject = LoadSelf + Jump bb2(v1) + bb1(v4:BasicObject): + EntryPoint JIT(0) + Jump bb2(v4) + bb2(v6:BasicObject): + PatchPoint SingleRactorMode + PatchPoint StableConstantNames(0x1000, S) + v20:StringExact[VALUE(0x1008)] = Const Value(VALUE(0x1008)) + PatchPoint MethodRedefined(String@0x1010, bytesize@0x1018, cme:0x1020) + PatchPoint NoSingletonClass(String@0x1010) + v24:CInt64 = LoadField v20, :len@0x1048 + v25:Fixnum = BoxFixnum v24 + IncrCounter inline_cfunc_optimized_send_count + CheckInterrupts + Return v25 + "); + } } From 96c804de1f6a495c339f85069a648d37dadfbc80 Mon Sep 17 00:00:00 2001 From: Alan Wu Date: Mon, 8 Dec 2025 22:08:04 -0500 Subject: [PATCH 04/12] ZJIT: Remove unused includes from zjit.c --- zjit.c | 8 -------- 1 file changed, 8 deletions(-) diff --git a/zjit.c b/zjit.c index 05fb3e1f028ff6..cb5a01734f9721 100644 --- a/zjit.c +++ b/zjit.c @@ -23,14 +23,6 @@ #include "ruby/debug.h" #include "internal/cont.h" -// For mmapp(), sysconf() -#ifndef _WIN32 -#include -#include -#endif - -#include - // This build config impacts the pointer tagging scheme and we only want to // support one scheme for simplicity. STATIC_ASSERT(pointer_tagging_scheme, USE_FLONUM); From b0ea9070d4f42f1844af7eac347ddfb9b4760439 Mon Sep 17 00:00:00 2001 From: Alan Wu Date: Mon, 8 Dec 2025 22:09:38 -0500 Subject: [PATCH 05/12] YJIT: For rustc build, remove cargo touch(1) workaround --- yjit/yjit.mk | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-) diff --git a/yjit/yjit.mk b/yjit/yjit.mk index e019e4a08cf7b8..777dcdd43e4ffc 100644 --- a/yjit/yjit.mk +++ b/yjit/yjit.mk @@ -9,12 +9,6 @@ YJIT_SRC_FILES = $(wildcard \ $(top_srcdir)/jit/src/lib.rs \ ) -# Because of Cargo cache, if the actual binary is not changed from the -# previous build, the mtime is preserved as the cached file. -# This means the target is not updated actually, and it will need to -# rebuild at the next build. -YJIT_LIB_TOUCH = touch $@ - # Absolute path to match RUST_LIB rules to avoid picking # the "target" dir in the source directory through VPATH. BUILD_YJIT_LIBS = $(TOP_BUILD_DIR)/$(YJIT_LIBS) @@ -24,8 +18,7 @@ ifneq ($(strip $(YJIT_LIBS)),) yjit-libs: $(BUILD_YJIT_LIBS) $(BUILD_YJIT_LIBS): $(YJIT_SRC_FILES) $(ECHO) 'building Rust YJIT (release mode)' - +$(Q) $(RUSTC) $(YJIT_RUSTC_ARGS) - $(YJIT_LIB_TOUCH) + $(Q) $(RUSTC) $(YJIT_RUSTC_ARGS) endif ifneq ($(YJIT_SUPPORT),no) From 1bab216062e94e8856ddf3760806b189b4ac5b09 Mon Sep 17 00:00:00 2001 From: Alan Wu Date: Mon, 8 Dec 2025 22:12:12 -0500 Subject: [PATCH 06/12] ZJIT: For rustc build, remove cargo touch(1) workaround --- zjit/zjit.mk | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-) diff --git a/zjit/zjit.mk b/zjit/zjit.mk index 01274dc3073642..68fc5778ec6994 100644 --- a/zjit/zjit.mk +++ b/zjit/zjit.mk @@ -14,12 +14,6 @@ ZJIT_SRC_FILES = $(wildcard \ $(RUST_LIB): $(ZJIT_SRC_FILES) -# Because of Cargo cache, if the actual binary is not changed from the -# previous build, the mtime is preserved as the cached file. -# This means the target is not updated actually, and it will need to -# rebuild at the next build. -ZJIT_LIB_TOUCH = touch $@ - # Absolute path to match RUST_LIB rules to avoid picking # the "target" dir in the source directory through VPATH. BUILD_ZJIT_LIBS = $(TOP_BUILD_DIR)/$(ZJIT_LIBS) @@ -28,8 +22,7 @@ BUILD_ZJIT_LIBS = $(TOP_BUILD_DIR)/$(ZJIT_LIBS) ifneq ($(strip $(ZJIT_LIBS)),) $(BUILD_ZJIT_LIBS): $(ZJIT_SRC_FILES) $(ECHO) 'building Rust ZJIT (release mode)' - +$(Q) $(RUSTC) $(ZJIT_RUSTC_ARGS) - $(ZJIT_LIB_TOUCH) + $(Q) $(RUSTC) $(ZJIT_RUSTC_ARGS) endif # By using ZJIT_BENCH_OPTS instead of RUN_OPTS, you can skip passing the options to `make install` From 121d0da02580693ac5f872578284e26e2668b577 Mon Sep 17 00:00:00 2001 From: Alan Wu Date: Mon, 8 Dec 2025 22:30:22 -0500 Subject: [PATCH 07/12] JITs: Move cargo-specific variables into conditional --- defs/jit.mk | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/defs/jit.mk b/defs/jit.mk index a537d803002856..67cc6fdce76215 100644 --- a/defs/jit.mk +++ b/defs/jit.mk @@ -1,11 +1,5 @@ # Make recipes that deal with the rust code of YJIT and ZJIT. -# Because of Cargo cache, if the actual binary is not changed from the -# previous build, the mtime is preserved as the cached file. -# This means the target is not updated actually, and it will need to -# rebuild at the next build. -RUST_LIB_TOUCH = touch $@ - ifneq ($(JIT_CARGO_SUPPORT),no) # Show Cargo progress when doing `make V=1` @@ -13,6 +7,12 @@ CARGO_VERBOSE_0 = -q CARGO_VERBOSE_1 = CARGO_VERBOSE = $(CARGO_VERBOSE_$(V)) +# Because of Cargo cache, if the actual binary is not changed from the +# previous build, the mtime is preserved as the cached file. +# This means the target is not updated actually, and it will need to +# rebuild at the next build. +RUST_LIB_TOUCH = touch $@ + # NOTE: MACOSX_DEPLOYMENT_TARGET to match `rustc --print deployment-target` to avoid the warning below. # ld: warning: object file (target/debug/libjit.a()) was built for # newer macOS version (15.2) than being linked (15.0) @@ -30,7 +30,7 @@ $(RUST_LIB): $(srcdir)/ruby.rs MACOSX_DEPLOYMENT_TARGET=11.0 \ $(CARGO) $(CARGO_VERBOSE) build --manifest-path '$(top_srcdir)/Cargo.toml' $(CARGO_BUILD_ARGS) $(RUST_LIB_TOUCH) -endif +endif # ifneq ($(JIT_CARGO_SUPPORT),no) RUST_LIB_SYMBOLS = $(RUST_LIB:.a=).symbols $(RUST_LIBOBJ): $(RUST_LIB) From 029a48176cf9fd367d52d8c9f87cb9f77d425a43 Mon Sep 17 00:00:00 2001 From: Alan Wu Date: Mon, 8 Dec 2025 22:25:05 -0500 Subject: [PATCH 08/12] JITs: Drop cargo and use just rustc for release combo build So we don't expose builders to network flakiness which cannot be worked around using cargo's --offline flag. --- Cargo.toml | 15 +++++++-- common.mk | 6 ++-- configure.ac | 80 +++++++++++++++++++++++++------------------- defs/jit.mk | 25 ++++++++++++++ template/Makefile.in | 2 ++ yjit/Cargo.toml | 4 +-- yjit/yjit.mk | 11 +++++- zjit/Cargo.toml | 4 +-- zjit/zjit.mk | 11 +++++- 9 files changed, 110 insertions(+), 48 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index ec2ce880ca4c48..521129d92d2863 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,6 +1,15 @@ -# Using Cargo's workspace feature to build all the Rust code in -# into a single package. -# TODO(alan) notes about rust version requirements. Undecided yet. +# This is the root Cargo [workspace](https://doc.rust-lang.org/cargo/reference/workspaces.html) +# and the root package for all the rust code that are statically linked into ruby. Rust tooling +# limitations means all Rust code need to share a single archive library (staticlib) at the +# integration point with non-rust code. (See rustlang/rust#44322 and #104707 for a taste of +# the linking challenges.) +# +# Do not add required dependencies. This is a policy that helps downstream consumers and give +# us tight control over what we ship. All of the optional dependencies are used exclusively +# during development. +# +# Release builds avoid Cargo entirely because offline builds can fail even when none of the +# optional dependencies are built (rust-lang/cargo#10352). [workspace] members = ["zjit", "yjit", "jit"] diff --git a/common.mk b/common.mk index 35c23159800160..50abfeab8a4df6 100644 --- a/common.mk +++ b/common.mk @@ -268,9 +268,8 @@ MAKE_LINK = $(MINIRUBY) -rfileutils -e "include FileUtils::Verbose" \ # For release builds YJIT_RUSTC_ARGS = --crate-name=yjit \ - --crate-type=staticlib \ + $(JIT_RUST_FLAGS) \ --edition=2021 \ - --cfg 'feature="stats_allocator"' \ -g \ -C lto=thin \ -C opt-level=3 \ @@ -279,9 +278,8 @@ YJIT_RUSTC_ARGS = --crate-name=yjit \ '$(top_srcdir)/yjit/src/lib.rs' ZJIT_RUSTC_ARGS = --crate-name=zjit \ - --crate-type=staticlib \ + $(JIT_RUST_FLAGS) \ --edition=2024 \ - --cfg 'feature="stats_allocator"' \ -g \ -C lto=thin \ -C opt-level=3 \ diff --git a/configure.ac b/configure.ac index 14b0234ef0f9aa..ef4b76e5376883 100644 --- a/configure.ac +++ b/configure.ac @@ -3896,7 +3896,6 @@ AC_SUBST(INSTALL_STATIC_LIBRARY) [begin]_group "JIT section" && { AC_CHECK_PROG(RUSTC, [rustc], [rustc], [no]) dnl no ac_tool_prefix -AC_CHECK_TOOL(CARGO, [cargo], [no]) dnl check if rustc is recent enough to build YJIT (rustc >= 1.58.0) JIT_RUSTC_OK=no @@ -3963,11 +3962,7 @@ AC_ARG_ENABLE(zjit, # 1.85.0 is the first stable version that supports the 2024 edition. AS_IF([test "$RUSTC" != "no" && echo "#[cfg(target_arch = \"$JIT_TARGET_ARCH\")] fn main() {}" | $RUSTC - --edition=2024 --emit asm=/dev/null 2>/dev/null], - AS_IF([test "$gnumake" = "yes" -a \( "$YJIT_SUPPORT" = "no" -o "$CARGO" != "no" \)], [ - # When only building ZJIT, we don't need cargo; it's required for YJIT+ZJIT build. - # Assume that if rustc is new enough, then cargo is also. - # TODO(alan): Get rid of dependency on cargo in YJIT+ZJIT build. Cargo's offline mode - # still too unreliable: https://github.com/rust-lang/cargo/issues/10352 + AS_IF([test "$gnumake" = "yes"], [ rb_zjit_build_possible=yes ]) ) @@ -4053,36 +4048,49 @@ AS_CASE(["${ZJIT_SUPPORT}"], AC_DEFINE(USE_ZJIT, 0) ]) -# if YJIT+ZJIT release build, or any build that requires Cargo -AS_IF([test x"$JIT_CARGO_SUPPORT" != "xno" -o \( x"$YJIT_SUPPORT" != "xno" -a x"$ZJIT_SUPPORT" != "xno" \)], [ - AS_IF([test x"$CARGO" = "xno"], - AC_MSG_ERROR([this build configuration requires cargo. Installation instructions available at https://www.rust-lang.org/tools/install])) - - YJIT_LIBS= - ZJIT_LIBS= - - # There's more processing below to get the feature set for the - # top-level crate, so capture at this point for feature set of - # just the zjit crate. - ZJIT_TEST_FEATURES="${rb_cargo_features}" +JIT_RUST_FLAGS='--crate-type=staticlib --cfg feature=\"stats_allocator\"' +RLIB_DIR= +AS_CASE(["$JIT_CARGO_SUPPORT:$YJIT_SUPPORT:$ZJIT_SUPPORT"], + [no:yes:yes], [ # release build of YJIT+ZJIT + YJIT_LIBS= + ZJIT_LIBS= + JIT_RUST_FLAGS="--crate-type=rlib" + RLIB_DIR="target/release" + RUST_LIB="target/release/libruby.a" + ], + [no:*], [], + [*], [ # JIT_CARGO_SUPPORT not "no" -- cargo required. + AC_CHECK_TOOL(CARGO, [cargo], [no]) + AS_IF([test x"$CARGO" = "xno"], + AC_MSG_ERROR([this build configuration requires cargo. Installation instructions available at https://www.rust-lang.org/tools/install])) + + YJIT_LIBS= + ZJIT_LIBS= + + # There's more processing below to get the feature set for the + # top-level crate, so capture at this point for feature set of + # just the zjit crate. + ZJIT_TEST_FEATURES="${rb_cargo_features}" + + AS_IF([test x"${YJIT_SUPPORT}" != x"no"], [ + rb_cargo_features="$rb_cargo_features,yjit" + ]) + AS_IF([test x"${ZJIT_SUPPORT}" != x"no"], [ + AC_SUBST(ZJIT_TEST_FEATURES) + rb_cargo_features="$rb_cargo_features,zjit" + ]) + # if YJIT and ZJIT release mode + AS_IF([test "${YJIT_SUPPORT}:${ZJIT_SUPPORT}" = "yes:yes"], [ + JIT_CARGO_SUPPORT=release + ]) + CARGO_BUILD_ARGS="--profile ${JIT_CARGO_SUPPORT} --features ${rb_cargo_features}" + AS_IF([test "${JIT_CARGO_SUPPORT}" = "dev"], [ + RUST_LIB="target/debug/libruby.a" + ], [ + RUST_LIB="target/${JIT_CARGO_SUPPORT}/libruby.a" + ]) + ], - AS_IF([test x"${YJIT_SUPPORT}" != x"no"], [ - rb_cargo_features="$rb_cargo_features,yjit" - ]) - AS_IF([test x"${ZJIT_SUPPORT}" != x"no"], [ - AC_SUBST(ZJIT_TEST_FEATURES) - rb_cargo_features="$rb_cargo_features,zjit" - ]) - # if YJIT and ZJIT release mode - AS_IF([test "${YJIT_SUPPORT}:${ZJIT_SUPPORT}" = "yes:yes"], [ - JIT_CARGO_SUPPORT=release - ]) - CARGO_BUILD_ARGS="--profile ${JIT_CARGO_SUPPORT} --features ${rb_cargo_features}" - AS_IF([test "${JIT_CARGO_SUPPORT}" = "dev"], [ - RUST_LIB="target/debug/libruby.a" - ], [ - RUST_LIB="target/${JIT_CARGO_SUPPORT}/libruby.a" - ]) ]) # In case either we're linking rust code @@ -4098,6 +4106,7 @@ AS_IF([test -n "$RUST_LIB"], [ dnl These variables end up in ::RbConfig::CONFIG AC_SUBST(RUSTC)dnl Rust compiler command +AC_SUBST(JIT_RUST_FLAGS)dnl the common rustc flags for JIT crates such as zjit AC_SUBST(CARGO)dnl Cargo command for Rust builds AC_SUBST(CARGO_BUILD_ARGS)dnl for selecting Rust build profiles AC_SUBST(YJIT_SUPPORT)dnl what flavor of YJIT the Ruby build includes @@ -4108,6 +4117,7 @@ AC_SUBST(ZJIT_LIBS)dnl path to the .a library of ZJIT AC_SUBST(ZJIT_OBJ)dnl for optionally building the C parts of ZJIT AC_SUBST(JIT_OBJ)dnl for optionally building C glue code for Rust FFI AC_SUBST(RUST_LIB)dnl path to the rust .a library that contains either or both JITs +AC_SUBST(RLIB_DIR)dnl subpath of build directory for .rlib files AC_SUBST(JIT_CARGO_SUPPORT)dnl "no" or the cargo profile of the rust code } diff --git a/defs/jit.mk b/defs/jit.mk index 67cc6fdce76215..e893098ca26486 100644 --- a/defs/jit.mk +++ b/defs/jit.mk @@ -30,6 +30,31 @@ $(RUST_LIB): $(srcdir)/ruby.rs MACOSX_DEPLOYMENT_TARGET=11.0 \ $(CARGO) $(CARGO_VERBOSE) build --manifest-path '$(top_srcdir)/Cargo.toml' $(CARGO_BUILD_ARGS) $(RUST_LIB_TOUCH) +else ifneq ($(strip $(RLIB_DIR)),) # combo build + +$(RUST_LIB): $(srcdir)/ruby.rs + $(ECHO) 'building $(@F)' + $(Q) $(RUSTC) --edition=2024 \ + '-L$(@D)' \ + --extern=yjit \ + --extern=zjit \ + --crate-type=staticlib \ + --cfg 'feature="yjit"' \ + --cfg 'feature="zjit"' \ + '--out-dir=$(@D)' \ + '$(top_srcdir)/ruby.rs' + +# Absolute path to avoid VPATH ambiguity +JIT_RLIB = $(TOP_BUILD_DIR)/$(RLIB_DIR)/libjit.rlib +$(YJIT_RLIB): $(JIT_RLIB) +$(ZJIT_RLIB): $(JIT_RLIB) +$(JIT_RLIB): + $(ECHO) 'building $(@F)' + $(Q) $(RUSTC) --crate-name=jit \ + --edition=2024 \ + $(JIT_RUST_FLAGS) \ + '--out-dir=$(@D)' \ + '$(top_srcdir)/jit/src/lib.rs' endif # ifneq ($(JIT_CARGO_SUPPORT),no) RUST_LIB_SYMBOLS = $(RUST_LIB:.a=).symbols diff --git a/template/Makefile.in b/template/Makefile.in index 7bc40b9d019604..fd41ddca9d90d5 100644 --- a/template/Makefile.in +++ b/template/Makefile.in @@ -114,6 +114,8 @@ JIT_CARGO_SUPPORT=@JIT_CARGO_SUPPORT@ CARGO_TARGET_DIR=@abs_top_builddir@/target CARGO_BUILD_ARGS=@CARGO_BUILD_ARGS@ ZJIT_TEST_FEATURES=@ZJIT_TEST_FEATURES@ +JIT_RUST_FLAGS=@JIT_RUST_FLAGS@ +RLIB_DIR=@RLIB_DIR@ RUST_LIB=@RUST_LIB@ RUST_LIBOBJ = $(RUST_LIB:.a=.@OBJEXT@) LDFLAGS = @STATIC@ $(CFLAGS) @LDFLAGS@ diff --git a/yjit/Cargo.toml b/yjit/Cargo.toml index e2f1d84ffd3d8c..d3124e608c6a3c 100644 --- a/yjit/Cargo.toml +++ b/yjit/Cargo.toml @@ -10,8 +10,8 @@ rust-version = "1.58.0" # Minimally supported rust version publish = false # Don't publish to crates.io [dependencies] -# No required dependencies to simplify build process. TODO: Link to yet to be -# written rationale. Optional For development and testing purposes +# No required dependencies to simplify build process. +# Optional For development and testing purposes. capstone = { version = "0.13.0", optional = true } jit = { version = "0.1.0", path = "../jit" } diff --git a/yjit/yjit.mk b/yjit/yjit.mk index 777dcdd43e4ffc..22d256fee78c14 100644 --- a/yjit/yjit.mk +++ b/yjit/yjit.mk @@ -19,7 +19,16 @@ yjit-libs: $(BUILD_YJIT_LIBS) $(BUILD_YJIT_LIBS): $(YJIT_SRC_FILES) $(ECHO) 'building Rust YJIT (release mode)' $(Q) $(RUSTC) $(YJIT_RUSTC_ARGS) -endif +else ifneq ($(strip $(RLIB_DIR)),) # combo build +# Absolute path to avoid VPATH ambiguity +YJIT_RLIB = $(TOP_BUILD_DIR)/$(RLIB_DIR)/libyjit.rlib + +$(YJIT_RLIB): $(YJIT_SRC_FILES) + $(ECHO) 'building $(@F)' + $(Q) $(RUSTC) '-L$(@D)' --extern=jit $(YJIT_RUSTC_ARGS) + +$(RUST_LIB): $(YJIT_RLIB) +endif # ifneq ($(strip $(YJIT_LIBS)),) ifneq ($(YJIT_SUPPORT),no) $(RUST_LIB): $(YJIT_SRC_FILES) diff --git a/zjit/Cargo.toml b/zjit/Cargo.toml index c97c845a6eff7f..ef656c5252027c 100644 --- a/zjit/Cargo.toml +++ b/zjit/Cargo.toml @@ -6,8 +6,8 @@ rust-version = "1.85.0" # Minimally supported rust version publish = false # Don't publish to crates.io [dependencies] -# No required dependencies to simplify build process. TODO: Link to yet to be -# written rationale. Optional For development and testing purposes +# No required dependencies to simplify build process. +# Optional For development and testing purposes. capstone = { version = "0.13.0", optional = true } jit = { version = "0.1.0", path = "../jit" } diff --git a/zjit/zjit.mk b/zjit/zjit.mk index 68fc5778ec6994..2116775a91a182 100644 --- a/zjit/zjit.mk +++ b/zjit/zjit.mk @@ -23,7 +23,16 @@ ifneq ($(strip $(ZJIT_LIBS)),) $(BUILD_ZJIT_LIBS): $(ZJIT_SRC_FILES) $(ECHO) 'building Rust ZJIT (release mode)' $(Q) $(RUSTC) $(ZJIT_RUSTC_ARGS) -endif +else ifneq ($(strip $(RLIB_DIR)),) # combo build +# Absolute path to avoid VPATH ambiguity +ZJIT_RLIB = $(TOP_BUILD_DIR)/$(RLIB_DIR)/libzjit.rlib + +$(ZJIT_RLIB): $(ZJIT_SRC_FILES) + $(ECHO) 'building $(@F)' + $(Q) $(RUSTC) '-L$(@D)' --extern=jit $(ZJIT_RUSTC_ARGS) + +$(RUST_LIB): $(ZJIT_RLIB) +endif # ifneq ($(strip $(ZJIT_LIBS)),) # By using ZJIT_BENCH_OPTS instead of RUN_OPTS, you can skip passing the options to `make install` ZJIT_BENCH_OPTS = $(RUN_OPTS) --enable-gems From b208f46f48f88cf4d0b813edbfab07214d085e89 Mon Sep 17 00:00:00 2001 From: Alan Wu Date: Wed, 10 Dec 2025 16:26:22 -0500 Subject: [PATCH 09/12] ZJIT: Don't fold LoadField with negative offsets and use byte_add No point doing the manual size unit conversion for add. Sorry, no new tests since there is no way to generate a LoadField with a negative offset from ruby code AFAICT. Careful with the `as` casts. --- zjit/src/hir.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/zjit/src/hir.rs b/zjit/src/hir.rs index deae6c3b653681..bc6f1fb5f564fa 100644 --- a/zjit/src/hir.rs +++ b/zjit/src/hir.rs @@ -3682,14 +3682,14 @@ impl Function { // Don't bother re-inferring the type of val; we already know it. continue; } - Insn::LoadField { recv, offset, return_type, .. } if return_type.is_subtype(types::BasicObject) => { + Insn::LoadField { recv, offset, return_type, .. } if return_type.is_subtype(types::BasicObject) && + u32::try_from(offset).is_ok() => { + let offset = (offset as u32).to_usize(); let recv_type = self.type_of(recv); match recv_type.ruby_object() { Some(recv_obj) if recv_obj.is_frozen() => { let recv_ptr = recv_obj.as_ptr() as *const VALUE; - // Rust pointer .add() scales by size_of::() and offset is - // already scaled by SIZEOF_VALUE, so undo that. - let val = unsafe { *recv_ptr.add(offset as usize / SIZEOF_VALUE) }; + let val = unsafe { recv_ptr.byte_add(offset).read() }; self.new_insn(Insn::Const { val: Const::Value(val) }) } _ => insn_id, From 5828872ec4814a5a07f5f4c7686c543127619197 Mon Sep 17 00:00:00 2001 From: John Hawthorn Date: Wed, 10 Dec 2025 13:13:21 -0800 Subject: [PATCH 10/12] Update Ractor warning message Although the Ractor API is still experimental and may change, and there may be some implementation issues, we should no longer say that there are many. Hopefully we can remove this warning entirely for Ruby 4.1 --- bootstraptest/test_ractor.rb | 2 +- ractor.rb | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/bootstraptest/test_ractor.rb b/bootstraptest/test_ractor.rb index 13c4652d3760a8..0b7a43272c3ca9 100644 --- a/bootstraptest/test_ractor.rb +++ b/bootstraptest/test_ractor.rb @@ -1661,7 +1661,7 @@ def hello = nil } # check experimental warning -assert_match /\Atest_ractor\.rb:1:\s+warning:\s+Ractor is experimental/, %q{ +assert_match /\Atest_ractor\.rb:1:\s+warning:\s+Ractor API is experimental/, %q{ Warning[:experimental] = $VERBOSE = true STDERR.reopen(STDOUT) eval("Ractor.new{}.value", nil, "test_ractor.rb", 1) diff --git a/ractor.rb b/ractor.rb index d992f0a04702c9..70ce1a9fffecb9 100644 --- a/ractor.rb +++ b/ractor.rb @@ -230,8 +230,8 @@ def self.new(*args, name: nil, &block) b = block # TODO: builtin bug raise ArgumentError, "must be called with a block" unless block if __builtin_cexpr!("RBOOL(ruby_single_main_ractor)") - Kernel.warn("Ractor is experimental, and the behavior may change in future versions of Ruby! " \ - "Also there are many implementation issues.", uplevel: 0, category: :experimental) + Kernel.warn("Ractor API is experimental and may change in future versions of Ruby.", + uplevel: 0, category: :experimental) end loc = caller_locations(1, 1).first loc = "#{loc.path}:#{loc.lineno}" From 1c29fbeca00d4be63e3f537609b88a8f10115ae4 Mon Sep 17 00:00:00 2001 From: John Hawthorn Date: Wed, 10 Dec 2025 12:58:23 -0800 Subject: [PATCH 11/12] GC_DEBUG_STRESS_TO_CLASS should only be for debug I believe this was accidentally left in as part of 2beb3798bac52624c3170138f8ef65869f1da6c0 --- gc/default/default.c | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/gc/default/default.c b/gc/default/default.c index a03fda859cf4ca..6a2035dccc9c99 100644 --- a/gc/default/default.c +++ b/gc/default/default.c @@ -316,7 +316,7 @@ int ruby_rgengc_debug; #endif #ifndef GC_DEBUG_STRESS_TO_CLASS -# define GC_DEBUG_STRESS_TO_CLASS 1 +# define GC_DEBUG_STRESS_TO_CLASS RUBY_DEBUG #endif typedef enum { @@ -9428,6 +9428,7 @@ rb_gc_impl_after_fork(void *objspace_ptr, rb_pid_t pid) VALUE rb_ident_hash_new_with_size(st_index_t size); +#if GC_DEBUG_STRESS_TO_CLASS /* * call-seq: * GC.add_stress_to_class(class[, ...]) @@ -9477,6 +9478,7 @@ rb_gcdebug_remove_stress_to_class(int argc, VALUE *argv, VALUE self) return Qnil; } +#endif void * rb_gc_impl_objspace_alloc(void) @@ -9582,10 +9584,10 @@ rb_gc_impl_init(void) rb_define_singleton_method(rb_mGC, "verify_compaction_references", rb_f_notimplement, -1); } - if (GC_DEBUG_STRESS_TO_CLASS) { - rb_define_singleton_method(rb_mGC, "add_stress_to_class", rb_gcdebug_add_stress_to_class, -1); - rb_define_singleton_method(rb_mGC, "remove_stress_to_class", rb_gcdebug_remove_stress_to_class, -1); - } +#if GC_DEBUG_STRESS_TO_CLASS + rb_define_singleton_method(rb_mGC, "add_stress_to_class", rb_gcdebug_add_stress_to_class, -1); + rb_define_singleton_method(rb_mGC, "remove_stress_to_class", rb_gcdebug_remove_stress_to_class, -1); +#endif /* internal methods */ rb_define_singleton_method(rb_mGC, "verify_internal_consistency", gc_verify_internal_consistency_m, 0); From c7d56e90d381f0bf115a5c76cbef9df6ae19f4c9 Mon Sep 17 00:00:00 2001 From: Takashi Kokubun Date: Wed, 10 Dec 2025 16:07:39 -0800 Subject: [PATCH 12/12] ZJIT: Re-compile ISEQs invalidated by PatchPoint (#15459) --- zjit/src/backend/arm64/mod.rs | 4 +- zjit/src/backend/lir.rs | 8 +-- zjit/src/backend/x86_64/mod.rs | 4 +- zjit/src/codegen.rs | 117 ++++++++++++++++++++++++--------- zjit/src/gc.rs | 61 ++++++++++++----- zjit/src/invariants.rs | 74 ++++++++++++++------- zjit/src/payload.rs | 52 +++++++++++---- zjit/src/stats.rs | 9 ++- 8 files changed, 236 insertions(+), 93 deletions(-) diff --git a/zjit/src/backend/arm64/mod.rs b/zjit/src/backend/arm64/mod.rs index f927f261cf39b8..55a65e3ea6161e 100644 --- a/zjit/src/backend/arm64/mod.rs +++ b/zjit/src/backend/arm64/mod.rs @@ -903,8 +903,8 @@ impl Assembler { } } } - &mut Insn::PatchPoint { ref target, invariant, payload } => { - split_patch_point(asm, target, invariant, payload); + &mut Insn::PatchPoint { ref target, invariant, version } => { + split_patch_point(asm, target, invariant, version); } _ => { asm.push_insn(insn); diff --git a/zjit/src/backend/lir.rs b/zjit/src/backend/lir.rs index d7c6740d13a1e4..f4ed3d7cb78ce6 100644 --- a/zjit/src/backend/lir.rs +++ b/zjit/src/backend/lir.rs @@ -9,7 +9,7 @@ use crate::cruby::{Qundef, RUBY_OFFSET_CFP_PC, RUBY_OFFSET_CFP_SP, SIZEOF_VALUE_ use crate::hir::{Invariant, SideExitReason}; use crate::options::{TraceExits, debug, get_option}; use crate::cruby::VALUE; -use crate::payload::IseqPayload; +use crate::payload::IseqVersionRef; use crate::stats::{exit_counter_ptr, exit_counter_ptr_for_opcode, side_exit_counter, CompileError}; use crate::virtualmem::CodePtr; use crate::asm::{CodeBlock, Label}; @@ -531,7 +531,7 @@ pub enum Insn { Or { left: Opnd, right: Opnd, out: Opnd }, /// Patch point that will be rewritten to a jump to a side exit on invalidation. - PatchPoint { target: Target, invariant: Invariant, payload: *mut IseqPayload }, + PatchPoint { target: Target, invariant: Invariant, version: IseqVersionRef }, /// Make sure the last PatchPoint has enough space to insert a jump. /// We insert this instruction at the end of each block so that the jump @@ -2358,8 +2358,8 @@ impl Assembler { out } - pub fn patch_point(&mut self, target: Target, invariant: Invariant, payload: *mut IseqPayload) { - self.push_insn(Insn::PatchPoint { target, invariant, payload }); + pub fn patch_point(&mut self, target: Target, invariant: Invariant, version: IseqVersionRef) { + self.push_insn(Insn::PatchPoint { target, invariant, version }); } pub fn pad_patch_point(&mut self) { diff --git a/zjit/src/backend/x86_64/mod.rs b/zjit/src/backend/x86_64/mod.rs index 5e975e1bd03ccf..9f780617cc4ac5 100644 --- a/zjit/src/backend/x86_64/mod.rs +++ b/zjit/src/backend/x86_64/mod.rs @@ -640,8 +640,8 @@ impl Assembler { }; asm.store(dest, src); } - &mut Insn::PatchPoint { ref target, invariant, payload } => { - split_patch_point(asm, target, invariant, payload); + &mut Insn::PatchPoint { ref target, invariant, version } => { + split_patch_point(asm, target, invariant, version); } _ => { asm.push_insn(insn); diff --git a/zjit/src/codegen.rs b/zjit/src/codegen.rs index 11b60c546c9716..dbcffb04273a75 100644 --- a/zjit/src/codegen.rs +++ b/zjit/src/codegen.rs @@ -13,7 +13,7 @@ use crate::invariants::{ track_single_ractor_assumption, track_stable_constant_names_assumption, track_no_singleton_class_assumption }; use crate::gc::append_gc_offsets; -use crate::payload::{get_or_create_iseq_payload, get_or_create_iseq_payload_ptr, IseqCodePtrs, IseqPayload, IseqStatus}; +use crate::payload::{get_or_create_iseq_payload, IseqCodePtrs, IseqVersion, IseqVersionRef, IseqStatus}; use crate::state::ZJITState; use crate::stats::{CompileError, exit_counter_for_compile_error, exit_counter_for_unhandled_hir_insn, incr_counter, incr_counter_by, send_fallback_counter, send_fallback_counter_for_method_type, send_fallback_counter_ptr_for_opcode, send_without_block_fallback_counter_for_method_type, send_without_block_fallback_counter_for_optimized_method_type}; use crate::stats::{counter_ptr, with_time_stat, Counter, Counter::{compile_time_ns, exit_compile_error}}; @@ -25,6 +25,9 @@ use crate::hir_type::{types, Type}; use crate::options::get_option; use crate::cast::IntoUsize; +/// At the moment, we support recompiling each ISEQ only once. +pub const MAX_ISEQ_VERSIONS: usize = 2; + /// Sentinel program counter stored in C frames when runtime checks are enabled. const PC_POISON: Option<*const VALUE> = if cfg!(feature = "runtime_checks") { Some(usize::MAX as *const VALUE) @@ -37,6 +40,9 @@ struct JITState { /// Instruction sequence for the method being compiled iseq: IseqPtr, + /// ISEQ version that is being compiled, which will be used by PatchPoint + version: IseqVersionRef, + /// Low-level IR Operands indexed by High-level IR's Instruction ID opnds: Vec>, @@ -52,9 +58,10 @@ struct JITState { impl JITState { /// Create a new JITState instance - fn new(iseq: IseqPtr, num_insns: usize, num_blocks: usize) -> Self { + fn new(iseq: IseqPtr, version: IseqVersionRef, num_insns: usize, num_blocks: usize) -> Self { JITState { iseq, + version, opnds: vec![None; num_insns], labels: vec![None; num_blocks], jit_entries: Vec::default(), @@ -134,11 +141,10 @@ fn gen_iseq_entry_point(cb: &mut CodeBlock, iseq: IseqPtr, jit_exception: bool) } /// Stub a branch for a JIT-to-JIT call -fn gen_iseq_call(cb: &mut CodeBlock, caller_iseq: IseqPtr, iseq_call: &IseqCallRef) -> Result<(), CompileError> { +pub fn gen_iseq_call(cb: &mut CodeBlock, iseq_call: &IseqCallRef) -> Result<(), CompileError> { // Compile a function stub let stub_ptr = gen_function_stub(cb, iseq_call.clone()).inspect_err(|err| { - debug!("{err:?}: gen_function_stub failed: {} -> {}", - iseq_get_location(caller_iseq, 0), iseq_get_location(iseq_call.iseq.get(), 0)); + debug!("{err:?}: gen_function_stub failed: {}", iseq_get_location(iseq_call.iseq.get(), 0)); })?; // Update the JIT-to-JIT call to call the stub @@ -197,29 +203,36 @@ pub fn gen_entry_trampoline(cb: &mut CodeBlock) -> Result fn gen_iseq(cb: &mut CodeBlock, iseq: IseqPtr, function: Option<&Function>) -> Result { // Return an existing pointer if it's already compiled let payload = get_or_create_iseq_payload(iseq); - match &payload.status { - IseqStatus::Compiled(code_ptrs) => return Ok(code_ptrs.clone()), - IseqStatus::CantCompile(err) => return Err(err.clone()), - IseqStatus::NotCompiled => {}, + let last_status = payload.versions.last().map(|version| &unsafe { version.as_ref() }.status); + match last_status { + Some(IseqStatus::Compiled(code_ptrs)) => return Ok(code_ptrs.clone()), + Some(IseqStatus::CantCompile(err)) => return Err(err.clone()), + _ => {}, + } + // If the ISEQ already hax MAX_ISEQ_VERSIONS, do not compile a new version. + if payload.versions.len() == MAX_ISEQ_VERSIONS { + return Err(CompileError::IseqVersionLimitReached); } // Compile the ISEQ - let code_ptrs = gen_iseq_body(cb, iseq, function, payload); + let mut version = IseqVersion::new(iseq); + let code_ptrs = gen_iseq_body(cb, iseq, version, function); match &code_ptrs { Ok(code_ptrs) => { - payload.status = IseqStatus::Compiled(code_ptrs.clone()); + unsafe { version.as_mut() }.status = IseqStatus::Compiled(code_ptrs.clone()); incr_counter!(compiled_iseq_count); } Err(err) => { - payload.status = IseqStatus::CantCompile(err.clone()); + unsafe { version.as_mut() }.status = IseqStatus::CantCompile(err.clone()); incr_counter!(failed_iseq_count); } } + payload.versions.push(version); code_ptrs } /// Compile an ISEQ into machine code -fn gen_iseq_body(cb: &mut CodeBlock, iseq: IseqPtr, function: Option<&Function>, payload: &mut IseqPayload) -> Result { +fn gen_iseq_body(cb: &mut CodeBlock, iseq: IseqPtr, mut version: IseqVersionRef, function: Option<&Function>) -> Result { // If we ran out of code region, we shouldn't attempt to generate new code. if cb.has_dropped_bytes() { return Err(CompileError::OutOfMemory); @@ -232,23 +245,23 @@ fn gen_iseq_body(cb: &mut CodeBlock, iseq: IseqPtr, function: Option<&Function>, }; // Compile the High-level IR - let (iseq_code_ptrs, gc_offsets, iseq_calls) = gen_function(cb, iseq, function)?; + let (iseq_code_ptrs, gc_offsets, iseq_calls) = gen_function(cb, iseq, version, function)?; // Stub callee ISEQs for JIT-to-JIT calls for iseq_call in iseq_calls.iter() { - gen_iseq_call(cb, iseq, iseq_call)?; + gen_iseq_call(cb, iseq_call)?; } // Prepare for GC - payload.iseq_calls.extend(iseq_calls); - append_gc_offsets(iseq, &gc_offsets); + unsafe { version.as_mut() }.outgoing.extend(iseq_calls); + append_gc_offsets(iseq, version, &gc_offsets); Ok(iseq_code_ptrs) } /// Compile a function -fn gen_function(cb: &mut CodeBlock, iseq: IseqPtr, function: &Function) -> Result<(IseqCodePtrs, Vec, Vec), CompileError> { +fn gen_function(cb: &mut CodeBlock, iseq: IseqPtr, version: IseqVersionRef, function: &Function) -> Result<(IseqCodePtrs, Vec, Vec), CompileError> { let num_spilled_params = max_num_params(function).saturating_sub(ALLOC_REGS.len()); - let mut jit = JITState::new(iseq, function.num_insns(), function.num_blocks()); + let mut jit = JITState::new(iseq, version, function.num_insns(), function.num_blocks()); let mut asm = Assembler::new_with_stack_slots(num_spilled_params); // Compile each basic block @@ -724,17 +737,16 @@ fn gen_invokebuiltin(jit: &JITState, asm: &mut Assembler, state: &FrameState, bf /// Record a patch point that should be invalidated on a given invariant fn gen_patch_point(jit: &mut JITState, asm: &mut Assembler, invariant: &Invariant, state: &FrameState) { - let payload_ptr = get_or_create_iseq_payload_ptr(jit.iseq); let invariant = *invariant; let exit = build_side_exit(jit, state); // Let compile_exits compile a side exit. Let scratch_split lower it with split_patch_point. - asm.patch_point(Target::SideExit { exit, reason: PatchPoint(invariant) }, invariant, payload_ptr); + asm.patch_point(Target::SideExit { exit, reason: PatchPoint(invariant) }, invariant, jit.version); } /// This is used by scratch_split to lower PatchPoint into PadPatchPoint and PosMarker. /// It's called at scratch_split so that we can use the Label after side-exit deduplication in compile_exits. -pub fn split_patch_point(asm: &mut Assembler, target: &Target, invariant: Invariant, payload_ptr: *mut IseqPayload) { +pub fn split_patch_point(asm: &mut Assembler, target: &Target, invariant: Invariant, version: IseqVersionRef) { let Target::Label(exit_label) = *target else { unreachable!("PatchPoint's target should have been lowered to Target::Label by compile_exits: {target:?}"); }; @@ -747,25 +759,25 @@ pub fn split_patch_point(asm: &mut Assembler, target: &Target, invariant: Invari let side_exit_ptr = cb.resolve_label(exit_label); match invariant { Invariant::BOPRedefined { klass, bop } => { - track_bop_assumption(klass, bop, code_ptr, side_exit_ptr, payload_ptr); + track_bop_assumption(klass, bop, code_ptr, side_exit_ptr, version); } Invariant::MethodRedefined { klass: _, method: _, cme } => { - track_cme_assumption(cme, code_ptr, side_exit_ptr, payload_ptr); + track_cme_assumption(cme, code_ptr, side_exit_ptr, version); } Invariant::StableConstantNames { idlist } => { - track_stable_constant_names_assumption(idlist, code_ptr, side_exit_ptr, payload_ptr); + track_stable_constant_names_assumption(idlist, code_ptr, side_exit_ptr, version); } Invariant::NoTracePoint => { - track_no_trace_point_assumption(code_ptr, side_exit_ptr, payload_ptr); + track_no_trace_point_assumption(code_ptr, side_exit_ptr, version); } Invariant::NoEPEscape(iseq) => { - track_no_ep_escape_assumption(iseq, code_ptr, side_exit_ptr, payload_ptr); + track_no_ep_escape_assumption(iseq, code_ptr, side_exit_ptr, version); } Invariant::SingleRactorMode => { - track_single_ractor_assumption(code_ptr, side_exit_ptr, payload_ptr); + track_single_ractor_assumption(code_ptr, side_exit_ptr, version); } Invariant::NoSingletonClass { klass } => { - track_no_singleton_class_assumption(klass, code_ptr, side_exit_ptr, payload_ptr); + track_no_singleton_class_assumption(klass, code_ptr, side_exit_ptr, version); } } }); @@ -2383,8 +2395,9 @@ c_callable! { // 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); - let compile_error = match &payload.status { - IseqStatus::CantCompile(err) => Some(err), + let last_status = payload.versions.last().map(|version| &unsafe { version.as_ref() }.status); + let compile_error = match last_status { + Some(IseqStatus::CantCompile(err)) => Some(err), _ if cb.has_dropped_bytes() => Some(&CompileError::OutOfMemory), _ => None, }; @@ -2398,7 +2411,15 @@ c_callable! { // 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_call)); + if code_ptr.is_ok() { + if let Some(version) = payload.versions.last_mut() { + unsafe { version.as_mut() }.incoming.push(iseq_call); + } + } let code_ptr = code_ptr.unwrap_or_else(|compile_error| { + // We'll use this Rc again, so increment the ref count decremented by from_raw. + unsafe { Rc::increment_strong_count(iseq_call_ptr as *const IseqCall); } + prepare_for_exit(iseq, cfp, sp, &compile_error); ZJITState::get_exit_trampoline_with_counter() }); @@ -2715,3 +2736,37 @@ impl IseqCall { }); } } + +#[cfg(test)] +mod tests { + use crate::codegen::MAX_ISEQ_VERSIONS; + use crate::cruby::test_utils::*; + use crate::payload::*; + + #[test] + fn test_max_iseq_versions() { + eval(&format!(" + TEST = -1 + def test = TEST + + # compile and invalidate MAX+1 times + i = 0 + while i < {MAX_ISEQ_VERSIONS} + 1 + test; test # compile a version + + Object.send(:remove_const, :TEST) + TEST = i + + i += 1 + end + ")); + + // It should not exceed MAX_ISEQ_VERSIONS + let iseq = get_method_iseq("self", "test"); + let payload = get_or_create_iseq_payload(iseq); + assert_eq!(payload.versions.len(), MAX_ISEQ_VERSIONS); + + // The last call should not discard the JIT code + assert!(matches!(unsafe { payload.versions.last().unwrap().as_ref() }.status, IseqStatus::Compiled(_))); + } +} diff --git a/zjit/src/gc.rs b/zjit/src/gc.rs index 93a9b10e562c4a..48d841a1048c39 100644 --- a/zjit/src/gc.rs +++ b/zjit/src/gc.rs @@ -1,8 +1,9 @@ //! This module is responsible for marking/moving objects on GC. +use std::ptr::null; use std::{ffi::c_void, ops::Range}; use crate::{cruby::*, state::ZJITState, stats::with_time_stat, virtualmem::CodePtr}; -use crate::payload::{IseqPayload, get_or_create_iseq_payload, payload_ptr_as_mut}; +use crate::payload::{IseqPayload, IseqVersionRef, get_or_create_iseq_payload}; use crate::stats::Counter::gc_time_ns; use crate::state::gc_mark_raw_samples; @@ -50,7 +51,13 @@ pub extern "C" fn rb_zjit_iseq_free(iseq: IseqPtr) { if !ZJITState::has_instance() { return; } + // TODO(Shopify/ruby#682): Free `IseqPayload` + let payload = get_or_create_iseq_payload(iseq); + for version in payload.versions.iter_mut() { + unsafe { version.as_mut() }.iseq = null(); + } + let invariants = ZJITState::get_invariants(); invariants.forget_iseq(iseq); } @@ -93,14 +100,16 @@ fn iseq_mark(payload: &IseqPayload) { // Mark objects baked in JIT code let cb = ZJITState::get_code_block(); - for &offset in payload.gc_offsets.iter() { - let value_ptr: *const u8 = offset.raw_ptr(cb); - // Creating an unaligned pointer is well defined unlike in C. - let value_ptr = value_ptr as *const VALUE; - - unsafe { - let object = value_ptr.read_unaligned(); - rb_gc_mark_movable(object); + for version in payload.versions.iter() { + for &offset in unsafe { version.as_ref() }.gc_offsets.iter() { + let value_ptr: *const u8 = offset.raw_ptr(cb); + // Creating an unaligned pointer is well defined unlike in C. + let value_ptr = value_ptr as *const VALUE; + + unsafe { + let object = value_ptr.read_unaligned(); + rb_gc_mark_movable(object); + } } } } @@ -115,8 +124,26 @@ fn iseq_update_references(payload: &mut IseqPayload) { } }); - // Move ISEQ references in IseqCall - for iseq_call in payload.iseq_calls.iter_mut() { + for &version in payload.versions.iter() { + iseq_version_update_references(version); + } +} + +fn iseq_version_update_references(mut version: IseqVersionRef) { + // Move ISEQ in the payload + unsafe { version.as_mut() }.iseq = unsafe { rb_gc_location(version.as_ref().iseq.into()) }.as_iseq(); + + // Move ISEQ references in incoming IseqCalls + for iseq_call in unsafe { version.as_mut() }.incoming.iter_mut() { + let old_iseq = iseq_call.iseq.get(); + let new_iseq = unsafe { rb_gc_location(VALUE(old_iseq as usize)) }.0 as IseqPtr; + if old_iseq != new_iseq { + iseq_call.iseq.set(new_iseq); + } + } + + // Move ISEQ references in outgoing IseqCalls + for iseq_call in unsafe { version.as_mut() }.outgoing.iter_mut() { let old_iseq = iseq_call.iseq.get(); let new_iseq = unsafe { rb_gc_location(VALUE(old_iseq as usize)) }.0 as IseqPtr; if old_iseq != new_iseq { @@ -126,7 +153,7 @@ fn iseq_update_references(payload: &mut IseqPayload) { // Move objects baked in JIT code let cb = ZJITState::get_code_block(); - for &offset in payload.gc_offsets.iter() { + for &offset in unsafe { version.as_ref() }.gc_offsets.iter() { let value_ptr: *const u8 = offset.raw_ptr(cb); // Creating an unaligned pointer is well defined unlike in C. let value_ptr = value_ptr as *const VALUE; @@ -146,9 +173,8 @@ fn iseq_update_references(payload: &mut IseqPayload) { } /// Append a set of gc_offsets to the iseq's payload -pub fn append_gc_offsets(iseq: IseqPtr, offsets: &Vec) { - let payload = get_or_create_iseq_payload(iseq); - payload.gc_offsets.extend(offsets); +pub fn append_gc_offsets(iseq: IseqPtr, mut version: IseqVersionRef, offsets: &Vec) { + unsafe { version.as_mut() }.gc_offsets.extend(offsets); // Call writebarrier on each newly added value let cb = ZJITState::get_code_block(); @@ -166,9 +192,8 @@ pub fn append_gc_offsets(iseq: IseqPtr, offsets: &Vec) { /// We do this when invalidation rewrites some code with a jump instruction /// and GC offsets are corrupted by the rewrite, assuming no on-stack code /// will step into the instruction with the GC offsets after invalidation. -pub fn remove_gc_offsets(payload_ptr: *mut IseqPayload, removed_range: &Range) { - let payload = payload_ptr_as_mut(payload_ptr); - payload.gc_offsets.retain(|&gc_offset| { +pub fn remove_gc_offsets(mut version: IseqVersionRef, removed_range: &Range) { + unsafe { version.as_mut() }.gc_offsets.retain(|&gc_offset| { let offset_range = gc_offset..(gc_offset.add_bytes(SIZEOF_VALUE)); !ranges_overlap(&offset_range, removed_range) }); diff --git a/zjit/src/invariants.rs b/zjit/src/invariants.rs index b6f5abe58bcd8e..749e0a9e1d38d9 100644 --- a/zjit/src/invariants.rs +++ b/zjit/src/invariants.rs @@ -3,7 +3,9 @@ use std::{collections::{HashMap, HashSet}, mem}; use crate::{backend::lir::{Assembler, asm_comment}, cruby::{ID, IseqPtr, RedefinitionFlag, VALUE, iseq_name, rb_callable_method_entry_t, rb_gc_location, ruby_basic_operators, src_loc, with_vm_lock}, hir::Invariant, options::debug, state::{ZJITState, zjit_enabled_p}, virtualmem::CodePtr}; -use crate::payload::IseqPayload; +use crate::payload::{IseqVersionRef, IseqStatus, get_or_create_iseq_payload}; +use crate::codegen::{MAX_ISEQ_VERSIONS, gen_iseq_call}; +use crate::cruby::{rb_iseq_reset_jit_func, iseq_get_location}; use crate::stats::with_time_stat; use crate::stats::Counter::invalidation_time_ns; use crate::gc::remove_gc_offsets; @@ -19,7 +21,25 @@ macro_rules! compile_patch_points { asm.compile(cb).expect("can write existing code"); }); // Stop marking GC offsets corrupted by the jump instruction - remove_gc_offsets(patch_point.payload_ptr, &written_range); + remove_gc_offsets(patch_point.version, &written_range); + + // If the ISEQ doesn't have max versions, invalidate this version. + let mut version = patch_point.version; + let iseq = unsafe { version.as_ref() }.iseq; + if !iseq.is_null() { + let payload = get_or_create_iseq_payload(iseq); + if unsafe { version.as_ref() }.status != IseqStatus::Invalidated && payload.versions.len() < MAX_ISEQ_VERSIONS { + unsafe { version.as_mut() }.status = IseqStatus::Invalidated; + unsafe { rb_iseq_reset_jit_func(version.as_ref().iseq) }; + + // Recompile JIT-to-JIT calls into the invalidated ISEQ + for incoming in unsafe { version.as_ref() }.incoming.iter() { + if let Err(err) = gen_iseq_call($cb, incoming) { + debug!("{err:?}: gen_iseq_call failed on PatchPoint: {}", iseq_get_location(incoming.iseq.get(), 0)); + } + } + } + } } }); }; @@ -32,17 +52,17 @@ struct PatchPoint { patch_point_ptr: CodePtr, /// Code pointer to a side exit side_exit_ptr: CodePtr, - /// Raw pointer to the ISEQ payload - payload_ptr: *mut IseqPayload, + /// ISEQ version to be invalidated + version: IseqVersionRef, } impl PatchPoint { /// PatchPointer constructor - fn new(patch_point_ptr: CodePtr, side_exit_ptr: CodePtr, payload_ptr: *mut IseqPayload) -> PatchPoint { + fn new(patch_point_ptr: CodePtr, side_exit_ptr: CodePtr, version: IseqVersionRef) -> PatchPoint { Self { patch_point_ptr, side_exit_ptr, - payload_ptr, + version, } } } @@ -206,13 +226,13 @@ pub fn track_no_ep_escape_assumption( iseq: IseqPtr, patch_point_ptr: CodePtr, side_exit_ptr: CodePtr, - payload_ptr: *mut IseqPayload, + version: IseqVersionRef, ) { let invariants = ZJITState::get_invariants(); invariants.no_ep_escape_iseq_patch_points.entry(iseq).or_default().insert(PatchPoint::new( patch_point_ptr, side_exit_ptr, - payload_ptr, + version, )); } @@ -227,13 +247,13 @@ pub fn track_bop_assumption( bop: ruby_basic_operators, patch_point_ptr: CodePtr, side_exit_ptr: CodePtr, - payload_ptr: *mut IseqPayload, + version: IseqVersionRef, ) { let invariants = ZJITState::get_invariants(); invariants.bop_patch_points.entry((klass, bop)).or_default().insert(PatchPoint::new( patch_point_ptr, side_exit_ptr, - payload_ptr, + version, )); } @@ -242,13 +262,13 @@ pub fn track_cme_assumption( cme: *const rb_callable_method_entry_t, patch_point_ptr: CodePtr, side_exit_ptr: CodePtr, - payload_ptr: *mut IseqPayload, + version: IseqVersionRef, ) { let invariants = ZJITState::get_invariants(); invariants.cme_patch_points.entry(cme).or_default().insert(PatchPoint::new( patch_point_ptr, side_exit_ptr, - payload_ptr, + version, )); } @@ -257,7 +277,7 @@ pub fn track_stable_constant_names_assumption( idlist: *const ID, patch_point_ptr: CodePtr, side_exit_ptr: CodePtr, - payload_ptr: *mut IseqPayload, + version: IseqVersionRef, ) { let invariants = ZJITState::get_invariants(); @@ -271,7 +291,7 @@ pub fn track_stable_constant_names_assumption( invariants.constant_state_patch_points.entry(id).or_default().insert(PatchPoint::new( patch_point_ptr, side_exit_ptr, - payload_ptr, + version, )); idx += 1; @@ -283,13 +303,13 @@ pub fn track_no_singleton_class_assumption( klass: VALUE, patch_point_ptr: CodePtr, side_exit_ptr: CodePtr, - payload_ptr: *mut IseqPayload, + version: IseqVersionRef, ) { let invariants = ZJITState::get_invariants(); invariants.no_singleton_class_patch_points.entry(klass).or_default().insert(PatchPoint::new( patch_point_ptr, side_exit_ptr, - payload_ptr, + version, )); } @@ -339,12 +359,16 @@ pub extern "C" fn rb_zjit_constant_state_changed(id: ID) { } /// Track the JIT code that assumes that the interpreter is running with only one ractor -pub fn track_single_ractor_assumption(patch_point_ptr: CodePtr, side_exit_ptr: CodePtr, payload_ptr: *mut IseqPayload) { +pub fn track_single_ractor_assumption( + patch_point_ptr: CodePtr, + side_exit_ptr: CodePtr, + version: IseqVersionRef, +) { let invariants = ZJITState::get_invariants(); invariants.single_ractor_patch_points.insert(PatchPoint::new( patch_point_ptr, side_exit_ptr, - payload_ptr, + version, )); } @@ -368,19 +392,23 @@ pub extern "C" fn rb_zjit_before_ractor_spawn() { }); } -pub fn track_no_trace_point_assumption(patch_point_ptr: CodePtr, side_exit_ptr: CodePtr, payload_ptr: *mut IseqPayload) { +pub fn track_no_trace_point_assumption( + patch_point_ptr: CodePtr, + side_exit_ptr: CodePtr, + version: IseqVersionRef, +) { let invariants = ZJITState::get_invariants(); invariants.no_trace_point_patch_points.insert(PatchPoint::new( patch_point_ptr, side_exit_ptr, - payload_ptr, + version, )); } #[unsafe(no_mangle)] pub extern "C" fn rb_zjit_tracing_invalidate_all() { use crate::payload::{get_or_create_iseq_payload, IseqStatus}; - use crate::cruby::{for_each_iseq, rb_iseq_reset_jit_func}; + use crate::cruby::for_each_iseq; if !zjit_enabled_p() { return; @@ -393,7 +421,9 @@ pub extern "C" fn rb_zjit_tracing_invalidate_all() { for_each_iseq(|iseq| { let payload = get_or_create_iseq_payload(iseq); - payload.status = IseqStatus::NotCompiled; + if let Some(version) = payload.versions.last_mut() { + unsafe { version.as_mut() }.status = IseqStatus::Invalidated; + } unsafe { rb_iseq_reset_jit_func(iseq) }; }); diff --git a/zjit/src/payload.rs b/zjit/src/payload.rs index 1fb3f919946dbb..8540d5e35c498e 100644 --- a/zjit/src/payload.rs +++ b/zjit/src/payload.rs @@ -1,4 +1,5 @@ use std::ffi::c_void; +use std::ptr::NonNull; use crate::codegen::IseqCallRef; use crate::stats::CompileError; use crate::{cruby::*, profile::IseqProfile, virtualmem::CodePtr}; @@ -6,27 +7,55 @@ use crate::{cruby::*, profile::IseqProfile, virtualmem::CodePtr}; /// 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 versions. Different versions should have different assumptions. + pub versions: Vec, +} + +impl IseqPayload { + fn new(iseq_size: u32) -> Self { + Self { + profile: IseqProfile::new(iseq_size), + versions: vec![], + } + } +} + +/// JIT code version. When the same ISEQ is compiled with a different assumption, a new version is created. +#[derive(Debug)] +pub struct IseqVersion { + /// ISEQ pointer. Stored here to minimize the size of PatchPoint. + pub iseq: IseqPtr, + + /// Compilation status of the ISEQ. It has the JIT code address of the first block if Compiled. + pub status: IseqStatus, /// GC offsets of the JIT code. These are the addresses of objects that need to be marked. pub gc_offsets: Vec, - /// JIT-to-JIT calls in the ISEQ. The IseqPayload's ISEQ is the caller of it. - pub iseq_calls: Vec, + /// JIT-to-JIT calls from the ISEQ. The IseqPayload's ISEQ is the caller of it. + pub outgoing: Vec, + + /// JIT-to-JIT calls to the ISEQ. The IseqPayload's ISEQ is the callee of it. + pub incoming: Vec, } -impl IseqPayload { - fn new(iseq_size: u32) -> Self { - Self { +/// We use a raw pointer instead of Rc to save space for refcount +pub type IseqVersionRef = NonNull; + +impl IseqVersion { + /// Allocate a new IseqVersion to be compiled + pub fn new(iseq: IseqPtr) -> IseqVersionRef { + let version = Self { + iseq, status: IseqStatus::NotCompiled, - profile: IseqProfile::new(iseq_size), gc_offsets: vec![], - iseq_calls: vec![], - } + outgoing: vec![], + incoming: vec![], + }; + let version_ptr = Box::into_raw(Box::new(version)); + NonNull::new(version_ptr).expect("no null from Box") } } @@ -44,6 +73,7 @@ pub enum IseqStatus { Compiled(IseqCodePtrs), CantCompile(CompileError), NotCompiled, + Invalidated, } /// Get a pointer to the payload object associated with an ISEQ. Create one if none exists. diff --git a/zjit/src/stats.rs b/zjit/src/stats.rs index 37345f93435185..25f0c638be2998 100644 --- a/zjit/src/stats.rs +++ b/zjit/src/stats.rs @@ -285,6 +285,7 @@ make_counters! { } // compile_error_: Compile error reasons + compile_error_iseq_version_limit_reached, compile_error_iseq_stack_too_large, compile_error_exception_handler, compile_error_out_of_memory, @@ -428,6 +429,7 @@ pub fn send_fallback_counter_ptr_for_opcode(opcode: u32) -> *mut u64 { /// Reason why ZJIT failed to produce any JIT code #[derive(Clone, Debug, PartialEq)] pub enum CompileError { + IseqVersionLimitReached, IseqStackTooLarge, ExceptionHandler, OutOfMemory, @@ -441,9 +443,10 @@ pub fn exit_counter_for_compile_error(compile_error: &CompileError) -> Counter { use crate::stats::CompileError::*; use crate::stats::Counter::*; match compile_error { - IseqStackTooLarge => compile_error_iseq_stack_too_large, - ExceptionHandler => compile_error_exception_handler, - OutOfMemory => compile_error_out_of_memory, + IseqVersionLimitReached => compile_error_iseq_version_limit_reached, + IseqStackTooLarge => compile_error_iseq_stack_too_large, + ExceptionHandler => compile_error_exception_handler, + OutOfMemory => compile_error_out_of_memory, ParseError(parse_error) => match parse_error { StackUnderflow(_) => compile_error_parse_stack_underflow, MalformedIseq(_) => compile_error_parse_malformed_iseq,