Skip to content

Commit 9d283af

Browse files
codexByron
authored andcommitted
perf(gix): retain native repository handles on Repo
The native adapter previously called `gix.open_opts()` for every operation, repeating configuration parsing and losing shared index/object-store state. Each `Repo` now owns a native handle, associated with `Git` through a weak reference. `close()` releases it and pickling excludes native resources. Reuse preserves storage/environment guards. Metadata and environment changes, and successful raw CLI launches, invalidate the configuration view and trigger `reload()` on the retained handle. Config queries keep a separate fresh view without synthetic safety settings. Includes reload conservatively because GixPython does not expose their source paths. A per-repository lock prevents concurrent refresh races; Gix handles read sharing and index/ODB refresh. Validated Gix first: 260 affected tests and 14 subtests passed, with 3 skips; 38 native regressions passed again after the metadata guard was completed. CLI compatibility: 222 affected tests and 14 subtests passed, with 3 skips. Ruff lint/format, mypy and basedpyright passed.
1 parent 2992d9a commit 9d283af

5 files changed

Lines changed: 143 additions & 23 deletions

File tree

‎doc/gix-backend.md‎

Lines changed: 24 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -172,9 +172,8 @@ only an absent top-level `gix` selects CLI mode.
172172
operations and their complete journey on one already-open `git.Repo`, plus
173173
separate direct opening and discovery from `git/objects`. Fresh high-level
174174
wrappers preserve the cost of actual operations; imports, fixture preparation
175-
and parity preflight are outside timing. Native repository reopening remains
176-
inside operation timing, so retaining native state in the future can improve
177-
these measurements without changing the workload.
175+
and parity preflight are outside timing. Native repository refresh remains inside operation timing, so these
176+
measurements include the cost of keeping retained state current.
178177

179178
At `244e418da6cc43de129cbe2908d11be4c5ad457a`, using official GixPython
180179
0.1.0 and the same existing CPython 3.12.14/macOS arm64 interpreter for both
@@ -213,8 +212,8 @@ ratios are observational on shared runners.
213212

214213
### Test-suite setup measurements
215214

216-
Native repository reuse remains future work. The fixture optimizations below
217-
are implemented as separate commits, each validated with GixPython before CLI.
215+
The fixture optimizations below are implemented as separate commits, each
216+
validated with GixPython before CLI.
218217
The original local measurements use official GixPython 0.1.0 and existing
219218
CPython 3.12.14 on macOS arm64 at `89c609cf92335e75652d7725682e215a9ea080e5`.
220219

@@ -224,22 +223,26 @@ expected failure. Operation reporting counted 114,800 native operations and
224223
39,002 CLI fallback decisions. Those decisions are not a complete subprocess
225224
count: raw `repo.git` calls and subprocesses started by Git are not all counted.
226225

227-
### Retain native repository state
228-
229-
`git._backend._repository()` calls `gix.open_opts()` for every operation;
230-
`git.Repo` does not retain a native repository. This deliberately avoids stale
231-
configuration and storage views after CLI mutations, but also loses native
232-
state reuse. In a 1,000-call benchmark without coverage, an object read using
233-
one native handle took 0.046 ms per call, compared with 0.635 ms through the
234-
current stream adapter. Opening and validating a native repository alone took
235-
0.418 ms per call. These are small repeated-read measurements, not an estimate
236-
of whole-suite speedup.
237-
238-
Consider retaining native repository/object-store state per `git.Repo`, with
239-
explicit ownership, `close()` behavior, thread semantics, and invalidation for
240-
CLI/native mutations, configuration changes, and environment overrides.
241-
Preserve the current safety checks and diagnostics. Establish those contracts
242-
before replacing the current fresh-open policy.
226+
### Retain native repository state (implemented)
227+
228+
Each `git.Repo` owns a native `gix.Repository`. Managed operations reuse it;
229+
`close()` releases it, and a later operation can reopen it. Pickling excludes
230+
native resources and restores the command's weak owner reference. Gix provides
231+
thread-safe handle access and automatic index/ODB snapshot refresh. A per-Repo
232+
lock serializes handle refresh, without serializing read operations.
233+
234+
Configuration and storage metadata changes, environment changes and raw CLI
235+
launches cause `reload()` before reuse. Configuration queries use a separate
236+
fresh handle so synthetic safety settings never appear as user settings.
237+
Configurations with includes conservatively reload on each operation because
238+
the binding does not expose all included source paths. Storage overrides,
239+
reftable and compatibility object formats retain their existing CLI guards.
240+
241+
An earlier 1,000-call benchmark without coverage measured an object read using
242+
one native handle at 0.046 ms per call, compared with 0.635 ms through the
243+
fresh-open stream adapter. Opening and validating a native repository alone
244+
took 0.418 ms per call. These small repeated-read measurements do not estimate
245+
whole-suite speedup.
243246

244247
### Reuse prepared test fixtures (implemented)
245248

‎git/_backend.py‎

Lines changed: 49 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,14 @@ def _fallback(method: str, reason: str) -> Any:
5959

6060

6161
def _repository(command: Any, env: Dict[str, Any], *, query_config: bool = False) -> Any:
62+
owner = command._repo() if command._repo is not None else None
63+
if owner is not None:
64+
with owner._gix_lock:
65+
return _open_repository(command, env, query_config=query_config)
66+
return _open_repository(command, env, query_config=query_config)
67+
68+
69+
def _open_repository(command: Any, env: Dict[str, Any], *, query_config: bool = False) -> Any:
6270
if command._git_options or command._persistent_git_options:
6371
raise _Unsupported("global command options")
6472
overrides = {**command.environment(), **env}
@@ -82,17 +90,56 @@ def _repository(command: Any, env: Dict[str, Any], *, query_config: bool = False
8290
options = options.config_overrides(
8391
["core.fsmonitor=false", "gc.auto=0", "maintenance.auto=false", "core.hooksPath=" + os.devnull]
8492
)
85-
repo = gix.open_opts(path, options)
93+
owner = command._repo() if command._repo is not None else None
94+
state = None
95+
repo = None
96+
if owner is not None and not query_config:
97+
# Gix refreshes index/ODB snapshots itself; configuration is loaded at open.
98+
paths = [
99+
path,
100+
os.fspath(owner.common_dir),
101+
os.path.join(owner.common_dir, "config"),
102+
os.path.join(path, "config.worktree"),
103+
]
104+
paths += [
105+
effective.get("GIT_CONFIG_SYSTEM", "/etc/gitconfig"),
106+
effective.get("GIT_CONFIG_GLOBAL", os.path.expanduser("~/.gitconfig")),
107+
os.path.join(effective.get("XDG_CONFIG_HOME", os.path.expanduser("~/.config")), "git", "config"),
108+
os.path.join(owner.common_dir, "objects", "info", "alternates"),
109+
os.path.join(path, "commondir"),
110+
os.path.join(path, "gitdir"),
111+
]
112+
stamps: List[Any] = []
113+
for filename in paths:
114+
try:
115+
info = os.stat(filename)
116+
stamps.append((info.st_dev, info.st_ino, info.st_size, info.st_mtime_ns, info.st_ctime_ns))
117+
except FileNotFoundError:
118+
stamps.append(None)
119+
state = (path, tuple(sorted(effective.items())), tuple(stamps))
120+
repo = owner._gix_repository
121+
if repo is not None and os.path.abspath(repo.git_dir()) != os.path.abspath(path):
122+
repo = None
123+
if repo is not None and state != owner._gix_state:
124+
repo.reload()
125+
if repo is None:
126+
repo = gix.open_opts(path, options)
86127
snapshot = repo.config_snapshot()
87128
if snapshot.string("extensions.refStorage") == b"reftable":
88129
raise _Unsupported("reftable (GIX-1)")
89130
if snapshot.string("extensions.compatObjectFormat") is not None:
90131
raise _Unsupported("compatibility object format (GIX-19)")
91-
if effective.get("GIT_WORK_TREE"):
132+
if effective.get("GIT_WORK_TREE") and (owner is None or query_config or state != owner._gix_state):
92133
workdir = effective["GIT_WORK_TREE"]
93134
if not os.path.isabs(workdir):
94135
workdir = os.path.join(command.working_dir or os.getcwd(), workdir)
95136
repo.set_workdir(workdir)
137+
if owner is not None and not query_config:
138+
owner._gix_repository = repo
139+
# ponytail: includes can load arbitrary files; reopen until Gix exposes their source paths.
140+
if state != owner._gix_state:
141+
included = re.search(rb"\[include(?:if)?[\s\]]", repo.config_snapshot().plumbing().to_bstring(), re.I)
142+
owner._gix_state = None if included else state
96143
return repo
97144

98145

‎git/cmd.py‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -688,13 +688,15 @@ class Git(metaclass=_GitMeta):
688688
"_git_options",
689689
"_persistent_git_options",
690690
"_environment",
691+
"_repo",
691692
)
692693

693694
_excluded_ = (
694695
"cat_file_all",
695696
"cat_file_header",
696697
"_version_info",
697698
"_version_info_token",
699+
"_repo",
698700
)
699701

700702
# Match Git's leading transport selector, including an empty helper name.
@@ -1230,6 +1232,7 @@ def __init__(self, working_dir: Union[None, PathLike] = None) -> None:
12301232

12311233
# Extra environment variables to pass to git commands
12321234
self._environment: Dict[str, Optional[str]] = {}
1235+
self._repo: Any = None # Weak reference; the Repo owns native resources.
12331236

12341237
# Cached version slots
12351238
self._version_info: Union[Tuple[int, ...], None] = None
@@ -1656,6 +1659,9 @@ def execute(
16561659
raise GitCommandNotFound(redacted_command, err) from err
16571660
else:
16581661
_backend.record("Git.execute", "CLI process")
1662+
owner = self._repo() if self._repo is not None else None
1663+
if owner is not None:
1664+
owner._gix_state = None
16591665
# Replace with a typeguard for Popen[bytes]?
16601666
proc.stdout = cast(BinaryIO, proc.stdout)
16611667
proc.stderr = cast(BinaryIO, proc.stderr)

‎git/repo/base.py‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,9 @@
1515
import shlex
1616
import sys
1717
import tempfile
18+
from threading import RLock
1819
import warnings
20+
import weakref
1921

2022
import gitdb
2123
import gitdb.util
@@ -124,6 +126,8 @@ class Repo:
124126

125127
# Must exist, or __del__ will fail in case we raise on `__init__()`.
126128
git = cast("Git", None)
129+
_gix_repository: Any = None
130+
_gix_state: Any = None
127131

128132
working_dir: PathLike
129133
"""The working directory of the git command."""
@@ -421,7 +425,9 @@ def __init__(
421425
break
422426

423427
self.working_dir = self._working_tree_dir or self.common_dir
428+
self._gix_lock = RLock()
424429
self.git = self.GitCommandWrapperType(self.working_dir)
430+
self.git._repo = weakref.ref(self)
425431
self.git._environment.update(GIT_DIR=git_dir, **environment)
426432
if self._working_tree_dir is not None:
427433
self.git.update_environment(GIT_WORK_TREE=os.fspath(self._working_tree_dir))
@@ -447,6 +453,18 @@ def __init__(
447453
)
448454
self.odb = odbt(rootpath)
449455

456+
def __getstate__(self) -> Dict[str, Any]:
457+
return {
458+
key: value
459+
for key, value in self.__dict__.items()
460+
if key not in ("_gix_repository", "_gix_state", "_gix_lock")
461+
}
462+
463+
def __setstate__(self, state: Dict[str, Any]) -> None:
464+
self.__dict__.update(state)
465+
self._gix_lock = RLock()
466+
self.git._repo = weakref.ref(self)
467+
450468
def __enter__(self) -> "Repo":
451469
return self
452470

@@ -460,6 +478,7 @@ def __del__(self) -> None:
460478
pass
461479

462480
def close(self) -> None:
481+
self._gix_repository = self._gix_state = None
463482
if self.git:
464483
self.git.clear_cache()
465484
# Tempfiles objects on Windows are holding references to open files until

‎test/test_gix_backend.py‎

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,8 @@
22

33
from io import BytesIO
44
from pathlib import Path
5+
import pickle
6+
from concurrent.futures import ThreadPoolExecutor
57
import tempfile
68
from unittest.mock import patch
79

@@ -55,6 +57,49 @@ def test_native_worktree_inventory_includes_bare_main(repo, tmp_path):
5557
assert current.git._call_process_safe("worktree", "list", "--porcelain", "-z") == expected
5658

5759

60+
def test_native_repository_lifetime_and_refresh(repo, tmp_path):
61+
handle = repo._gix_repository
62+
assert isinstance(handle, gix.Repository)
63+
with patch.object(gix, "open_opts", side_effect=AssertionError("unexpected reopen")):
64+
assert repo.git._call_process_safe("rev_parse", "--show-object-format") == repo.object_format
65+
assert repo._gix_repository is handle
66+
with ThreadPoolExecutor(max_workers=2) as pool:
67+
assert list(
68+
pool.map(lambda _: repo.git._call_process_safe("rev_parse", "--show-object-format"), range(2))
69+
) == [
70+
repo.object_format,
71+
repo.object_format,
72+
]
73+
Path(repo.working_dir, "file").write_text("payload")
74+
repo.index.add(["file"])
75+
first = repo.index.commit("native", skip_hooks=True)
76+
assert repo.commit().hexsha == first.hexsha
77+
repo.git.commit("--allow-empty", "-m", "CLI", "--no-verify")
78+
assert repo.commit().message == "CLI\n"
79+
assert repo._gix_repository is handle
80+
assert set(repo.index.entries) == {("file", 0)}
81+
82+
with repo.config_writer() as writer:
83+
writer.set_value("core", "abbrev", "9")
84+
assert _backend._repository(repo.git, {}).config_snapshot().integer("core.abbrev") == 9
85+
assert repo._gix_repository is handle
86+
include = tmp_path / "included"
87+
include.write_text("[test]\nvalue = first\n")
88+
repo.git.config("include.path", str(include))
89+
assert _backend._repository(repo.git, {}).config_snapshot().string("test.value") == b"first"
90+
include.write_text("[test]\nvalue = second\n")
91+
assert _backend._repository(repo.git, {}).config_snapshot().string("test.value") == b"second"
92+
93+
with pickle.loads(pickle.dumps(repo)) as restored:
94+
assert restored._gix_repository is None
95+
assert restored.commit().hexsha == repo.commit().hexsha
96+
assert restored._gix_repository is not None
97+
repo.close()
98+
assert repo._gix_repository is None
99+
assert repo.commit().message == "CLI\n"
100+
assert repo._gix_repository is not handle
101+
102+
58103
def test_native_objects_trees_commits_and_index_reads(repo):
59104
actor = Actor("Example", "example@example.invalid")
60105
paths = ["file", "dir/with space", "dir/unicode-é", "--option"]

0 commit comments

Comments
 (0)