| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
Sorry, something went wrong.
|
@DavidCEllis what do you think of AA-Turner@opt-annotationlib? Instead of the lazy object, it uses self-replacing functions, so that the 'lazy' cost is only paid once. I think for ast.unparse you could probably just get away with a local import, but I haven't benchmarked/tested this. def _Stringifier(*args, **kwds):
# This function replaces itself with the real class when first called
global _Stringifier
from annotationlib._stringifier import Stringifier as _Stringifier
return _Stringifier(*args, **kwds)A |
Sorry, something went wrong.
|
I don't mind the self replacing functions if that's a more recognisable pattern. I personally prefer the class as there's no observable placeholder object that needs to be replaced. Inspecting will only give you the actual function/module and you can't make a reference to the self-replacing function that doesn't get replaced. [Edit: I also like that it puts information about all of the imports at the top of the module, so you can see that the module may import functools without having to search for the inline import statement.] The lazy object also only pays the cost once by assigning to the object after the first import - it's basically a by-hand written version of an instance of my general lazy importer module. |
Sorry, something went wrong.
|
Benchmarking again with the recent changes to ast, the vast majority of the improvement comes just from deferring the functools import. We can gain another ~5ms by splitting the module and deferring the ast import. I'll leave it up to @JelleZijlstra to decide which he prefers. Current HEADimport time: self [us] | cumulative | imported package
import time: 1537 | 1537 | _ast
import time: 3552 | 5089 | ast
import time: 4438 | 4438 | types
import time: 5144 | 9581 | enum
import time: 133 | 133 | itertools
import time: 3329 | 3329 | keyword
import time: 79 | 79 | _operator
import time: 3214 | 3293 | operator
import time: 3374 | 3374 | reprlib
import time: 273 | 273 | _collections
import time: 6617 | 17016 | collections
import time: 72 | 72 | _functools
import time: 3908 | 20995 | functools
import time: 3612 | 39275 | annotationlib
This PR ('lazy')import time: self [us] | cumulative | imported package
import time: 3438 | 3438 | types
import time: 4468 | 7906 | enum
import time: 3768 | 3768 | keyword
import time: 5120 | 16793 | annotationlib_lazy
Self replacing functionsimport time: self [us] | cumulative | imported package
import time: 3419 | 3419 | types
import time: 4419 | 7838 | enum
import time: 2701 | 2701 | keyword
import time: 5530 | 16067 | annotationlib_self_replacing
Use a local import for functoolsimport time: self [us] | cumulative | imported package
import time: 1687 | 1687 | _ast
import time: 3630 | 5316 | ast
import time: 3850 | 3850 | types
import time: 4770 | 8619 | enum
import time: 3127 | 3127 | keyword
import time: 3694 | 20755 | annotationlib_defer_functools
|
Sorry, something went wrong.
|
I'm not sure this is worth it any more:
|
Sorry, something went wrong.
|
Probably fair on the scale of things, although perhaps you could skip the functools import all together with something similar to how dataclasses avoids importing typing? Lines 808 to 814 in d30052a I'll note that needing annotationlib doesn't mean you also need typing. I use annotations for a dataclasses-like project where I've been very careful to keep import time down and so avoid importing typing at runtime. With Python 3.14, annotationlib on the other hand is somewhat unavoidable in the case that FORWARDREF is needed. |
Sorry, something went wrong.
|
Good idea regarding functools, I just did that myself in #132059 since the change is so simple. I feel deferring the ast import in annotationlib goes too far. We need ast for both the FORWARDREF and STRING formats in get_annotations(), and if you're using annotationlib, you probably want one of those. However, I do think it's realistic to defer import annotationlib in typing.py, I'll see if I can make that happen. |
Sorry, something went wrong.
|
With the STRING format you do need ast and that's unavoidable as the fake globals always uses _Stringifier in that case. With FORWARDREF however, _Stringifier will only be used if there are actual forward references that need to be replaced, so the ast import is only needed in this case. Example with this PR: import sys
from annotationlib import get_annotations, Format
class NoForwardRef:
a: int
class YesForwardRef:
a: Any
print(get_annotations(NoForwardRef, format=Format.FORWARDREF))
print("ast" in sys.modules)
print(get_annotations(YesForwardRef, format=Format.FORWARDREF))
print("ast" in sys.modules){'a': <class 'int'>}
False
{'a': ForwardRef('Any')}
TrueI think it's likely many tools that currently get annotations directly will need to switch to using annotationlib.get_annotations(obj, format=Format.FORWARDREF) to avoid the potential NameError or AttributeError results. This doesn't necessarily mean they want or need ForwardRef in all cases, they just need to 'work' as they did with 3.13 and not raise new exceptions if someone removes from __future__ import annotations. Still arguable if it goes too far with the improvement to ast's own import time but I just wanted to point out that with 3.14 using annotationlib doesn't necessarily mean you want the elements that require ast. |
Sorry, something went wrong.
Sorry, something went wrong.
|
I think this can be closed. The functools import has been deferred separately and ast import time has been improved so it's less of a sledgehammer to import time and typing is no longer being directly hit by this. The module could still be broken up into a package, but I feel that about a fair chunk of the larger slower stdlib modules and doing what this branch does feels more 'hacky' than structured at this point. |
Sorry, something went wrong.
| Back | FazBrowse Home | New Git URL |
This PR converts annotationlib.py into a package with annotationlib/__init__.py and annotationlib/_stringifier.py.
Discussed here: https://discuss.python.org/t/pep-749-implementing-pep-649/54974/63
This is done in order to move the definition of _Stringifier into a new submodule in order to defer the import of ast in the main module.
The outcome of this is that ast should only be imported if any of the following occur:
Note: I've used a class with a __getattr__ method as a way of deferring imports in the current PR but I'd be happy to change that to something else if there's a more standard pattern.
My machine isn't a super stable benchmarking machine so take these only as rough estimates (they're slightly different to those posted in the discuss thread as it's a different run).
This branch:
Main:
Footnotes
or any of the similar methods such as call_annotate_function ↩