Skip to content

Commit 717239a

Browse files
committed
ZJIT: Re-profile whole ISEQ in non-final versions
Reset each existing instruction profile and reinstall zjit_* instructions after compiling a non-final version. Do the same for inlined ISEQs so materialized frames can collect fresh profiles. Let interpreter profiling instructions collect the samples and make recompile exits only observe whether the exiting instruction has finished profiling.
1 parent 6e055f5 commit 717239a

7 files changed

Lines changed: 74 additions & 51 deletions

File tree

zjit/bindgen/src/main.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -110,6 +110,7 @@ fn main() {
110110
.allowlist_function("rb_funcallv")
111111
.allowlist_function("rb_protect")
112112
.allowlist_function("rb_zjit_profile_disable")
113+
.allowlist_function("rb_zjit_profile_enable")
113114
.allowlist_function("rb_zjit_insn_to_bare_insn")
114115
.allowlist_function("rb_zjit_hash_new_size")
115116
.allowlist_function("rb_zjit_class_allocate_instance_fastpath")

zjit/src/codegen.rs

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ use crate::invariants::{
1717
};
1818
use crate::gc::append_gc_offsets;
1919
use crate::payload::{IseqCodePtrs, IseqStatus, IseqVersion, IseqVersionRef, JITFrame, get_or_create_iseq_payload};
20+
use crate::profile::reset_profiles_remaining;
2021
use crate::state::ZJITState;
2122
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_for_super_method_type, send_fallback_counter_ptr_for_opcode, send_fallback_counter_for_optimized_method_type};
2223
use crate::stats::{counter_ptr, with_time_stat, trace_compile_phase, Counter, Counter::{compile_time_ns, exit_compile_error}};
@@ -3361,6 +3362,10 @@ fn compile_iseq(iseq: IseqPtr) -> Result<Function, CompileError> {
33613362
trace_compile_phase("optimize", || function.optimize());
33623363
}
33633364
function.dump_hir();
3365+
let non_final_version = get_or_create_iseq_payload(iseq).versions.len() + 1 < max_iseq_versions();
3366+
if non_final_version {
3367+
reset_profiles_remaining(iseq);
3368+
}
33643369
Ok(function)
33653370
}
33663371

zjit/src/codegen_tests.rs

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -122,6 +122,28 @@ fn test_putobject() {
122122
"), @"1");
123123
}
124124

125+
#[test]
126+
fn test_recompile_exit_waits_for_interpreter_profiles() {
127+
set_call_threshold(2);
128+
eval("
129+
def recompile_profile_window(a, b) = a + b
130+
recompile_profile_window(1, 2)
131+
recompile_profile_window(1, 2)
132+
");
133+
134+
let iseq = get_method_iseq("self", "recompile_profile_window");
135+
let num_profiles = get_option!(num_profiles);
136+
for _ in 0..num_profiles {
137+
eval("recompile_profile_window(1.5, 2.5)");
138+
}
139+
let payload = get_or_create_iseq_payload(iseq);
140+
assert!(!unsafe { payload.versions.last().unwrap().as_ref() }.is_invalidated());
141+
142+
eval("recompile_profile_window(1.5, 2.5)");
143+
let payload = get_or_create_iseq_payload(iseq);
144+
assert!(unsafe { payload.versions.last().unwrap().as_ref() }.is_invalidated());
145+
}
146+
125147
#[test]
126148
fn test_dupstring() {
127149
eval(r##"

zjit/src/cruby_bindings.inc.rs

Lines changed: 1 addition & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

zjit/src/hir.rs

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,9 +6,7 @@
66
#![allow(clippy::if_same_then_else)]
77
#![allow(clippy::match_like_matches_macro)]
88
use crate::{
9-
backend::lir::C_ARG_OPNDS,
10-
cast::IntoUsize, codegen::{max_iseq_versions}, cruby::*, invariants::{self, iseq_seen_ep_escape}, payload::get_or_create_iseq_payload, options::{debug, get_option, DumpHIR, InlineDepth}, state::ZJITState, json::Json,
11-
state,
9+
backend::lir::C_ARG_OPNDS, cast::IntoUsize, codegen::max_iseq_versions, cruby::*, invariants::{self, iseq_seen_ep_escape}, json::Json, options::{DumpHIR, InlineDepth, debug, get_option}, payload::get_or_create_iseq_payload, profile::reset_profiles_remaining, state::{self, ZJITState},
1210
};
1311
use std::{
1412
cell::RefCell, collections::{HashMap, HashSet, VecDeque}, ffi::{c_void, c_uint, c_int, CStr}, fmt::Display, mem::{align_of, size_of}, ptr, slice::Iter,
@@ -9751,6 +9749,10 @@ fn add_iseq_to_hir(
97519749
None => {},
97529750
}
97539751
}
9752+
if matches!(mode, AddIseqMode::Inlined { .. }) {
9753+
// Materialized inlined frames also need fresh interpreter profiles.
9754+
reset_profiles_remaining(iseq);
9755+
}
97549756

97559757
Ok(AddIseqResult { body_entry_block, profiles })
97569758
}

zjit/src/hir/opt_tests.rs

Lines changed: 28 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -5602,11 +5602,15 @@ mod hir_opt_tests {
56025602
v36:NilClass = Const Value(nil)
56035603
Jump bb8(v36, v13)
56045604
bb8(v26:BasicObject, v27:BasicObject):
5605-
v39:BasicObject = Send v24, &block, :then, v26 # SendFallbackReason: Send: block argument is not nil
5605+
v56:NilClass = GuardBitEquals v26, Value(nil) recompile
5606+
PatchPoint MethodRedefined(Integer@0x1008, then@0x1010, cme:0x1018)
5607+
PushInlineFrame v24 (0x1040)
5608+
v76:BasicObject = InvokeBuiltin <inline_expr>, v24
56065609
CheckInterrupts
5607-
Return v39
5610+
PopInlineFrame
5611+
Return v76
56085612
bb4(v44:BasicObject, v45:Falsy, v46:BasicObject):
5609-
v50:StaticSymbol[:skip] = Const Value(VALUE(0x1008))
5613+
v50:StaticSymbol[:skip] = Const Value(VALUE(0x1048))
56105614
CheckInterrupts
56115615
Return v50
56125616
");
@@ -9175,12 +9179,17 @@ mod hir_opt_tests {
91759179
Jump bb3(v6, v7)
91769180
bb3(v9:HeapBasicObject, v10:BasicObject):
91779181
v17:Fixnum[5] = Const Value(5)
9182+
v21:CBool = HasType v10, ObjectSubclass[class_exact:C]
9183+
CondBranch v21, bb5(), bb6()
9184+
bb5():
9185+
v24:ObjectSubclass[class_exact:C] = RefineType v10, ObjectSubclass[class_exact:C]
91789186
PatchPoint MethodRedefined(C@0x1008, foo=@0x1010, cme:0x1018)
9179-
v28:ObjectSubclass[class_exact:C] = GuardType v10, ObjectSubclass[class_exact:C] recompile
9180-
v30:CShape = LoadField v28, :shape_id@0x1040
9181-
v31:CShape[0x1041] = GuardBitEquals v30, CShape(0x1041)
9182-
StoreField v28, :@foo@0x1042, v17
9183-
WriteBarrier v28, v17
9187+
SetIvar v24, :@foo, v17
9188+
Jump bb4(v17)
9189+
bb6():
9190+
v27:BasicObject = Send v10, :foo=, v17 # SendFallbackReason: Send: polymorphic fallback
9191+
Jump bb4(v27)
9192+
bb4(v20:BasicObject):
91849193
CheckInterrupts
91859194
Return v17
91869195
");
@@ -18809,27 +18818,25 @@ mod hir_opt_tests {
1880918818
bb4():
1881018819
PatchPoint NoEPEscape(f)
1881118820
v44:Fixnum[1] = Const Value(1)
18812-
v48:CBool = HasType v12, Flonum
18821+
v48:CBool = HasType v12, Fixnum
1881318822
CondBranch v48, bb10(), bb11()
1881418823
bb10():
18815-
v51:Flonum = RefineType v12, Flonum
18816-
PatchPoint MethodRedefined(Float@0x1008, +@0x1010, cme:0x1018)
18817-
v86:Float = FloatAdd v51, v44
18824+
v51:Fixnum = RefineType v12, Fixnum
18825+
PatchPoint MethodRedefined(Integer@0x1008, +@0x1010, cme:0x1018)
18826+
v86:Fixnum = FixnumAdd v51, v44
1881818827
Jump bb9(v86)
1881918828
bb11():
18820-
v54:CBool = HasType v12, Fixnum
18829+
v54:CBool = HasType v12, Flonum
1882118830
CondBranch v54, bb12(), bb13()
1882218831
bb12():
18823-
v57:Fixnum = RefineType v12, Fixnum
18824-
PatchPoint MethodRedefined(Integer@0x1040, +@0x1010, cme:0x1048)
18825-
v89:Fixnum = FixnumAdd v57, v44
18832+
v57:Flonum = RefineType v12, Flonum
18833+
PatchPoint MethodRedefined(Float@0x1040, +@0x1010, cme:0x1048)
18834+
v89:Float = FloatAdd v57, v44
1882618835
Jump bb9(v89)
1882718836
bb13():
18828-
PatchPoint MethodRedefined(Float@0x1008, +@0x1010, cme:0x1018)
18829-
v92:Flonum = GuardType v12, Flonum recompile
18830-
v93:Float = FloatAdd v92, v44
18831-
Jump bb9(v93)
18832-
bb9(v47:Float|Fixnum):
18837+
v60:BasicObject = Send v12, :+, v44 # SendFallbackReason: Send: polymorphic fallback
18838+
Jump bb9(v60)
18839+
bb9(v47:BasicObject):
1883318840
PatchPoint SingleRactorMode
1883418841
v69:CShape = LoadField v11, :shape_id@0x1001
1883518842
v70:CShape[0x1002] = Const CShape(0x1002)

zjit/src/profile.rs

Lines changed: 12 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -122,36 +122,21 @@ fn profile_insn(bare_opcode: ruby_vminsn_type, ec: EcPtr) {
122122
}
123123
}
124124

125-
/// Profile the instruction at the current CFP for a recompile side exit.
125+
/// Return whether the interpreter finished profiling the current instruction.
126126
pub fn profile_recompile_insn(ec: EcPtr) -> bool {
127-
let profiler = &mut Profiler::new(ec);
128-
let pc = unsafe { get_cfp_pc(profiler.cfp) };
129-
let bare_opcode = unsafe {
130-
rb_zjit_insn_to_bare_insn(rb_iseq_opcode_at_pc(profiler.iseq, pc))
131-
} as ruby_vminsn_type;
132-
let profile = &mut get_or_create_iseq_payload(profiler.iseq).profile;
133-
134-
let is_send = matches!(bare_opcode, YARVINSN_send | YARVINSN_opt_send_without_block);
135-
// For now, send recompile exits only fill in missing profiles. Once the send site
136-
// has finished profiling, don't recompile it on later exits.
137-
if is_send && profile.done_profiling_at(profiler.insn_idx) {
138-
return false;
139-
}
140-
// For now, non-send recompile exits reset the profiling counter before requesting recompilation
141-
// so that we can collect enough samples.
142-
if !is_send && profile.done_profiling_at(profiler.insn_idx) {
143-
profile.entry_mut(profiler.insn_idx)
144-
.set_profiles_remaining(get_option!(num_profiles));
145-
}
127+
let profiler = &Profiler::new(ec);
128+
get_or_create_iseq_payload(profiler.iseq).profile.done_profiling_at(profiler.insn_idx)
129+
}
146130

147-
// If this opcode can't be sampled here, this exit has no profile data to collect.
148-
if !profile_insn_sample(bare_opcode, profiler, profile) {
149-
return false;
131+
/// Reset existing profile counters and install profiling instructions throughout an ISEQ.
132+
/// Newly reached instructions initialize their counters from the same option.
133+
pub(crate) fn reset_profiles_remaining(iseq: IseqPtr) {
134+
let profile = &mut get_or_create_iseq_payload(iseq).profile;
135+
let num_profiles = get_option!(num_profiles);
136+
for entry in &mut profile.entries {
137+
entry.profiles_remaining = num_profiles;
150138
}
151-
152-
let entry = profile.entry_mut(profiler.insn_idx);
153-
entry.profiles_remaining = entry.profiles_remaining.saturating_sub(1);
154-
entry.profiles_remaining == 0
139+
unsafe { rb_zjit_profile_enable(iseq) };
155140
}
156141

157142
/// Return the argc as stated in the calldata plus:

0 commit comments

Comments
 (0)