[Concept,12/20] buildman: Cache the kconfig-changed check per commit

Message ID 20260316154733.1587261-13-sjg@u-boot.org
State New
Headers
Series buildman: Add distributed builds |

Commit Message

Simon Glass March 16, 2026, 3:47 p.m. UTC
  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(-)

-- 
2.43.0
  

Patch

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(