Skip to content

Commit bb96a48

Browse files
committed
jit: inline the receiver type's __getattr__ hook for a missing attribute
`descroperation.py:242-245` reaches the hook only after the descriptor protocol has raised, so a hooked access cost one opaque residual holding the whole `object_getattr_miss` walk — the `__dict__` / `__doc__` / `__class__` special names, the metaclass loops, the terminal miss — and then a fresh interpreter frame for the hook, on every iteration. Every other user dunder already has a resolver into `try_walker_inline_resolved_user_call` (`__getitem__`, `__add__`, `__hash__`, `__index__`, `property.__get__`, `__eq__`); `__getattr__` had none. `mapdict::getattr_hook_fast_path` is the miss twin of `load_attr_fast_path`: it answers with the type's version tag and the instance map, which are what make "the name resolves nowhere and `__getattr__` is this one" a constant of the trace. `try_walker_inline_getattr_hook` emits those pins through `walker_guard_mapdict_instance_shape` and enters the hook. All three spellings `get_and_call_function` binds are folded — a plain `Function` leads with the receiver, a `classmethod` with the class, a `staticmethod` with nothing; a custom-descriptor hook stays on the residual. The name argument is an interned immortal block, the shape `pyopcode.py LOAD_ATTR` passes (`co_names_w[oparg]`, one object per code object). Per-access cost at N=600000, dynasm: plain hook 0.346s -> 0.088s, classmethod 0.410s -> 0.080s, staticmethod 0.347s -> 0.076s, each from one residual to none (an existing attribute reads 0.074s). synth/getattr_hook_binding moves 48.6x -> 7.4x (dynasm), 10.4x (cranelift), 8.7x (wasm); its ceiling goes 90 -> 25. `extra_tests/parity_tests/getattr_hook_inline_deopt.py` breaks each pin in turn mid-loop — a store that puts the name on the instance, a reassigned `__getattr__`, a class attribute that shadows the hook — and covers a raising hook, an inherited classmethod hook's bound class, and a hook that installs the attribute itself. Assisted-by: Claude
1 parent aaf59da commit bb96a48

5 files changed

Lines changed: 423 additions & 1 deletion

File tree

pyre/bench/synth/getattr_hook_binding.py

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,14 @@
1-
# pyre-check: max-pypy-ratio=90
1+
# pyre-check: max-pypy-ratio=25
22
# objspace.py:710 get_and_call_function: a __getattr__ (or __getattribute__)
33
# defined as a classmethod or staticmethod must be bound through __get__ before
44
# being called, exactly like any other special method, so it receives the
55
# arguments the descriptor protocol gives it.
6+
#
7+
# Each of the three accesses below used to cost one opaque residual holding the
8+
# whole `object_getattr_miss` walk plus a fresh frame for the hook. Inlining
9+
# the hook against the version-tag and map pins that make the miss constant
10+
# dropped the ratio from 48.6x/59.5x (dynasm/wasm) to 7.4x/10.4x/8.7x
11+
# (dynasm/cranelift/wasm); the bound is twice the slowest of those, rounded up.
612

713

814
class ClassmethodGetattr:
Lines changed: 165 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,165 @@
1+
# CPython-suite gap: the suite exercises __getattr__ semantics but never runs a
2+
# hooked access hot enough to be compiled, so nothing covers the compiled form.
3+
# parity-tests reason: this targets the pyre-specific guards a compiled
4+
# __getattr__ hook rests on.
5+
6+
"""A compiled `__getattr__` hook answers to the two pins that admitted it.
7+
8+
`objspace.py:710 get_and_call_function` reaches the hook only after the
9+
attribute resolves nowhere, so the compiled form pins the receiver's type
10+
version tag (the type keeps lacking the name, and keeps this hook) and the
11+
instance map (the receiver keeps lacking the name). Each loop below runs long
12+
enough to be compiled and then invalidates exactly one of those pins mid-loop:
13+
the values recorded before and after must differ at the iteration the pin was
14+
broken, which is what proves the guard deopts rather than the compiled answer
15+
being reused.
16+
17+
The AttributeError case is here for the same reason: a hook that raises for an
18+
unknown name is an ordinary outcome of an inlined body, not a shape the fold may
19+
quietly turn into a returned value.
20+
"""
21+
22+
N = 40000
23+
SWAP = N // 2
24+
25+
26+
class Instance:
27+
def __getattr__(self, name):
28+
return "hook:" + name
29+
30+
31+
class Hooked:
32+
@classmethod
33+
def __getattr__(cls, name):
34+
return "cm:%s:%s" % (cls.__name__, name)
35+
36+
37+
class Static:
38+
@staticmethod
39+
def __getattr__(name):
40+
return "sm:" + name
41+
42+
43+
class Raiser:
44+
def __getattr__(self, name):
45+
if name == "absent":
46+
raise AttributeError("no " + name)
47+
return "ok:" + name
48+
49+
50+
class Installer:
51+
def __getattr__(self, name):
52+
# The hook itself gives the instance the attribute, so every later
53+
# access must read the instance rather than hook again.
54+
self.installed = "real"
55+
return "hook:" + name
56+
57+
58+
def instance_shadow():
59+
"""A store during the loop puts the name on the instance."""
60+
obj = Instance()
61+
seen = []
62+
i = 0
63+
while i < N:
64+
seen.append(obj.later)
65+
if i == SWAP:
66+
obj.later = "instance"
67+
i += 1
68+
assert seen[0] == "hook:later", seen[0]
69+
assert seen[SWAP] == "hook:later", seen[SWAP]
70+
assert seen[SWAP + 1] == "instance", seen[SWAP + 1]
71+
assert seen[-1] == "instance", seen[-1]
72+
73+
74+
def hook_replaced():
75+
"""Reassigning `__getattr__` bumps the type's version tag."""
76+
77+
class Swapped(Hooked):
78+
pass
79+
80+
obj = Swapped()
81+
seen = []
82+
i = 0
83+
while i < N:
84+
seen.append(obj.zed)
85+
if i == SWAP:
86+
Swapped.__getattr__ = classmethod(lambda cls, name: "replaced")
87+
i += 1
88+
assert seen[0] == "cm:Swapped:zed", seen[0]
89+
assert seen[SWAP + 1] == "replaced", seen[SWAP + 1]
90+
91+
92+
def name_shadowed_on_type():
93+
"""A class attribute added during the loop wins over the hook."""
94+
95+
class Shadowed(Static):
96+
pass
97+
98+
obj = Shadowed()
99+
seen = []
100+
i = 0
101+
while i < N:
102+
seen.append(obj.zed)
103+
if i == SWAP:
104+
Shadowed.zed = "class"
105+
i += 1
106+
assert seen[0] == "sm:zed", seen[0]
107+
assert seen[SWAP + 1] == "class", seen[SWAP + 1]
108+
109+
110+
def bound_argument_follows_the_receiver_type():
111+
"""A classmethod hook binds the receiver's own class, not the base."""
112+
113+
class Sub(Hooked):
114+
pass
115+
116+
base = Hooked()
117+
sub = Sub()
118+
i = 0
119+
while i < N:
120+
assert base.q == "cm:Hooked:q"
121+
assert sub.q == "cm:Sub:q"
122+
i += 1
123+
124+
125+
def hook_raises():
126+
"""An AttributeError out of the hook reaches the caller every iteration."""
127+
obj = Raiser()
128+
hits = 0
129+
misses = 0
130+
i = 0
131+
while i < N:
132+
hits += len(obj.present)
133+
try:
134+
obj.absent
135+
except AttributeError as exc:
136+
assert str(exc) == "no absent", exc
137+
misses += 1
138+
i += 1
139+
assert hits == N * len("ok:present"), hits
140+
assert misses == N, misses
141+
142+
143+
def hook_installs_the_attribute():
144+
obj = Installer()
145+
seen = []
146+
i = 0
147+
while i < N:
148+
seen.append(obj.installed)
149+
i += 1
150+
assert seen[0] == "hook:installed", seen[0]
151+
assert seen[1] == "real", seen[1]
152+
assert seen[-1] == "real", seen[-1]
153+
154+
155+
def main():
156+
instance_shadow()
157+
hook_replaced()
158+
name_shadowed_on_type()
159+
bound_argument_follows_the_receiver_type()
160+
hook_raises()
161+
hook_installs_the_attribute()
162+
print("OK")
163+
164+
165+
main()

pyre/pyre-interpreter/src/objspace/std/mapdict.rs

Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1748,6 +1748,73 @@ pub unsafe fn load_attr_fast_path(
17481748
Some((w_type, version_tag, map, p.storageindex))
17491749
}
17501750

1751+
/// The miss twin of [`load_attr_fast_path`]: return the ingredients for
1752+
/// inlining the receiver type's `__getattr__` hook when `name` resolves
1753+
/// nowhere.
1754+
///
1755+
/// `baseobjspace::instance_getattr_hook_or_err` is the tail this stands in for
1756+
/// (`descroperation.py:242-245`): once the descriptor protocol has produced an
1757+
/// AttributeError, the type's `__getattr__` is looked up and called with the
1758+
/// receiver and the name. Reaching that tail is what the two returned pins
1759+
/// prove, and both are guards the caller owes:
1760+
///
1761+
/// * `version_tag` — the class lookup stays constant, so `name` keeps
1762+
/// resolving to nothing on the type and `__getattr__` keeps resolving to
1763+
/// the returned hook;
1764+
/// * `map` — the instance shape stays constant, so `name` keeps being absent
1765+
/// from this receiver's own storage.
1766+
///
1767+
/// Together they make the whole `object_getattr_miss` walk a compile-time
1768+
/// answer, which is the work the fold removes; the hook itself is what the
1769+
/// caller then inlines.
1770+
///
1771+
/// Returns `None` for every shape those two guards cannot cover: a non-mapdict
1772+
/// receiver, a custom `__getattribute__`, an uncacheable `version_tag`, a name
1773+
/// the type or the instance actually owns, or a type with no `__getattr__`.
1774+
///
1775+
/// # Safety
1776+
/// `w_obj` must be a live object.
1777+
pub unsafe fn getattr_hook_fast_path(
1778+
w_obj: PyObjectRef,
1779+
name: &str,
1780+
) -> Option<(PyObjectRef, u64, MapRef, PyObjectRef)> {
1781+
// mapdict.py:1495 `if map is not None:` — also filters non-instances.
1782+
let map = unsafe { mapdict_map_or_null(w_obj) };
1783+
if map.is_null() {
1784+
return None;
1785+
}
1786+
// mapdict.py:1496 `w_type = map.terminator.w_cls`.
1787+
let w_type = unsafe { (*(*map).terminator()).as_terminator() }.w_cls;
1788+
if w_type.is_null() {
1789+
return None;
1790+
}
1791+
// mapdict.py:1497-1499 — a custom `__getattribute__` runs its own lookup,
1792+
// which neither pin describes.
1793+
if unsafe { crate::baseobjspace::getattribute_if_not_from_object(w_type) }.is_some() {
1794+
return None;
1795+
}
1796+
// mapdict.py:1500-1501 `version_tag = w_type.version_tag(); if is not None:`.
1797+
let version_tag = unsafe { crate::baseobjspace::w_type_version_tag(w_type) };
1798+
if version_tag == 0 {
1799+
return None;
1800+
}
1801+
// The miss itself. A type-level hit is refused before the map is consulted:
1802+
// `classify_attr` reads a `__slots__` member under the `"slot"` name rather
1803+
// than its own, so a descriptor found here says nothing about what
1804+
// `find_map_attr(name)` below would answer.
1805+
if unsafe { crate::baseobjspace::lookup_in_type_where(w_type, name) }.is_some() {
1806+
return None;
1807+
}
1808+
// `classify_attr(w_type, None, false)` answers `(DICT, false)` — the
1809+
// no-descriptor arm (mapdict.py:1509-1510) — so this is the same
1810+
// `find_map_attr` call the hit path makes, read for its absence.
1811+
if unsafe { find_map_attr(map, Wtf8::new(name), DICT) }.is_some() {
1812+
return None;
1813+
}
1814+
let w_getattr = unsafe { crate::baseobjspace::lookup_in_type_where(w_type, "__getattr__") }?;
1815+
Some((w_type, version_tag, map, w_getattr))
1816+
}
1817+
17511818
/// The [`load_attr_fast_path`] twin for a receiver that keeps its attributes in
17521819
/// a `newdict(instance=True)` dictionary rather than in header mapdict storage
17531820
/// (`mapdict.py:1299-1303 make_instance_dict`). It applies the same

0 commit comments

Comments
 (0)