Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork411
Replace cached_property on slotted classes with a new descriptor instead of writing__getattr__#1488
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
base:main
Are you sure you want to change the base?
Conversation
The descriptor now carries the annotations or annotate function from the wrapped function.
codspeed-hqbot commentedDec 8, 2025 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
CodSpeed Performance ReportMerging#1488 willdegrade performances by 71.97%Comparing Summary
Benchmarks breakdown
|
DavidCEllis commentedDec 8, 2025
Oof, that's worse than I thought. I have one idea that should make it slightly better, but probably still significantly worse than the direct access it has on main. |
DavidCEllis commentedDec 8, 2025
Ah, it would have needed to be on main to see the difference. I haven't used codspeed before, sorry for the noise there. Locally it seemed to be significantly faster to construct classes at least. Probably less important than attribute access, but worth noting it's not uniformly worse performance-wise. |
hynek commentedDec 9, 2025
would you like me to transplant the benchmark to main? i'm a bit busy (just came back from vacation with the obvious results), but i'm open to mechanical work ;) |
DavidCEllis commentedDec 9, 2025
Sure, that would be great thanks. No rush if you're already swamped. I tried a few other things to see if they affected performance but it seems it's entirely linked with wrapping the slot in another descriptor. The only other thing I've tried so far that improved performance at all was replacing the new descriptor with a regular |
hynek commentedDec 14, 2025
done! |
Uh oh!
There was an error while loading.Please reload this page.
Summary
This PR reimplements
cached_propertyin slotted classes as a slot wrapping descriptor replacing the current generated__getattr__implementation.Accessing the attributes will probably be slightly slower due to going through this descriptor but if the performance is reasonable this willclose#1288#1325 and#1333.
This idea came up again as part of discussion onhttps://discuss.python.org/t/extensible-cached-property/105188/25. I hadn't realised it would work, but it looks like there was a partial attempt to implement it for attrs before in#1357
I've implemented it for my own attrs/dataclasses-likehere (the _SlottedCachedProperty code is mostly identical).
As I saw there were outstanding issues for it and the code is fairly similar I thought you might also find it useful. (Passing all of your existing tests also gave me some confidence that it does work correctly).
Changes:
_SlottedCachedPropertydescriptor class that replaces the slot descriptors after attrs has rebuilt the class.cached_propertyitself, other than reading/writing/deleting a wrapped slot rather than dict entry.__getattr__function you were making previously.__getattr__behaviour that are now just checking__getattr__works in Python.Outstanding issues:
This was already the case for the
__getattr__implementation but I'm now aware of one difference in behaviour this hasn't yet resolved as it would require changing the logic of when you create a new cached property. There's currently a test marked as xfail for this.This is because the cached_property replacement check
attrsdoes skips field names that already exist, sonameis skipped in the check and doesn't get replaced. The descriptor would behave like this, but it's never placed.Pull Request Check List
mainbranch –use a separate branch!Our CI fails if coverage is not 100%.
.pyi).typing-examples/baseline.pyor, if necessary,typing-examples/mypy.py.attr/__init__.pyi, they'vealso been re-imported inattrs/__init__.pyi.docs/api.rstby hand.@attr.s()and@attrs.define()have to be added by hand too.versionadded,versionchanged, ordeprecateddirectives.The next version is the second number in the current release + 1.
The first number represents the current year.
So if the current version on PyPI is 22.2.0, the next version is gonna be 22.3.0.
If the next version is the first in the new year, it'll be 23.1.0.
attrs.define()andattr.s(), you have to add version directives to both..rstand.mdfiles is written usingsemantic newlines.changelog.d.