msg250875 - (view) |
Author: Ethan Furman (ethan.furman) *  |
Date: 2015-09-17 07:18 |
Pulling in collections.OrderedDict has a significant startup cost (from what I've heard, and can easily believe by glancing at all the imports in collections.__init__). By keeping a separate list for the Enum member names using Enum in the stdlib becomes more attractive. |
|
|
msg250878 - (view) |
Author: Serhiy Storchaka (serhiy.storchaka) *  |
Date: 2015-09-17 09:13 |
This change has visible side effect: __members__ is no longer ordered. This should be reflected in the documentation and in What's New. |
|
|
msg250879 - (view) |
Author: STINNER Victor (vstinner) *  |
Date: 2015-09-17 10:34 |
> This change has visible side effect: __members__ is no longer ordered. This property of part of the PEP 435 (enum): "The special attribute __members__ is an ordered dictionary mapping names to members." IMHO if we really want to change this, it must be discussed on the python-dev mailing list. To keep __members__ ordered, we can delay the creation of the OrderedDict at the first access to __members__: start with a dict, and use the ordered _all_member_names_ to created the ordered dictionary. Attached patch implements this idea. I checked enum.Enum and enum.IntEnum classes and the @enum.unique decorated: __members__ is not used to declare the class. Hum, I had to modify a little bit @enum.unique to not access __members__. |
|
|
msg250895 - (view) |
Author: Eric Snow (eric.snow) *  |
Date: 2015-09-17 16:14 |
OrderedDict has a C implementation now. So try the following: try: from _collections import OrderedDict except ImportError: from collections import OrderedDict |
|
|
msg250898 - (view) |
Author: Ethan Furman (ethan.furman) *  |
Date: 2015-09-17 16:40 |
Is pulling in `_collections` not as resource intensive as pulling in `collections`? |
|
|
msg250900 - (view) |
Author: R. David Murray (r.david.murray) *  |
Date: 2015-09-17 17:22 |
_collections exists because it contains only the stuff that is needed at python startup. So it is loaded regardless, and thus is very cheap to import elsewhere :) |
|
|
msg250904 - (view) |
Author: Serhiy Storchaka (serhiy.storchaka) *  |
Date: 2015-09-17 17:43 |
_collections exists because it contains C implementations of some collections classes or helpers. _collections itself is imported only in collections and threading. And in threading the same idiom as proposed by Eric is used: try: from _collections import deque as _deque except ImportError: from collections import deque as _deque What is the cost of importing OrderedDict at all? Ethan, can you provide any measurement results? |
|
|
msg250905 - (view) |
Author: Ethan Furman (ethan.furman) *  |
Date: 2015-09-17 17:55 |
_collections sounds cool, but the flip side is any python without the C implemntation would still have the slower startup, right? No, I don't have measurements -- just that I have heard importing collections can have an effect on startup time. Of course, it's only a cost you pay once, so maybe it's not a big deal. On the other hand, I could make __members__ a caching property that doesn't import OrderedDict until it is used, and not use it internally -- and I can't think of any reason why the stdlib would need to access __members__ itself. |
|
|
msg250909 - (view) |
Author: Serhiy Storchaka (serhiy.storchaka) *  |
Date: 2015-09-17 18:27 |
I tried to measure import time with different patches and didn't notice any difference. |
|
|
msg250911 - (view) |
Author: Ethan Furman (ethan.furman) *  |
Date: 2015-09-17 18:39 |
In that case I'll go with _collections; if performance does become an issue with other pythons later we can add the caching property. Thanks for all the insights. |
|
|
msg250917 - (view) |
Author: Serhiy Storchaka (serhiy.storchaka) *  |
Date: 2015-09-17 20:07 |
If there is no any evidences, we shouldn't change a code. |
|
|
msg250933 - (view) |
Author: Ethan Furman (ethan.furman) *  |
Date: 2015-09-18 04:53 |
Serhiy, your objection is noted, thank you. |
|
|
msg250934 - (view) |
Author: Roundup Robot (python-dev)  |
Date: 2015-09-18 05:04 |
New changeset b77916d2d7cc by Ethan Furman in branch 'default': Close : use C implementation of OrderedDict https://hg.python.org/cpython/rev/b77916d2d7cc |
|
|
msg250935 - (view) |
Author: Zachary Ware (zach.ware) *  |
Date: 2015-09-18 05:14 |
That change could use a comment stating why it's doing things that way instead of the 'obvious' way. |
|
|
msg250938 - (view) |
Author: Stefan Behnel (scoder) *  |
Date: 2015-09-18 05:55 |
> _collections sounds cool, but the flip side is any python without the C implemntation would still have the slower startup, right? I wouldn't bother too much with that, certainly not given the order we are talking about here. Jython's startup time is slowed down mostly by the JVM startup time anyway. And if you use PyPy then it's exactly because you do *not* care about the startup time but about performance improvements for long running code. |
|
|
msg250939 - (view) |
Author: Roundup Robot (python-dev)  |
Date: 2015-09-18 05:56 |
New changeset c0363f849624 by Ethan Furman in branch 'default': Issue 25147: add reason for using _collections https://hg.python.org/cpython/rev/c0363f849624 |
|
|
msg250942 - (view) |
Author: Serhiy Storchaka (serhiy.storchaka) *  |
Date: 2015-09-18 06:25 |
But the comment is false. That change doesn't reduce startup cost. |
|
|
msg250943 - (view) |
Author: Stefan Behnel (scoder) *  |
Date: 2015-09-18 06:33 |
Let's say the change minimises the dependencies. That is a reasonable goal, too. |
|
|