git_config, project: coalesce SetString writes in BatchWrites

This change attempts to skip sequential git config calls in order to
further speed up git-repo's performance for VFS integration. On
git-main we currently call git config 26K+ times, causing roughly 5~6
seconds of latency (see b/543101585#comment23's TRACE_FILE for data).
Given the expected time of VFS integration is targeting 30s, 6 seconds
makes a huge difference.

During an initial `repo sync`, initializing each project (`_InitGitDir`
and `_InitRemote`) invokes `SetString` multiple times per project, and
`SyncAnalysisState` writes `repo.syncstate.*` keys once per sync.
Because the `git config` CLI has no batch-write mode to set multiple
keys in a single invocation, each `SetString` call previously spawned a
separate `git config` subprocess and rewrote `.repo_config.json` per key
(see b/543101585#comment23 and b/543101585#comment25 for trace profiles
and benchmark data).

Bug: 543101585
Bug: 564817989
Change-Id: Ia1d4613266fb6b7742e8db637039e72ed26a13a2
Reviewed-on: https://gerrit-review.googlesource.com/c/git-repo/+/657083
Tested-by: Gavin Mak <gavinmak@google.com>
Commit-Queue: Gavin Mak <gavinmak@google.com>
Reviewed-by: Brian Gan <brgan@google.com>

git_config.py

@@ -24,7 +24,7 @@ import ssl
 import subprocess
 import sys
 import tempfile
-from typing import Optional, Tuple, Union
+from typing import Any, Dict, Iterator, List, Optional, Tuple, Union
 import urllib.error
 import urllib.request
 
@@ -40,6 +40,8 @@ import repo_trace
 SYNC_STATE_PREFIX = "repo.syncstate."
 
 ID_RE = re.compile(r"^(?:[0-9a-f]{40}|[0-9a-f]{64})$")
+_INI_SECTION_RE = re.compile(r"^[A-Za-z0-9-]+$")
+_INI_KEY_RE = re.compile(r"^[A-Za-z][A-Za-z0-9-]*$")
 
 REVIEW_CACHE = {}
 
@@ -47,6 +49,85 @@ REVIEW_CACHE = {}
 _JSON_META_KEY = "__repo_meta__"
 
 
+def _SplitIniName(name: str) -> Optional[Tuple[str, str, str]]:
+    """Split a config key into INI components when plain-append safe.
+
+    Args:
+        name: Dotted git config key name (e.g. "remote.origin.url").
+
+    Returns:
+        A (section, subsection, var) tuple if the key can be written as a
+        plain INI entry, or None if it requires git config CLI handling.
+    """
+    parts = name.split(".")
+    if len(parts) < 2:
+        return None
+    section = parts[0]
+    var = parts[-1]
+    subsection = ".".join(parts[1:-1]) if len(parts) > 2 else ""
+    if not _INI_SECTION_RE.match(section) or not _INI_KEY_RE.match(var):
+        return None
+    if any(
+        ord(ch) < 0x20 or ord(ch) > 0x7E or ch in ('"', "\\")
+        for ch in subsection
+    ):
+        return None
+    return (section, subsection, var)
+
+
+def _IsPlainIniValue(val: Any) -> bool:
+    """Check whether a config value needs no INI quoting or escaping.
+
+    Args:
+        val: Candidate configuration value passed to SetString.
+
+    Returns:
+        True if val is a string that can be written verbatim without quotes
+        or backslash escapes; False otherwise.
+    """
+    if not isinstance(val, str):
+        return False
+    if val and (val[0] in (" ", "\t") or val[-1] in (" ", "\t")):
+        return False
+    return not any(
+        ord(ch) < 0x20 or ord(ch) > 0x7E or ch in ('"', "\\", ";", "#")
+        for ch in val
+    )
+
+
+def _FormatIniBatch(ops: List[Tuple[str, List[str]]], existing: bytes) -> bytes:
+    """Format queued key/value pairs as UTF-8 INI bytes to append.
+
+    Args:
+        ops: Ordered list of (name, values) pairs to append.
+        existing: Existing raw bytes of the target git config file.
+
+    Returns:
+        UTF-8 encoded INI stanza bytes ready to append after existing.
+    """
+    lines: List[str] = []
+    if existing and not existing.endswith(b"\n"):
+        lines.append("\n")
+    last_header = None
+    for name, vals in ops:
+        parsed = _SplitIniName(name)
+        assert parsed is not None
+        section, subsection, var = parsed
+        header_key = (section.lower(), subsection)
+        if header_key != last_header:
+            if subsection:
+                lines.append(f'[{section} "{subsection}"]\n')
+            else:
+                lines.append(f"[{section}]\n")
+            last_header = header_key
+        for val in vals:
+            if val == "":
+                lines.append(f"\t{var} =\n")
+            else:
+                lines.append(f"\t{var} = {val}\n")
+    return "".join(lines).encode("utf-8")
+
+
 def IsChange(rev):
     return rev.startswith(git_refs.R_CHANGES)
 
@@ -168,6 +249,9 @@ class GitConfig:
         self._cache_dict = None
         self._cache_fingerprint = None
         self._section_dict = None
+        self._batch_depth = 0
+        self._batch_ops: Dict[str, Tuple[str, List[str]]] = {}
+        self._batch_json_dirty = False
         self._remotes = {}
         self._branches = {}
 
@@ -190,9 +274,36 @@ class GitConfig:
         with contextlib.suppress(OSError):
             platform_utils.remove(self._json, missing_ok=True)
 
+    @contextlib.contextmanager
+    def BatchWrites(self) -> Iterator[None]:
+        """Coalesce compatible SetString writes under Git's config.lock.
+
+        Yields:
+            None while SetString writes are queued for batch flushing on exit.
+        """
+        self._batch_depth += 1
+        try:
+            yield
+        except BaseException:
+            self._batch_depth -= 1
+            if self._batch_depth == 0:
+                try:
+                    self._FlushBatch(save_json=True)
+                except Exception:
+                    self._batch_ops.clear()
+                    self._batch_json_dirty = False
+                    self.ClearCache()
+                    self._RemoveJson()
+            raise
+        else:
+            self._batch_depth -= 1
+            if self._batch_depth == 0:
+                self._FlushBatch(save_json=True)
+
     def Reload(self) -> None:
         """Clear cached state and reload configuration from disk."""
         self.ClearCache()
+        self._batch_json_dirty = False
         self._RemoveJson()
         _ = self._cache
 
@@ -314,6 +425,7 @@ class GitConfig:
 
         if (
             self._cache_dict is not None
+            and not self._batch_ops
             and self._ReadFileFingerprint() != self._cache_fingerprint
         ):
             self.ClearCache()
@@ -333,7 +445,15 @@ class GitConfig:
 
         if value is None:
             if old:
-                self._do("--unset-all", name, allow_missing=True)
+                if key in self._batch_ops:
+                    del self._batch_ops[key]
+                else:
+                    if self._batch_ops and (
+                        key == "include.path" or key.startswith("includeif.")
+                    ):
+                        self._FlushBatch(save_json=False)
+                    self._do("--unset-all", name, allow_missing=True)
+                    self._cache_fingerprint = self._ReadFileFingerprint()
                 del self._cache[key]
                 self._SyncCacheAfterSet(key)
 
@@ -345,22 +465,171 @@ class GitConfig:
                 self.SetString(name, value[0])
 
             elif old != value:
-                self._do("--replace-all", name, value[0])
-                try:
-                    for i in range(1, len(value)):
-                        self._do("--add", name, value[i])
-                except BaseException:
-                    self.ClearCache()
-                    self._RemoveJson()
-                    raise
-                self._cache[key] = list(value)
+                vals = list(value)
+                if self._CanQueueInBatch(key, name, vals, old):
+                    self._cache[key] = vals
+                    self._batch_ops[key] = (name, vals)
+                else:
+                    if self._batch_ops:
+                        self._batch_ops.pop(key, None)
+                        self._FlushBatch(save_json=False)
+                    self._do("--replace-all", name, vals[0])
+                    try:
+                        for i in range(1, len(vals)):
+                            self._do("--add", name, vals[i])
+                    except BaseException:
+                        self.ClearCache()
+                        self._RemoveJson()
+                        raise
+                    self._cache[key] = vals
+                    self._cache_fingerprint = self._ReadFileFingerprint()
                 self._SyncCacheAfterSet(key)
 
         elif len(old) != 1 or old[0] != value:
-            self._do("--replace-all", name, value)
-            self._cache[key] = [value]
+            vals = [value]
+            if self._CanQueueInBatch(key, name, vals, old):
+                self._cache[key] = vals
+                self._batch_ops[key] = (name, vals)
+            else:
+                if self._batch_ops:
+                    self._batch_ops.pop(key, None)
+                    self._FlushBatch(save_json=False)
+                self._do("--replace-all", name, value)
+                self._cache[key] = vals
+                self._cache_fingerprint = self._ReadFileFingerprint()
             self._SyncCacheAfterSet(key)
 
+    def _HasIncludes(self) -> bool:
+        """Return True if cached config contains include or includeIf keys."""
+        return self._cache_dict is not None and (
+            "include.path" in self._cache_dict
+            or any(k.startswith("includeif.") for k in self._cache_dict)
+        )
+
+    def _CanQueueInBatch(
+        self,
+        key: str,
+        name: str,
+        vals: List[Any],
+        old: List[Optional[str]],
+    ) -> bool:
+        """Check whether a SetString write can be queued for lockfile append.
+
+        Args:
+            key: Lowercased canonical key used in self._cache.
+            name: Original dotted key name preserving subsection case.
+            vals: New value list requested by SetString.
+            old: Existing cached values for key before this write.
+
+        Returns:
+            True if the write can be appended at batch flush time; False if it
+            must be flushed immediately through git config.
+        """
+        if self._batch_depth <= 0 or self.file == self._SYSTEM_CONFIG:
+            return False
+        if (
+            key == "include.path"
+            or key.startswith("includeif.")
+            or self._HasIncludes()
+        ):
+            return False
+        if old and key not in self._batch_ops:
+            return False
+        return _SplitIniName(name) is not None and all(
+            _IsPlainIniValue(v) for v in vals
+        )
+
+    def _FlushBatch(self, save_json: bool = True) -> None:
+        """Flush queued SetString writes via config.lock or git config CLI.
+
+        Args:
+            save_json: Whether to persist a dirty .repo_config.json cache
+                after flushing queued config updates.
+        """
+        ops = list(self._batch_ops.values())
+        self._batch_ops.clear()
+        if ops and not self._TryWriteBatchLocked(ops):
+            self._ApplyBatchViaGit(ops)
+        if save_json and self._batch_json_dirty:
+            self._batch_json_dirty = False
+            if self._cache_dict is not None:
+                self._SaveJson(self._cache_dict)
+
+    def _TryWriteBatchLocked(self, ops: List[Tuple[str, List[str]]]) -> bool:
+        """Atomically append queued entries under <file>.lock if unmodified.
+
+        Args:
+            ops: Ordered list of (name, values) pairs to append.
+
+        Returns:
+            True if the lockfile was written and renamed into place; False if
+            the lock was busy, the file changed externally, or an I/O error
+            occurred.
+        """
+        if self.file == self._SYSTEM_CONFIG or os.path.islink(self.file):
+            return False
+        lock_path = f"{self.file}.lock"
+        try:
+            fd = os.open(lock_path, os.O_WRONLY | os.O_CREAT | os.O_EXCL, 0o666)
+        except OSError:
+            return False
+
+        replaced = False
+        try:
+            existing = b""
+            curr_fp = None
+            if os.path.exists(self.file):
+                with open(self.file, "rb") as src_fp:
+                    existing = src_fp.read()
+                    st = os.fstat(src_fp.fileno())
+                curr_fp = (
+                    st.st_mtime_ns,
+                    st.st_ctime_ns,
+                    st.st_size,
+                    st.st_ino,
+                    hashlib.sha1(existing).hexdigest(),
+                )
+                if hasattr(os, "fchmod") and not platform_utils.isWindows():
+                    os.fchmod(fd, st.st_mode & 0o777)
+            if curr_fp != self._cache_fingerprint:
+                return False
+
+            payload = _FormatIniBatch(ops, existing)
+            with os.fdopen(fd, "wb") as lock_fp:
+                fd = -1
+                if existing:
+                    lock_fp.write(existing)
+                lock_fp.write(payload)
+            os.replace(lock_path, self.file)
+            replaced = True
+            self._cache_fingerprint = self._ReadFileFingerprint()
+            return True
+        except OSError:
+            return False
+        finally:
+            if fd >= 0:
+                os.close(fd)
+            if not replaced:
+                platform_utils.remove(lock_path, missing_ok=True)
+
+    def _ApplyBatchViaGit(self, ops: List[Tuple[str, List[str]]]) -> None:
+        """Apply queued batch operations via the git config CLI fallback.
+
+        Args:
+            ops: Ordered list of (name, values) pairs to write via git config.
+        """
+        try:
+            for name, vals in ops:
+                self._do("--replace-all", name, vals[0])
+                for extra in vals[1:]:
+                    self._do("--add", name, extra)
+        except BaseException:
+            self._batch_json_dirty = False
+            self.ClearCache()
+            self._RemoveJson()
+            raise
+        self.Reload()
+
     def _SyncCacheAfterSet(self, key: str) -> None:
         """Keep the in-memory section index and JSON cache coherent.
 
@@ -374,17 +643,16 @@ class GitConfig:
         elif key.startswith("branch."):
             self._branches = {}
 
-        has_includes = self._cache_dict is not None and (
-            "include.path" in self._cache_dict
-            or any(k.startswith("includeif.") for k in self._cache_dict)
-        )
         if (
             key == "include.path"
             or key.startswith("includeif.")
-            or has_includes
+            or self._HasIncludes()
         ):
             self.ClearCache()
             self._RemoveJson()
+            self._batch_json_dirty = False
+        elif self._batch_depth > 0:
+            self._batch_json_dirty = True
         else:
             self._SaveJson(self._cache)
 
@@ -476,7 +744,10 @@ class GitConfig:
         d = self._ReadJson()
         if d is None:
             d = self._ReadGit()
-            self._SaveJson(d)
+            if self._batch_depth > 0:
+                self._batch_json_dirty = True
+            else:
+                self._SaveJson(d)
         return d
 
     def _ReadFileFingerprint(self) -> Optional[Tuple[int, int, int, int, str]]:
@@ -916,17 +1187,18 @@ class Remote:
 
     def Save(self):
         """Save this remote to the configuration."""
-        self._Set("url", self.url)
-        # projectname is initialized for projects listed in the manifest, but
-        # not for others (e.g. the manifest project). This class is used for
-        # all of them.
-        if self.pushUrl is not None and self.projectname is not None:
-            self._Set("pushurl", self.pushUrl + "/" + self.projectname)
-        else:
-            self._Set("pushurl", self.pushUrl)
-        self._Set("review", self.review)
-        self._Set("projectname", self.projectname)
-        self._Set("fetch", list(map(str, self.fetch)))
+        with self._config.BatchWrites():
+            self._Set("url", self.url)
+            # projectname is initialized for projects listed in the manifest,
+            # but not for others (e.g. the manifest project). This class is
+            # used for all of them.
+            if self.pushUrl is not None and self.projectname is not None:
+                self._Set("pushurl", self.pushUrl + "/" + self.projectname)
+            else:
+                self._Set("pushurl", self.pushUrl)
+            self._Set("review", self.review)
+            self._Set("projectname", self.projectname)
+            self._Set("fetch", list(map(str, self.fetch)))
 
     def _Set(self, key, value):
         key = f"remote.{self.name}.{key}"
@@ -960,11 +1232,12 @@ class Branch:
 
     def Save(self):
         """Save this branch back into the configuration."""
-        if self.remote:
-            self._Set("remote", self.remote.name)
-        else:
-            self._Set("remote", None)
-        self._Set("merge", self.merge)
+        with self._config.BatchWrites():
+            if self.remote:
+                self._Set("remote", self.remote.name)
+            else:
+                self._Set("remote", None)
+            self._Set("merge", self.merge)
 
     def _Set(self, key, value):
         key = f"branch.{self.name}.{key}"
@@ -1001,23 +1274,24 @@ class SyncAnalysisState:
         """
         self._config = config
         now = datetime.datetime.now(datetime.timezone.utc)
-        self._Set("main.synctime", now.isoformat(timespec="microseconds"))
-        self._Set("main.version", "1")
-        self._Set("sys.argv", sys.argv)
-        for key, value in superproject_logging_data.items():
-            self._Set(f"superproject.{key}", value)
-        for key, value in options.__dict__.items():
-            self._Set(f"options.{key}", value)
-        config_items = config.DumpConfigDict().items()
-        EXTRACT_NAMESPACES = {"repo", "branch", "remote"}
-        self._SetDictionary(
-            {
-                k: v
-                for k, v in config_items
-                if not k.startswith(SYNC_STATE_PREFIX)
-                and k.split(".", 1)[0] in EXTRACT_NAMESPACES
-            }
-        )
+        with self._config.BatchWrites():
+            self._Set("main.synctime", now.isoformat(timespec="microseconds"))
+            self._Set("main.version", "1")
+            self._Set("sys.argv", sys.argv)
+            for key, value in superproject_logging_data.items():
+                self._Set(f"superproject.{key}", value)
+            for key, value in options.__dict__.items():
+                self._Set(f"options.{key}", value)
+            config_items = config.DumpConfigDict().items()
+            EXTRACT_NAMESPACES = {"repo", "branch", "remote"}
+            self._SetDictionary(
+                {
+                    k: v
+                    for k, v in config_items
+                    if not k.startswith(SYNC_STATE_PREFIX)
+                    and k.split(".", 1)[0] in EXTRACT_NAMESPACES
+                }
+            )
 
     def _SetDictionary(self, data):
         """Save all key/value pairs of |data| dictionary.

project.py

@@ -1926,24 +1926,26 @@ class Project:
 
         if is_new is None:
             is_new = not self.Exists
-        if is_new:
-            self._InitGitDir(force_sync=force_sync, quiet=quiet)
-        else:
-            try:
-                # At this point, it's possible that gitdir points to an old
-                # objdir (e.g. name changed, but objdir exists). Check
-                # references to ensure that's not the case. See
-                # https://issues.gerritcodereview.com/40013418 for more
-                # details.
-                self._CheckDirReference(self.objdir, self.gitdir)
+        with self.config.BatchWrites():
+            if is_new:
+                self._InitGitDir(force_sync=force_sync, quiet=quiet)
+            else:
+                try:
+                    # At this point, it's possible that gitdir points to an old
+                    # objdir (e.g. name changed, but objdir exists). Check
+                    # references to ensure that's not the case. See
+                    # https://issues.gerritcodereview.com/40013418 for more
+                    # details.
+                    self._CheckDirReference(self.objdir, self.gitdir)
 
-                self._UpdateHooks(quiet=quiet)
-            except error.GitError as e:
-                if not force_sync:
-                    raise e
-                # Let _InitGitDir fix the issue, force_sync is always True here.
-                self._InitGitDir(force_sync=True, quiet=quiet)
-        self._InitRemote()
+                    self._UpdateHooks(quiet=quiet)
+                except error.GitError as e:
+                    if not force_sync:
+                        raise e
+                    # Let _InitGitDir fix the issue, force_sync is always True
+                    # here.
+                    self._InitGitDir(force_sync=True, quiet=quiet)
+            self._InitRemote()
 
         if self.UseAlternates:
             # If gitdir/objects is a symlink, migrate it from the old layout.
@@ -4496,23 +4498,27 @@ class Project:
                         )
 
                 m = self.manifest.manifestProject.config
-                for key in ["user.name", "user.email"]:
-                    if m.Has(key, include_defaults=False):
-                        curr_config.SetString(key, m.GetString(key))
-                if not self.manifest.EnableGitLfs:
-                    curr_config.SetString(
-                        "filter.lfs.smudge", "git-lfs smudge --skip -- %f"
+                with curr_config.BatchWrites():
+                    for key in ["user.name", "user.email"]:
+                        if m.Has(key, include_defaults=False):
+                            curr_config.SetString(key, m.GetString(key))
+                    if not self.manifest.EnableGitLfs:
+                        curr_config.SetString(
+                            "filter.lfs.smudge",
+                            "git-lfs smudge --skip -- %f",
+                        )
+                        curr_config.SetString(
+                            "filter.lfs.process",
+                            "git-lfs filter-process --skip",
+                        )
+                    curr_config.SetBoolean(
+                        "core.bare", True if self.manifest.IsMirror else None
                     )
-                    curr_config.SetString(
-                        "filter.lfs.process", "git-lfs filter-process --skip"
-                    )
-                curr_config.SetBoolean(
-                    "core.bare", True if self.manifest.IsMirror else None
-                )
 
                 if tmp_gitdir:
                     platform_utils.rename(tmp_gitdir, self.gitdir)
                     tmp_gitdir = None
+                    self.config.ClearCache()
             if not init_obj_dir and self.objdir != self.gitdir:
                 # The project might be shared (obj_dir already initialized), but
                 # such information is not available here. Instead of passing it,
@@ -4614,27 +4620,28 @@ class Project:
                     raise
 
     def _InitRemote(self):
-        if self.remote.url:
-            remote = self.GetRemote()
-            remote.url = self.remote.url
-            remote.pushUrl = self.remote.pushUrl
-            remote.review = self.remote.review
-            remote.projectname = self.name
+        with self.config.BatchWrites():
+            if self.remote.url:
+                remote = self.GetRemote()
+                remote.url = self.remote.url
+                remote.pushUrl = self.remote.pushUrl
+                remote.review = self.remote.review
+                remote.projectname = self.name
 
-            if self.worktree:
-                remote.ResetFetch(mirror=False)
-            else:
-                remote.ResetFetch(mirror=True)
-            remote.Save()
+                if self.worktree:
+                    remote.ResetFetch(mirror=False)
+                else:
+                    remote.ResetFetch(mirror=True)
+                remote.Save()
 
-        # Disable auto-gc for depth=1 to prevent hangs during lazy fetches
-        # inside git checkout for partial clones.
-        effective_depth = (
-            self.clone_depth or self.manifest.manifestProject.depth
-        )
-        if effective_depth == 1:
-            self.config.SetBoolean("maintenance.auto", False)
-            self.config.SetInt("gc.auto", 0)
+            # Disable auto-gc for depth=1 to prevent hangs during lazy fetches
+            # inside git checkout for partial clones.
+            effective_depth = (
+                self.clone_depth or self.manifest.manifestProject.depth
+            )
+            if effective_depth == 1:
+                self.config.SetBoolean("maintenance.auto", False)
+                self.config.SetInt("gc.auto", 0)
 
     def _InitMRef(self):
         """Initialize the pseudo m/<manifest branch> ref."""

tests/test_git_config.py

@@ -17,7 +17,8 @@
 import json
 import os
 from pathlib import Path
-from typing import Any, Optional
+import types
+from typing import Any, List, Optional, Tuple
 
 import pytest
 import utils_for_test
@@ -563,3 +564,230 @@ def test_set_string_after_external_edit_preserves_external_keys(
     assert reader2.GetString("other.key") == "ext2"
     assert reader2.GetString("user.name") == "Alice"
     assert reader2.GetString("user.email") == "alice@example.com"
+
+
+def test_batch_writes_coalesces_new_keys_without_git_replace(
+    tmp_path: Path, monkeypatch: pytest.MonkeyPatch
+) -> None:
+    """Test BatchWrites writes new sections atomically with one JSON save."""
+    cfg_path = tmp_path / "config"
+    cfg_path.write_text("[core]\n\trepositoryformatversion = 0\n")
+    config = git_config.GitConfig(str(cfg_path))
+
+    orig_do = config._do
+    seen_do = []
+    save_count = 0
+    orig_save = config._SaveJson
+
+    def spy_do(*args: str) -> str:
+        seen_do.append(args)
+        return orig_do(*args)
+
+    def spy_save(cache: Any) -> None:
+        nonlocal save_count
+        save_count += 1
+        orig_save(cache)
+
+    monkeypatch.setattr(config, "_do", spy_do)
+    monkeypatch.setattr(config, "_SaveJson", spy_save)
+
+    with config.BatchWrites():
+        config.SetString("filter.lfs.smudge", "git-lfs smudge --skip -- %f")
+        config.SetString("filter.lfs.process", "git-lfs filter-process --skip")
+        config.SetString("remote.goog.url", "https://example.com/platform/art")
+        config.SetString(
+            "remote.goog.fetch",
+            [
+                "+refs/heads/*:refs/remotes/goog/*",
+                "+refs/tags/*:refs/tags/*",
+            ],
+        )
+        config.SetBoolean("maintenance.auto", False)
+
+    assert seen_do == [("--null", "--list")]
+    assert save_count == 1
+    assert not (tmp_path / "config.lock").exists()
+
+    reloaded = git_config.GitConfig(str(cfg_path))
+    fresh = reloaded._ReadGit()
+    assert fresh["filter.lfs.smudge"] == ["git-lfs smudge --skip -- %f"]
+    assert fresh["filter.lfs.process"] == ["git-lfs filter-process --skip"]
+    assert fresh["remote.goog.url"] == ["https://example.com/platform/art"]
+    assert fresh["remote.goog.fetch"] == [
+        "+refs/heads/*:refs/remotes/goog/*",
+        "+refs/tags/*:refs/tags/*",
+    ]
+    assert fresh["maintenance.auto"] == ["false"]
+
+
+def test_batch_writes_falls_back_on_lock_or_external_edit(
+    tmp_path: Path, monkeypatch: pytest.MonkeyPatch
+) -> None:
+    """Test BatchWrites preserves concurrent edits and respects config.lock."""
+    # 1. External edit between cache load and flush falls back to git config CLI
+    # so neither the external key nor the batched key is lost.
+    cfg_path = tmp_path / "config"
+    cfg_path.write_text("[core]\n\tfilemode = true\n")
+    config = git_config.GitConfig(str(cfg_path))
+
+    with config.BatchWrites():
+        config.SetString("remote.origin.url", "https://example.com/a")
+        with open(cfg_path, "a", encoding="utf-8") as fp:
+            fp.write("[external]\n\tadded = yes\n")
+
+    reloaded = git_config.GitConfig(str(cfg_path))
+    fresh = reloaded._ReadGit()
+    assert fresh["external.added"] == ["yes"]
+    assert fresh["remote.origin.url"] == ["https://example.com/a"]
+    assert config.GetString("external.added") == "yes"
+
+    # 2. Pre-existing config.lock is never clobbered; BatchWrites delegates to
+    # _do instead.
+    locked_path = tmp_path / "locked_config"
+    locked_path.write_text("[core]\n\tfilemode = true\n")
+    lock_file = tmp_path / "locked_config.lock"
+    lock_file.write_text("held-by-other-process")
+
+    locked_cfg = git_config.GitConfig(str(locked_path))
+    _ = locked_cfg.GetBoolean("core.filemode")
+    do_calls = []
+    monkeypatch.setattr(
+        locked_cfg,
+        "_do",
+        lambda *args: do_calls.append(args) or "core.filemode\ntrue\0",
+    )
+
+    with locked_cfg.BatchWrites():
+        locked_cfg.SetString("remote.origin.url", "https://example.com/b")
+
+    assert lock_file.read_text() == "held-by-other-process"
+    assert ("--replace-all", "remote.origin.url", "https://example.com/b") in (
+        do_calls
+    )
+
+
+def test_batch_writes_direct_do_refreshes_fingerprint_and_branch_save_batches(
+    tmp_path: Path, monkeypatch: pytest.MonkeyPatch
+) -> None:
+    """Direct _do in BatchWrites updates fingerprint; Save uses BatchWrites."""
+    cfg_path = tmp_path / "config"
+    cfg_path.write_text("[core]\n\tfilemode = true\n")
+    config = git_config.GitConfig(str(cfg_path))
+    _ = config.GetBoolean("core.filemode")
+
+    orig_do = config._do
+    seen_do = []
+
+    def spy_do(*args: str, **kwargs: Any) -> str:
+        seen_do.append(args)
+        return orig_do(*args, **kwargs)
+
+    monkeypatch.setattr(config, "_do", spy_do)
+
+    # Updating an existing key runs _do() directly inside BatchWrites; the next
+    # SetString must not see a stale _cache_fingerprint or re-run --list.
+    with config.BatchWrites():
+        config.SetString("core.filemode", "false")
+        config.SetString("new.section.key", "val")
+
+    assert seen_do == [("--replace-all", "core.filemode", "false")]
+    assert config.GetString("core.filemode") == "false"
+    assert config.GetString("new.section.key") == "val"
+
+    # Remote.Save() and Branch.Save() coalesce new keys via BatchWrites without
+    # spawning git config --replace-all subprocesses.
+    seen_do.clear()
+    remote = config.GetRemote("origin")
+    remote.url = "https://example.com/repo"
+    remote.ResetFetch()
+    remote.Save()
+
+    branch = config.GetBranch("main")
+    branch.remote = remote
+    branch.merge = "refs/heads/main"
+    branch.Save()
+
+    assert seen_do == []
+
+    # When include.path is present, _CanQueueInBatch returns False so writes go
+    # through git config immediately instead of queueing for lockfile append.
+    inc_file = tmp_path / "inc.gitconfig"
+    inc_file.write_text("[extra]\n\tflag = 1\n")
+    config.SetString("include.path", str(inc_file))
+    _ = config.GetString("extra.flag")
+    seen_do.clear()
+    with config.BatchWrites():
+        config.SetString("after.include", "yes")
+    assert ("--replace-all", "after.include", "yes") in seen_do
+
+
+def test_batch_writes_failure_paths(tmp_path: Path) -> None:
+    """BatchWrites clears state on failure and preserves body errors."""
+    # 1. Failed fallback flush clears poisoned in-memory cache and dirty flag.
+    fail_path = tmp_path / "fail_config"
+    fail_path.write_text("[core]\n\tfilemode = true\n")
+    lock_path = tmp_path / "fail_config.lock"
+    lock_path.write_text("locked")
+    fail_cfg = git_config.GitConfig(str(fail_path))
+    assert fail_cfg.GetBoolean("core.filemode") is True
+
+    with pytest.raises(error.GitError):
+        with fail_cfg.BatchWrites():
+            fail_cfg.SetString("remote.origin.url", "https://bad.example.com")
+
+    lock_path.unlink()
+    assert fail_cfg.GetString("remote.origin.url") is None
+    with fail_cfg.BatchWrites():
+        pass
+    assert (
+        git_config.GitConfig(str(fail_path)).GetString("remote.origin.url")
+        is None
+    )
+
+    # 2. If body raises and flush also fails, original body error propagates.
+    body_path = tmp_path / "body_config"
+    body_path.write_text("[core]\n\tfilemode = true\n")
+    body_lock = tmp_path / "body_config.lock"
+    body_lock.write_text("locked")
+    body_cfg = git_config.GitConfig(str(body_path))
+    assert body_cfg.GetBoolean("core.filemode") is True
+
+    with pytest.raises(RuntimeError, match="body boom"):
+        with body_cfg.BatchWrites():
+            body_cfg.SetString("remote.origin.url", "https://bad.example.com")
+            raise RuntimeError("body boom")
+
+    body_lock.unlink()
+    assert body_cfg.GetString("remote.origin.url") is None
+
+
+def test_batch_writes_sync_analysis_state(
+    tmp_path: Path, monkeypatch: pytest.MonkeyPatch
+) -> None:
+    """UpdateSyncAnalysisState coalesces new syncstate keys without CLI."""
+    state_cfg_path = tmp_path / "state_config"
+    state_cfg_path.write_text(
+        '[remote "origin"]\n\turl = https://example.com/repo\n',
+        encoding="utf-8",
+    )
+    state_cfg = git_config.GitConfig(str(state_cfg_path))
+    _ = state_cfg.GetString("remote.origin.url")
+    seen_do: List[Tuple[str, ...]] = []
+    orig_do = state_cfg._do
+    monkeypatch.setattr(
+        state_cfg,
+        "_do",
+        lambda *args, **kw: seen_do.append(args) or orig_do(*args, **kw),
+    )
+
+    opts = types.SimpleNamespace(jobs=8, quiet=True)
+    state_cfg.UpdateSyncAnalysisState(opts, {"superproject": False})
+
+    assert seen_do == []
+    fresh_state = git_config.GitConfig(str(state_cfg_path))._ReadGit()
+    assert fresh_state["repo.syncstate.main.version"] == ["1"]
+    assert fresh_state["repo.syncstate.options.jobs"] == ["8"]
+    assert fresh_state["repo.syncstate.options.quiet"] == ["true"]
+    assert fresh_state["repo.syncstate.remote.origin.url"] == [
+        "https://example.com/repo"
+    ]

tests/test_project.py

@@ -5169,11 +5169,16 @@ class InitGitDirConfigTests(unittest.TestCase):
                     super().__init__(proj_arg, cmdv, *args, **kwargs)
 
             with mock.patch.object(git_command, "GitCommand", SpyGitCommand):
-                proj._InitGitDir(quiet=True)
-                proj._InitRemote()
+                with proj.config.BatchWrites():
+                    proj._InitGitDir(quiet=True)
+                    proj._InitRemote()
 
             init_cmds = [cmd for cmd in seen_cmds if cmd and cmd[0] == "init"]
             self.assertEqual([["init", "-q", "--template="]], init_cmds)
+            self.assertFalse(
+                any("--replace-all" in cmd for cmd in seen_cmds),
+                f"Expected BatchWrites to avoid --replace-all, got {seen_cmds}",
+            )
             reloaded = git_config.GitConfig.ForRepository(gitdir=proj.gitdir)
             fresh = reloaded._ReadGit()
             self.assertEqual(