Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
bridges_compiled=0
descr_set_absent=0
descr_set_ambiguous=0
descr_set_stale_absent=0
fbw_blackhole_adopted_multi_frame=0
fbw_blackhole_adopted_single_frame=0
fbw_rolled_back_with_effects=0
fbw_store_journal_rollback_failed=0
field_pos_attached_misplaced=0
field_pos_spec_misplaced=0
guard_failures=1
internal_compile_panics=0
loops_aborted=1
loops_compiled=2
retraces_compiled=0
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
bridges_compiled=0
descr_set_absent=0
descr_set_ambiguous=0
descr_set_stale_absent=0
fbw_blackhole_adopted_multi_frame=0
fbw_blackhole_adopted_single_frame=0
fbw_rolled_back_with_effects=0
fbw_store_journal_rollback_failed=0
field_pos_attached_misplaced=0
field_pos_spec_misplaced=0
guard_failures=1
internal_compile_panics=0
loops_aborted=1
loops_compiled=2
retraces_compiled=0
38 changes: 38 additions & 0 deletions pyre/bench/synth/foriter_isinstance_class_property_replay.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
# `isinstance(obj, T)` first checks the object's real MRO and, on a miss,
# reads `obj.__class__`. Inheriting `object.__getattribute__` does not make
# that read pure: a class can still override `__class__` with a property.
#
# `helper` is admitted into the surrounding FOR_ITER body. The trailing
# opaque `id` call makes the first inline sub-walk abort and replay `helper`.
# If the `isinstance` call is incorrectly marked replay-safe, its property
# effect is not recorded and that one replay invokes it twice (N + 1 hits).

N = 5000
hits = [0]


class C:
def __init__(self):
self.x = 1

@property
def __class__(self):
hits[0] += 1
return str


def helper(obj):
# Memoize that C inherits object.__getattribute__; this was the incomplete
# proof used by the buggy replay-safe predicate.
obj.x
result = isinstance(obj, int)
id(obj)
return result


obj = C()
total = 0
for _ in range(N):
total += helper(obj)

print(hits[0], total)
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
bridges_compiled=0
descr_set_absent=0
descr_set_ambiguous=0
descr_set_stale_absent=0
fbw_blackhole_adopted_multi_frame=0
fbw_blackhole_adopted_single_frame=0
fbw_rolled_back_with_effects=0
fbw_store_journal_rollback_failed=0
field_pos_attached_misplaced=0
field_pos_spec_misplaced=0
guard_failures=1
internal_compile_panics=0
loops_aborted=1
loops_compiled=2
retraces_compiled=0
15 changes: 15 additions & 0 deletions pyre/bench/synth/foriter_str_subclass_replay.cranelift.jitstats
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
bridges_compiled=0
descr_set_absent=0
descr_set_ambiguous=0
descr_set_stale_absent=0
fbw_blackhole_adopted_multi_frame=0
fbw_blackhole_adopted_single_frame=0
fbw_rolled_back_with_effects=0
fbw_store_journal_rollback_failed=0
field_pos_attached_misplaced=0
field_pos_spec_misplaced=0
guard_failures=1
internal_compile_panics=0
loops_aborted=1
loops_compiled=2
retraces_compiled=0
15 changes: 15 additions & 0 deletions pyre/bench/synth/foriter_str_subclass_replay.dynasm.jitstats
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
bridges_compiled=0
descr_set_absent=0
descr_set_ambiguous=0
descr_set_stale_absent=0
fbw_blackhole_adopted_multi_frame=0
fbw_blackhole_adopted_single_frame=0
fbw_rolled_back_with_effects=0
fbw_store_journal_rollback_failed=0
field_pos_attached_misplaced=0
field_pos_spec_misplaced=0
guard_failures=1
internal_compile_panics=0
loops_aborted=1
loops_compiled=2
retraces_compiled=0
27 changes: 27 additions & 0 deletions pyre/bench/synth/foriter_str_subclass_replay.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
# The FOR_ITER inline replay exemption for `str(value)` applies only to exact
# immutable builtin scalars. A str subclass can override `__str__` and mutate
# visible state. The trailing opaque `id` call makes the first inline walk
# abort; treating the subclass as exact replays that override once (N + 1).

N = 5000
hits = [0]


class S(str):
def __str__(self):
hits[0] += 1
return "x"


def helper(value):
result = str(value)
id(value)
return len(result)


value = S("abc")
total = 0
for _ in range(N):
total += helper(value)

print(hits[0], total)
15 changes: 15 additions & 0 deletions pyre/bench/synth/foriter_str_subclass_replay.wasm.jitstats
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
bridges_compiled=0
descr_set_absent=0
descr_set_ambiguous=0
descr_set_stale_absent=0
fbw_blackhole_adopted_multi_frame=0
fbw_blackhole_adopted_single_frame=0
fbw_rolled_back_with_effects=0
fbw_store_journal_rollback_failed=0
field_pos_attached_misplaced=0
field_pos_spec_misplaced=0
guard_failures=1
internal_compile_panics=0
loops_aborted=1
loops_compiled=2
retraces_compiled=0
39 changes: 39 additions & 0 deletions pyre/pyre-interpreter/src/baseobjspace.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9973,6 +9973,27 @@ pub(crate) unsafe fn has_object_getattribute(w_type: PyObjectRef) -> bool {
getattribute_if_not_from_object(w_type).is_none()
}

/// Whether an `isinstance` miss can read `obj.__class__` without invoking
/// app-level code.
///
/// `abstractinst.py:p_recursive_isinstance_type_w` performs that attribute
/// read after the direct MRO test misses. Inheriting
/// `object.__getattribute__` is not sufficient: a subclass may still replace
/// `__class__` with a property. Require both the already-memoized default
/// `__getattribute__` and the canonical `object.__class__` descriptor.
/// `W_TypeObject.mutated` invalidates the memoized flag and the method cache
/// whenever either lookup shape changes.
pub unsafe fn isinstance_miss_class_lookup_is_pure(w_type: PyObjectRef) -> bool {
if !pyre_object::typeobject::w_type_get_uses_object_getattribute(w_type) {
return false;
}
let Some(actual_class_descr) = lookup_in_type_where(w_type, "__class__") else {
return false;
};
lookup_in_type_where(crate::typedef::w_object(), "__class__")
.is_some_and(|object_class_descr| std::ptr::eq(actual_class_descr, object_class_descr))
}

/// typeobject.py:328-348 `setattr_if_not_from_object` — the `__setattr__`
/// companion of [`getattribute_if_not_from_object`].
pub(crate) unsafe fn setattr_if_not_from_object(w_type: PyObjectRef) -> Option<PyObjectRef> {
Expand Down Expand Up @@ -19008,6 +19029,24 @@ mod tests {
}
}

#[test]
fn isinstance_miss_requires_canonical_object_class_descriptor() {
crate::typedef::init_typeobjects();
let t = crate::typedef::make_builtin_type("IsinstanceClassLookup", |_| {});
unsafe {
pyre_object::typeobject::w_type_set_uses_object_getattribute(t, true);
assert!(isinstance_miss_class_lookup_is_pure(t));

// A `property` is the app-visible hazardous case. Any MRO
// replacement is enough to prove the identity gate itself: only
// the canonical object getset may enter the replay-safe arm.
crate::type_dict_store(t, "__class__", pyre_object::w_int_new(7));
mutated(t, Some("__class__"));
pyre_object::typeobject::w_type_set_uses_object_getattribute(t, true);
assert!(!isinstance_miss_class_lookup_is_pure(t));
}
}

/// typeobject.py:303-326 — builtin types and a plain user class that
/// inherit object.__getattribute__ answer `None`, so the layout-agnostic
/// receiver gate does not route them through an app-level call.
Expand Down
25 changes: 15 additions & 10 deletions pyre/pyre-jit-trace/src/jitcode_dispatch/residual_call.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2830,11 +2830,12 @@ pub(crate) fn try_execute_residual_call_via_executor<Sym: WalkSym>(
&& !operand.is_null()
&& std::ptr::eq(callable, str_type)
&& unsafe {
pyre_object::is_int_or_long(operand)
|| pyre_object::is_float(operand)
|| pyre_object::is_complex(operand)
|| pyre_object::is_str(operand)
|| pyre_object::is_none(operand)
pyre_object::is_exact_builtin_instance(operand)
&& (pyre_object::is_int_or_long(operand)
|| pyre_object::is_float(operand)
|| pyre_object::is_complex(operand)
|| pyre_object::is_str(operand)
|| pyre_object::is_none(operand))
}
};
// PyPy traces `space.iter(w_exact_unicode)` through
Expand All @@ -2858,7 +2859,10 @@ pub(crate) fn try_execute_residual_call_via_executor<Sym: WalkSym>(
&& args.len() == 1
&& {
let operand = args[0] as pyre_object::PyObjectRef;
!operand.is_null() && unsafe { pyre_object::is_str(operand) }
!operand.is_null()
&& unsafe {
pyre_object::is_str(operand) && pyre_object::is_exact_builtin_instance(operand)
}
};
// PyPy's `space.ord` reads the immutable unicode payload directly. Match
// both canonical builtin-code identity and the observed exact string so a
Expand All @@ -2876,9 +2880,10 @@ pub(crate) fn try_execute_residual_call_via_executor<Sym: WalkSym>(
// then (only on a miss) `w_inst.__class__` (`abstractinst.py:74-86`). A
// hit is replay-safe directly. A miss is replay-safe only when the
// instance type has already proven and memoized that it inherits
// `object.__getattribute__`; type mutation invalidates that flag. This
// covers the ordinary `isinstance(C0(), int)` false result without blessing
// an override that can run Python and mutate live heap.
// `object.__getattribute__` AND its MRO resolves `__class__` to the
// canonical `object.__class__` descriptor. The first fact alone is not
// enough: `@property def __class__` retains the default getattribute path
// but runs user code here. Type mutation invalidates both lookup facts.
//
// Require the canonical callable and an ordinary class whose metaclass is
// exactly `type`; tuple/union classinfo and custom `__instancecheck__`
Expand All @@ -2899,7 +2904,7 @@ pub(crate) fn try_execute_residual_call_via_executor<Sym: WalkSym>(
})
&& pyre_interpreter::typedef::r#type(object).is_some_and(|actual| unsafe {
pyre_interpreter::baseobjspace::isinstance_w(object, classinfo)
|| pyre_object::typeobject::w_type_get_uses_object_getattribute(
|| pyre_interpreter::baseobjspace::isinstance_miss_class_lookup_is_pure(
actual.as_ptr(),
)
})
Expand Down
Loading