Skip to content

Commit 0356687

Browse files
tekknolagitekknolagi
committed
ZJIT: Add assume_no_singleton_classes to avoid invalidation loops
1 parent 1ca0660 commit 0356687

File tree

6 files changed

+334
-229
lines changed

6 files changed

+334
-229
lines changed

‎zjit/src/cruby_methods.rs‎

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -857,6 +857,10 @@ fn inline_kernel_respond_to_p(
857857
}
858858
(_, _) => return None, // not public and include_all not known, can't compile
859859
};
860+
// Check singleton class assumption first, before emitting other patchpoints
861+
if !fun.assume_no_singleton_classes(block, recv_class, state) {
862+
return None;
863+
}
860864
fun.push_insn(block, hir::Insn::PatchPoint { invariant: hir::Invariant::NoTracePoint, state });
861865
fun.push_insn(block, hir::Insn::PatchPoint {
862866
invariant: hir::Invariant::MethodRedefined {
@@ -865,11 +869,6 @@ fn inline_kernel_respond_to_p(
865869
cme: target_cme
866870
}, state
867871
});
868-
if recv_class.instance_can_have_singleton_class() {
869-
fun.push_insn(block, hir::Insn::PatchPoint {
870-
invariant: hir::Invariant::NoSingletonClass { klass: recv_class }, state
871-
});
872-
}
873872
Some(fun.push_insn(block, hir::Insn::Const { val: hir::Const::Value(result) }))
874873
}
875874

‎zjit/src/hir.rs‎

Lines changed: 70 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77
#![allow(clippy::match_like_matches_macro)]
88
use crate::{
99
backend::lir::C_ARG_OPNDS,
10-
cast::IntoUsize, codegen::local_idx_to_ep_offset, cruby::*, payload::{get_or_create_iseq_payload, IseqPayload}, options::{debug, get_option, DumpHIR}, state::ZJITState, json::Json
10+
cast::IntoUsize, codegen::local_idx_to_ep_offset, cruby::*, invariants::has_singleton_class_of, payload::{get_or_create_iseq_payload, IseqPayload}, options::{debug, get_option, DumpHIR}, state::ZJITState, json::Json
1111
};
1212
use std::{
1313
cell::RefCell, collections::{BTreeSet, HashMap, HashSet, VecDeque}, ffi::{c_void, c_uint, c_int, CStr}, fmt::Display, mem::{align_of, size_of}, ptr, slice::Iter
@@ -644,6 +644,9 @@ pub enum SendFallbackReason {
644644
ComplexArgPass,
645645
/// Caller has keyword arguments but callee doesn't expect them; need to convert to hash.
646646
UnexpectedKeywordArgs,
647+
/// A singleton class has been seen for the receiver class, so we skip the optimization
648+
/// to avoid an invalidation loop.
649+
SingletonClassSeen,
647650
/// Initial fallback reason for every instruction, which should be mutated to
648651
/// a more actionable reason when an attempt to specialize the instruction fails.
649652
Uncategorized(ruby_vminsn_type),
@@ -680,6 +683,7 @@ impl Display for SendFallbackReason {
680683
ArgcParamMismatch => write!(f, "Argument count does not match parameter count"),
681684
ComplexArgPass => write!(f, "Complex argument passing"),
682685
UnexpectedKeywordArgs => write!(f, "Unexpected Keyword Args"),
686+
SingletonClassSeen => write!(f, "Singleton class previously created for receiver class"),
683687
Uncategorized(insn) => write!(f, "Uncategorized({})", insn_name(*insn as usize)),
684688
}
685689
}
@@ -1891,6 +1895,24 @@ impl Function {
18911895
}
18921896
}
18931897

1898+
/// Assume that objects of a given class will have no singleton class.
1899+
/// Returns true if safe to assume so and emits a PatchPoint.
1900+
/// Returns false if we've already seen a singleton class for this class,
1901+
/// to avoid an invalidation loop.
1902+
pub fn assume_no_singleton_classes(&mut self, block: BlockId, klass: VALUE, state: InsnId) -> bool {
1903+
if !klass.instance_can_have_singleton_class() {
1904+
// This class can never have a singleton class, so no patchpoint needed.
1905+
return true;
1906+
}
1907+
if has_singleton_class_of(klass) {
1908+
// We've seen a singleton class for this klass. Disable the optimization
1909+
// to avoid an invalidation loop.
1910+
return false;
1911+
}
1912+
self.push_insn(block, Insn::PatchPoint { invariant: Invariant::NoSingletonClass { klass }, state });
1913+
true
1914+
}
1915+
18941916
/// Return a copy of the instruction where the instruction and its operands have been read from
18951917
/// the union-find table (to find the current most-optimized version of this instruction). See
18961918
/// [`UnionFind`] for more.
@@ -2531,8 +2553,8 @@ impl Function {
25312553
return false;
25322554
}
25332555
self.gen_patch_points_for_optimized_ccall(block, class, method_id, cme, state);
2534-
if class.instance_can_have_singleton_class() {
2535-
self.push_insn(block, Insn::PatchPoint { invariant: Invariant::NoSingletonClass { klass: class }, state });
2556+
if !self.assume_no_singleton_classes(block, class, state) {
2557+
return false;
25362558
}
25372559
true
25382560
}
@@ -2702,10 +2724,12 @@ impl Function {
27022724
if !can_direct_send(self, block, iseq, insn_id, args.as_slice()) {
27032725
self.push_insn_id(block, insn_id); continue;
27042726
}
2705-
self.push_insn(block, Insn::PatchPoint { invariant: Invariant::MethodRedefined { klass, method: mid, cme }, state });
2706-
if klass.instance_can_have_singleton_class() {
2707-
self.push_insn(block, Insn::PatchPoint { invariant: Invariant::NoSingletonClass { klass }, state });
2727+
// Check singleton class assumption first, before emitting other patchpoints
2728+
if !self.assume_no_singleton_classes(block, klass, state) {
2729+
self.set_dynamic_send_reason(insn_id, SingletonClassSeen);
2730+
self.push_insn_id(block, insn_id); continue;
27082731
}
2732+
self.push_insn(block, Insn::PatchPoint { invariant: Invariant::MethodRedefined { klass, method: mid, cme }, state });
27092733
if let Some(profiled_type) = profiled_type {
27102734
recv = self.push_insn(block, Insn::GuardType { val: recv, guard_type: Type::from_profiled_type(profiled_type), state });
27112735
}
@@ -2754,10 +2778,12 @@ impl Function {
27542778
// TODO(alan): Turn this into a ractor belonging guard to work better in multi ractor mode.
27552779
self.push_insn_id(block, insn_id); continue;
27562780
}
2757-
self.push_insn(block, Insn::PatchPoint { invariant: Invariant::MethodRedefined { klass, method: mid, cme }, state });
2758-
if klass.instance_can_have_singleton_class() {
2759-
self.push_insn(block, Insn::PatchPoint { invariant: Invariant::NoSingletonClass { klass }, state });
2781+
// Check singleton class assumption first, before emitting other patchpoints
2782+
if !self.assume_no_singleton_classes(block, klass, state) {
2783+
self.set_dynamic_send_reason(insn_id, SingletonClassSeen);
2784+
self.push_insn_id(block, insn_id); continue;
27602785
}
2786+
self.push_insn(block, Insn::PatchPoint { invariant: Invariant::MethodRedefined { klass, method: mid, cme }, state });
27612787

27622788
if let Some(profiled_type) = profiled_type {
27632789
recv = self.push_insn(block, Insn::GuardType { val: recv, guard_type: Type::from_profiled_type(profiled_type), state });
@@ -2788,11 +2814,13 @@ impl Function {
27882814
if self.is_metaclass(klass) && !self.assume_single_ractor_mode(block, state) {
27892815
self.push_insn_id(block, insn_id); continue;
27902816
}
2817+
// Check singleton class assumption first, before emitting other patchpoints
2818+
if !self.assume_no_singleton_classes(block, klass, state) {
2819+
self.set_dynamic_send_reason(insn_id, SingletonClassSeen);
2820+
self.push_insn_id(block, insn_id); continue;
2821+
}
27912822

27922823
self.push_insn(block, Insn::PatchPoint { invariant: Invariant::MethodRedefined { klass, method: mid, cme }, state });
2793-
if klass.instance_can_have_singleton_class() {
2794-
self.push_insn(block, Insn::PatchPoint { invariant: Invariant::NoSingletonClass { klass }, state });
2795-
}
27962824
if let Some(profiled_type) = profiled_type {
27972825
recv = self.push_insn(block, Insn::GuardType { val: recv, guard_type: Type::from_profiled_type(profiled_type), state });
27982826
}
@@ -2835,10 +2863,12 @@ impl Function {
28352863
// No (monomorphic/skewed polymorphic) profile info
28362864
self.push_insn_id(block, insn_id); continue;
28372865
};
2838-
self.push_insn(block, Insn::PatchPoint { invariant: Invariant::MethodRedefined { klass, method: mid, cme }, state });
2839-
if klass.instance_can_have_singleton_class() {
2840-
self.push_insn(block, Insn::PatchPoint { invariant: Invariant::NoSingletonClass { klass }, state });
2866+
// Check singleton class assumption first, before emitting other patchpoints
2867+
if !self.assume_no_singleton_classes(block, klass, state) {
2868+
self.set_dynamic_send_reason(insn_id, SingletonClassSeen);
2869+
self.push_insn_id(block, insn_id); continue;
28412870
}
2871+
self.push_insn(block, Insn::PatchPoint { invariant: Invariant::MethodRedefined { klass, method: mid, cme }, state });
28422872
if let Some(profiled_type) = profiled_type {
28432873
recv = self.push_insn(block, Insn::GuardType { val: recv, guard_type: Type::from_profiled_type(profiled_type), state });
28442874
}
@@ -3375,11 +3405,14 @@ impl Function {
33753405
return Err(());
33763406
}
33773407

3408+
// Check singleton class assumption first, before emitting other patchpoints
3409+
if !fun.assume_no_singleton_classes(block, recv_class, state) {
3410+
fun.set_dynamic_send_reason(send_insn_id, SingletonClassSeen);
3411+
return Err(());
3412+
}
3413+
33783414
// Commit to the replacement. Put PatchPoint.
33793415
fun.gen_patch_points_for_optimized_ccall(block, recv_class, method_id, cme, state);
3380-
if recv_class.instance_can_have_singleton_class() {
3381-
fun.push_insn(block, Insn::PatchPoint { invariant: Invariant::NoSingletonClass { klass: recv_class }, state });
3382-
}
33833416

33843417
if let Some(profiled_type) = profiled_type {
33853418
// Guard receiver class
@@ -3410,11 +3443,15 @@ impl Function {
34103443
-1 => {
34113444
// The method gets a pointer to the first argument
34123445
// func(int argc, VALUE *argv, VALUE recv)
3413-
fun.gen_patch_points_for_optimized_ccall(block, recv_class, method_id, cme, state);
34143446

3415-
if recv_class.instance_can_have_singleton_class() {
3416-
fun.push_insn(block, Insn::PatchPoint { invariant: Invariant::NoSingletonClass { klass: recv_class }, state });
3447+
// Check singleton class assumption first, before emitting other patchpoints
3448+
if !fun.assume_no_singleton_classes(block, recv_class, state) {
3449+
fun.set_dynamic_send_reason(send_insn_id, SingletonClassSeen);
3450+
return Err(());
34173451
}
3452+
3453+
fun.gen_patch_points_for_optimized_ccall(block, recv_class, method_id, cme, state);
3454+
34183455
if let Some(profiled_type) = profiled_type {
34193456
// Guard receiver class
34203457
recv = fun.push_insn(block, Insn::GuardType { val: recv, guard_type: Type::from_profiled_type(profiled_type), state });
@@ -3522,11 +3559,14 @@ impl Function {
35223559
return Err(());
35233560
}
35243561

3562+
// Check singleton class assumption first, before emitting other patchpoints
3563+
if !fun.assume_no_singleton_classes(block, recv_class, state) {
3564+
fun.set_dynamic_send_reason(send_insn_id, SingletonClassSeen);
3565+
return Err(());
3566+
}
3567+
35253568
// Commit to the replacement. Put PatchPoint.
35263569
fun.gen_patch_points_for_optimized_ccall(block, recv_class, method_id, cme, state);
3527-
if recv_class.instance_can_have_singleton_class() {
3528-
fun.push_insn(block, Insn::PatchPoint { invariant: Invariant::NoSingletonClass { klass: recv_class }, state });
3529-
}
35303570

35313571
let props = ZJITState::get_method_annotations().get_cfunc_properties(cme);
35323572
if props.is_none() && get_option!(stats) {
@@ -3599,11 +3639,14 @@ impl Function {
35993639
fun.set_dynamic_send_reason(send_insn_id, ComplexArgPass);
36003640
return Err(());
36013641
} else {
3642+
// Check singleton class assumption first, before emitting other patchpoints
3643+
if !fun.assume_no_singleton_classes(block, recv_class, state) {
3644+
fun.set_dynamic_send_reason(send_insn_id, SingletonClassSeen);
3645+
return Err(());
3646+
}
3647+
36023648
fun.gen_patch_points_for_optimized_ccall(block, recv_class, method_id, cme, state);
36033649

3604-
if recv_class.instance_can_have_singleton_class() {
3605-
fun.push_insn(block, Insn::PatchPoint { invariant: Invariant::NoSingletonClass { klass: recv_class }, state });
3606-
}
36073650
if let Some(profiled_type) = profiled_type {
36083651
// Guard receiver class
36093652
recv = fun.push_insn(block, Insn::GuardType { val: recv, guard_type: Type::from_profiled_type(profiled_type), state });

0 commit comments

Comments
 (0)