mirror of
https://gerrit.googlesource.com/git-repo
synced 2026-11-14 16:27:13 +00:00
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 <gavinmak@google.com> Reviewed-by: Brian Gan <brgan@google.com> Commit-Queue: Gavin Mak <gavinmak@google.com>
This commit is contained in:
1 parent
c9cfb541aa
commit
2aca835f1f
4 files changed
+289
-14
No files matched your search
+34
-11
@@ -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-"
|
||||
|
||||
+22
-2
@@ -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,
|
||||
|
||||
@@ -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."""
|
||||
|
||||
+169
-1
@@ -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:
|
||||
|
||||
Reference in new issue
Block a user