From: Simon Glass <sjg@chromium.org> The kconfig_changed_since() check does an os.walk() of the entire source tree which is very slow when dozens of threads do it simultaneously for the same commit. Add a per-commit cache so only the first thread to check a given commit does the walk; all other threads reuse the result. Also skip the check entirely when do_config is already True (e.g. first commit) since defconfig will run anyway. Prune dotfiles and 'build' directories from the walk to reduce the search space. Signed-off-by: Simon Glass <sjg@chromium.org> --- tools/buildman/builderthread.py | 72 +++++++++++++++++++++++++-------- tools/buildman/func_test.py | 6 ++- tools/buildman/test.py | 3 ++ 3 files changed, 63 insertions(+), 18 deletions(-) diff --git a/tools/buildman/builderthread.py b/tools/buildman/builderthread.py index dcf2d8f9ac5..13c98612c81 100644 --- a/tools/buildman/builderthread.py +++ b/tools/buildman/builderthread.py @@ -52,20 +52,19 @@ BuildSetup = namedtuple('BuildSetup', ['env', 'args', 'config_args', 'cwd', RETURN_CODE_RETRY = -1 BASE_ELF_FILENAMES = ['u-boot', 'spl/u-boot-spl', 'tpl/u-boot-tpl'] +# Per-commit cache for the kconfig_changed_since() result. The answer only +# depends on the commit (did any Kconfig file change since the previous +# checkout?), so one thread can do the walk and every other thread building +# the same commit reuses the boolean. +_kconfig_cache = {} +_kconfig_cache_lock = threading.Lock() -def kconfig_changed_since(fname, srcdir='.', target=None): - """Check if any Kconfig or defconfig files are newer than the given file. - Args: - fname (str): Path to file to compare against (typically '.config') - srcdir (str): Source directory to search for Kconfig/defconfig files - target (str): Board target name; if provided, only check that board's - defconfig file (e.g. 'sandbox' checks 'configs/sandbox_defconfig') +def _kconfig_changed_uncached(fname, srcdir, target): + """Check if any Kconfig or defconfig files are newer than fname. - Returns: - bool: True if any Kconfig* file (or the board's defconfig) in srcdir - is newer than fname, False otherwise. Also returns False if fname - doesn't exist. + This does the real work — an os.walk() of srcdir. It should only be + called once per commit; the result is cached by kconfig_changed_since(). """ if not os.path.exists(fname): return False @@ -78,8 +77,9 @@ def kconfig_changed_since(fname, srcdir='.', target=None): if os.path.getmtime(defconfig) > ref_time: return True - # Check all Kconfig files - for dirpath, _, filenames in os.walk(srcdir): + for dirpath, dirnames, filenames in os.walk(srcdir): + # Prune in-place so os.walk() skips dotdirs and build dirs + dirnames[:] = [d for d in dirnames if d[0] != '.' and d != 'build'] for filename in filenames: if filename.startswith('Kconfig'): filepath = os.path.join(dirpath, filename) @@ -87,6 +87,42 @@ def kconfig_changed_since(fname, srcdir='.', target=None): return True return False + +def reset_kconfig_cache(): + """Reset the cached kconfig results, for testing""" + _kconfig_cache.clear() + + +def kconfig_changed_since(fname, srcdir='.', target=None, commit_upto=None): + """Check if any Kconfig or defconfig files are newer than the given file. + + Args: + fname (str): Path to file to compare against (typically '.config') + srcdir (str): Source directory to search for Kconfig/defconfig files + target (str): Board target name; if provided, only check that board's + defconfig file (e.g. 'sandbox' checks 'configs/sandbox_defconfig') + commit_upto (int or None): Commit index for caching. When set, only + the first thread to check a given commit does the walk; all + other threads reuse that result. + + Returns: + bool: True if any Kconfig* file (or the board's defconfig) in srcdir + is newer than fname, False otherwise. Also returns False if fname + doesn't exist. + """ + if commit_upto is None: + return _kconfig_changed_uncached(fname, srcdir, target) + + if commit_upto in _kconfig_cache: + return _kconfig_cache[commit_upto] + + with _kconfig_cache_lock: + if commit_upto in _kconfig_cache: + return _kconfig_cache[commit_upto] + result = _kconfig_changed_uncached(fname, srcdir, target) + _kconfig_cache[commit_upto] = result + return result + # Common extensions for images COMMON_EXTS = ['.bin', '.rom', '.itb', '.img'] @@ -625,11 +661,15 @@ class BuilderThread(threading.Thread): if self.toolchain: commit = self._checkout(commit_upto, req.work_dir) - # Check if Kconfig files have changed since last config - if self.builder.kconfig_check: + # Check if Kconfig files have changed since last config. Skip + # when do_config is already True (e.g. first commit) since + # defconfig will run anyway. This avoids an expensive os.walk() + # of the source tree that can be very slow when many threads + # do it simultaneously. + if self.builder.kconfig_check and not do_config: config_file = os.path.join(out_dir, '.config') if kconfig_changed_since(config_file, req.work_dir, - req.brd.target): + req.brd.target, commit_upto): kconfig_reconfig = True do_config = True diff --git a/tools/buildman/func_test.py b/tools/buildman/func_test.py index 7db4c086207..aa206cf75df 100644 --- a/tools/buildman/func_test.py +++ b/tools/buildman/func_test.py @@ -1634,7 +1634,8 @@ something: me call_count = [0] config_exists = [False] - def mock_kconfig_changed(fname, _srcdir='.', _target=None): + def mock_kconfig_changed(fname, _srcdir='.', _target=None, + _commit_upto=None): """Mock for kconfig_changed_since that checks if .config exists Args: @@ -1671,7 +1672,8 @@ something: me """Test that -Z flag disables Kconfig change detection""" call_count = [0] - def mock_kconfig_changed(_fname, _srcdir='.', _target=None): + def mock_kconfig_changed(_fname, _srcdir='.', _target=None, + _commit_upto=None): """Mock for kconfig_changed_since that always returns True Returns: diff --git a/tools/buildman/test.py b/tools/buildman/test.py index 37930ad9720..998f2227281 100644 --- a/tools/buildman/test.py +++ b/tools/buildman/test.py @@ -1148,6 +1148,7 @@ class TestBuildMisc(TestBuildBase): def test_kconfig_changed_since(self): """Test the kconfig_changed_since() function""" + builderthread.reset_kconfig_cache() with tempfile.TemporaryDirectory() as tmpdir: # Create a reference file ref_file = os.path.join(tmpdir, 'done') @@ -1163,6 +1164,7 @@ class TestBuildMisc(TestBuildBase): # Create a Kconfig file newer than the reference kconfig = os.path.join(tmpdir, 'Kconfig') tools.write_file(kconfig, b'config TEST\n') + builderthread.reset_kconfig_cache() # Should now return True since Kconfig is newer self.assertTrue( @@ -1187,6 +1189,7 @@ class TestBuildMisc(TestBuildBase): time.sleep(0.1) tools.write_file(os.path.join(subdir, 'Kconfig.sub'), b'config SUBTEST\n') + builderthread.reset_kconfig_cache() # Should return True due to newer Kconfig.sub in subdir self.assertTrue( -- 2.43.0