bpo-45417: fix quadratic behaviour when creating an enum #28907
Conversation
creating an enum class used to be quadratic in the number of fields, in two different places: in the dicitonary implementation for the enum class, and when setting the name of all members. fix the former by switching to a dictionary instead of a list to store the fields while building up the dictionary. fix the latter by trying to use the already existing dictionary when looking up a field, instead of looping of all fields.
|
A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated. Once you have made the requested changes, please leave a comment on this pull request containing the phrase |
|
@ethanfurman ah, thank you for the explanation, I figured it was something like that. Typo is fixed! |
|
There are other list-based uses of
Do you want to update those to use a dictionary or should I? |
hm, I am a getting a bit confused 😅! I think The uses of |
|
Thanks @cfbolz for the PR, and @ethanfurman for merging it 🌮🎉.. I'm working now to backport this PR to: 3.10. |
|
Sorry @cfbolz and @ethanfurman, I had trouble checking out the |
There are two changes: Changes in the actual code: - _member_names changed from a list to a dict in python/cpython#28907 - we instance-check and remove by list-specific or dict-specific way Change in the tests only: - accessing other enum members via instance attributes is no longer possible - we access them via the class instead - we leave the original test in a try-except block Some of the Python enum changes might get reverted, see python/cpython#93910 But the fix is backwards compatible.
There are two changes: Changes in the actual code: - _member_names changed from a list to a dict in python/cpython#28907 - we instance-check and remove by list-specific or dict-specific way Change in the tests only: - accessing other enum members via instance attributes is no longer possible - we access them via the class instead - we leave the original test in a try-except block Some of the Python enum changes might get reverted, see python/cpython#93910 But the fix is backwards compatible. Fixes googleapis#326
* Adjust to enum changes in Python 3.11.0b3 There are two changes: Changes in the actual code: - _member_names changed from a list to a dict in python/cpython#28907 - we instance-check and remove by list-specific or dict-specific way Change in the tests only: - accessing other enum members via instance attributes is no longer possible - we access them via the class instead - we leave the original test in a try-except block Some of the Python enum changes might get reverted, see python/cpython#93910 But the fix is backwards compatible. Fixes #326 * ci: unit test session with python 3.11.0-beta.3 * ci: add python v3.11.0-beta.3 to noxfile.py * another attempt to get python 3.11.0b3 working in github actions * ci: use python 3.8 for docs check * ci: fix docs build * fix ci * mark python 3.11 tests as required * add python 3.11 to setup.py * fix docs build * remove python 3.11 test for unitcpp * remove python 3.11 test for unitcpp * remove python 3.11 test for unitcpp * attempt to fix exclude in github action Co-authored-by: Anthonios Partheniou <partheniou@google.com>
We got a bug report in PyPy that creating really big enums was slow, and worse, the performance was growing quadratically in the number of fields. We actually even found two places with quadratic behavior. Both are fixed in this PR:
_EnumDictclass, the_member_namesattribute was a list that was continually growing, and every new field was checked for existance using "in" in that list. Replace it with a dict (with None keys) to get both insertion order tracking as well as O(1) lookup._proto_member, there was code that goes through all members to find something. Use the existing dictionary for that lookup if possible.I am open to ideas how I would test that the slow behavior is fixed without relying on brittle timing checks.
(there is also a third place that behaves quadratically: When using
auto(), the growing_EnumDict._last_valueslist is continually copied for every auto member. Removing that copy fixes the slowness (and tests pass), but I don't understand why it is there in the first place, so I am bit wary of touching it.)https://bugs.python.org/issue45417