gh-109653: Improve import performance of ``tkinter`` library with lazy imports by sharktide · Pull Request #148409 · python/cpython
sharktide
marked this pull request as ready for review
@sharktide please also add a test verifying that the lazy modules are indeed not imported. See #144387 for how to do it.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
EDIT: I can’t generate a direct comparison graph currently, but expect a 20-30% performance increase.
tkinter is not a common module while enum is and that's the rationale why we wanted to improve the import time of enum. I would also not say "expect a XX" increase without convincing arguments (it could be more, it could be less, but I don't think it's relevant to shoot a random number like that).
The change to import re adds more functions that are less readable IMO. I don't think it's worth it. For traceback it may be ok but os is a cheap module so I don't think it's worth either.
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 I have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.
FWIW The 20-30% is likely in the right ballpark, here's the tuna-visualized output from running
python3.15 -Ximporttime -c 'import tkinter' 2> out
using uv's build of python3.15a8.
We should definitely keep traceback lazy as it import _colorize module. Otherwise agreed, not sure if the added complexity is worth it for this specific module.
The added complexity is minimal. It's just a very basic wrapper around a variable. I was able to make it in ~5 minutes on mobile.
The added complexity is minimal.
It's not. It's about code readability. It's not because you did in 5 minutes that it's not complex. The problem is that we have more indirect references.
Here's a smaller diff that does it the way tokenize.py used to. It adds a fast functools import.
Details
diff --git a/Lib/tkinter/__init__.py b/Lib/tkinter/__init__.py index 63340aafa77..f2313a5cfa9 100644 --- a/Lib/tkinter/__init__.py +++ b/Lib/tkinter/__init__.py @@ -33,6 +33,7 @@ from tkinter.constants import * import collections import enum +import functools import sys import types @@ -51,28 +52,16 @@ WRITABLE = _tkinter.WRITABLE EXCEPTION = _tkinter.EXCEPTION -_magic_re = None -_space_re = None +_MAGIC_RE_PAT = r'([\\{}])' +_SPACE_RE_PAT = r'(?a)([\s])' + + +@functools.lru_cache +def _compile(expr): + return re.compile(expr) + -def _get_magic_re(): - """ - Internal function that acts as a wrapper - for the re import to make it lazy. - """ - global _magic_re - if _magic_re is None: - _magic_re = re.compile(r'([\\{}])') - return _magic_re -def _get_space_re(): - """ - Internal function that acts as a wrapper - for the re import to make it lazy. - """ - global _space_re - if _space_re is None: - _space_re = re.compile(r'([\s])', re.ASCII) - return _space_re def _join(value): """Internal function.""" @@ -83,7 +72,7 @@ def _stringify(value): if isinstance(value, (list, tuple)): if len(value) == 1: value = _stringify(value[0]) - if _get_magic_re().search(value): + if _compile(_MAGIC_RE_PAT).search(value): value = '{%s}' % value else: value = '{%s}' % _join(value) @@ -94,14 +83,14 @@ def _stringify(value): value = str(value) if not value: value = '{}' - elif _get_magic_re().search(value): + elif _compile(_MAGIC_RE_PAT).search(value): # add '\' before special characters and spaces - value = _get_magic_re().sub(r'\\\1', value) + value = _compile(_MAGIC_RE_PAT).sub(r'\\\1', value) value = value.replace('\n', r'\n') - value = _get_space_re().sub(r'\\\1', value) + value = _compile(_SPACE_RE_PAT).sub(r'\\\1', value) if value[0] == '"': value = '\\' + value - elif value[0] == '"' or _get_space_re().search(value): + elif value[0] == '"' or _compile(_SPACE_RE_PAT).search(value): value = '{%s}' % value return value
AlexWaygood
changed the title
gh-109653: Improve import preformance of
gh-109653: Improve import performance of tkinter library with lazy importstkinter library with lazy imports
I just ran some tests in a headless windows environment, and the 3.15 main branch was 23ms, this pr was 18ms, @hugovk’s idea was also 18ms.
IMO it’s worth making these imports lazy, up to y’all which impl you like better. For readability, I prefer mine because it’s almost the same as the original code, save for the two wrapper functions, but they preserve the same names as before so it is easier to understand for me at least, but @hugovk’s idea is also just as good.
P.S. The system I tested on has low-end specs and it’s headless (I use it for cybersecurity testing), but nevertheless those times are what I got.
I am honestly against the change for re and will still be -1 unless we revert those parts. I don't think it's worth changing the re part. The functools approach is a bit clearer to read but honestly I don't think it's really necessary. Removing the local imports to traceback and os are okayish (it'll be easier to read the source code), but otherwise, it just adds un-necessary complexity.
Importing tkinter is unlikely to be the bottleneck in tkinter-based applications.


