gh-109653: typing.py: improve import time by creating soft-deprecated members on demand

[AlexWaygood]

This cuts about 1/3 off the import time of typing.py for me locally (PGO-optimised, non-debug build). 0.012s -> 0.008s.

Benchmark script

(There's probably a better way of benchmarking import times, but this seems to work pretty well.)

importsysimporttimeimportstatisticsimportsubprocesstimes= [] for_inrange(500): ret=subprocess.run( [sys.executable, "-c", f"import time; t0 = time.perf_counter(); import typing; print(time.perf_counter() - t0)"], check=True, capture_output=True, text=True ) times.append(float(ret.stdout.strip())) print(statistics.mean(times))

• Issue: Improve import time of various stdlib modules #109653

[bedevere-bot]

🤖 New build scheduled with the buildbot fleet by @AlexWaygood for commit 180461f 🤖

If you want to schedule another build, you need to add the 🔨 test-with-refleak-buildbots label again.

[AA-Turner]

Would it be going to far to use this approach for all of the _alias types?

If it would be, perhaps we could defer the collections types? By doing so (& changing _overload_registry to use .setdefault()), we'd be able to avoid importing collections and just use _collections_abc (which is already imported in e.g. os).

A

[AlexWaygood]

Would it be going to far to use this approach for all of the _alias types?

The main reason why the changes here save us so much is because we can avoid unconditionally importing re and contextlib -- the creation of these aliases themselves is pretty cheap, I think. So it would probably only be worth it for the others if we could remove more dependencies on the rest of the stdlib. And I'm not sure about this:

changing _overload_registry to use .setdefault()

IIRC, defaultdict is a fair bit faster than setdefault(), and the _overload_registry is quite performance-sensitive. (We spent a lot of time in #31716 trying to figure out how to implement it without having a huge performance regression for typing.overload!)

[AA-Turner]

I can just about see a path to inlining functools, but it would also rely on changing _overload_registry -- so I think this PR is a sensible limit for now -- there are no other 'heavy' imports, as far as I can see.

A

[AlexWaygood]

FWIW @AA-Turner, here's a diff (relative to this PR branch) that shaves another 0.002s off the import time by avoiding the collections import. But it's a much bigger diff, for a smaller gain, so I'd prefer to defer it to its own PR and consider it separately. I'm honestly not sure it's worth the cost to readability, personally:

Details

diff --git a/Lib/typing.py b/Lib/typing.py index 84b741bfb0..0218b715f6 100644 --- a/Lib/typing.py+++ b/Lib/typing.py@@ -19,9 +19,7 @@ """ from abc import abstractmethod, ABCMeta -import collections-from collections import defaultdict-import collections.abc+import _collections_abc import copyreg import functools import operator @@ -40,6 +38,11 @@ Generic, ) +try:+ from _collections import defaultdict+except ImportError:+ from collections import defaultdict+ # Please keep __all__ alphabetized within each category. __all__ = [ # Super-special typing primitives. @@ -219,7 +222,7 @@ def _should_unflatten_callable_args(typ, args): we need to unflatten it. """ return ( - typ.__origin__ is collections.abc.Callable+ typ.__origin__ is _collections_abc.Callable and not (len(args) == 2 and _is_param_expr(args[0])) ) @@ -1317,7 +1320,7 @@ def _make_substitution(self, args, new_arg_by_param): subargs.append(new_arg_by_param[x]) new_arg = old_arg[tuple(subargs)] - if self.__origin__ == collections.abc.Callable and isinstance(new_arg, tuple):+ if self.__origin__ == _collections_abc.Callable and isinstance(new_arg, tuple): # Consider the following `Callable`. # C = Callable[[int], str] # Here, `C.__args__` should be (int, str) - NOT ([int], str). @@ -1885,7 +1888,7 @@ def _proto_hook(cls, other): # ...or in annotations, if it is a sub-protocol. annotations = getattr(base, '__annotations__',{}) - if (isinstance(annotations, collections.abc.Mapping) and+ if (isinstance(annotations, _collections_abc.Mapping) and attr in annotations and issubclass(other, Generic) and getattr(other, '_is_protocol', False)): break @@ -2521,18 +2524,18 @@ class Other(Leaf): # Error reported by type checker # Various ABCs mimicking those in collections.abc. _alias = _SpecialGenericAlias -Hashable = _alias(collections.abc.Hashable, 0) # Not generic.-Awaitable = _alias(collections.abc.Awaitable, 1)-Coroutine = _alias(collections.abc.Coroutine, 3)-AsyncIterable = _alias(collections.abc.AsyncIterable, 1)-AsyncIterator = _alias(collections.abc.AsyncIterator, 1)-Iterable = _alias(collections.abc.Iterable, 1)-Iterator = _alias(collections.abc.Iterator, 1)-Reversible = _alias(collections.abc.Reversible, 1)-Sized = _alias(collections.abc.Sized, 0) # Not generic.-Container = _alias(collections.abc.Container, 1)-Collection = _alias(collections.abc.Collection, 1)-Callable = _CallableType(collections.abc.Callable, 2)+Hashable = _alias(_collections_abc.Hashable, 0) # Not generic.+Awaitable = _alias(_collections_abc.Awaitable, 1)+Coroutine = _alias(_collections_abc.Coroutine, 3)+AsyncIterable = _alias(_collections_abc.AsyncIterable, 1)+AsyncIterator = _alias(_collections_abc.AsyncIterator, 1)+Iterable = _alias(_collections_abc.Iterable, 1)+Iterator = _alias(_collections_abc.Iterator, 1)+Reversible = _alias(_collections_abc.Reversible, 1)+Sized = _alias(_collections_abc.Sized, 0) # Not generic.+Container = _alias(_collections_abc.Container, 1)+Collection = _alias(_collections_abc.Collection, 1)+Callable = _CallableType(_collections_abc.Callable, 2) Callable.__doc__ = \ """Deprecated alias to collections.abc.Callable. @@ -2547,15 +2550,15 @@ class Other(Leaf): # Error reported by type checker There is no syntax to indicate optional or keyword arguments; such function types are rarely used as callback types. """ -AbstractSet = _alias(collections.abc.Set, 1, name='AbstractSet')-MutableSet = _alias(collections.abc.MutableSet, 1)+AbstractSet = _alias(_collections_abc.Set, 1, name='AbstractSet')+MutableSet = _alias(_collections_abc.MutableSet, 1) # NOTE: Mapping is only covariant in the value type. -Mapping = _alias(collections.abc.Mapping, 2)-MutableMapping = _alias(collections.abc.MutableMapping, 2)-Sequence = _alias(collections.abc.Sequence, 1)-MutableSequence = _alias(collections.abc.MutableSequence, 1)+Mapping = _alias(_collections_abc.Mapping, 2)+MutableMapping = _alias(_collections_abc.MutableMapping, 2)+Sequence = _alias(_collections_abc.Sequence, 1)+MutableSequence = _alias(_collections_abc.MutableSequence, 1) ByteString = _DeprecatedGenericAlias( - collections.abc.ByteString, 0, removal_version=(3, 14) # Not generic.+ _collections_abc.ByteString, 0, removal_version=(3, 14) # Not generic. ) # Tuple accepts variable number of parameters. Tuple = _TupleType(tuple, -1, inst=False, name='Tuple') @@ -2571,20 +2574,15 @@ class Other(Leaf): # Error reported by type checker To specify a variable-length tuple of homogeneous type, use Tuple[T, ...]. """ List = _alias(list, 1, inst=False, name='List') -Deque = _alias(collections.deque, 1, name='Deque') Set = _alias(set, 1, inst=False, name='Set') FrozenSet = _alias(frozenset, 1, inst=False, name='FrozenSet') -MappingView = _alias(collections.abc.MappingView, 1)-KeysView = _alias(collections.abc.KeysView, 1)-ItemsView = _alias(collections.abc.ItemsView, 2)-ValuesView = _alias(collections.abc.ValuesView, 1)+MappingView = _alias(_collections_abc.MappingView, 1)+KeysView = _alias(_collections_abc.KeysView, 1)+ItemsView = _alias(_collections_abc.ItemsView, 2)+ValuesView = _alias(_collections_abc.ValuesView, 1) Dict = _alias(dict, 2, inst=False, name='Dict') -DefaultDict = _alias(collections.defaultdict, 2, name='DefaultDict')-OrderedDict = _alias(collections.OrderedDict, 2)-Counter = _alias(collections.Counter, 1)-ChainMap = _alias(collections.ChainMap, 2)-Generator = _alias(collections.abc.Generator, 3)-AsyncGenerator = _alias(collections.abc.AsyncGenerator, 2)+Generator = _alias(_collections_abc.Generator, 3)+AsyncGenerator = _alias(_collections_abc.AsyncGenerator, 2) Type = _alias(type, 1, inst=False, name='Type') Type.__doc__ = \ """Deprecated alias to builtins.type. @@ -2692,8 +2690,8 @@ def _make_nmtuple(name, types, module, defaults = ()): fields = [n for n, t in types] types ={n: _type_check(t, f"field{n} annotation must be a type") for n, t in types} - nm_tpl = collections.namedtuple(name, fields,- defaults=defaults, module=module)+ from collections import namedtuple+ nm_tpl = namedtuple(name, fields, defaults=defaults, module=module) nm_tpl.__annotations__ = nm_tpl.__new__.__annotations__ = types return nm_tpl @@ -3432,6 +3430,17 @@ def __getattr__(attr): elif attr in{"ContextManager", "AsyncContextManager"}: import contextlib obj = _alias(getattr(contextlib, f"Abstract{attr}"), 1, name=attr) + elif attr in{"Deque", "DefaultDict", "OrderedDict", "Counter", "ChainMap"}:+ import collections+ match attr:+ case "Deque":+ obj = _alias(collections.deque, 1, name="Deque")+ case "DefaultDict":+ obj = _alias(collections.defaultdict, 2, name="DefaultDict")+ case "OrderedDict" | "ChainMap":+ obj = _alias(getattr(collections, attr), 2)+ case "Counter":+ obj = _alias(collections.Counter, 1) else: raise AttributeError(f"Module 'typing' has no attribute{attr!r}") globals()[attr] = obj

[AA-Turner]

a smaller gain

Though still 25% (on the 8ms post-this PR) or 50% total incl. this PR.

I agree this PR is both bigger bang-for-buck and should come first (I've no comments save applying Tom's suggestion), but for e.g. CLIs, milli-seconds can matter in feeling responsive, so I think worth at least considering the further improvement!

A

[JelleZijlstra]

I'm a bit skeptical of this because it relies on avoiding imports of common standard library modules only: how many real applications are there going to be that want to import typing but not re or contextlib?

On the other hand, the change in this PR is fairly small and self-contained, so I won't oppose it. But we shouldn't go through extremely lengths to avoid importing common stdlib modules.

[AlexWaygood]

I'm a bit skeptical of this because it relies on avoiding imports of common standard library modules only: how many real applications are there going to be that want to import typing but not re or contextlib?

Well, for one example, from a quick grep, pluggy appears to import neither re nor contextlib in their src/pluggy/ directory. The pluggy maintainers were the original people who raised concern about the import time of typing, among other stdlib modules, in https://discuss.python.org/t/deferred-computation-evalution-for-toplevels-imports-and-dataclasses/34173

More than that, though, I think that it's generally good to do this kind of thing (where it's not significantly detrimental to code readability). It's best to reduce dependencies between stdlib modules wherever possible, and make it clearer exactly why certain stdlib modules depend on others. It helps reduce the extent to which the stdlib is a massive import cycle.

But we shouldn't go through extremely lengths to avoid importing common stdlib modules.

I definitely agree with you there :)

[JelleZijlstra]

Well, for one example, from a quick grep, pluggy appears to import neither re nor contextlib in their src/pluggy/ directory.

But they do use importlib.metadata, which imports both re (in importlib/metadata/_adapters.py) and contextlib (in importlib/metadata/__init__.py). That's just one example of course, but it illustrates my point that a lot of applications won't benefit from deferring these imports.

[AlexWaygood]

But they do use importlib.metadata, which imports both re (in importlib/metadata/_adapters.py) and contextlib (in importlib/metadata/__init__.py). That's just one example of course, but it illustrates my point that a lot of applications won't benefit from deferring these imports.

Sure, it's unlikely that this PR is going to hugely improve the import time of pluggy by itself. But the conclusion that leads me to isn't "Working on reducing the number of imports in typing is pointless", it's "Both typing and importlib.metadata should work on optimising their import times". importlib.metadata could use exactly the same rationale for not working on reducing the number of imports they have ("Well, typing still imports re, so what's the point?"). One of the two has to come first.

[JelleZijlstra]

Those are good points. Let's do it.

[hauntsaninja]

Nice! I'm fine with this, but #109651 (comment) crosses the line for me.

[AlexWaygood]

Thanks all for the helpful reviews!

[bedevere-bot]

⚠️⚠️⚠️ Buildbot failure ⚠️⚠️⚠️

Hi! The buildbot PPC64LE Fedora Stable 3.x has failed when building commit e8be0c9.

What do you need to do:

1. Don't panic.
2. Check the buildbot page in the devguide if you don't know what the buildbots are or how they work.
3. Go to the page of the buildbot that failed (https://buildbot.python.org/all/#builders/90/builds/3814) and take a look at the build logs.
4. Check if the failure is related to this commit (e8be0c9) or if it is a false positive.
5. If the failure is related to this commit, please, reflect that on the issue and make a new Pull Request with a fix.

You can take a look at the buildbot page here:

https://buildbot.python.org/all/#builders/90/builds/3814

Failed tests:

• test_math

Summary of the results of the build (if available):

==

Click to see traceback logs

remote: Enumerating objects: 11, done. remote: Counting objects: 9% (1/11) remote: Counting objects: 18% (2/11) remote: Counting objects: 27% (3/11) remote: Counting objects: 36% (4/11) remote: Counting objects: 45% (5/11) remote: Counting objects: 54% (6/11) remote: Counting objects: 63% (7/11) remote: Counting objects: 72% (8/11) remote: Counting objects: 81% (9/11) remote: Counting objects: 90% (10/11) remote: Counting objects: 100% (11/11) remote: Counting objects: 100% (11/11), done. remote: Compressing objects: 12% (1/8) remote: Compressing objects: 25% (2/8) remote: Compressing objects: 37% (3/8) remote: Compressing objects: 50% (4/8) remote: Compressing objects: 62% (5/8) remote: Compressing objects: 75% (6/8) remote: Compressing objects: 87% (7/8) remote: Compressing objects: 100% (8/8) remote: Compressing objects: 100% (8/8), done. remote: Total 11 (delta 3), reused 9 (delta 3), pack-reused 0  From https://github.com/python/cpython * branch main -> FETCH_HEAD Note: switching to 'e8be0c9c5a7c2327b3dd64009f45ee0682322dcb'. You are in 'detached HEAD' state. You can look around, make experimental changes and commit them, and you can discard any commits you make in this state without impacting any branches by switching back to a branch. If you want to create a new branch to retain commits you create, you may do so (now or later) by using -c with the switch command. Example: git switch -c <new-branch-name> Or undo this operation with: git switch - Turn off this advice by setting config variable advice.detachedHead to false HEAD is now at e8be0c9c5a gh-109653: `typing.py`: improve import time by creating soft-deprecated members on demand (#109651) Switched to and reset branch 'main' ../Objects/longobject.c: In function ‘long_format_binary’: ../Objects/longobject.c:2120:13: warning: ‘kind’ may be used uninitialized [-Wmaybe-uninitialized] 2120 | else if (kind == PyUnicode_1BYTE_KIND){|^ ../Objects/longobject.c:1996:9: note: ‘kind’ was declared here 1996 | int kind; |^~~~../Objects/longobject.c: In function ‘long_to_decimal_string_internal’: ../Objects/longobject.c:1943:13: warning: ‘kind’ may be used uninitialized [-Wmaybe-uninitialized] 1943 | else if (kind == PyUnicode_1BYTE_KIND){|^ ../Objects/longobject.c:1767:9: note: ‘kind’ was declared here 1767 | int kind; |^~~~ Kill <WorkerThread #2 running test=test_gdb pid=713028 time=6 min 8 sec> process group Kill <WorkerThread #8 running test=test_tokenize pid=713190 time=5 min 57 sec> process group Kill <WorkerThread #9 running test=test_unparse pid=715520 time=3 min 58 sec> process group Kill <WorkerThread #10 running test=test_tarfile pid=718168 time=2 min 48 sec> process group Warning -- Failed to join <WorkerThread #2 running test=test_gdb pid=713028 time=6 min 38 sec> in 30.0 sec Warning -- Failed to join <WorkerThread #9 running test=test_unparse pid=715520 time=4 min 30 sec> in 31.0 sec Warning -- Failed to join <WorkerThread #10 running test=test_tarfile pid=718168 time=3 min 20 sec> in 32.0 sec make: *** [Makefile:2039: buildbottest] Error 5