Skip to content

Commit 3090bb1

Browse files
codexByron
authored andcommitted
perf(cmd): share minimum Git version checks across wrappers
Reuse successful minimum-version checks for equivalent `Git` wrappers, keyed by the resolved executable and its metadata, working directory, effective environment and `Git.refresh()` generation. Keep public `version_info` caching per instance and reject unsupported versions before repository creation. Executable replacement and newly preferred PATH candidates trigger new probes; ambiguous Windows search and global options retain the existing behavior. This is shared CLI housekeeping for both backends: it caches actual Git answers and records no native success. It does not implement a missing Gix capability. The cache is bounded to 128 contexts; external launcher state still requires an explicit `Git.refresh()` when executable/environment metadata is unchanged. The fixed pair of extra warm version checks drops from two CLI launches to zero. With the Gix-only fallbacks restored, warm opening/discovery launch counts fall from 2/4 to 1/3 for Gix and 11/13 to 9/11 for CLI. Update the benchmark ceilings accordingly while retaining real reference-format and negative-path queries. Cold contexts can still add a version probe. Validation: 68 affected version, refresh, guard, counter and ceiling tests passed with Gix first (1.50s), then CLI (1.74s). All 11 benchmark result digests match and the adjusted ceilings pass; `git diff --check` passed.
1 parent 608e773 commit 3090bb1

5 files changed

Lines changed: 130 additions & 12 deletions

File tree

‎doc/gix-backend.md‎

Lines changed: 25 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -105,7 +105,7 @@ the call phase, so that column can still include their fixture work. Counts
105105
cover the current Python process; they are not aggregated across xdist workers.
106106
For a stable CI assertion, the pinned repository benchmark uses per-measurement
107107
ceilings in `test/performance/cli-budget.json`: the warm Gix journey launches
108-
one CLI process, opening launches two, and nested discovery launches four.
108+
one CLI process, opening launches one, and nested discovery launches three.
109109
Reduce these ceilings as conversions land. The other operation rows have zero
110110
ceilings except the one-process patch diff.
111111

@@ -119,8 +119,9 @@ accounting. The same run recorded 21,409 fallback decisions. These totals
119119
include explicit CLI tests and fixture commands, including non-Git commands
120120
passed directly to `Git.execute`; they are a local baseline, not a count of
121121
only fallback calls. The fixed warm benchmark instead isolates a user journey:
122-
23 CLI launches versus one Gix launch, opening 11 versus two, and nested
123-
discovery 13 versus four after restoring the reference-format fallback.
122+
23 CLI launches versus one Gix launch, opening nine versus one, and nested
123+
discovery eleven versus three after restoring the reference-format fallback
124+
and sharing successful version checks.
124125
The historical full-suite counts above predate this correction. A constant
125126
`files` answer was not a Gix query; `GIX-1` records the missing binding.
126127

@@ -169,6 +170,27 @@ an import error; only an absent top-level `gix` selects CLI mode.
169170

170171
## Performance work
171172

173+
### Shared minimum-version checks
174+
175+
Both installations reuse successful minimum-Git-version checks across fresh
176+
`Git` wrappers with the same resolved executable, executable metadata, working
177+
directory, effective environment and `Git.refresh()` generation. Two extra
178+
checks in the fixed warm probe now launch zero processes instead of two;
179+
two equivalent cold wrappers launch one probe. Public `version_info` caching
180+
remains per instance, and old versions still fail before repository creation.
181+
182+
This is shared CLI housekeeping: it caches an actual Git answer and records
183+
no native success. It is not a Gix implementation of a missing capability.
184+
The pinned warm Gix opening/discovery ceilings fall from 2/4 to 1/3 here;
185+
the reference-format query and rejected-candidate diagnostics remain real CLI
186+
calls. Cold contexts can still require a version probe.
187+
188+
Global Git options and ambiguous Windows executable search retain per-instance
189+
probes; explicit absolute executables can share checks on Windows. The shared
190+
cache is bounded to 128 contexts. Call `Git.refresh()` if a launcher's reported
191+
version changes through external state without changing its executable or
192+
environment metadata.
193+
172194
### Existing-repository benchmark
173195

174196
[`test/performance/README.md`](../test/performance/README.md) describes the

‎git/cmd.py‎

Lines changed: 61 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
import logging
1414
import os
1515
import re
16+
import shutil
1617
import signal
1718
import subprocess
1819
import sys
@@ -699,6 +700,9 @@ class Git(metaclass=_GitMeta):
699700
"_repo",
700701
)
701702

703+
_version_info: Optional[Tuple[int, ...]]
704+
_version_info_token: object
705+
702706
# Match Git's leading transport selector, including an empty helper name.
703707
# Git also selects the command-executing ext helper for an ext:// URL.
704708
re_unsafe_protocol = re.compile(r"([A-Za-z0-9][A-Za-z0-9+.-]*|)::|ext://")
@@ -791,6 +795,7 @@ def __setstate__(self, d: Dict[str, Any]) -> None:
791795
"""
792796

793797
_refresh_token = object() # Since None would match an initial _version_info_token.
798+
_version_check_cache: Dict[Tuple[Any, ...], Tuple[int, ...]] = {}
794799

795800
@classmethod
796801
def refresh(cls, path: Union[None, PathLike] = None) -> bool:
@@ -1145,8 +1150,61 @@ def _check_operand(value: Any, label: str = "operand") -> str:
11451150
return value
11461151

11471152
def _require_version(self) -> None:
1148-
if self.version_info < (2, 52):
1153+
key = None
1154+
if self._version_info_token is not self._refresh_token:
1155+
key = self._version_check_key()
1156+
if key is not None:
1157+
version = self._version_check_cache.get(key)
1158+
if version is not None:
1159+
self._version_info = version
1160+
self._version_info_token = key[0]
1161+
version = self.version_info
1162+
if version < (2, 52):
11491163
raise UnsupportedOperation("GitPython requires Git 2.52 or newer for repository operations")
1164+
if key is not None and key not in self._version_check_cache:
1165+
# ponytail: clear at 128 contexts; use LRU if diverse environments churn.
1166+
if len(self._version_check_cache) >= 128:
1167+
self._version_check_cache.clear()
1168+
self._version_check_cache[key] = version
1169+
1170+
def _version_check_key(self) -> Optional[Tuple[Any, ...]]:
1171+
"""Identify the executable and context of a minimum-version probe."""
1172+
executable = self.GIT_PYTHON_GIT_EXECUTABLE
1173+
if not executable or self._git_options or self._persistent_git_options:
1174+
return None
1175+
environment = {**os.environ, "LANGUAGE": "C", "LC_ALL": "C", **self._environment}
1176+
try:
1177+
cwd = os.path.realpath(self._working_dir or os.getcwd())
1178+
if not os.access(cwd, os.X_OK):
1179+
return None
1180+
resolved: Optional[str]
1181+
if os.path.isabs(executable):
1182+
resolved = executable
1183+
elif sys.platform == "win32":
1184+
# CreateProcess searches in the parent context, unlike execvpe.
1185+
return None
1186+
elif os.path.dirname(executable):
1187+
resolved = os.path.join(cwd, executable)
1188+
else:
1189+
path = environment.get("PATH")
1190+
path = os.defpath if path is None else path
1191+
resolved = shutil.which(
1192+
executable, path=os.pathsep.join(os.path.join(cwd, p) for p in path.split(os.pathsep))
1193+
)
1194+
if resolved is None:
1195+
return None
1196+
info = os.stat(resolved)
1197+
except OSError:
1198+
return None
1199+
return (
1200+
self._refresh_token,
1201+
type(self),
1202+
executable,
1203+
resolved,
1204+
cwd,
1205+
tuple(sorted(environment.items())),
1206+
(info.st_dev, info.st_ino, info.st_size, info.st_mtime_ns, info.st_ctime_ns),
1207+
)
11501208

11511209
def _call_process_safe(
11521210
self,
@@ -1235,8 +1293,8 @@ def __init__(self, working_dir: Union[None, PathLike] = None) -> None:
12351293
self._repo: Any = None # Weak reference; the Repo owns native resources.
12361294

12371295
# Cached version slots
1238-
self._version_info: Union[Tuple[int, ...], None] = None
1239-
self._version_info_token: object = None
1296+
self._version_info = None
1297+
self._version_info_token = None
12401298

12411299
# Cached command slots
12421300
self.cat_file_header: Union[None, TBD] = None

‎test/performance/README.md‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -88,15 +88,16 @@ Direct subprocesses in the harness and Git's own child processes are excluded.
8888
The checked-in `cli-budget.json` sets maximum Gix launch counts for this pinned
8989
fixture. CI enforces the ceilings: reductions pass, increases fail with the
9090
measurement and counts. Lower ceilings when an optimization lands to retain
91-
the gain. Warm ceilings are journey/patch 1, opening 2, discovery 4,
91+
the gain. Warm ceilings are journey/patch 1, opening 1, discovery 3,
9292
and zero for the other operations. New measurements need an explicit ceiling;
9393
do not automatically raise an existing ceiling to accept a regression.
9494

9595
The opening/discovery ceilings include a real CLI reference-format query
96-
(`GIX-1`) and version validation. Nested candidates rejected by Gix also use
96+
(`GIX-1`). Nested candidates rejected by Gix also use
9797
Git for compatible discovery diagnostics (`GIX-14`). GixPython must expose
9898
the missing capabilities before those calls can disappear; Python inference
9999
does not count as a native implementation.
100+
Warm version checks reuse real CLI answers; cold contexts can add a version probe.
100101

101102
Timing is observational: shared CI runners and local activity introduce noise.
102103
The CI job publishes all means, standard deviations and ratios, retains raw

‎test/performance/cli-budget.json‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@
1010
"diff_and_stats": 0,
1111
"patch_diff": 1,
1212
"worktree_status": 0,
13-
"open_repository": 2,
14-
"discover_repository": 4
13+
"open_repository": 1,
14+
"discover_repository": 3
1515
}
1616
}

‎test/test_git.py‎

Lines changed: 39 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24,8 +24,8 @@
2424

2525
import ddt
2626

27-
from git import Git, GitCommandError, GitCommandNotFound, Repo, cmd, refresh
28-
from git.exc import UnsafeOptionError
27+
from git import Git, GitCommandError, GitCommandNotFound, Repo, _backend, cmd, refresh
28+
from git.exc import UnsafeOptionError, UnsupportedOperation
2929
from git.util import cwd, finalize_process
3030

3131
from test.lib import TestBase, fixture_path, with_rw_directory
@@ -825,6 +825,43 @@ def test_version_info_cache_is_per_instance(self):
825825
git2.version_info
826826
git1.version_info
827827

828+
def test_minimum_version_shares_context_and_detects_replacement(self):
829+
def calls():
830+
return _backend.statistics().get(("Git.execute", "CLI process"), 0)
831+
832+
with _rollback_refresh(), _fake_git(2, 54, 0) as path:
833+
refresh(path)
834+
count = calls()
835+
Git()._require_version()
836+
Git()._require_version()
837+
self.assertEqual(calls() - count, 1)
838+
custom = Git()
839+
custom.update_environment(GITPYTHON_VERSION_CONTEXT="different")
840+
for command in (Git(Path(path).parent), custom):
841+
count = calls()
842+
command._require_version()
843+
self.assertEqual(calls() - count, 1)
844+
with _fake_git(2, 51, 0) as older:
845+
shutil.copy(older, path)
846+
with self.assertRaises(UnsupportedOperation):
847+
Git()._require_version()
848+
849+
def test_minimum_version_detects_new_executable_earlier_in_path(self):
850+
with contextlib.ExitStack() as stack:
851+
stack.enter_context(_rollback_refresh())
852+
older = Path(stack.enter_context(_fake_git(2, 51, 0)))
853+
newer = Path(stack.enter_context(_fake_git(2, 54, 0)))
854+
stack.enter_context(mock.patch.dict(os.environ, {"PATH": f"{older.parent}{os.pathsep}{newer.parent}"}))
855+
stack.enter_context(_patch_out_env("GIT_PYTHON_GIT_EXECUTABLE"))
856+
if sys.platform == "win32":
857+
stack.enter_context(mock.patch.object(Git, "git_exec_name", "git.cmd"))
858+
_rename_with_stem(newer, "git")
859+
refresh()
860+
Git()._require_version()
861+
_rename_with_stem(older, "git")
862+
with self.assertRaises(UnsupportedOperation):
863+
Git()._require_version()
864+
828865
def test_version_info_cache_is_not_pickled(self):
829866
with _rollback_refresh():
830867
with _fake_git(123, 456, 789) as path:

0 commit comments

Comments
 (0)