Skip to content

gh-118013: Use weakrefs for the cache key in inspect._shadowed_dict - #118202

Merged
AlexWaygood merged 9 commits into
python:mainfrom
AlexWaygood:inspect-weakref-cache-key
Apr 24, 2024
Merged

gh-118013: Use weakrefs for the cache key in inspect._shadowed_dict#118202
AlexWaygood merged 9 commits into
python:mainfrom
AlexWaygood:inspect-weakref-cache-key

Conversation

@AlexWaygood

@AlexWaygoodAlexWaygood commented Apr 23, 2024

Copy link
Copy Markdown
Member

Slower than main, but significantly faster than a straight revert of #104267.

@AlexWaygood
AlexWaygood requested a review from carljmApril 23, 2024 23:12

@carljmcarljm left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me, once we get news/tests/benchmarks.

Comment threadLib/inspect.py
@AlexWaygood
AlexWaygood marked this pull request as ready for review April 24, 2024 11:23
@AlexWaygood

Copy link
Copy Markdown
MemberAuthor

Benchmarks

All timings done with a fresh PGO-optimised build on main using a MacBook pro.

getattr_static benchmarks:

Benchmark script
fromtimeitimporttimeitfromstatisticsimportmean, stdevfrominspectimportgetattr_staticclassFoo:
@propertydefx(self) ->int:
return42classBar:
x=42classWithParentClassX(Bar): ...
classBaz:
def__init__(self):
self.x=42classWithParentX(Baz): ...
classMissing: ...
classSlotted:
__slots__= ('x',)
def__init__(self):
self.x=42classMethod:
defx(self): ...
classClsMethod:
@classmethoddefx(cls): ...
classStMethod:
@staticmethoddefx(): ...
importgcgc.disable()
times= []
defstats():
ts= [t*1e8fortinsorted(times)[:5]]
returnf'{round(mean(ts)):4} ± {round(stdev(ts)):2} ns 'defbench(obj):
# Warmup:for_inrange(5):
number=100timeit(lambda: getattr_static(obj, 'x', None), number=number)
# Actual bench:for_inrange(50):
number=1000t=timeit(lambda: getattr_static(obj, 'x', None), number=number) /numbertimes.append(t)
bench_name= (
f'type[{obj.__name__}]'ifisinstance(obj, type)
elseobj.__class__.__name__
)
print(f"{bench_name: <25}: {stats()}")
times.clear()
bench(Foo)
bench(Foo())
bench(Bar)
bench(Bar())
bench(WithParentClassX())
bench(Baz())
bench(WithParentX())
bench(Missing)
bench(Missing())
bench(Slotted())
bench(Method())
bench(StMethod())
bench(ClsMethod())
Results on main
type[Foo] : 26 ± 1 ns Foo : 44 ± 0 ns type[Bar] : 26 ± 0 ns Bar : 40 ± 0 ns WithParentClassX : 52 ± 0 ns Baz : 49 ± 0 ns WithParentX : 59 ± 0 ns type[Missing] : 56 ± 0 ns Missing : 50 ± 0 ns Slotted : 57 ± 0 ns Method : 39 ± 0 ns StMethod : 39 ± 0 ns ClsMethod : 39 ± 0 ns
Results with this PR
type[Foo] : 39 ± 0 ns Foo : 60 ± 1 ns type[Bar] : 33 ± 0 ns Bar : 56 ± 0 ns WithParentClassX : 79 ± 0 ns Baz : 72 ± 0 ns WithParentX : 92 ± 0 ns type[Missing] : 86 ± 0 ns Missing : 74 ± 0 ns Slotted : 74 ± 0 ns Method : 56 ± 0 ns StMethod : 56 ± 0 ns ClsMethod : 56 ± 0 ns
Results with a clean revert of #104267
type[Foo] : 43 ± 0 ns Foo : 66 ± 1 ns type[Bar] : 37 ± 0 ns Bar : 65 ± 0 ns WithParentClassX : 91 ± 0 ns Baz : 84 ± 0 ns WithParentX : 111 ± 0 ns type[Missing] : 103 ± 0 ns Missing : 85 ± 0 ns Slotted : 78 ± 0 ns Method : 65 ± 0 ns StMethod : 65 ± 0 ns ClsMethod : 65 ± 0 ns

Runtime-checkable protocol benchmarks

Benchmark script
fromtypingimportProtocol, runtime_checkable@runtime_checkableclassFoo(Protocol):
a: intb: intc: intd: inte: intf: intg: inth: inti: intj: intk: intl: intm: intn: into: intp: intq: intr: ints: intt: intu: intv: intw: intx: inty: intz: intclassBar:
def__init__(self):
forattrnamein'abcdefghijklmnopqrstuvwxyz':
setattr(self, attrname, 42)
bars= [Bar() for_inrange(100_000)]
importtimestart=time.perf_counter()
forbarinbars:
isinstance(bar, Foo)
print(time.perf_counter() -start)

Results on main: 1.3446190829854459

Results with this PR: 1.9680303339846432

Results with a clean revert of #104267: 2.243281374918297

@AlexWaygood
AlexWaygood requested a review from carljmApril 24, 2024 11:39
@AlexWaygoodAlexWaygood added the needs backport to 3.12 only security fixes label Apr 24, 2024
@AlexWaygood

Copy link
Copy Markdown
MemberAuthor

(The getattr_static() benchmark script was originally contributed by @sobolevn in #103193 (comment))

Comment threadLib/inspect.py

@carljmcarljm left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎉

Comment threadLib/test/test_inspect/test_inspect.py
@AlexWaygood
AlexWaygood merged commit 8227883 into python:mainApr 24, 2024
@AlexWaygood
AlexWaygood deleted the inspect-weakref-cache-key branch April 24, 2024 14:55
@miss-islington-app

Copy link
Copy Markdown

Thanks @AlexWaygood for the PR 🌮🎉.. I'm working now to backport this PR to: 3.12.
🐍🍒⛏🤖

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Apr 24, 2024
…_dict` (pythonGH-118202)
(cherry picked from commit 8227883)
Co-authored-by: Alex Waygood <Alex.Waygood@Gmail.com>
@bedevere-app

Copy link
Copy Markdown

GH-118232 is a backport of this pull request to the 3.12 branch.

@bedevere-appbedevere-appBot removed the needs backport to 3.12 only security fixes label Apr 24, 2024
AlexWaygood added a commit that referenced this pull request Apr 24, 2024
…d_dict` (GH-118202) (#118232)
Co-authored-by: Alex Waygood <Alex.Waygood@Gmail.com>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@AlexWaygood@carljm@sobolevn