Skip to content

Commit 4f3501f

Browse files
committed
Try to call Override less
Eliminate a kwarg 'explain' from the call to Builder() for Mkdir. Arg is not used (maybe it was in the past, though have not found the evidence for this), which meant we had an override dict in this case. Suprisingly, this ended up in the override logic a *lot* in a build of a large sample project - over 2000 extra calls to scons_subst_once that weren't needed. Builder and Executor both have places where they call Override(). While the Override factory indeed returns quickly if the override dict is empty, avoid the function call entirely in this case by checking (there was an old TODO comment to this effect in Builder.__init__.py). The Executor init method declared multiple paramters with mutable defaults, which means they "persist". While there's no evidence this is calling a real problem, eliminate part of this footgun by defaulting overridelist and builder_kw to None and checking for that (targets and sources still default to a mutable empty list, a TBD perhaps). Signed-off-by: Mats Wichmann <[email protected]>
1 parent 8c15e8c commit 4f3501f

File tree

6 files changed

+28
-19
lines changed

6 files changed

+28
-19
lines changed

CHANGES.txt

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -13,11 +13,16 @@ NOTE: Since SCons 4.9.0, Python 3.7.0 or above is required.
1313
RELEASE VERSION/DATE TO BE FILLED IN LATER
1414

1515
From John Doe:
16-
17-
- Whatever John Doe did.
16+
- Whatever John Doe did.
1817

1918
From William Deegan:
20-
- Fix SCons Docbook schema to work with lxml > 5
19+
- Fix SCons Docbook schema to work with lxml > 5
20+
21+
From Mats Wichmann:
22+
- Reduce unneeded computation of overrides. The Mkdir builder used an
23+
unknown argument ('explain') on creation, causing it to be considered
24+
an override. Also, if override dict is empty, don't even call the
25+
Override factory function.
2126

2227

2328
RELEASE 4.10.1 - Sun, 16 Nov 2025 10:51:57 -0700

RELEASE.txt

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,11 @@ IMPROVEMENTS
4141
documentation: performance improvements (describe the circumstances
4242
under which they would be observed), or major code cleanups
4343

44+
- Reduce unneeded computation of overrides. The Mkdir builder used an
45+
unknown argument ('explain') on creation, causing it to be considered
46+
an override. Also, if override dict is empty, don't even call the
47+
Override factory function.
48+
4449
PACKAGING
4550
---------
4651

SCons/Builder.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -670,8 +670,8 @@ def prependDirIfRelative(f, srcdir=kw['srcdir']):
670670
else:
671671
env_kw = self.overrides
672672

673-
# TODO if env_kw: then the following line. there's no purpose in calling if no overrides.
674-
env = env.Override(env_kw)
673+
if env_kw: # there's no purpose in calling if no overrides.
674+
env = env.Override(env_kw)
675675
return self._execute(env, target, source, OverrideWarner(kw), ekw)
676676

677677
def adjust_suffix(self, suff):

SCons/Environment.py

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -859,9 +859,12 @@ def Override(self, overrides):
859859
A proxy environment of type :class:`OverrideEnvironment`.
860860
or the current environment if *overrides* is empty.
861861
"""
862-
if not overrides: return self
862+
# belt-and-suspenders - main callers should already have checked:
863+
if not overrides:
864+
return self
863865
o = copy_non_reserved_keywords(overrides)
864-
if not o: return self
866+
if not o:
867+
return self
865868
overrides = {}
866869
merges = None
867870
for key, value in o.items():

SCons/Executor.py

Lines changed: 8 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -168,21 +168,18 @@ class Executor(metaclass=NoSlotsPyPy):
168168
'_do_execute',
169169
'_execute_str')
170170

171-
def __init__(self, action, env=None, overridelist=[{}],
172-
targets=[], sources=[], builder_kw={}) -> None:
171+
def __init__(self, action, env=None, overridelist=None,
172+
targets=[], sources=[], builder_kw=None) -> None:
173173
if SCons.Debug.track_instances: logInstanceCreation(self, 'Executor.Executor')
174174
self.set_action_list(action)
175175
self.pre_actions = []
176176
self.post_actions = []
177177
self.env = env
178-
self.overridelist = overridelist
179-
if targets or sources:
180-
self.batches = [Batch(targets[:], sources[:])]
181-
else:
182-
self.batches = []
183-
self.builder_kw = builder_kw
184-
self._do_execute = 1
185-
self._execute_str = 1
178+
self.overridelist = [{}] if overridelist is None else overridelist
179+
self.batches = [Batch(targets[:], sources[:])] if targets or sources else []
180+
self.builder_kw = {} if builder_kw is None else builder_kw
181+
self._do_execute: int = 1 # map key
182+
self._execute_str: int = 1 # map key
186183
self._memo = {}
187184

188185
def get_lvars(self):
@@ -358,7 +355,7 @@ def get_build_env(self):
358355

359356
import SCons.Defaults
360357
env = self.env or SCons.Defaults.DefaultEnvironment()
361-
build_env = env.Override(overrides)
358+
build_env = env.Override(overrides) if overrides else env
362359

363360
self._memo['get_build_env'] = build_env
364361

SCons/Node/FS.py

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -381,7 +381,6 @@ def get_MkdirBuilder():
381381
# calling SCons.Defaults.DefaultEnvironment() when necessary.
382382
MkdirBuilder = SCons.Builder.Builder(action = Mkdir,
383383
env = None,
384-
explain = None,
385384
is_explicit = None,
386385
target_scanner = SCons.Defaults.DirEntryScanner,
387386
name = "MkdirBuilder")

0 commit comments

Comments
 (0)