Change in semantics and much worse performance for enum members.

[markshannon]

Given 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

• gh-93910: [Enum] remove member.member deprecation #103236
• [3.11] gh-93910: [Enum] remove member.member deprecation (GH-103236) #103299

[ethanfurman]

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]

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]

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]

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]

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]

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

[markshannon]

OOI, why is it necessary that code like if self == self.START: no longer works?

[pablogsal]

I am marking this as a release blocker so we can discuss this properly. If we don't reach consensus quickly, please, let's send an email to python-dev so we can collectively reach an agreement.

[hroncok]

For the record, in #87328 (comment) @ethanfurman said:

DeprecationWarning will be active in 3.10 and 3.11 with removal in 3.12.

This was not followed, for reasons I don't understand.

This once again breaks real projects, e.g. googleapis/proto-plus-python#326

[JelleZijlstra]

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?

The permission was granted in python/steering-council#80 (linked in the last line of the linked message).

But I don't think the SC's permission means we must make the change in 3.11. A 9x performance regression is very severe and doesn't seem worth the minor benefit of disallowing Color.RED.GREEN.

[gpshead]

Agreed, the existing checked in solution to getting rid of Color.RED.BLUE behavior wart makes the default case performance in all code worse. So we shouldn't "fix" the wart that way. Within 3.11-beta the best thing to do is revert that change.

Finding a well performing attribute of an attribute wart fix remains for a future version, if it ever gets fixed.

[warsaw]

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?

That SC exception very specifically was for removing Color.RED.BLUE in 3.11 without a deprecation warning in the code. At the time, there was no discussion about any potential performance impact.

The SC rejection notice for PEP 663 had this to say about str() and format():

One aspect of the PEP we do agree with is the alignment of IntEnum’s str() and format(). It is confusing that they give different results, and that is worth the small compatibility breakage to fix. We are uncertain whether it’s better to change str to be more like format or vice versa. Our recommendation is to take this discussion to python-dev and see if consensus on this topic can be reached.

This one's a little less definitive (others on that edition of the SC can also chime in; this was a unanimous decision and agreed upon communication). This was saying that we thought it would be good to align IntEnum's str() and format() output in 3.11, although we urged consensus building on python-dev in order to decide how exactly to resolve that misalignment. We thought it was worth a compatibility break (implied: without warning or transition period) to align those in 3.11.

But I don't think the SC's permission means we must make the change in 3.11. A 9x performance regression is very severe and doesn't seem worth the minor benefit of disallowing Color.RED.GREEN.

Personally, I agree with this, but I don't have a vote on the SC any more. I would argue to not incur the performance loss in 3.11 and take the time to remove the wart performantly in 3.12, if that's even possible.

[ethanfurman]

@markshannon How did you measure the performance? If you give me the same steps so I have something to work towards I'll try to remedy the performance impact.

Out of curiosity, are regular Python properties slower, or not speeding up, with the work being done? The new Enum members are basically properties, so I'm a bit surprised they're so much slower

[markshannon]

Using the script above building with ./configure (non-debug, no-pgo, no-lto build) for 3.10 and 3.11 on ubuntu linux.

[markshannon]

We haven't sped up regular properties up for 3.11, but have for 3.12. #93912

[markshannon]

Could someone point me to a reference explaining why accessing one enum value from another is considered a wart?

I'm curious what is wrong with code like if state == state.START: or while state != state.END: that one might write in generic state machine code.

[AlexWaygood]

Could someone point me to a reference explaining why accessing one enum value from another is considered a wart?

It can lead to confusing behaviour, as is detailed in the documentation here:

>>> from enum import Enum
>>> class FieldTypes(Enum):
...     name = 0
...     value = 1
...     size = 2
...
>>> FieldTypes.value.size
<FieldTypes.size: 2>
>>> FieldTypes.size.value
2

[markshannon]

That a.b.c != a.c.b is only surprising if you expect the . operator to be commutative for some reason.
It would be better to recommend avoid using value as an enum member, IMO. Possibly even rejecting value at class creation time.

If you pick silly enough names, weird things will happen.

>>> class FieldTypes(Enum):
...     __class__ = 0

[Yhg1s]

The performance of defining enums also matters, in particular for the startup time of programs.

[davfsa]

I'll get performance benchmarks for those two as soon as possible

[ethanfurman]

We did manage to keep the same schematics as was seen in 3.8 and that @markshannon pointed out in the beginning of this issue:

Python 3.12.0a0 (heads/main-dirty:e0ae9dd, Oct 11 2022, 10:38:47) [GCC 11.2.0] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> import hikari
>>> 
>>> # The following is just an example of an enum used in the library
>>> hikari.GuildFeature
<enum GuildFeature>
>>> hikari.GuildFeature.__dict__["VERIFIED"] is hikari.GuildFeature.VERIFIED
True

The __dict__ access truism is an implementation detail, and does not need to persist in the new version.

>>> hikari.GuildFeature.VERIFIED.VERIFIED
<GuildFeature.VERIFIED: 'VERIFIED'>
>>> 

The VERIFIED.VERIFIED behavior is not desired and will raise a DeprecationWarning in 3.12.

If your implementation passes the 3.12 test suite and is faster in attribute lookup and enum creation time, a PR would be welcome.

[davfsa]

@markshannon @Yhg1s Here are your requested benchmarks :)

Script

import pyperf

import_basic_enum = "import enum"
import_hikari_enum = "from hikari.internal import enums"

create_basic_enum = [
    "class BasicPyEnum(str, enum.Enum):",
    '    a = "0"',
    '    b = "1"',
    '    c = "2"',
    '    d = "3"',
    '    e = "4"',
    '    f = "5"',
    '    g = "6"',
    '    h = "7"',
    '    i = "8"',
    '    j = "9"',
    '    k = "10"',
    '    l = "11"',
    '    m = "12"',
    '    n = "13"',
    '    o = "14"',
    '    p = "15"',
    '    q = "16"',
    '    r = "17"',
    '    s = "18"',
    '    t = "19"',
    '    u = "20"',
    '    v = "21"',
    '    w = "22"',
    '    x = "23"',
    '    y = "24"',
    '    z = "25"',
]

create_hikari_enum = [
    "class BasicHikariEnum(str, enums.Enum):",
    '    a = "0"',
    '    b = "1"',
    '    c = "2"',
    '    d = "3"',
    '    e = "4"',
    '    f = "5"',
    '    g = "6"',
    '    h = "7"',
    '    i = "8"',
    '    j = "9"',
    '    k = "10"',
    '    l = "11"',
    '    m = "12"',
    '    n = "13"',
    '    o = "14"',
    '    p = "15"',
    '    q = "16"',
    '    r = "17"',
    '    s = "18"',
    '    t = "19"',
    '    u = "20"',
    '    v = "21"',
    '    w = "22"',
    '    x = "23"',
    '    y = "24"',
    '    z = "25"',
]

full_basic_setup = [import_basic_enum, *create_basic_enum]
full_hikari_setup = [import_hikari_enum, *create_hikari_enum]

runner = pyperf.Runner()


runner.timeit(
    name="BasicPyEnum creation",
    stmt=create_basic_enum,
    setup=import_basic_enum,
)
runner.timeit(
    name="BasicHikariEnum creation",
    stmt=create_hikari_enum,
    setup=import_hikari_enum,
)

runner.timeit(
    name="BasicPyEnum.z",
    stmt="BasicPyEnum.z",
    setup=full_basic_setup,
)
runner.timeit(
    name="BasicHikariEnum.z",
    stmt="BasicHikariEnum.z",
    setup=full_hikari_setup,
)

runner.timeit(
    name="BasicPyEnum.__call__('25')",
    stmt="BasicPyEnum('25')",
    setup=full_basic_setup,
)
runner.timeit(
    name="BasicHikariEnum.__call__('25')",
    stmt="BasicHikariEnum('25')",
    setup=full_hikari_setup,
)


runner.timeit(
    name="BasicPyEnum._value2member_map_['25']",
    stmt="BasicPyEnum._value2member_map_['25']",
    setup=full_basic_setup,
)
runner.timeit(
    name="BasicHikariEnum._value_to_member_map_['25']",
    stmt="BasicHikariEnum._value_to_member_map_['25']",
    setup=full_hikari_setup,
)

runner.timeit(
    name="BasicPyEnum.__getitem__['z']",
    stmt="BasicPyEnum['z']",
    setup=full_basic_setup,
)
runner.timeit(
    name="BasicHikariEnum.__getitem__['z']",
    stmt="BasicHikariEnum['z']",
    setup=full_hikari_setup,
)

Results (3.12, main branch)

.....................
BasicPyEnum creation: Mean +- std dev: 179 us +- 3 us
.....................
BasicHikariEnum creation: Mean +- std dev: 39.5 us +- 0.6 us
.....................
BasicPyEnum.z: Mean +- std dev: 73.8 ns +- 1.1 ns
.....................
BasicHikariEnum.z: Mean +- std dev: 15.3 ns +- 0.5 ns
.....................
BasicPyEnum.__call__('25'): Mean +- std dev: 207 ns +- 6 ns
.....................
BasicHikariEnum.__call__('25'): Mean +- std dev: 99.5 ns +- 2.1 ns
.....................
BasicPyEnum._value2member_map_['25']: Mean +- std dev: 24.6 ns +- 1.3 ns
.....................
BasicHikariEnum._value_to_member_map_['25']: Mean +- std dev: 24.1 ns +- 1.2 ns
.....................
BasicPyEnum.__getitem__['z']: Mean +- std dev: 52.4 ns +- 1.9 ns
.....................
BasicHikariEnum.__getitem__['z']: Mean +- std dev: 52.3 ns +- 1.5 ns

Ill start working on a PR to add this to main :)

[davfsa]

Little update. Been working on porting it to CPython and ran into a bunch of issues at the beginning, so I decided to just rewrite the implementation and merge both ideas. This left the following performance benchmarks:

Results (3.12)

.....................
BasicPyEnum creation: Mean +- std dev: 73.4 us +- 1.3 us
.....................
BasicPyEnum.z: Mean +- std dev: 15.4 ns +- 0.3 ns
.....................
BasicPyEnum.__call__('25'): Mean +- std dev: 149 ns +- 6 ns
.....................
BasicPyEnum._value2member_map_['25']: Mean +- std dev: 24.4 ns +- 1.8 ns
.....................
BasicPyEnum.__getitem__['z']: Mean +- std dev: 51.9 ns +- 2.3 ns

Results like the one for __call__ im not overly happy with, but the issue is that its purely due to the functional api taking in so many arguments that causes the slow down (the * makes up for 40ns alone).

There is still some features that need to be implemented, as well as making sure it passes the test suite, so still slowly but surely working on that.

I also wanted to have a second look at the creation logic and hopefully make it faster. This might not be possible due to the checks that are in place.

[markshannon]

The performance of enums is still far too slow. Maybe not as bad as before, but it is still poor enough that I will have to recommend anyone who cares about performance not use enums.

Using the example above, #93910 (comment)
Enums are about 6 times slower than the "fast" enum, which is twice as slow as the simple class:

class FasterColor:
    RED = "RED"
    BLUE = "BLUE"
    GREEN = "GREEN"

So, enums are about 12 times slower than a simple class with string attributes.
Is this really what we want?

And I still get get what what is wrong with Color.RED.RED.
The object to accessing attributes from enum instances seem to be dogmatic, rather than rational.

[warsaw]

Hey @markshannon, could you maybe take a look at flufl.enum? I re-released that late last year. It's definitely simpler than the built-in enum, so it may lack features folks want from the built-in stdlib. It may also have some different semantics than people want. But I think for simple enum use cases, it should fit the bill. If there are opportunities for improving the performance of this library, I'm all ears.

[ethanfurman]

@markshannon worte

And I still get get what what is wrong with Color.RED.RED.

Ignoring the reasoning for the moment, would allowing Color.RED.RED improve performance?

So, enums are about 12 times slower than a simple class with string attributes. Is this really what we want?

Well, we could make normal class access slower so enums don't look so bad. ;-)

Seriously, I don't understand the point of the question: enums have certain features and those features cost a certain amount of resources. Faster is obviously better, but what are you suggesting we give up to achieve the speed?

The performance of enums is still far too slow. Maybe not as bad as before, but it is still poor enough that I will have to
recommend anyone who cares about performance not use enums.

Do we have a real-world performance test for this? I'm curious about when enums are so heavily used that their performance is an issue.

[ethanfurman]

@markshannon
Having said all that, the above PR stores the member directly in the enum __dict__. Can you test it for performance?

Also, and somewhat ironically, I did try to do a member.member access in a custom enum, and was quite irritated that it didn't work. ☹️

[markshannon]

Ignoring the reasoning for the moment, would allowing Color.RED.RED improve performance?

In general, fewer special cases means fewer tests and less overhead.

Unfortunately it seems that some sort of special descriptor is needed if you want to prevent assignment of attribute.
I assume this should be illegal:

Color.RED = "Pink"

So we aren't going to get performance on a par with simple classes, at least not for 3.12.

Making enum.property.__get__ simpler would help considerably.

If you were to allow Color.RED.RED you could skip the instance is None check.
Also, making sure that self.member is always set, would skip another check:

def __get__(self, instance, ownerclass=None):
    return self.member

That won't fix the problem, but it will help.
We could add specialization for Python descriptors, maybe in time for 3.12
The combination of the two should get the overhead of using enums down to reasonable levels.

[Gobot1234]

Just as a passerby to this conversation, I'm curious as to why enum needs it's own property class.
It appears to me most of the work (in a future version) could be handled by making the Enum members type/class attributes (i.e. create a subclass of EnumType for each Enum subclass and add the members there instead of on the Enum) as this prevents Color.RED.RED access natively, you can prevent reassignment using a __setattr__ hook inside of EnumType, and it still allows you to define a member called name. The only downside I can think of to this method is that Colours.__dict__ looks like mappingproxy({'__module__': '__main__', '__doc__': None}) (Colours.__class__.__dict__ is where the magic happens
mappingproxy({'_value2member_map_': {1: Colours.RED}, '_member_map_': {'RED': Colours.RED}, ..., 'RED': Colours.RED}))

[ethanfurman]

The enum.property descriptor is complex because it's handling three different cases:

1. attribute is a member (i.e. Color.BLUE)
2. attribute is a non-member (i.e. Color.BLUE.name)
3. 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.

[JelleZijlstra]

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?

[ethanfurman]

I'm pretty sure this has been resolved. Please open a new issue if anything still needs handling.