GeezerVison is showing github.com — Change in semantics and much worse performance for enum members. · Issue #93910 · python/cpython
Change in semantics and much worse performance for enum members. #93910DescriptionGiven the enum:
class Colours(Enum):
RED = 1
In Python 3.9 and 3.10:
>>> Colours.__dict__["RED"] is Colours.RED
True
>>> Colours.RED.RED
<Colours.RED: 1>
In Python 3.11:
>>> Colours.__dict__["RED"] is Colours.RED
False
>>> Colours.RED.RED
Traceback (most recent call last):
File "<stdin>", line 1, in <module>
File "/home/mark/repos/cpython/Lib/enum.py", line 198, in __get__
raise AttributeError(
^^^^^^^^^^^^^^^^^^^^^
AttributeError: <enum 'Colours'> member has no attribute 'RED'
While these might seem like minor semantic changes, there is also a large performance impact.
Lookup of Colours.RED is simple and efficient in 3.10, but involves a lot of indirection and dispatching through the enum.property class in 3.11.
The performance impact is likely to get worse in 3.12, as we optimize more kinds of attributes.
Introduced in c314e60, I believe.
@pablogsal
@ethanfurman
Linked PRs
Activityethanfurman commented on Jun 22, 2022 Those changes are intentional, and remedy an issue present since 3.5. As for performance, the work @markshannon and others have done has already had a considerable impact on making enum faster in 3.12. markshannon commented on Jun 22, 2022 What issue does the change remedy?
I'm puzzled by your claim that the work we've done has sped up enums for 3.12? It hasn't. Without c314e60, it would have. markshannon commented on Jun 22, 2022 It looks like the change in Colours.RED.RED has been brought up before: #87328
Failing on 3.11 seems to be in violation of PEP 387. A warning should be issued until 3.12.
What I don't understand is why this change is necessary at all.
What benefits does it bring? Accessing class attributes from instances is fairly universal, why should enums be special? ethanfurman commented on Jun 22, 2022 The rudimentary timings I did on my system for member access showed enums steadily getting faster over the releases, with a big jump in performance between 3.11 and 3.12.
The Colors.RED.RED behavior was illegal/missing in 3.4, added in 3.5 for performance (but warned against), and SC permission recieved for skipping the programmatic warning in 3.11
Enums are special, and this change brings them back into line with the original, intended, specification. markshannon commented on Jun 24, 2022 The performance of enum lookup is much slower than it should be in 3.11.
It would be a shame to have to advise people not to use enums if they care about performance.
The SC permission pertains to changes in repr(), str(), and format(), AFAICT. No mention is made of the the change in semantics of attribute lookup and performance.
@warsaw Could you confirm? markshannon commented on Jun 24, 2022 I'm seeing an approx x9 slowdown on attribute access.
class Color(Enum):
RED = "Red"
BLUE = "Blue"
GREEN = "Green"
class FastColor:
RED = Color.RED
BLUE = Color.BLUE
GREEN = Color.GREEN
def f():
for _ in range(1000):
Color.RED
Color.BLUE
Color.GREEN
import timeit
print(timeit.timeit('f()', number=10000, globals=globals()))
Color = FastColor
print(timeit.timeit('f()', number=10000, globals=globals()))
With an optimized, but non pgo, no lto, build.
0.8739701807498932
0.09454824822023511
ethanfurman commented on Apr 4, 2023 The enum.property descriptor is complex because it's handling three different cases:
- attribute is a member (i.e.
Color.BLUE)
- attribute is a non-member (i.e.
Color.BLUE.name)
- attribute is both (i.e.
Fields.name (member) and Fields.name.name (name of member))
The current enum code uses an enum.property for all members; the PR I asked you to benchmark returns to the behavior of only using an enum.proprtey in cases where both a member and a non-member attribute share the same name (i.e. the name in Fields.name and Fields.name.name) -- in other words, with that PR the vast majority of enums would have their members stored directly in the class __dict__. It also restores the Color.RED.RED behavior, but if the performance gain is substantial then practicality beats purity. For that matter, even if it isn't I have been convinced the member.member access is a good idea, and will finish the PR and merge it. Reacted by James Hilton-Balfe JelleZijlstra commented on May 22, 2023 This is still marked as a "deferred blocker", but I'm not really clear on what the concrete item is that is still a blocker. Would it be better to close this very long issue and open a new one for any concrete problem that still needs fixing? Reacted by Erlend E. Aasland ethanfurman commented on Dec 20, 2023 I'm pretty sure this has been resolved. Please open a new issue if anything still needs handling.
MetadataMetadataStatusShow more project fields StatusShow more project fields
DevelopmentNo branches or pull requestsIssue actions
|