gh-109653: Improve import performance of tkinter library with lazy imports#148409
gh-109653: Improve import performance of tkinter library with lazy imports#148409sharktide wants to merge 15 commits intopython:mainfrom
tkinter library with lazy imports#148409Conversation
|
cc @AlexWaygood |
|
@sharktide please also add a test verifying that the lazy modules are indeed not imported. See #144387 for how to do it. |
picnixz
left a comment
There was a problem hiding this comment.
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 |
|
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. |
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 Detailsdiff --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 |
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 Importing |




Similar to #109789 but for Tkinter
EDIT: I can’t generate a direct comparison graph currently, but expect a 20-30% performance increase.