From 2aca835f1fdf132578c2f195184801bcaaa88894 Mon Sep 17 00:00:00 2001 From: Gavin Mak Date: Thu, 12 Nov 2026 22:01:45 +0000 Subject: [PATCH] sync: warm init.templateDir cache for sync workers When initializing missing project object directories in parallel, each sync worker cold-misses the init.templateDir lookup cache and spawns `git config --get init.templatedir`. Relying on parent process memory alone also misses workers started via `spawn` or `forkserver`. Resolve the init.templateDir lookup once in the parent process when parallel sync workers may initialize an object directory (either because an objdir is missing or `--force-sync` is active), pass the cached entry through `parallel_context`, and seed it in `InitWorker`. Bug: 543101585 Change-Id: I6bac2352a9fd4b62d25ed2a89b670ac21aa70f4e Reviewed-on: https://gerrit-review.googlesource.com/c/git-repo/+/657084 Tested-by: Gavin Mak Reviewed-by: Brian Gan Commit-Queue: Gavin Mak --- project.py | 45 +++++++--- subcmds/sync.py | 24 +++++- tests/test_project.py | 64 ++++++++++++++ tests/test_subcmds_sync.py | 170 ++++++++++++++++++++++++++++++++++++- 4 files changed, 289 insertions(+), 14 deletions(-) diff --git a/project.py b/project.py index 4da098e8d..ea3304c50 100644 --- a/project.py +++ b/project.py @@ -120,7 +120,9 @@ def _GitTemplateEnvKey() -> Tuple[Any, ...]: return base + extra -@functools.lru_cache(maxsize=None) +_INIT_TEMPLATE_CACHE: Dict[Tuple[Any, ...], bool] = {} + + def _HasCustomGitTemplateDir(env_key: Tuple[Any, ...]) -> bool: """Return whether Git has a custom template directory configured. @@ -130,19 +132,26 @@ def _HasCustomGitTemplateDir(env_key: Tuple[Any, ...]) -> bool: Returns: True if a non-empty GIT_TEMPLATE_DIR or init.templatedir is configured. """ + cached = _INIT_TEMPLATE_CACHE.get(env_key) + if cached is not None: + return cached + git_template_dir = env_key[0] if git_template_dir is not None: - return bool(git_template_dir) + result = bool(git_template_dir) + else: + p = git_command.GitCommand( + None, + ["config", "--get", "init.templatedir"], + capture_stdout=True, + capture_stderr=True, + bare=True, + add_event_log=False, + ) + result = p.Wait() == 0 and bool(p.stdout.strip()) - p = git_command.GitCommand( - None, - ["config", "--get", "init.templatedir"], - capture_stdout=True, - capture_stderr=True, - bare=True, - add_event_log=False, - ) - return p.Wait() == 0 and bool(p.stdout.strip()) + _INIT_TEMPLATE_CACHE[env_key] = result + return result def _FirstLines(lines: List[str], limit: int = 10) -> str: @@ -4368,6 +4377,20 @@ class Project: project=self.name, ) + @staticmethod + def WarmInitTemplateCache() -> Tuple[Tuple[Any, ...], bool]: + """Resolve and cache the init.templateDir lookup for sync workers.""" + env_key = _GitTemplateEnvKey() + return env_key, _HasCustomGitTemplateDir(env_key) + + @staticmethod + def SeedInitTemplateCache( + cache_entry: Tuple[Tuple[Any, ...], bool], + ) -> None: + """Seed the init.templateDir lookup cache in a worker process.""" + env_key, has_custom = cache_entry + _INIT_TEMPLATE_CACHE[env_key] = has_custom + def _InitGitDir(self, mirror_git=None, force_sync=False, quiet=False): # Prefix for temporary directories created during gitdir initialization. TMP_GITDIR_PREFIX = ".tmp-project-initgitdir-" diff --git a/subcmds/sync.py b/subcmds/sync.py index bc14d3cdd..f536c775a 100644 --- a/subcmds/sync.py +++ b/subcmds/sync.py @@ -28,7 +28,7 @@ import subprocess import sys import tempfile import time -from typing import Dict, List, NamedTuple, Optional, Set, Tuple, Union +from typing import Dict, Iterable, List, NamedTuple, Optional, Set, Tuple, Union import urllib.error import urllib.parse import urllib.request @@ -1189,7 +1189,21 @@ later is required to fix a server side protocol bug. # job at the same time. Then the connection may fail if too many # connections are pending and execeeded the socket listening backlog, # especially on MacOS. - _ = len(cls.get_parallel_context()["sync_dict"]) + context = cls.get_parallel_context() + _ = len(context["sync_dict"]) + init_template_cache = context.get("init_template_cache") + if init_template_cache is not None: + Project.SeedInitTemplateCache(init_template_cache) + + def _WarmInitTemplateCache( + self, objdirs: Iterable[str], force_sync: Optional[bool] + ) -> None: + """Populate parallel_context with init.templateDir state if needed.""" + context = self.get_parallel_context() + if "init_template_cache" in context: + return + if force_sync is True or any(not os.path.exists(d) for d in objdirs): + context["init_template_cache"] = Project.WarmInitTemplateCache() def _Fetch(self, projects, opt, err_event, ssh_proxy, errors): ret = True @@ -1260,6 +1274,8 @@ later is required to fix a server side protocol bug. # pass it as an argument to _FetchProjectList below as # multiprocessing is unable to pickle those. self.get_parallel_context()["ssh_proxy"] = ssh_proxy + if jobs > 1: + self._WarmInitTemplateCache(objdir_project_map, opt.force_sync) sync_progress_thread.start() if not opt.quiet: @@ -3581,6 +3597,10 @@ later is required to fix a server side protocol bug. continue jobs = max(1, min(opt.jobs, len(work_items))) + if jobs > 1: + self._WarmInitTemplateCache( + objdir_project_map, opt.force_sync + ) callback = functools.partial( self._ProcessSyncInterleavedResults, finished_relpaths, diff --git a/tests/test_project.py b/tests/test_project.py index 53442d0d9..82cef09d9 100644 --- a/tests/test_project.py +++ b/tests/test_project.py @@ -5258,6 +5258,70 @@ class InitGitDirConfigTests(unittest.TestCase): os.path.exists(os.path.join(hooks_dir, "post-checkout")) ) + def test_warm_and_seed_init_template_cache(self) -> None: + """Warming in parent and seeding in worker avoids re-querying git.""" + with tempfile.TemporaryDirectory() as topdir, mock.patch.dict( + project._INIT_TEMPLATE_CACHE, {}, clear=True + ), mock.patch.dict( + os.environ, + { + "GIT_CONFIG_NOSYSTEM": "1", + "GIT_CONFIG_GLOBAL": os.path.join(topdir, "empty_gitconfig"), + }, + clear=False, + ): + os.environ.pop("GIT_TEMPLATE_DIR", None) + os.environ.pop("GIT_CONFIG_PARAMETERS", None) + os.environ.pop("GIT_CONFIG_COUNT", None) + seen_cmds: List[List[str]] = [] + + class SpyGitCommand(git_command.GitCommand): + def __init__( + self, + proj_arg: Any, + cmdv: List[str], + *args: Any, + **kwargs: Any, + ) -> None: + seen_cmds.append(list(cmdv)) + super().__init__(proj_arg, cmdv, *args, **kwargs) + + with mock.patch.object(git_command, "GitCommand", SpyGitCommand): + entry = project.Project.WarmInitTemplateCache() + + self.assertFalse(entry[1]) + self.assertEqual( + [["config", "--get", "init.templatedir"]], seen_cmds + ) + + # Simulate a fresh spawn/forkserver worker seeded via InitWorker. + seen_cmds.clear() + project._INIT_TEMPLATE_CACHE.clear() + project.Project.SeedInitTemplateCache(entry) + proj = self._make_uninitialized_project(topdir, clone_depth=1) + with mock.patch.object(git_command, "GitCommand", SpyGitCommand): + proj._InitGitDir(quiet=True) + + tpl_cmds = [cmd for cmd in seen_cmds if "init.templatedir" in cmd] + init_cmds = [cmd for cmd in seen_cmds if cmd and cmd[0] == "init"] + self.assertEqual([], tpl_cmds) + self.assertEqual([["init", "-q", "--template="]], init_cmds) + + # Seeding True omits --template= without querying git config. + seen_cmds.clear() + project._INIT_TEMPLATE_CACHE.clear() + project.Project.SeedInitTemplateCache((entry[0], True)) + proj2 = self._make_uninitialized_project( + os.path.join(topdir, "sub"), clone_depth=1 + ) + with mock.patch.object(git_command, "GitCommand", SpyGitCommand): + proj2._InitGitDir(quiet=True) + + tpl_cmds2 = [cmd for cmd in seen_cmds if "init.templatedir" in cmd] + init_cmds2 = [cmd for cmd in seen_cmds if cmd and cmd[0] == "init"] + self.assertEqual([], tpl_cmds2) + self.assertEqual([["init", "-q"]], init_cmds2) + class SparseCheckoutTests(unittest.TestCase): """Check sparse-checkout handling.""" diff --git a/tests/test_subcmds_sync.py b/tests/test_subcmds_sync.py index 67a8ac610..cbfde7d67 100644 --- a/tests/test_subcmds_sync.py +++ b/tests/test_subcmds_sync.py @@ -24,7 +24,17 @@ import subprocess import tempfile import threading import time -from typing import Any, Dict, Iterator, List, Optional, Set, Tuple, Type +from typing import ( + Any, + Callable, + Dict, + Iterator, + List, + Optional, + Set, + Tuple, + Type, +) import unittest from unittest import mock import xmlrpc.client @@ -2308,6 +2318,11 @@ class InterleavedSyncTest(unittest.TestCase): mock.patch.object(self.cmd, "_ValidateOptionsWithManifest").start() mock.patch.object(sync, "_PostRepoUpgrade").start() mock.patch.object(sync, "_PostRepoFetch").start() + self.mock_warm_init_template = mock.patch.object( + sync.Project, + "WarmInitTemplateCache", + return_value=(("env",), False), + ).start() # Mock parallel context for worker tests. self.parallel_context_patcher = mock.patch.object( @@ -2721,6 +2736,159 @@ class InterleavedSyncTest(unittest.TestCase): expected_sets = {frozenset([0, 2]), frozenset([1])} self.assertEqual(work_items_sets, expected_sets) + def test_interleaved_init_template_cache_warmup(self) -> None: + """Test _SyncInterleaved warms init.templateDir cache when needed.""" + projB_sub = FakeProject( + "projB/sub", name="projB_sub", objdir="objB_sub" + ) + all_projects = [self.projA, self.projB, self.projA_sub, projB_sub] + mock.patch.object( + self.cmd, "GetProjects", return_value=all_projects + ).start() + + def execute_side_effect( + jobs: int, + target: object, + work_items: List[List[int]], + **kwargs: Any, + ) -> bool: + synced_relpaths_set = kwargs["callback"].args[0] + projects_in_pass = self.cmd.get_parallel_context()["projects"] + for item in work_items: + for project_idx in item: + synced_relpaths_set.add( + projects_in_pass[project_idx].relpath + ) + return True + + mock.patch.object( + self.cmd, "ExecuteInParallel", side_effect=execute_side_effect + ).start() + + # Multi-level cold sync with -j2 warms once across both levels. + opt, args = self.cmd.OptionParser.parse_args(["--interleaved", "-j2"]) + opt.quiet = True + self.cmd._SyncInterleaved( + opt, + args, + [], + self.manifest, + self.manifest.manifestProject, + all_projects, + {}, + ) + self.mock_warm_init_template.assert_called_once() + self.assertEqual( + self.mock_context["init_template_cache"], (("env",), False) + ) + + # Single-job sync skips warming. + self.mock_warm_init_template.reset_mock() + self.mock_context.pop("init_template_cache", None) + opt_j1, args_j1 = self.cmd.OptionParser.parse_args( + ["--interleaved", "-j1"] + ) + opt_j1.quiet = True + self.cmd._SyncInterleaved( + opt_j1, + args_j1, + [], + self.manifest, + self.manifest.manifestProject, + all_projects, + {}, + ) + self.mock_warm_init_template.assert_not_called() + + # Existing objdirs skip warming unless --force-sync is passed. + for p in all_projects: + p.objdir = os.path.join(self.repodir, p.name) + os.makedirs(p.objdir, exist_ok=True) + + self.cmd._SyncInterleaved( + opt, + args, + [], + self.manifest, + self.manifest.manifestProject, + all_projects, + {}, + ) + self.mock_warm_init_template.assert_not_called() + + opt_force, args_force = self.cmd.OptionParser.parse_args( + ["--interleaved", "-j2", "--force-sync"] + ) + opt_force.quiet = True + self.cmd._SyncInterleaved( + opt_force, + args_force, + [], + self.manifest, + self.manifest.manifestProject, + all_projects, + {}, + ) + self.mock_warm_init_template.assert_called_once() + + def test_fetch_and_init_worker_seed_template_cache(self) -> None: + """Test _Fetch populates parallel_context and InitWorker seeds it.""" + self.parallel_context_patcher.stop() + self.outer_client.manifest.IsArchive = True + p1 = FakeProject("p1", objdir=os.path.join(self.repodir, "missing1")) + p2 = FakeProject("p2", objdir=os.path.join(self.repodir, "missing2")) + + def execute_side_effect( + jobs: int, + func: object, + inputs: List[List[int]], + callback: Callable[..., bool], + **kwargs: Any, + ) -> bool: + if jobs > 1 and kwargs.get("initializer"): + kwargs["initializer"]() + return callback(None, kwargs["output"], []) + + with mock.patch.object( + sync.multiprocessing, "Manager" + ) as mock_mgr, mock.patch.object( + self.cmd, "ExecuteInParallel", side_effect=execute_side_effect + ), mock.patch.object( + sync.Project, "SeedInitTemplateCache" + ) as mock_seed: + mock_mgr.return_value.dict.return_value = {} + + # Cold parallel fetch warms once and seeds worker via InitWorker. + opt, _ = self.cmd.OptionParser.parse_args(["-j2"]) + opt.jobs_network = 2 + opt.quiet = True + self.cmd._Fetch([p1, p2], opt, mock.Mock(), None, []) + self.mock_warm_init_template.assert_called_once() + mock_seed.assert_called_once_with((("env",), False)) + + # Single-worker fetch neither warms nor seeds. + self.mock_warm_init_template.reset_mock() + mock_seed.reset_mock() + opt.jobs_network = 1 + self.cmd._Fetch([p1, p2], opt, mock.Mock(), None, []) + self.mock_warm_init_template.assert_not_called() + mock_seed.assert_not_called() + + # Warm parallel fetch (existing objdirs, no --force-sync) skips + # warming, and InitWorker does not seed when unset. + os.makedirs(p1.objdir, exist_ok=True) + os.makedirs(p2.objdir, exist_ok=True) + opt.jobs_network = 2 + self.cmd._Fetch([p1, p2], opt, mock.Mock(), None, []) + self.mock_warm_init_template.assert_not_called() + mock_seed.assert_not_called() + + # --force-sync warms and seeds even when objdirs exist. + opt.force_sync = True + self.cmd._Fetch([p1, p2], opt, mock.Mock(), None, []) + self.mock_warm_init_template.assert_called_once() + mock_seed.assert_called_once_with((("env",), False)) + def _get_opts(self, args=None): """Helper to get default options for worker tests.""" if args is None: