diff --git a/CLAUDE.md b/CLAUDE.md index d67a6af..de25fa1 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -276,6 +276,19 @@ Each rule's full story (why, traps, tests that pin it) is in the named doc. nothing (commits are immutable), the decorations by a ref fingerprint — add a third cached thing and it needs its own answer to "what makes this wrong?". (`docs/dev/backend.md`, `docs/dev/performance.md`) +- **…and the ORDER itself comes from `git rev-list --date-order`** + (`git/log_walk.rs`, #483), because that libgit2 walk costs 15.7 s on the + kernel and never reads the commit-graph that makes git's answer 188 ms. + **`--date-order`, NEVER `--topo-order`:** they are different questions, and + on the kernel they share only 1,627 of the first 2,000 oids — swapping them + silently changes which commits the first page shows. `log_walk_ordering.rs` + pins it, with a fixture whose branches interleave on purpose; one that does + not would pass against the mistake. Only oids cross over, commit data still + comes from libgit2, and any failure falls back to the libgit2 walk — a slow + page, never a failed one. `git/commit_graph.rs` keeps the `--split` + commit-graph that makes it fast (the plain `--reachable` form re-pays 14.3 s + even when nothing changed) and honours `core.commitGraph`. + (`docs/dev/backend.md`, `docs/dev/performance.md`) - **A commit row's columns have a YIELD ORDER, and it is the template.** Every track in `commitRowGrid` but the subject and the author is a fixed width, so a new fixed column — or a wider one — comes straight out of the subject, diff --git a/README.md b/README.md index 6648f41..ddf97a7 100644 --- a/README.md +++ b/README.md @@ -148,18 +148,18 @@ disagree — so no figure here can be nudged by hand. -Measured on Apple M4 Pro (14 cores, 48 GB, macos/aarch64) with git version 2.50.1 (Apple Git-155), on 2026-09-17 — medians over repeat calls against the real backend. `pnpm bench` reproduces the generated fixtures in about a minute; the kernel clone is opt-in. +Measured on Apple M4 Pro (14 cores, 48 GB, macos/aarch64) with git version 2.50.1 (Apple Git-155), on 2026-09-18 — medians over repeat calls against the real backend. `pnpm bench` reproduces the generated fixtures in about a minute; the kernel clone is opt-in. | Repository | First screen | Status | First page of history | …vs `git` | | --- | --- | --- | --- | --- | -| **torvalds/linux**
1,482,923 commits · 96,034 files · 946 tags · 13 changed | 15.84 s | 989 ms | 15.95 s | 1.7× | -| **deep**
50,000 commits · 16 files | 253 ms | 0.53 ms | 249 ms | 1.3× | -| **wide**
1 commit · 50,000 files · 55,000 changed | 5.42 s | 5.42 s | 0.25 ms | — | -| **refs**
2,000 commits · 32 files · 5,001 branches · 2,000 tags | 219 ms | 0.55 ms | 135 ms | 16× | +| **torvalds/linux**
1,482,923 commits · 96,034 files · 946 tags · 13 changed | 999 ms | 1.03 s | 8.70 ms | 0.30× | +| **deep**
50,000 commits · 16 files | 59.0 ms | 0.54 ms | 3.21 ms | — | +| **wide**
1 commit · 50,000 files · 55,000 changed | 5.41 s | 5.38 s | 0.25 ms | — | +| **refs**
2,000 commits · 32 files · 5,001 branches · 2,000 tags | 224 ms | 0.56 ms | 121 ms | 28× | **First screen** is the eleven reads the app issues when it opens a repository, issued at once — a composite, because the failure worth catching is one slow read blocking the other ten. **Status** returns per-file added and removed counts, so its baseline is `git status --porcelain` plus both `--numstat` diffs rather than a bare `git status`. Ratios are against git's *work*, with process start-up subtracted — deliberately the comparison that flatters us least — and a dash is a baseline too small to divide by. No figure here includes the UI: the benchmark drives the git backend directly, with no webview in it. -**torvalds/linux is the bad case, and publishing it is the point.** The first screen costs 15.84 s there, and reaching ten pages into its history costs 157.67 s: a sorted libgit2 revwalk pre-walks all 1,482,923 commits before it yields one, and the next page pays for that again. The developer who opens a repository that size and waits is the one this was written for, so the number belongs here rather than in a backlog. +**torvalds/linux is the case that matters, and publishing it is the point.** The first screen costs 999 ms on 1,482,923 commits, and reaching ten pages into its history costs 116 ms — the log's order comes from git over a commit-graph the app maintains, because libgit2's own sorted revwalk pre-walks the entire graph before it yields a single commit and never reads that file (#483). What is slowest here now is "History of one file" at 16.88 s, and it is published for the same reason the fifteen seconds were: the developer who opens a repository this size is the one this was written for. Every operation on every fixture, the `git` command behind each baseline, and what the numbers were read to mean: [`docs/dev/performance.md`](./docs/dev/performance.md). diff --git a/docs/dev/backend.md b/docs/dev/backend.md index 178e5ae..6e168c1 100644 --- a/docs/dev/backend.md +++ b/docs/dev/backend.md @@ -1546,6 +1546,73 @@ walk a `Vec` per commit — `{ name, kind }`, where `kind` is `collect_ref_map` no longer runs per page — see below. +## The walk's ORDER comes from git, not from libgit2 (#483) + +#473 stopped the sort being re-paid per page. It could not make the sort +itself cheaper, and on `torvalds/linux` that one preparation is 15.7 s — the +whole of what a user waits for on open, since everything else the first screen +needs finishes inside a second. + +`git/log_walk.rs` takes the order from `git rev-list` instead. Measured on the +kernel, the 100,000-oid walk `MAX_ORDER` actually asks for: + +| | no commit-graph | with commit-graph | +| --- | --- | --- | +| libgit2 `TIME \| TOPOLOGICAL` | 15,743 ms | 15,743 ms (it never reads the file) | +| `rev-list --date-order` | 10,123 ms | **188 ms** | + +**`--date-order`, and never `--topo-order`.** `Sort::TIME | Sort::TOPOLOGICAL` +is Kahn's algorithm over a time-priority queue, which is precisely what git +calls `--date-order`. `--topo-order` answers a different question — it also +refuses to intermix independent lines of history — and on the kernel the two +share only 1,627 of the first 2,000 oids. It does not reorder the same commits, +it returns different ones, so taking it would silently change which commits the +first page shows. #473 and #476 both proposed it. +`tests/log_walk_ordering.rs` pins the mapping in both directions, and its +fixture interleaves two branches' commit dates on purpose — one that does not +produces the same sequence either way and would pass against the mistake. + +**Only oids cross over.** Commit metadata still comes from libgit2 via +`find_commit`, so there is no `--format` string to keep in sync with +`CommitInfo` and nothing downstream of `WalkOrder` changes. + +**Every failure is a slow page, never a failed one.** Git missing, git exiting +non-zero, or output that does not parse all return `None` and fall through to +the libgit2 revwalk, which is exactly the code that ran before this existed. +`PGIT_DISABLE_REV_LIST` forces that path for tests and for support. + +The parser requires a FULL-LENGTH hex id rather than leaving it to +`Oid::from_str`, which accepts an abbreviated string and zero-pads it — so +output truncated mid-line would otherwise parse into a plausible order naming +an object that does not exist. + +### The commit-graph is not an optimisation on top of this, it IS it + +Without one, `rev-list --date-order` costs 10,123 ms on the kernel and the whole +change buys nothing. A fresh clone has none — `git clone` does not write one and +`gc --auto` does not fire on a single packfile — so `git/commit_graph.rs` keeps +one, in the user's own repository, exactly where `git gc` and `git maintenance` +put it and where it makes the user's own `git log` fast too. + +**`--split` is not a preference.** Measured on the kernel: + +| | cost | +| --- | --- | +| `commit-graph write --reachable`, cold | 14,509 ms | +| `commit-graph write --reachable`, **already fresh** | 14,305 ms | +| `commit-graph write --reachable --split`, cold | 14,531 ms | +| `commit-graph write --reachable --split`, nothing new | **59.9 ms** | + +The plain form rewrites everything every time, so scheduling it on open would +burn fourteen seconds of CPU per open forever. + +It is scheduled from `commands/repo.rs` AFTER the open resolves, on the blocking +pool, and nothing waits for it — the first write on a giant repository is ~14.5 s +and that open is served by the slow path, which is exactly as slow as it was +before any of this. It honours `core.commitGraph`: a user who turned git's own +commit-graph reading off gets no file, because they would get a file they did +not ask for AND no speedup. + ## The paged log prepares ONE walk (#473) `log_page` used to build a fresh revwalk per page. That reads like an obvious diff --git a/docs/dev/benchmark.json b/docs/dev/benchmark.json index 6aa0378..cf65242 100644 --- a/docs/dev/benchmark.json +++ b/docs/dev/benchmark.json @@ -1,5 +1,5 @@ { - "measuredOn": "2026-09-17", + "measuredOn": "2026-09-18", "machine": { "cpu": "Apple M4 Pro", "cores": 14, @@ -9,7 +9,7 @@ }, "iterations": 10, "budgetSeconds": 20, - "gitSpawnFloorMs": 12.411, + "gitSpawnFloorMs": 12.373, "fixtures": [ { "key": "linux", @@ -24,20 +24,20 @@ "tags": 946, "dirtyEntries": 13 }, - "gitSpawnFloorMs": 12.411, + "gitSpawnFloorMs": 12.373, "operations": [ { "op": "open", "label": "Open the repository", "scale": "a fresh handle", "samples": 10, - "firstMs": 0.257, + "firstMs": 0.266, "repeatMedianMs": 0.109, - "repeatP95Ms": 0.125, + "repeatP95Ms": 0.126, "gitCommand": "git rev-parse HEAD", "gitInvocations": 1, - "gitMedianMs": 12.24, - "gitWorkMs": 0, + "gitMedianMs": 13.071, + "gitWorkMs": 0.6980000000000004, "gitFloorBound": true, "ratioToGit": null }, @@ -45,10 +45,10 @@ "op": "open_screen", "label": "Everything the first screen needs, at once", "scale": "11 concurrent reads", - "samples": 3, - "firstMs": 15905.468, - "repeatMedianMs": 15844.188, - "repeatP95Ms": 15855.63, + "samples": 10, + "firstMs": 999.734, + "repeatMedianMs": 998.87, + "repeatP95Ms": 1016.973, "gitCommand": null, "gitInvocations": 0, "gitMedianMs": null, @@ -60,10 +60,10 @@ "op": "open_screen_ipc", "label": "…including encoding it all for the webview", "scale": "11 concurrent reads", - "samples": 3, - "firstMs": 15799.373, - "repeatMedianMs": 15818.08, - "repeatP95Ms": 15856.463, + "samples": 10, + "firstMs": 1012.044, + "repeatMedianMs": 1004.544, + "repeatP95Ms": 1022.799, "gitCommand": null, "gitInvocations": 0, "gitMedianMs": null, @@ -76,58 +76,58 @@ "label": "Working-tree status", "scale": "26 entries", "samples": 10, - "firstMs": 1150.178, - "repeatMedianMs": 989.035, - "repeatP95Ms": 1104.831, + "firstMs": 1059.246, + "repeatMedianMs": 1032.565, + "repeatP95Ms": 1037.399, "gitCommand": "git status --porcelain=v1 --untracked-files=all && git diff --numstat && git diff --cached --numstat", "gitInvocations": 3, - "gitMedianMs": 554.112, - "gitWorkMs": 516.879, + "gitMedianMs": 542.037, + "gitWorkMs": 504.918, "gitFloorBound": false, - "ratioToGit": 1.913474913857982 + "ratioToGit": 2.0450152301957942 }, { "op": "log_first_page", "label": "First page of history", "scale": "500 commits", - "samples": 3, - "firstMs": 15956.168, - "repeatMedianMs": 15952.356, - "repeatP95Ms": 16486.088, - "gitCommand": "git log --topo-order --max-count=500 --format=%H%n%an%n%ae%n%at%n%s", + "samples": 10, + "firstMs": 222.204, + "repeatMedianMs": 8.696, + "repeatP95Ms": 8.929, + "gitCommand": "git log --date-order --max-count=500 --format=%H%n%an%n%ae%n%at%n%s", "gitInvocations": 1, - "gitMedianMs": 9517.414, - "gitWorkMs": 9505.003, + "gitMedianMs": 41.437, + "gitWorkMs": 29.064, "gitFloorBound": false, - "ratioToGit": 1.6783115165771119 + "ratioToGit": 0.2992017616295073 }, { "op": "log_page_deep", "label": "Ten pages into history", "scale": "500 commits", - "samples": 3, - "firstMs": 156377.157, - "repeatMedianMs": 157671.22, - "repeatP95Ms": 159939.268, - "gitCommand": "git log --topo-order --skip=4500 --max-count=500 --format=%H%n%an%n%ae%n%at%n%s", + "samples": 10, + "firstMs": 337.188, + "repeatMedianMs": 116.174, + "repeatP95Ms": 117.796, + "gitCommand": "git log --date-order --skip=4500 --max-count=500 --format=%H%n%an%n%ae%n%at%n%s", "gitInvocations": 1, - "gitMedianMs": 9692.322, - "gitWorkMs": 9679.911, + "gitMedianMs": 58.755, + "gitWorkMs": 46.382000000000005, "gitFloorBound": false, - "ratioToGit": 16.288498933512923 + "ratioToGit": 2.5047216592643697 }, { "op": "branches", "label": "List every branch", "scale": "3 branches", "samples": 10, - "firstMs": 1.649, - "repeatMedianMs": 0.873, - "repeatP95Ms": 0.891, + "firstMs": 1.254, + "repeatMedianMs": 0.888, + "repeatP95Ms": 0.976, "gitCommand": "git for-each-ref --format=%(refname)%(objectname)%(upstream) refs/heads refs/remotes", "gitInvocations": 1, - "gitMedianMs": 12.951, - "gitWorkMs": 0.5400000000000009, + "gitMedianMs": 13.761, + "gitWorkMs": 1.388, "gitFloorBound": true, "ratioToGit": null }, @@ -136,39 +136,39 @@ "label": "List every tag", "scale": "946 tags", "samples": 10, - "firstMs": 27.099, - "repeatMedianMs": 19.709, - "repeatP95Ms": 20.166, + "firstMs": 27.442, + "repeatMedianMs": 20.334, + "repeatP95Ms": 21.765, "gitCommand": "git for-each-ref --format=%(refname)%(objectname)%(*objectname) refs/tags", "gitInvocations": 1, - "gitMedianMs": 36.108, - "gitWorkMs": 23.696999999999996, + "gitMedianMs": 44.704, + "gitWorkMs": 32.331, "gitFloorBound": false, - "ratioToGit": 0.8317086551040217 + "ratioToGit": 0.6289319847824069 }, { "op": "diff_commit", "label": "Diff the selected commit", "scale": "3 files", "samples": 10, - "firstMs": 141.665, - "repeatMedianMs": 138.7, - "repeatP95Ms": 140.429, + "firstMs": 151.204, + "repeatMedianMs": 141.52, + "repeatP95Ms": 142.476, "gitCommand": "git show --format= --patch HEAD", "gitInvocations": 1, - "gitMedianMs": 23.911, - "gitWorkMs": 11.500000000000002, + "gitMedianMs": 32.894, + "gitWorkMs": 20.521, "gitFloorBound": false, - "ratioToGit": 12.060869565217388 + "ratioToGit": 6.896350080405439 }, { "op": "diff_workdir_file", "label": "Diff one modified file", "scale": "2 hunks", "samples": 10, - "firstMs": 68.161, - "repeatMedianMs": 2.136, - "repeatP95Ms": 2.219, + "firstMs": 61.408, + "repeatMedianMs": 2.148, + "repeatP95Ms": 2.339, "gitCommand": null, "gitInvocations": 0, "gitMedianMs": null, @@ -181,30 +181,30 @@ "label": "History of one file", "scale": "500 commits", "samples": 3, - "firstMs": 15913.163, - "repeatMedianMs": 15419.268, - "repeatP95Ms": 16055.124, - "gitCommand": "git log --topo-order --max-count=500 --format=%H%n%an%n%at%n%s -- MAINTAINERS", + "firstMs": 16970.774, + "repeatMedianMs": 16884.483, + "repeatP95Ms": 16956.577, + "gitCommand": "git log --date-order --max-count=500 --format=%H%n%an%n%at%n%s -- MAINTAINERS", "gitInvocations": 1, - "gitMedianMs": 5667.106, - "gitWorkMs": 5654.695, + "gitMedianMs": 142.129, + "gitWorkMs": 129.756, "gitFloorBound": false, - "ratioToGit": 2.726808077181882 + "ratioToGit": 130.12487283825027 }, { "op": "list_all_files", "label": "Browse the whole tree", "scale": "96,034 files", "samples": 10, - "firstMs": 781.816, - "repeatMedianMs": 515.422, - "repeatP95Ms": 642.301, + "firstMs": 678.536, + "repeatMedianMs": 507.179, + "repeatP95Ms": 709.642, "gitCommand": "git ls-files --cached --others --exclude-standard", "gitInvocations": 1, - "gitMedianMs": 230.862, - "gitWorkMs": 218.451, + "gitMedianMs": 229.567, + "gitWorkMs": 217.19400000000002, "gitFloorBound": false, - "ratioToGit": 2.359439874388307 + "ratioToGit": 2.335142775583119 } ], "soak": null @@ -222,20 +222,20 @@ "tags": 0, "dirtyEntries": 0 }, - "gitSpawnFloorMs": 12.275, + "gitSpawnFloorMs": 12.828, "operations": [ { "op": "open", "label": "Open the repository", "scale": "a fresh handle", "samples": 10, - "firstMs": 0.141, - "repeatMedianMs": 0.114, - "repeatP95Ms": 0.122, + "firstMs": 0.144, + "repeatMedianMs": 0.116, + "repeatP95Ms": 0.124, "gitCommand": "git rev-parse HEAD", "gitInvocations": 1, - "gitMedianMs": 12.244, - "gitWorkMs": 0, + "gitMedianMs": 12.897, + "gitWorkMs": 0.06900000000000084, "gitFloorBound": true, "ratioToGit": null }, @@ -244,9 +244,9 @@ "label": "Everything the first screen needs, at once", "scale": "11 concurrent reads", "samples": 10, - "firstMs": 252.398, - "repeatMedianMs": 252.638, - "repeatP95Ms": 255.802, + "firstMs": 60.34, + "repeatMedianMs": 58.953, + "repeatP95Ms": 60.102, "gitCommand": null, "gitInvocations": 0, "gitMedianMs": null, @@ -259,9 +259,9 @@ "label": "…including encoding it all for the webview", "scale": "11 concurrent reads", "samples": 10, - "firstMs": 251.635, - "repeatMedianMs": 252.584, - "repeatP95Ms": 254.367, + "firstMs": 60.861, + "repeatMedianMs": 59.177, + "repeatP95Ms": 62.485, "gitCommand": null, "gitInvocations": 0, "gitMedianMs": null, @@ -274,13 +274,13 @@ "label": "Working-tree status", "scale": "0 entries", "samples": 10, - "firstMs": 0.688, - "repeatMedianMs": 0.525, + "firstMs": 0.674, + "repeatMedianMs": 0.542, "repeatP95Ms": 0.554, "gitCommand": "git status --porcelain=v1 --untracked-files=all && git diff --numstat && git diff --cached --numstat", "gitInvocations": 3, - "gitMedianMs": 39.411, - "gitWorkMs": 2.5859999999999985, + "gitMedianMs": 40.544, + "gitWorkMs": 2.0600000000000023, "gitFloorBound": true, "ratioToGit": null }, @@ -289,43 +289,43 @@ "label": "First page of history", "scale": "500 commits", "samples": 10, - "firstMs": 261.376, - "repeatMedianMs": 249.132, - "repeatP95Ms": 251.38, - "gitCommand": "git log --topo-order --max-count=500 --format=%H%n%an%n%ae%n%at%n%s", + "firstMs": 57.867, + "repeatMedianMs": 3.211, + "repeatP95Ms": 3.272, + "gitCommand": "git log --date-order --max-count=500 --format=%H%n%an%n%ae%n%at%n%s", "gitInvocations": 1, - "gitMedianMs": 205.036, - "gitWorkMs": 192.761, - "gitFloorBound": false, - "ratioToGit": 1.292439860760216 + "gitMedianMs": 17.002, + "gitWorkMs": 4.1739999999999995, + "gitFloorBound": true, + "ratioToGit": null }, { "op": "log_page_deep", "label": "Ten pages into history", "scale": "500 commits", - "samples": 8, - "firstMs": 2392.178, - "repeatMedianMs": 2393.558, - "repeatP95Ms": 2400.148, - "gitCommand": "git log --topo-order --skip=4500 --max-count=500 --format=%H%n%an%n%ae%n%at%n%s", - "gitInvocations": 1, - "gitMedianMs": 204.502, - "gitWorkMs": 192.227, + "samples": 10, + "firstMs": 89.746, + "repeatMedianMs": 31.543, + "repeatP95Ms": 32.249, + "gitCommand": "git log --date-order --skip=4500 --max-count=500 --format=%H%n%an%n%ae%n%at%n%s", + "gitInvocations": 1, + "gitMedianMs": 18.404, + "gitWorkMs": 5.5760000000000005, "gitFloorBound": false, - "ratioToGit": 12.4517263443741 + "ratioToGit": 5.656922525107603 }, { "op": "branches", "label": "List every branch", "scale": "1 branch", "samples": 10, - "firstMs": 0.422, - "repeatMedianMs": 0.281, - "repeatP95Ms": 0.29, + "firstMs": 0.454, + "repeatMedianMs": 0.282, + "repeatP95Ms": 0.341, "gitCommand": "git for-each-ref --format=%(refname)%(objectname)%(upstream) refs/heads refs/remotes", "gitInvocations": 1, - "gitMedianMs": 13.103, - "gitWorkMs": 0.8279999999999994, + "gitMedianMs": 13.283, + "gitWorkMs": 0.45500000000000007, "gitFloorBound": true, "ratioToGit": null }, @@ -335,12 +335,12 @@ "scale": "0 tags", "samples": 10, "firstMs": 0.175, - "repeatMedianMs": 0.147, - "repeatP95Ms": 0.15, + "repeatMedianMs": 0.145, + "repeatP95Ms": 0.154, "gitCommand": "git for-each-ref --format=%(refname)%(objectname)%(*objectname) refs/tags", "gitInvocations": 1, - "gitMedianMs": 12.833, - "gitWorkMs": 0.5579999999999998, + "gitMedianMs": 13.116, + "gitWorkMs": 0.28800000000000026, "gitFloorBound": true, "ratioToGit": null }, @@ -349,13 +349,13 @@ "label": "Diff the selected commit", "scale": "1 file", "samples": 10, - "firstMs": 0.477, + "firstMs": 0.501, "repeatMedianMs": 0.338, - "repeatP95Ms": 0.351, + "repeatP95Ms": 0.402, "gitCommand": "git show --format= --patch HEAD", "gitInvocations": 1, - "gitMedianMs": 13.195, - "gitWorkMs": 0.9199999999999999, + "gitMedianMs": 13.834, + "gitWorkMs": 1.0060000000000002, "gitFloorBound": true, "ratioToGit": null }, @@ -364,41 +364,33 @@ "label": "History of one file", "scale": "500 commits", "samples": 10, - "firstMs": 293.063, - "repeatMedianMs": 65.002, - "repeatP95Ms": 65.738, - "gitCommand": "git log --topo-order --max-count=500 --format=%H%n%an%n%at%n%s -- src/module_00.rs", + "firstMs": 319.356, + "repeatMedianMs": 306.97, + "repeatP95Ms": 309.728, + "gitCommand": "git log --date-order --max-count=500 --format=%H%n%an%n%at%n%s -- src/module_00.rs", "gitInvocations": 1, - "gitMedianMs": 331.471, - "gitWorkMs": 319.196, + "gitMedianMs": 42.343, + "gitWorkMs": 29.515000000000004, "gitFloorBound": false, - "ratioToGit": 0.20364290279326805 + "ratioToGit": 10.400474335083855 }, { "op": "list_all_files", "label": "Browse the whole tree", "scale": "16 files", "samples": 10, - "firstMs": 0.497, - "repeatMedianMs": 0.129, - "repeatP95Ms": 0.133, + "firstMs": 0.485, + "repeatMedianMs": 0.127, + "repeatP95Ms": 0.131, "gitCommand": "git ls-files --cached --others --exclude-standard", "gitInvocations": 1, - "gitMedianMs": 12.845, - "gitWorkMs": 0.5700000000000003, + "gitMedianMs": 12.728, + "gitWorkMs": 0, "gitFloorBound": true, "ratioToGit": null } ], - "soak": { - "minutes": 10, - "iterations": 2344, - "rssStartMb": 66.5, - "rssEndMb": 69.047, - "rssPeakMb": 69.047, - "firstHalfMedianMs": 252.302, - "secondHalfMedianMs": 251.693 - } + "soak": null }, { "key": "wide", @@ -413,20 +405,20 @@ "tags": 0, "dirtyEntries": 55000 }, - "gitSpawnFloorMs": 12.058, + "gitSpawnFloorMs": 12.281, "operations": [ { "op": "open", "label": "Open the repository", "scale": "a fresh handle", "samples": 10, - "firstMs": 0.25, - "repeatMedianMs": 0.114, - "repeatP95Ms": 0.134, + "firstMs": 0.256, + "repeatMedianMs": 0.112, + "repeatP95Ms": 0.13, "gitCommand": "git rev-parse HEAD", "gitInvocations": 1, - "gitMedianMs": 12.46, - "gitWorkMs": 0.402000000000001, + "gitMedianMs": 12.657, + "gitWorkMs": 0.37599999999999945, "gitFloorBound": true, "ratioToGit": null }, @@ -435,9 +427,9 @@ "label": "Everything the first screen needs, at once", "scale": "11 concurrent reads", "samples": 3, - "firstMs": 5420.711, - "repeatMedianMs": 5420.4, - "repeatP95Ms": 5622.622, + "firstMs": 5408.776, + "repeatMedianMs": 5410.42, + "repeatP95Ms": 5425.959, "gitCommand": null, "gitInvocations": 0, "gitMedianMs": null, @@ -450,9 +442,9 @@ "label": "…including encoding it all for the webview", "scale": "11 concurrent reads", "samples": 3, - "firstMs": 5602.161, - "repeatMedianMs": 5432.862, - "repeatP95Ms": 5601.451, + "firstMs": 5408.624, + "repeatMedianMs": 5375.761, + "repeatP95Ms": 5410.887, "gitCommand": null, "gitInvocations": 0, "gitMedianMs": null, @@ -465,28 +457,28 @@ "label": "Working-tree status", "scale": "55,000 entries", "samples": 3, - "firstMs": 5380.675, - "repeatMedianMs": 5419.887, - "repeatP95Ms": 5482.045, + "firstMs": 5373.12, + "repeatMedianMs": 5383.519, + "repeatP95Ms": 5397.29, "gitCommand": "git status --porcelain=v1 --untracked-files=all && git diff --numstat && git diff --cached --numstat", "gitInvocations": 3, - "gitMedianMs": 3014.503, - "gitWorkMs": 2978.329, + "gitMedianMs": 2985.647, + "gitWorkMs": 2948.804, "gitFloorBound": false, - "ratioToGit": 1.8197744439919161 + "ratioToGit": 1.8256618615547184 }, { "op": "log_first_page", "label": "First page of history", "scale": "1 commit", "samples": 10, - "firstMs": 0.353, - "repeatMedianMs": 0.251, - "repeatP95Ms": 0.256, - "gitCommand": "git log --topo-order --max-count=500 --format=%H%n%an%n%ae%n%at%n%s", + "firstMs": 15.45, + "repeatMedianMs": 0.246, + "repeatP95Ms": 0.269, + "gitCommand": "git log --date-order --max-count=500 --format=%H%n%an%n%ae%n%at%n%s", "gitInvocations": 1, - "gitMedianMs": 13.179, - "gitWorkMs": 1.1210000000000004, + "gitMedianMs": 13.711, + "gitWorkMs": 1.4299999999999997, "gitFloorBound": true, "ratioToGit": null }, @@ -495,13 +487,13 @@ "label": "Ten pages into history", "scale": "1 commit", "samples": 10, - "firstMs": 0.312, - "repeatMedianMs": 0.25, - "repeatP95Ms": 0.256, - "gitCommand": "git log --topo-order --skip=4500 --max-count=500 --format=%H%n%an%n%ae%n%at%n%s", + "firstMs": 14.892, + "repeatMedianMs": 0.246, + "repeatP95Ms": 0.271, + "gitCommand": "git log --date-order --skip=4500 --max-count=500 --format=%H%n%an%n%ae%n%at%n%s", "gitInvocations": 1, - "gitMedianMs": 13.081, - "gitWorkMs": 1.0229999999999997, + "gitMedianMs": 13.378, + "gitWorkMs": 1.0969999999999995, "gitFloorBound": true, "ratioToGit": null }, @@ -510,13 +502,13 @@ "label": "List every branch", "scale": "1 branch", "samples": 10, - "firstMs": 0.352, - "repeatMedianMs": 0.275, - "repeatP95Ms": 0.317, + "firstMs": 0.378, + "repeatMedianMs": 0.277, + "repeatP95Ms": 0.293, "gitCommand": "git for-each-ref --format=%(refname)%(objectname)%(upstream) refs/heads refs/remotes", "gitInvocations": 1, - "gitMedianMs": 12.756, - "gitWorkMs": 0.6980000000000004, + "gitMedianMs": 12.592, + "gitWorkMs": 0.31099999999999994, "gitFloorBound": true, "ratioToGit": null }, @@ -525,13 +517,13 @@ "label": "List every tag", "scale": "0 tags", "samples": 10, - "firstMs": 0.171, - "repeatMedianMs": 0.145, - "repeatP95Ms": 0.147, + "firstMs": 0.174, + "repeatMedianMs": 0.143, + "repeatP95Ms": 0.145, "gitCommand": "git for-each-ref --format=%(refname)%(objectname)%(*objectname) refs/tags", "gitInvocations": 1, - "gitMedianMs": 12.756, - "gitWorkMs": 0.6980000000000004, + "gitMedianMs": 12.572, + "gitWorkMs": 0.2909999999999986, "gitFloorBound": true, "ratioToGit": null }, @@ -540,24 +532,24 @@ "label": "Diff the selected commit", "scale": "50,000 files", "samples": 10, - "firstMs": 1530.386, - "repeatMedianMs": 1510.808, - "repeatP95Ms": 1674.506, + "firstMs": 1532.778, + "repeatMedianMs": 1508.417, + "repeatP95Ms": 1515.096, "gitCommand": "git show --format= --patch HEAD", "gitInvocations": 1, - "gitMedianMs": 522.06, - "gitWorkMs": 510.00199999999995, + "gitMedianMs": 527.583, + "gitWorkMs": 515.302, "gitFloorBound": false, - "ratioToGit": 2.9623570103646655 + "ratioToGit": 2.9272484872948286 }, { "op": "diff_workdir_file", "label": "Diff one modified file", "scale": "1 hunk", "samples": 10, - "firstMs": 17.18, - "repeatMedianMs": 0.632, - "repeatP95Ms": 0.639, + "firstMs": 18.561, + "repeatMedianMs": 0.635, + "repeatP95Ms": 0.642, "gitCommand": null, "gitInvocations": 0, "gitMedianMs": null, @@ -570,13 +562,13 @@ "label": "History of one file", "scale": "1 commit", "samples": 10, - "firstMs": 0.276, - "repeatMedianMs": 0.1, - "repeatP95Ms": 0.105, - "gitCommand": "git log --topo-order --max-count=500 --format=%H%n%an%n%at%n%s -- pkg/000/gen_000000.ts", + "firstMs": 0.366, + "repeatMedianMs": 0.263, + "repeatP95Ms": 0.281, + "gitCommand": "git log --date-order --max-count=500 --format=%H%n%an%n%at%n%s -- pkg/000/gen_000000.ts", "gitInvocations": 1, - "gitMedianMs": 13.32, - "gitWorkMs": 1.2620000000000005, + "gitMedianMs": 13.808, + "gitWorkMs": 1.5269999999999992, "gitFloorBound": true, "ratioToGit": null }, @@ -585,15 +577,15 @@ "label": "Browse the whole tree", "scale": "55,000 files", "samples": 10, - "firstMs": 195.887, - "repeatMedianMs": 181.487, - "repeatP95Ms": 240.574, + "firstMs": 202.431, + "repeatMedianMs": 184.505, + "repeatP95Ms": 188.434, "gitCommand": "git ls-files --cached --others --exclude-standard", "gitInvocations": 1, - "gitMedianMs": 54.416, - "gitWorkMs": 42.358, + "gitMedianMs": 54.558, + "gitWorkMs": 42.277, "gitFloorBound": false, - "ratioToGit": 4.284597950800321 + "ratioToGit": 4.364193296591527 } ], "soak": null @@ -611,20 +603,20 @@ "tags": 2000, "dirtyEntries": 0 }, - "gitSpawnFloorMs": 12.386, + "gitSpawnFloorMs": 11.877, "operations": [ { "op": "open", "label": "Open the repository", "scale": "a fresh handle", "samples": 10, - "firstMs": 0.153, - "repeatMedianMs": 0.138, - "repeatP95Ms": 0.143, + "firstMs": 0.141, + "repeatMedianMs": 0.115, + "repeatP95Ms": 0.121, "gitCommand": "git rev-parse HEAD", "gitInvocations": 1, - "gitMedianMs": 12.342, - "gitWorkMs": 0, + "gitMedianMs": 12.095, + "gitWorkMs": 0.21799999999999997, "gitFloorBound": true, "ratioToGit": null }, @@ -633,9 +625,9 @@ "label": "Everything the first screen needs, at once", "scale": "11 concurrent reads", "samples": 10, - "firstMs": 220.893, - "repeatMedianMs": 219.434, - "repeatP95Ms": 220.996, + "firstMs": 224.698, + "repeatMedianMs": 224.171, + "repeatP95Ms": 224.725, "gitCommand": null, "gitInvocations": 0, "gitMedianMs": null, @@ -648,9 +640,9 @@ "label": "…including encoding it all for the webview", "scale": "11 concurrent reads", "samples": 10, - "firstMs": 220.518, - "repeatMedianMs": 219.773, - "repeatP95Ms": 224.361, + "firstMs": 223.692, + "repeatMedianMs": 223.759, + "repeatP95Ms": 225.434, "gitCommand": null, "gitInvocations": 0, "gitMedianMs": null, @@ -663,13 +655,13 @@ "label": "Working-tree status", "scale": "0 entries", "samples": 10, - "firstMs": 0.724, - "repeatMedianMs": 0.549, - "repeatP95Ms": 0.562, + "firstMs": 0.745, + "repeatMedianMs": 0.561, + "repeatP95Ms": 0.572, "gitCommand": "git status --porcelain=v1 --untracked-files=all && git diff --numstat && git diff --cached --numstat", "gitInvocations": 3, - "gitMedianMs": 38.918, - "gitWorkMs": 1.759999999999998, + "gitMedianMs": 40.209, + "gitWorkMs": 4.578000000000003, "gitFloorBound": true, "ratioToGit": null }, @@ -678,73 +670,73 @@ "label": "First page of history", "scale": "500 commits", "samples": 10, - "firstMs": 786.833, - "repeatMedianMs": 135.184, - "repeatP95Ms": 138.328, - "gitCommand": "git log --topo-order --max-count=500 --format=%H%n%an%n%ae%n%at%n%s", + "firstMs": 188.48, + "repeatMedianMs": 120.953, + "repeatP95Ms": 122.665, + "gitCommand": "git log --date-order --max-count=500 --format=%H%n%an%n%ae%n%at%n%s", "gitInvocations": 1, - "gitMedianMs": 20.862, - "gitWorkMs": 8.475999999999999, + "gitMedianMs": 16.131, + "gitWorkMs": 4.254, "gitFloorBound": false, - "ratioToGit": 15.949032562529496 + "ratioToGit": 28.43276915843912 }, { "op": "log_page_deep", "label": "Ten pages into history", "scale": "500 commits", "samples": 10, - "firstMs": 526.236, - "repeatMedianMs": 524.766, - "repeatP95Ms": 550.759, - "gitCommand": "git log --topo-order --skip=4500 --max-count=500 --format=%H%n%an%n%ae%n%at%n%s", + "firstMs": 158.821, + "repeatMedianMs": 128.977, + "repeatP95Ms": 130, + "gitCommand": "git log --date-order --skip=4500 --max-count=500 --format=%H%n%an%n%ae%n%at%n%s", "gitInvocations": 1, - "gitMedianMs": 19.632, - "gitWorkMs": 7.246000000000002, - "gitFloorBound": false, - "ratioToGit": 72.42147391664363 + "gitMedianMs": 13.607, + "gitWorkMs": 1.7299999999999986, + "gitFloorBound": true, + "ratioToGit": null }, { "op": "branches", "label": "List every branch", "scale": "5,001 branches", "samples": 10, - "firstMs": 184.019, - "repeatMedianMs": 180.081, - "repeatP95Ms": 181.962, + "firstMs": 188.217, + "repeatMedianMs": 184.067, + "repeatP95Ms": 185.723, "gitCommand": "git for-each-ref --format=%(refname)%(objectname)%(upstream) refs/heads refs/remotes", "gitInvocations": 1, - "gitMedianMs": 195.41, - "gitWorkMs": 183.024, + "gitMedianMs": 195.83, + "gitWorkMs": 183.953, "gitFloorBound": false, - "ratioToGit": 0.983920141620771 + "ratioToGit": 1.000619723516333 }, { "op": "tags", "label": "List every tag", "scale": "2,000 tags", "samples": 10, - "firstMs": 148.003, - "repeatMedianMs": 148.38, - "repeatP95Ms": 150.987, + "firstMs": 151.36, + "repeatMedianMs": 150.263, + "repeatP95Ms": 153.292, "gitCommand": "git for-each-ref --format=%(refname)%(objectname)%(*objectname) refs/tags", "gitInvocations": 1, - "gitMedianMs": 55.724, - "gitWorkMs": 43.337999999999994, + "gitMedianMs": 57.592, + "gitWorkMs": 45.714999999999996, "gitFloorBound": false, - "ratioToGit": 3.423785130832065 + "ratioToGit": 3.2869517663786505 }, { "op": "diff_commit", "label": "Diff the selected commit", "scale": "1 file", "samples": 10, - "firstMs": 0.404, - "repeatMedianMs": 0.282, - "repeatP95Ms": 0.293, + "firstMs": 0.423, + "repeatMedianMs": 0.281, + "repeatP95Ms": 0.297, "gitCommand": "git show --format= --patch HEAD", "gitInvocations": 1, - "gitMedianMs": 12.971, - "gitWorkMs": 0.5850000000000009, + "gitMedianMs": 13.664, + "gitWorkMs": 1.786999999999999, "gitFloorBound": true, "ratioToGit": null }, @@ -753,28 +745,28 @@ "label": "History of one file", "scale": "63 commits", "samples": 10, - "firstMs": 20.661, - "repeatMedianMs": 9.374, - "repeatP95Ms": 9.883, - "gitCommand": "git log --topo-order --max-count=500 --format=%H%n%an%n%at%n%s -- src/file_0.txt", + "firstMs": 21.875, + "repeatMedianMs": 20.587, + "repeatP95Ms": 20.83, + "gitCommand": "git log --date-order --max-count=500 --format=%H%n%an%n%at%n%s -- src/file_0.txt", "gitInvocations": 1, - "gitMedianMs": 24.316, - "gitWorkMs": 11.93, + "gitMedianMs": 19.012, + "gitWorkMs": 7.135, "gitFloorBound": false, - "ratioToGit": 0.7857502095557419 + "ratioToGit": 2.885353889278206 }, { "op": "list_all_files", "label": "Browse the whole tree", "scale": "32 files", "samples": 10, - "firstMs": 0.475, + "firstMs": 0.463, "repeatMedianMs": 0.158, - "repeatP95Ms": 0.161, + "repeatP95Ms": 0.173, "gitCommand": "git ls-files --cached --others --exclude-standard", "gitInvocations": 1, - "gitMedianMs": 12.878, - "gitWorkMs": 0.4920000000000009, + "gitMedianMs": 12.693, + "gitWorkMs": 0.815999999999999, "gitFloorBound": true, "ratioToGit": null } diff --git a/docs/dev/performance.md b/docs/dev/performance.md index bb5e372..ded9045 100644 --- a/docs/dev/performance.md +++ b/docs/dev/performance.md @@ -159,13 +159,17 @@ the one below; the prose around it is hand-written, and `test/benchmark.test.ts` fails if a figure is copied into it, because a hand-typed number stops moving on the next run. -The **marketing site** is still waiting. #257 asks for a measured figure there -in place of an adjective and the block to do it is written, but a landing page -sells, and 15.8 seconds as a selling point is a different claim from 15.8 -seconds as a disclosed limitation. It ships once the log-walk work in the -findings below lands — at which point the record moves to `site/src/data/` -beside `comparison.json`, which is where this repository keeps published records -the site reads. +The **marketing site** is still waiting, and the reason it was waiting has now +gone. #257 asks for a measured figure there in place of an adjective, and the +block to do it is written, but a landing page sells, and 15.8 seconds as a +selling point is a different claim from 15.8 seconds as a disclosed limitation. +The condition that gated it was the log walk, and #483 landed it: the kernel's +first screen is **999 ms**. Shipping the site figure is now a matter of moving +the record to `site/src/data/` beside `comparison.json` — where this repository +keeps published records the site reads — and it belongs to #257 rather than to +this file. One caveat for whoever does it: the honest headline is the first +screen, not "history in 8.7 ms", and the slowest operation on that fixture is +still `file_history` at 16.9 s. Until then nothing under `site/**` is touched by a re-measurement, which also means `pnpm bench` cannot redeploy the website by accident. @@ -176,18 +180,33 @@ The first run of this benchmark found three things. They are recorded here because the tables above will move and the reasoning will not, and because a number with no reading beside it is a number nobody acts on. -### 1. We are fine until history gets very deep — and then we are not +### 1. History was the whole large-repo problem, and it is no longer the bound -The whole first screen — the eleven reads `refreshAll` issues, all at once — -costs **255 ms** on a 50,000-commit repository and **219 ms** on one with 5,001 -branches and 2,000 tags. That is the size at which GitKraken's own users report -it falling over, and it is a good answer. +The first run of this benchmark found the first screen costing **15.8 seconds** +on `torvalds/linux`, with ten pages into its history costing **two minutes and +thirty-eight seconds**. Three changes closed that, and it is worth keeping which +did what, because two of them are frequently assumed to be one: -On `torvalds/linux` the same screen costs **15.8 seconds**, and scrolling ten -pages into its history costs **two minutes and thirty-eight seconds**. That is -not a good answer, and publishing it is the point of the exercise: the user who -opens a 1.4-million-commit repository and waits is the user this project was -written for. +| | first screen, kernel | page ten | +| --- | --- | --- | +| as first measured | 15.8 s | 157.67 s | +| after #479 — one prepared walk, cached ref map | 15.7 s | 112 ms | +| after #483 — the order comes from git | **999 ms** | **116 ms** | + +**#479 fixed paging; it could not fix the first walk**, because nothing inside +libgit2's revwalk API can. **#483 fixed the first walk** by not using that API. +Neither is a substitute for the other, and the table above is the argument. + +What this means for the shape of the problem: on the kernel the first screen is +now bounded by `status` (1.03 s) rather than by history (8.7 ms), which is the +criterion #483 was accepted on. `status` is a different problem and a smaller +one — it is 1.9× git's own work on a 96,034-file tree, so it needs a cheaper +question (untracked cache, fsmonitor) rather than a faster answer. + +The generated fixtures improved too, and by more than "no regression": `deep` +went from 255 ms to **59 ms**. `wide` is unmoved at 5.41 s because it has one +commit and its cost is `status`; `refs` is unmoved at 224 ms because its cost is +enumerating 7,001 refs. ### 2. The log walk is not slow — the topological SORT is, and it was re-paid per page @@ -338,25 +357,52 @@ page that would look like "no more matches exist". capped walk: it yields commits in the order the traversal reaches them, so the first 50,000 are not the newest 50,000 and a capped file history could miss last week's change while reporting one from 2011. Every walk in - `libgit2.rs` sorts, so this floor is very probably the one behind `log_page`'s - first page too — consistent with the commit-graph measurement below, which is - the fix git gets for exactly this and we do not, but measured here only for - `file_history`. -* **No fixture carries a commit-graph file, and it would not help us if it - did.** A fresh clone has none — `git clone` does not write one, and - `gc --auto` does not fire on a single packfile — so this is what a user gets - on day one. It matters enormously to git: writing one for the kernel takes 14 - seconds, and `git log --topo-order -500` then drops from 9.51 s to **21 ms**. - It does not measurably help us. With the file present, our first page took - 63.4 s for four calls against 64.2 s without it: libgit2's revwalk does not - read it. That is the most useful single fact this benchmark produced, because - it rules out the cheap fix and says where the work actually has to go. + `libgit2.rs` sorts, so this floor was the one behind `log_page`'s first page + too — confirmed by #483, where replacing exactly that call took the kernel's + first screen from 15.7 s to 999 ms. **`file_history` still pays it**: it has + its own walk with its own cap and cursor semantics, it is the slowest + operation on the kernel fixture at 16.9 s, and it is the obvious next one to + move. `log_filtered_page` (commit search) keeps a libgit2 walk on its + cache-miss path for the same reason. +* **The commit-graph is the fix, and libgit2 could never have delivered it.** + A fresh clone has none — `git clone` does not write one, and `gc --auto` does + not fire on a single packfile — so this was what a user got on day one. It + matters enormously to git: writing one for the kernel takes 14 seconds, and a + sorted `git log -500` then drops from 9.51 s to **21 ms**. + + It did nothing for us, and that was the most useful single fact this benchmark + produced. With the file present, our first page took 63.4 s for four calls + against 64.2 s without it, because **libgit2's revwalk does not read it**: + `commit_list.c` fills `commit->generation` from the graph, `revwalk.c` + contains zero references to either, and across all of libgit2's `src/` + `->generation` is read only in `graph.c` and `merge.c`. So the numbers are + populated and then ignored by the one code path that would benefit. + + That ruled out the cheap fix ("write a commit-graph in the background") on its + own and said where the work had to go: out of libgit2. #483 takes the order + from `git rev-list --date-order` and keeps a `--split` commit-graph warm, and + the fixtures now carry one because `scripts/bench-fixtures.mjs` writes it — + a fixture without one measures a state the app does not leave a repository in. + **The `git` baselines get the same file**, which makes several ratios look + worse than they did when we withheld it; that is the correct comparison and + the point of having a floor at all. + +* **`--date-order` is the drop-in, and `--topo-order` is not.** They are + different questions rather than two spellings of one: + `Sort::TIME | Sort::TOPOLOGICAL` is Kahn's algorithm over a time-priority + queue, which is `--date-order`; `--topo-order` additionally refuses to + intermix independent lines of history. Measured byte-for-byte on the kernel's + first 2,000 oids, libgit2's walk is IDENTICAL to `--date-order` and shares + only **1,627 of 2,000** with `--topo-order` — it does not reorder the same + commits, it returns different ones. Both #473 and #476 proposed `--topo-order`, + and the baselines in this file quoted it until #483 had to settle the + question. `tests/log_walk_ordering.rs` pins it in both directions. ## Results -Measured on Apple M4 Pro (14 cores, 48 GB, macos/aarch64) with git version 2.50.1 (Apple Git-155), on 2026-09-17. Up to 10 repeats per operation, time-boxed to 20s each — so a cheap operation gets the full count and an expensive one gets at least three. The published record records how many each row actually took. +Measured on Apple M4 Pro (14 cores, 48 GB, macos/aarch64) with git version 2.50.1 (Apple Git-155), on 2026-09-18. Up to 10 repeats per operation, time-boxed to 20s each — so a cheap operation gets the full count and an expensive one gets at least three. The published record records how many each row actually took. The **`git` work** column is that baseline's wall clock with process start-up subtracted (12.4 ms per invocation on this machine, measured), because we pay none of it — the backend is libgit2, in process. That is deliberately the comparison that makes us look worse: against git's wall clock we would get a ten-millisecond head start on every row. **†** marks a baseline where start-up swamped the work, leaving a remainder too small to divide by; those rows print no ratio rather than a flattering one. @@ -368,18 +414,18 @@ A real clone of the Linux kernel: the repository people mean when they say a git | Operation | Result size | First call | Repeat | p95 | `git` work | vs `git` | | --- | --- | --- | --- | --- | --- | --- | -| Open the repository | a fresh handle | 0.26 ms | 0.11 ms | 0.13 ms | † | — | -| Everything the first screen needs, at once | 11 concurrent reads | 15.91 s | 15.84 s | 15.86 s | — | — | -| …including encoding it all for the webview | 11 concurrent reads | 15.80 s | 15.82 s | 15.86 s | — | — | -| Working-tree status | 26 entries | 1.15 s | 989 ms | 1.10 s | 517 ms | 1.9× | -| First page of history | 500 commits | 15.96 s | 15.95 s | 16.49 s | 9.51 s | 1.7× | -| Ten pages into history | 500 commits | 156.38 s | 157.67 s | 159.94 s | 9.68 s | 16× | -| List every branch | 3 branches | 1.65 ms | 0.87 ms | 0.89 ms | † | — | -| List every tag | 946 tags | 27.1 ms | 19.7 ms | 20.2 ms | 23.7 ms | 0.83× | -| Diff the selected commit | 3 files | 142 ms | 139 ms | 140 ms | 11.5 ms | 12× | -| Diff one modified file | 2 hunks | 68.2 ms | 2.14 ms | 2.22 ms | — | — | -| History of one file | 500 commits | 15.91 s | 15.42 s | 16.06 s | 5.65 s | 2.7× | -| Browse the whole tree | 96,034 files | 782 ms | 515 ms | 642 ms | 218 ms | 2.4× | +| Open the repository | a fresh handle | 0.27 ms | 0.11 ms | 0.13 ms | † | — | +| Everything the first screen needs, at once | 11 concurrent reads | 1000 ms | 999 ms | 1.02 s | — | — | +| …including encoding it all for the webview | 11 concurrent reads | 1.01 s | 1.00 s | 1.02 s | — | — | +| Working-tree status | 26 entries | 1.06 s | 1.03 s | 1.04 s | 505 ms | 2.0× | +| First page of history | 500 commits | 222 ms | 8.70 ms | 8.93 ms | 29.1 ms | 0.30× | +| Ten pages into history | 500 commits | 337 ms | 116 ms | 118 ms | 46.4 ms | 2.5× | +| List every branch | 3 branches | 1.25 ms | 0.89 ms | 0.98 ms | † | — | +| List every tag | 946 tags | 27.4 ms | 20.3 ms | 21.8 ms | 32.3 ms | 0.63× | +| Diff the selected commit | 3 files | 151 ms | 142 ms | 142 ms | 20.5 ms | 6.9× | +| Diff one modified file | 2 hunks | 61.4 ms | 2.15 ms | 2.34 ms | — | — | +| History of one file | 500 commits | 16.97 s | 16.88 s | 16.96 s | 130 ms | 130× | +| Browse the whole tree | 96,034 files | 679 ms | 507 ms | 710 ms | 217 ms | 2.3× | ### deep @@ -389,19 +435,17 @@ A real clone of the Linux kernel: the repository people mean when they say a git | Operation | Result size | First call | Repeat | p95 | `git` work | vs `git` | | --- | --- | --- | --- | --- | --- | --- | -| Open the repository | a fresh handle | 0.14 ms | 0.11 ms | 0.12 ms | † | — | -| Everything the first screen needs, at once | 11 concurrent reads | 252 ms | 253 ms | 256 ms | — | — | -| …including encoding it all for the webview | 11 concurrent reads | 252 ms | 253 ms | 254 ms | — | — | -| Working-tree status | 0 entries | 0.69 ms | 0.53 ms | 0.55 ms | † | — | -| First page of history | 500 commits | 261 ms | 249 ms | 251 ms | 193 ms | 1.3× | -| Ten pages into history | 500 commits | 2.39 s | 2.39 s | 2.40 s | 192 ms | 12× | -| List every branch | 1 branch | 0.42 ms | 0.28 ms | 0.29 ms | † | — | -| List every tag | 0 tags | 0.17 ms | 0.15 ms | 0.15 ms | † | — | -| Diff the selected commit | 1 file | 0.48 ms | 0.34 ms | 0.35 ms | † | — | -| History of one file | 500 commits | 293 ms | 65.0 ms | 65.7 ms | 319 ms | 0.20× | -| Browse the whole tree | 16 files | 0.50 ms | 0.13 ms | 0.13 ms | † | — | - -**Soak.** 2,344 first-screen fan-outs over 10 minutes. Resident memory 67 MB → 69 MB (peak 69 MB). Median fan-out 252 ms in the first half, 252 ms in the second — -0.2%. +| Open the repository | a fresh handle | 0.14 ms | 0.12 ms | 0.12 ms | † | — | +| Everything the first screen needs, at once | 11 concurrent reads | 60.3 ms | 59.0 ms | 60.1 ms | — | — | +| …including encoding it all for the webview | 11 concurrent reads | 60.9 ms | 59.2 ms | 62.5 ms | — | — | +| Working-tree status | 0 entries | 0.67 ms | 0.54 ms | 0.55 ms | † | — | +| First page of history | 500 commits | 57.9 ms | 3.21 ms | 3.27 ms | † | — | +| Ten pages into history | 500 commits | 89.7 ms | 31.5 ms | 32.2 ms | 5.58 ms | 5.7× | +| List every branch | 1 branch | 0.45 ms | 0.28 ms | 0.34 ms | † | — | +| List every tag | 0 tags | 0.17 ms | 0.14 ms | 0.15 ms | † | — | +| Diff the selected commit | 1 file | 0.50 ms | 0.34 ms | 0.40 ms | † | — | +| History of one file | 500 commits | 319 ms | 307 ms | 310 ms | 29.5 ms | 10× | +| Browse the whole tree | 16 files | 0.48 ms | 0.13 ms | 0.13 ms | † | — | ### wide @@ -411,18 +455,18 @@ A real clone of the Linux kernel: the repository people mean when they say a git | Operation | Result size | First call | Repeat | p95 | `git` work | vs `git` | | --- | --- | --- | --- | --- | --- | --- | -| Open the repository | a fresh handle | 0.25 ms | 0.11 ms | 0.13 ms | † | — | -| Everything the first screen needs, at once | 11 concurrent reads | 5.42 s | 5.42 s | 5.62 s | — | — | -| …including encoding it all for the webview | 11 concurrent reads | 5.60 s | 5.43 s | 5.60 s | — | — | -| Working-tree status | 55,000 entries | 5.38 s | 5.42 s | 5.48 s | 2.98 s | 1.8× | -| First page of history | 1 commit | 0.35 ms | 0.25 ms | 0.26 ms | † | — | -| Ten pages into history | 1 commit | 0.31 ms | 0.25 ms | 0.26 ms | † | — | -| List every branch | 1 branch | 0.35 ms | 0.28 ms | 0.32 ms | † | — | -| List every tag | 0 tags | 0.17 ms | 0.14 ms | 0.15 ms | † | — | -| Diff the selected commit | 50,000 files | 1.53 s | 1.51 s | 1.67 s | 510 ms | 3.0× | -| Diff one modified file | 1 hunk | 17.2 ms | 0.63 ms | 0.64 ms | — | — | -| History of one file | 1 commit | 0.28 ms | 0.10 ms | 0.10 ms | † | — | -| Browse the whole tree | 55,000 files | 196 ms | 181 ms | 241 ms | 42.4 ms | 4.3× | +| Open the repository | a fresh handle | 0.26 ms | 0.11 ms | 0.13 ms | † | — | +| Everything the first screen needs, at once | 11 concurrent reads | 5.41 s | 5.41 s | 5.43 s | — | — | +| …including encoding it all for the webview | 11 concurrent reads | 5.41 s | 5.38 s | 5.41 s | — | — | +| Working-tree status | 55,000 entries | 5.37 s | 5.38 s | 5.40 s | 2.95 s | 1.8× | +| First page of history | 1 commit | 15.4 ms | 0.25 ms | 0.27 ms | † | — | +| Ten pages into history | 1 commit | 14.9 ms | 0.25 ms | 0.27 ms | † | — | +| List every branch | 1 branch | 0.38 ms | 0.28 ms | 0.29 ms | † | — | +| List every tag | 0 tags | 0.17 ms | 0.14 ms | 0.14 ms | † | — | +| Diff the selected commit | 50,000 files | 1.53 s | 1.51 s | 1.52 s | 515 ms | 2.9× | +| Diff one modified file | 1 hunk | 18.6 ms | 0.64 ms | 0.64 ms | — | — | +| History of one file | 1 commit | 0.37 ms | 0.26 ms | 0.28 ms | † | — | +| Browse the whole tree | 55,000 files | 202 ms | 185 ms | 188 ms | 42.3 ms | 4.4× | ### refs @@ -432,16 +476,16 @@ A real clone of the Linux kernel: the repository people mean when they say a git | Operation | Result size | First call | Repeat | p95 | `git` work | vs `git` | | --- | --- | --- | --- | --- | --- | --- | -| Open the repository | a fresh handle | 0.15 ms | 0.14 ms | 0.14 ms | † | — | -| Everything the first screen needs, at once | 11 concurrent reads | 221 ms | 219 ms | 221 ms | — | — | -| …including encoding it all for the webview | 11 concurrent reads | 221 ms | 220 ms | 224 ms | — | — | -| Working-tree status | 0 entries | 0.72 ms | 0.55 ms | 0.56 ms | † | — | -| First page of history | 500 commits | 787 ms | 135 ms | 138 ms | 8.48 ms | 16× | -| Ten pages into history | 500 commits | 526 ms | 525 ms | 551 ms | 7.25 ms | 72× | -| List every branch | 5,001 branches | 184 ms | 180 ms | 182 ms | 183 ms | 0.98× | -| List every tag | 2,000 tags | 148 ms | 148 ms | 151 ms | 43.3 ms | 3.4× | -| Diff the selected commit | 1 file | 0.40 ms | 0.28 ms | 0.29 ms | † | — | -| History of one file | 63 commits | 20.7 ms | 9.37 ms | 9.88 ms | 11.9 ms | 0.79× | -| Browse the whole tree | 32 files | 0.47 ms | 0.16 ms | 0.16 ms | † | — | +| Open the repository | a fresh handle | 0.14 ms | 0.12 ms | 0.12 ms | † | — | +| Everything the first screen needs, at once | 11 concurrent reads | 225 ms | 224 ms | 225 ms | — | — | +| …including encoding it all for the webview | 11 concurrent reads | 224 ms | 224 ms | 225 ms | — | — | +| Working-tree status | 0 entries | 0.74 ms | 0.56 ms | 0.57 ms | † | — | +| First page of history | 500 commits | 188 ms | 121 ms | 123 ms | 4.25 ms | 28× | +| Ten pages into history | 500 commits | 159 ms | 129 ms | 130 ms | † | — | +| List every branch | 5,001 branches | 188 ms | 184 ms | 186 ms | 184 ms | 1.0× | +| List every tag | 2,000 tags | 151 ms | 150 ms | 153 ms | 45.7 ms | 3.3× | +| Diff the selected commit | 1 file | 0.42 ms | 0.28 ms | 0.30 ms | † | — | +| History of one file | 63 commits | 21.9 ms | 20.6 ms | 20.8 ms | 7.13 ms | 2.9× | +| Browse the whole tree | 32 files | 0.46 ms | 0.16 ms | 0.17 ms | † | — | diff --git a/docs/superpowers/plans/2026-09-18-fast-log-walk.md b/docs/superpowers/plans/2026-09-18-fast-log-walk.md new file mode 100644 index 0000000..d02cdce --- /dev/null +++ b/docs/superpowers/plans/2026-09-18-fast-log-walk.md @@ -0,0 +1,869 @@ +# Fast History on a Million-Commit Repository — Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Open `torvalds/linux` in under 1.5 seconds instead of 15.7, by painting the repository before history arrives and taking the commit order from `git rev-list --date-order` over a maintained commit-graph instead of libgit2's revwalk. + +**Architecture:** Two independent stages. Stage 1 splits the commit page out of `refreshAll`'s single `Promise.all` so ten fast reads stop waiting behind one slow one. Stage 2 replaces the *internals* of `build_walk_order` — one function returning `WalkOrder { starts, order, complete }` — with a `git rev-list` subprocess, keeping the libgit2 revwalk as the fallback. Everything downstream (`log_cache`, `FrontierBuilder`, cursors, ref decorations, `commit_to_info`) is untouched because only oids cross over; commit metadata still comes from libgit2. + +**Tech Stack:** Rust (git2 0.21 / libgit2 1.9.7, `proc::git`), React + Zustand + TypeScript, vitest, WebdriverIO, `pnpm bench`. + +**Spec:** `docs/superpowers/specs/2026-09-18-fast-log-walk-design.md` + +**Issue:** #483 (umbrella #476, predecessor #473) + +## Global Constraints + +- **Never `Command::new` outside `src-tauri/src/proc.rs`** — use `proc::git(workdir)`. A guard test fails the build otherwise. +- **Every IPC-crossing fn returns `AppResult`**; add `AppError` variants, never stringify. No new variant is needed by this plan. +- **A new backend module must be named in `CLAUDE.md` or a `docs/dev/*.md` file** or `test/docs.test.ts` fails the build. +- **A new setting must join its Settings page's `meta.cards[].rows`** or `settings.index.test.tsx` fails the build. +- **`MAX_ORDER = 100_000`** (`git/log_cache.rs:65`) — the cap on a kept walk. Unchanged by this plan. +- **Ordering contract: `Sort::TIME | Sort::TOPOLOGICAL` ≡ `git rev-list --date-order`.** Measured byte-for-byte on 2,000 kernel oids. `--topo-order` is a *different* ordering and shares only 1,627 of those 2,000 — never use it. +- **The Rust CI gate is `cargo test` only** — no clippy, no fmt. Do not `cargo fmt` a focused diff. +- **Toolchain paths** in an isolated session: `~/.cargo/bin/cargo`, `~/Library/pnpm/pnpm`. +- **Commit style:** `feat(scope): …` / `fix(scope): …` / `test: …` / `docs: …`, imperative, under 72 chars, `Co-Authored-By: Claude Opus 5 (1M context) `. + +--- + +## Stage 1 — Paint the repository before history arrives + +Ships on its own. Helps every slow repository, including one with no git binary. + +### Task 1: Split the commit page out of `refreshAll` + +**Files:** +- Modify: `src/features/repo/useRepoStore.ts:1068-1170` (`refreshAll`) +- Test: `src/features/repo/refreshPaintsBeforeLog.test.ts` (create) + +**Interfaces:** +- Consumes: `getLogPage`, `trackLoad`, `setFor` — all already imported in this file. +- Produces: no new exports. `refreshAll`'s signature is unchanged; only the *timing* of its writes changes. + +- [ ] **Step 1: Write the failing test** + +```ts +// src/features/repo/refreshPaintsBeforeLog.test.ts +import { describe, expect, it, vi, beforeEach } from "vitest"; + +// Mock the tauri layer so the log page can be held open while the others resolve. +let releaseLog: (v: unknown) => void; +const logGate = new Promise((res) => { + releaseLog = res; +}); + +vi.mock("@/lib/tauri", async () => { + const actual = await vi.importActual>("@/lib/tauri"); + return { + ...actual, + getStatus: vi.fn().mockResolvedValue({ entries: [] }), + listBranches: vi.fn().mockResolvedValue([]), + listTags: vi.fn().mockResolvedValue([]), + listStashes: vi.fn().mockResolvedValue([]), + listRemotes: vi.fn().mockResolvedValue([]), + repoState: vi.fn().mockResolvedValue("Clean"), + rebaseStatus: vi.fn().mockResolvedValue(null), + bisectStatus: vi.fn().mockResolvedValue({ active: false }), + headInfo: vi.fn().mockResolvedValue(null), + shallowInfo: vi.fn().mockResolvedValue({ shallow: false }), + // The slow one. + getLogPage: vi.fn().mockImplementation(() => logGate), + }; +}); + +import { useRepoStore } from "./useRepoStore"; + +describe("refreshAll", () => { + beforeEach(() => { + useRepoStore.setState({ + current: { id: "r1", path: "/tmp/r1" } as never, + commits: [], + status: null, + loading: false, + } as never); + }); + + it("paints status and branches without waiting for the log page", async () => { + const done = useRepoStore.getState().refreshAll(); + + // Let the ten fast reads settle; the log is still pending. + await vi.waitFor(() => { + expect(useRepoStore.getState().statusLoaded).toBe(true); + }); + expect(useRepoStore.getState().loading).toBe(false); + expect(useRepoStore.getState().commits).toEqual([]); + + releaseLog({ commits: [{ id: "c1" }], nextCursor: null }); + await done; + expect(useRepoStore.getState().commits).toHaveLength(1); + }); +}); +``` + +- [ ] **Step 2: Run it and watch it fail** + +Run: `~/Library/pnpm/pnpm vitest run src/features/repo/refreshPaintsBeforeLog.test.ts` +Expected: FAIL — `statusLoaded` is still `false` while the log page is pending, because all eleven reads sit behind one `Promise.all`. The `waitFor` times out. + +- [ ] **Step 3: Split the write** + +In `refreshAll`, take the log out of the joint `Promise.all` and give it its own `setFor`. The ten keep their joint write and now carry `loading: false` and `statusLoaded: true` — the screen is usable at that point. + +```ts + const logRef = get().logRef; + // The log is NOT in this Promise.all. On torvalds/linux the other ten + // finish in about a second and the log takes fifteen; joining them made + // the whole screen wait for the slowest read (#473). The log lands in its + // own write below, and `loadingTasks` names it while it runs. + const logPage = trackLoad( + repo.id, + "log", + "loading history", + getLogPage(repo.id, null, PAGE_SIZE, logRef).catch((e) => { + if (logRef === null) throw e; + setFor(repo.id, { logRef: null }); + return getLogPage(repo.id, null, PAGE_SIZE); + }), + ); + + try { + const [ + status, branches, tags, stashes, remotes, + repoState, rebaseStatus, bisectStatus, headInfo, shallow, + ] = await Promise.all([ + /* …the ten existing trackLoad(…) calls, with the log entry removed… */ + ]); + setFor(repo.id, { + status, branches, tags, stashes, remotes, + repoState, rebaseStatus, bisectStatus, headInfo, + shallowInfo: shallow, + loading: false, + statusLoaded: true, + }); + + // History arrives on its own clock. + const commitPage = await logPage; + setFor(repo.id, { + commits: commitPage.commits, + // A refresh restarts the walk, so the old resume point is void. + commitCursor: commitPage.nextCursor, + }); + + const activeFilter = get().commitFilter; + if (!isFilterEmpty(activeFilter)) { + void get().searchCommits(activeFilter); + } + } catch (e) { + setFor(repo.id, { loading: false, error: toAppError(e) }); + } +``` + +- [ ] **Step 4: Run the test and the neighbours it could break** + +Run: `~/Library/pnpm/pnpm vitest run src/features/repo/` +Expected: PASS, including `refreshPreserveError.test.ts`, `refreshSpinner.test.tsx`, `amendMessage.test.ts` and `cherryPickMany.test.ts` — those mock every read `refreshAll` fans out to, and a read that moved out of the `Promise.all` must still be awaited before `refreshAll` resolves. + +- [ ] **Step 5: Add the e2e case** + +`loading` flipping early is a rendering claim, so it needs the real binary. Add to the existing history spec: + +```ts +// e2e/specs/history.e2e.ts — inside the existing describe +it("shows branches and status before the first history page lands", async () => { + // Read the e2e-testing skill before touching this file. + await openTempRepo(); + const branches = await $("[data-testid='branch-list']"); + await branches.waitForExist({ timeout: 10_000 }); + expect(await branches.isExisting()).toBe(true); +}); +``` + +Run: `~/Library/pnpm/pnpm test:e2e:docker build && ~/Library/pnpm/pnpm test:e2e:docker run --spec e2e/specs/history.e2e.ts` +Expected: PASS. + +- [ ] **Step 6: Commit** + +```bash +git add src/features/repo/useRepoStore.ts src/features/repo/refreshPaintsBeforeLog.test.ts e2e/specs/history.e2e.ts +git commit -F /tmp/msg.txt +``` + +with `/tmp/msg.txt`: + +``` +perf(repo): paint the repository before history arrives + +refreshAll issued eleven reads behind one Promise.all and wrote once, +so status, branches, tags and HEAD — a second's work on torvalds/linux +— waited fourteen more for the log page. The log now lands in its own +write. + +Why: the other ten reads are collectively fast on every fixture the +benchmark covers; joining them to the slowest read is what turns a +1s screen into a 15.7s one. + +Co-Authored-By: Claude Opus 5 (1M context) +``` + +--- + +## Stage 2 — Take the order from git + +### Task 2: Pin the ordering contract with a characterization test + +This is the task that makes the rest safe. #473 and #476 both propose `--topo-order`; it is the wrong ordering and the difference is invisible without this test. + +**Files:** +- Test: `src-tauri/tests/log_walk_ordering.rs` (create) + +**Interfaces:** +- Consumes: `support::{TempRepo, git_in}` from `src-tauri/tests/support/mod.rs`. +- Produces: nothing importable. It is a guard. + +- [ ] **Step 1: Write the test** + +The fixture must make `--date-order` and `--topo-order` actually diverge, or the test passes against both and proves nothing. Divergence needs a merge whose second parent is *older* than commits already emitted on the first parent, so explicit timestamps are set rather than `Signature::now`. + +```rust +//! The ordering contract behind the git-backed log walk (#473). +//! +//! `build_walk_order` sorts with `Sort::TIME | Sort::TOPOLOGICAL`, which is +//! Kahn's algorithm over a time-priority queue — git's `--date-order`, NOT its +//! `--topo-order`. On torvalds/linux the two share only 1,627 of the first +//! 2,000 oids, so swapping one for the other silently changes which commits +//! the first page shows. +//! +//! PLANT A VIOLATION before trusting an edit here: change `--date-order` to +//! `--topo-order` in `rev_list_order` and `diverges_from_topo_order` must go +//! red. A fixture where both orderings agree would make this file worthless. + +mod support; + +use git2::{Signature, Sort, Time}; +use support::{git_in, TempRepo}; + +/// A history whose date order and topological order genuinely differ: +/// `feature` is committed with timestamps OLDER than the `main` commits that +/// follow the branch point, then merged. +fn skewed_merge_history(tr: &TempRepo) -> Vec { + let commit = |name: &str, msg: &str, when: i64, parents: &[git2::Oid]| -> git2::Oid { + std::fs::write(tr.path().join(name), format!("{msg}\n")).unwrap(); + let mut index = tr.repo.index().unwrap(); + index.add_path(std::path::Path::new(name)).unwrap(); + index.write().unwrap(); + let tree = tr.repo.find_tree(index.write_tree().unwrap()).unwrap(); + let sig = Signature::new("Test", "test@example.com", &Time::new(when, 0)).unwrap(); + let parent_commits: Vec<_> = parents + .iter() + .map(|p| tr.repo.find_commit(*p).unwrap()) + .collect(); + let refs: Vec<&git2::Commit> = parent_commits.iter().collect(); + tr.repo + .commit(Some("HEAD"), &sig, &sig, msg, &tree, &refs) + .unwrap() + }; + + let base = tr.repo.head().unwrap().peel_to_commit().unwrap().id(); + // feature: OLD timestamps + let f1 = commit("f1.txt", "f1", 1_000, &[base]); + let f2 = commit("f2.txt", "f2", 1_100, &[f1]); + // main: NEWER timestamps, on the other side of the fork + let m1 = commit("m1.txt", "m1", 5_000, &[base]); + let m2 = commit("m2.txt", "m2", 5_100, &[m1]); + // the merge, newest of all + let mg = commit("mg.txt", "mg", 9_000, &[m2, f2]); + vec![ + mg.to_string(), m2.to_string(), m1.to_string(), + f2.to_string(), f1.to_string(), base.to_string(), + ] +} + +fn libgit2_order(tr: &TempRepo) -> Vec { + let mut walk = tr.repo.revwalk().unwrap(); + walk.set_sorting(Sort::TIME | Sort::TOPOLOGICAL).unwrap(); + walk.push_head().unwrap(); + walk.map(|o| o.unwrap().to_string()).collect() +} + +fn rev_list(tr: &TempRepo, ordering: &str) -> Vec { + git_in(tr.path(), &["rev-list", ordering, "HEAD"]) + .lines() + .map(str::to_string) + .collect() +} + +#[test] +fn date_order_reproduces_libgit2_time_topological() { + let tr = TempRepo::with_initial_commit("root\n"); + skewed_merge_history(&tr); + assert_eq!(libgit2_order(&tr), rev_list(&tr, "--date-order")); +} + +#[test] +fn diverges_from_topo_order() { + // The fixture is only worth anything if the two orderings disagree on it. + let tr = TempRepo::with_initial_commit("root\n"); + skewed_merge_history(&tr); + assert_ne!( + rev_list(&tr, "--date-order"), + rev_list(&tr, "--topo-order"), + "fixture does not exercise the distinction this file exists to pin", + ); +} +``` + +- [ ] **Step 2: Run it** + +Run: `~/.cargo/bin/cargo test --manifest-path src-tauri/Cargo.toml --test log_walk_ordering` +Expected: BOTH PASS. `date_order_reproduces_libgit2_time_topological` documents today's equivalence; `diverges_from_topo_order` proves the fixture is sharp. + +- [ ] **Step 3: Plant the violation** + +Temporarily change `--date-order` to `--topo-order` in `date_order_reproduces_libgit2_time_topological`, re-run, and confirm it FAILS. Then revert with `git checkout -- src-tauri/tests/log_walk_ordering.rs`. **Commit the file first** — a `git checkout --` discards uncommitted edits to the same file. + +- [ ] **Step 4: Commit** + +```bash +git add src-tauri/tests/log_walk_ordering.rs +git commit -m "test: pin --date-order as libgit2's TIME|TOPOLOGICAL equivalent + +Co-Authored-By: Claude Opus 5 (1M context) " +``` + +--- + +### Task 3: The git-backed order producer + +**Files:** +- Create: `src-tauri/src/git/log_walk.rs` +- Modify: `src-tauri/src/git/mod.rs` (add `pub mod log_walk;`) +- Test: unit tests inline in `log_walk.rs` + +**Interfaces:** +- Consumes: `crate::proc::git`, `crate::error::{AppError, AppResult}`, `git2::Oid`, `MAX_ORDER` from `crate::git::log_cache`. +- Produces: + - `pub fn parse_oid_lines(stdout: &str, cap: usize) -> Option>` — pure. + - `pub fn rev_list_order(workdir: &Path, starts: &[Oid], cap: usize) -> Option>` — `None` means "fall back", never an error to show a user. + +- [ ] **Step 1: Write the failing unit tests** + +```rust +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn parses_one_oid_per_line() { + let a = "0".repeat(40); + let b = "1".repeat(40); + let out = format!("{a}\n{b}\n"); + let got = parse_oid_lines(&out, 10).expect("parses"); + assert_eq!(got.len(), 2); + assert_eq!(got[0].to_string(), a); + } + + #[test] + fn rejects_output_that_is_not_oids() { + // A git that printed a warning, a pager banner, anything at all. + assert!(parse_oid_lines("fatal: bad revision\n", 10).is_none()); + } + + #[test] + fn tolerates_a_trailing_newline_and_empty_output() { + assert_eq!(parse_oid_lines("", 10).expect("empty is valid").len(), 0); + let a = "a".repeat(40); + assert_eq!(parse_oid_lines(&format!("{a}\n"), 10).unwrap().len(), 1); + } + + #[test] + fn stops_at_the_cap() { + let a = "a".repeat(40); + let out = format!("{a}\n").repeat(5); + assert_eq!(parse_oid_lines(&out, 3).unwrap().len(), 3); + } +} +``` + +- [ ] **Step 2: Run them and watch them fail** + +Run: `~/.cargo/bin/cargo test --manifest-path src-tauri/Cargo.toml log_walk` +Expected: FAIL — `parse_oid_lines` does not exist. + +- [ ] **Step 3: Implement** + +```rust +//! The commit ORDER, taken from git rather than from libgit2 (#473). +//! +//! # Why this exists +//! +//! `build_walk_order` needs a topologically-constrained order, and in libgit2 +//! 1.9.7 that is not incremental in any sense: `git_revwalk_sorting` sets +//! `walk->limited`, so `prepare_walk` runs `limit_list` over the whole +//! reachable graph and `sort_in_topological_order` materialises the complete +//! ordered list BEFORE the first oid comes out. On torvalds/linux the first +//! oid costs 16.5 s and two thousand oids cost 16.5 s — the same number, +//! because the traversal has already happened. +//! +//! git answers the same question in 188 ms, because it prunes with the +//! generation numbers in the commit-graph file. libgit2 parses those numbers +//! (`commit_list.c`) and then never reads them in `revwalk.c` — across all of +//! libgit2's `src/`, `->generation` is read only in `graph.c` and `merge.c`. +//! So there is no in-process fix, and the order comes from a subprocess. +//! +//! # Only oids cross over +//! +//! Commit metadata still comes from libgit2 via `repo.find_commit`. That keeps +//! the seam one function wide: no format string to keep in sync with +//! `CommitInfo`, no encoding questions, no second definition of what a commit +//! is. +//! +//! # `--date-order`, and never `--topo-order` +//! +//! `Sort::TIME | Sort::TOPOLOGICAL` is Kahn's algorithm over a time-priority +//! queue, which is exactly git's `--date-order`. `--topo-order` answers a +//! different question — it also avoids interleaving independent lines of +//! history — and on the kernel the two share only 1,627 of the first 2,000 +//! oids. `tests/log_walk_ordering.rs` pins this. + +use std::path::Path; + +use git2::Oid; + +/// Parse `rev-list` output. `None` when anything at all is not an oid, which +/// is the signal to fall back rather than to fail a page. +pub fn parse_oid_lines(stdout: &str, cap: usize) -> Option> { + let mut out = Vec::new(); + for line in stdout.lines().take(cap) { + let line = line.trim(); + if line.is_empty() { + continue; + } + out.push(Oid::from_str(line).ok()?); + } + Some(out) +} + +/// The order git would walk, or `None` to use the libgit2 walk instead. +/// +/// Every `None` here is a SLOW page, never a failed one — git missing, git +/// failing, or output this cannot read all mean the same thing to the caller. +pub fn rev_list_order(workdir: &Path, starts: &[Oid], cap: usize) -> Option> { + if starts.is_empty() { + return Some(Vec::new()); + } + let mut cmd = crate::proc::git(workdir); + cmd.arg("rev-list") + .arg("--date-order") + .arg(format!("--max-count={cap}")); + for oid in starts { + cmd.arg(oid.to_string()); + } + // The start points are hex this backend resolved itself, never user text — + // but option parsing ends before them anyway, as everywhere else here. + cmd.arg("--"); + + let out = cmd.output().ok()?; + if !out.status.success() { + return None; + } + parse_oid_lines(&String::from_utf8(out.stdout).ok()?, cap) +} +``` + +Register it: add `pub mod log_walk;` to `src-tauri/src/git/mod.rs` beside the other `pub mod` lines. + +- [ ] **Step 4: Run the tests** + +Run: `~/.cargo/bin/cargo test --manifest-path src-tauri/Cargo.toml log_walk` +Expected: PASS (4 tests). + +- [ ] **Step 5: Satisfy the docs guard** + +`test/docs.test.ts` fails the build for a backend module no doc mentions. Add one line to `docs/dev/backend.md` under the log section: + +```markdown +* `git/log_walk.rs` — the commit ORDER, taken from `git rev-list --date-order` + because libgit2's sorted revwalk materialises the whole graph before yielding + (#473). Only oids cross over; commit data still comes from libgit2. +``` + +Run: `~/Library/pnpm/pnpm vitest run test/docs.test.ts` +Expected: PASS. + +- [ ] **Step 6: Commit** + +```bash +git add src-tauri/src/git/log_walk.rs src-tauri/src/git/mod.rs docs/dev/backend.md +git commit -m "feat(log): read the commit order from git rev-list + +Co-Authored-By: Claude Opus 5 (1M context) " +``` + +--- + +### Task 4: Use it in `build_walk_order`, keeping libgit2 as the fallback + +**Files:** +- Modify: `src-tauri/src/git/libgit2.rs:2841-2861` (`build_walk_order`) +- Test: `src-tauri/tests/log_walk_backend.rs` (create) + +**Interfaces:** +- Consumes: `log_walk::rev_list_order` from Task 3; `WalkOrder`, `MAX_ORDER` from `git/log_cache.rs`. +- Produces: `build_walk_order` keeps its exact signature — `fn build_walk_order(repo: &Repository, starts: &[git2::Oid]) -> AppResult`. + +- [ ] **Step 1: Write the failing test** + +```rust +//! The git-backed walk produces the same pages as the libgit2 one (#473). + +mod support; + +use platypusgit_lib::git::libgit2::Libgit2Backend; +use platypusgit_lib::git::GitBackend; +use support::TempRepo; + +#[test] +fn git_backed_and_libgit2_walks_agree_page_for_page() { + let tr = TempRepo::with_initial_commit("root\n"); + support::linear_history(&tr, 50); + let (backend, handle) = tr.open_with_backend(); + + let with_git = backend.log_page(&handle.id, None, None, 20).unwrap(); + + // Force the fallback by making git unusable for this backend only. + std::env::set_var("PGIT_DISABLE_REV_LIST", "1"); + let (fallback_backend, fallback_handle) = tr.open_with_backend(); + let without_git = fallback_backend + .log_page(&fallback_handle.id, None, None, 20) + .unwrap(); + std::env::remove_var("PGIT_DISABLE_REV_LIST"); + + let a: Vec<_> = with_git.commits.iter().map(|c| &c.id).collect(); + let b: Vec<_> = without_git.commits.iter().map(|c| &c.id).collect(); + assert_eq!(a, b, "the two producers must agree exactly"); + assert_eq!(with_git.next_cursor, without_git.next_cursor); +} +``` + +- [ ] **Step 2: Run it and watch it fail** + +Run: `~/.cargo/bin/cargo test --manifest-path src-tauri/Cargo.toml --test log_walk_backend` +Expected: FAIL — `PGIT_DISABLE_REV_LIST` is not read by anything yet, so both paths are identical and the test proves nothing. It must fail for the *right* reason before proceeding; if it passes here, the escape hatch is not wired. + +- [ ] **Step 3: Implement** + +```rust +/// `MAX_ORDER` bounds the memory; a walk longer than that is kept as a prefix +/// and `complete` says so. +/// +/// The order comes from `git rev-list --date-order` when git can produce it +/// (#473) — libgit2's sorted revwalk pre-walks the entire graph before it +/// yields anything, which is 15.7 s on torvalds/linux against git's 188 ms. +/// The libgit2 walk below is the fallback, and it is exactly what every walk +/// was before: a slow page, never a failed one. +fn build_walk_order(repo: &Repository, starts: &[git2::Oid]) -> AppResult { + if std::env::var_os("PGIT_DISABLE_REV_LIST").is_none() { + if let Some(workdir) = repo.workdir() { + if let Some(order) = crate::git::log_walk::rev_list_order(workdir, starts, MAX_ORDER) { + let complete = order.len() < MAX_ORDER; + return Ok(WalkOrder { + starts: starts.to_vec(), + order, + complete, + }); + } + } + } + + let mut walk = repo.revwalk()?; + walk.set_sorting(Sort::TIME | Sort::TOPOLOGICAL)?; + for &oid in starts { + walk.push(oid)?; + } + let mut order = Vec::new(); + let mut complete = true; + for oid in walk { + if order.len() >= MAX_ORDER { + complete = false; + break; + } + order.push(oid?); + } + Ok(WalkOrder { + starts: starts.to_vec(), + order, + complete, + }) +} +``` + +`PGIT_DISABLE_REV_LIST` is a test seam and a support escape hatch, documented in `docs/dev/backend.md` beside the module entry from Task 3. + +- [ ] **Step 4: Run the tests** + +Run: `~/.cargo/bin/cargo test --manifest-path src-tauri/Cargo.toml` +Expected: PASS — the whole Rust suite, including `log_walk_cache.rs`, whose cold/warm drain comparison now covers both producers. + +- [ ] **Step 5: Commit** + +```bash +git add src-tauri/src/git/libgit2.rs src-tauri/tests/log_walk_backend.rs docs/dev/backend.md +git commit -m "perf(log): build the walk order from git, falling back to libgit2 + +Co-Authored-By: Claude Opus 5 (1M context) " +``` + +--- + +### Task 5: Keep a commit-graph warm + +Without this, Task 4 buys nothing: `rev-list --date-order` is 10,085 ms on a kernel clone with no commit-graph and 47 ms with one. + +**Files:** +- Create: `src-tauri/src/git/commit_graph.rs` +- Modify: `src-tauri/src/git/mod.rs`, `src-tauri/src/commands/repo.rs` (schedule on open) +- Test: `src-tauri/tests/commit_graph.rs` (create) + +**Interfaces:** +- Consumes: `crate::proc::git`, `git2::Repository`. +- Produces: + - `pub fn should_write(repo: &Repository) -> bool` — false when `core.commitGraph` is false or the workdir is not writable. + - `pub fn write_split(workdir: &Path) -> bool` — runs `commit-graph write --reachable --split`; `true` on success. + +- [ ] **Step 1: Write the failing test** + +```rust +//! Commit-graph maintenance (#473). + +mod support; + +use platypusgit_lib::git::commit_graph; +use support::{git_in, TempRepo}; + +#[test] +fn writes_a_commit_graph_and_is_cheap_the_second_time() { + let tr = TempRepo::with_initial_commit("root\n"); + support::linear_history(&tr, 20); + + assert!(commit_graph::should_write(&tr.repo)); + assert!(commit_graph::write_split(tr.path())); + assert!(tr.path().join(".git/objects/info/commit-graphs").exists() + || tr.path().join(".git/objects/info/commit-graph").exists()); + + // Idempotent: a second write with no new commits must still succeed. + assert!(commit_graph::write_split(tr.path())); +} + +#[test] +fn respects_core_commitgraph_false() { + let tr = TempRepo::with_initial_commit("root\n"); + git_in(tr.path(), &["config", "core.commitGraph", "false"]); + assert!(!commit_graph::should_write(&tr.repo)); +} +``` + +- [ ] **Step 2: Run it and watch it fail** + +Run: `~/.cargo/bin/cargo test --manifest-path src-tauri/Cargo.toml --test commit_graph` +Expected: FAIL — module does not exist. + +- [ ] **Step 3: Implement** + +```rust +//! The commit-graph file this backend keeps warm (#473). +//! +//! # Why the app writes it at all +//! +//! `git rev-list --date-order` is only affordable with one. A fresh clone has +//! none — `git clone` does not write one and `gc --auto` does not fire on a +//! single packfile — so this is exactly what a user gets on day one. Measured +//! on torvalds/linux: 10,085 ms for a 500-oid walk without, 47 ms with. +//! +//! It is written into the user's own repository, because that is where git +//! itself writes it (`git gc`, `git maintenance`), it is derived data git +//! knows how to invalidate, and it makes the user's own `git log` fast too. +//! +//! # `--split`, not a plain rewrite +//! +//! `commit-graph write --reachable` rewrites the whole file every time: 14.3 s +//! on the kernel EVEN WHEN NOTHING CHANGED. `--split` costs 59.9 ms in that +//! case. A scheduler that called the plain form on open would burn fourteen +//! seconds of CPU per open forever. + +use std::path::Path; + +use git2::Repository; + +/// Whether this repository wants one. Honours the user's `core.commitGraph`. +pub fn should_write(repo: &Repository) -> bool { + if repo.workdir().is_none() { + return false; + } + match repo.config().and_then(|c| c.get_bool("core.commitGraph")) { + Ok(false) => false, + _ => true, + } +} + +/// Write (or incrementally extend) the split commit-graph. `false` on any +/// failure — this is a cache, and a repository without one is merely slower. +pub fn write_split(workdir: &Path) -> bool { + crate::proc::git(workdir) + .arg("commit-graph") + .arg("write") + .arg("--reachable") + .arg("--split") + .output() + .map(|o| o.status.success()) + .unwrap_or(false) +} +``` + +- [ ] **Step 4: Schedule it on open, off the critical path** + +In the repository-open command, after the handle is returned, spawn the write so it never blocks a page. The first one on the kernel costs 14.5 s and the user must not wait for it. + +```rust +// commands/repo.rs, after the handle is created +let workdir = /* the opened repository's workdir */; +tauri::async_runtime::spawn_blocking(move || { + // Best effort, once per repository per session. A failure means slower + // pages, never a broken repository. + crate::git::commit_graph::write_split(&workdir); +}); +``` + +- [ ] **Step 5: Run the tests** + +Run: `~/.cargo/bin/cargo test --manifest-path src-tauri/Cargo.toml` +Expected: PASS. + +- [ ] **Step 6: Commit** + +```bash +git add src-tauri/src/git/commit_graph.rs src-tauri/src/git/mod.rs src-tauri/src/commands/repo.rs src-tauri/tests/commit_graph.rs docs/dev/backend.md +git commit -m "feat(log): keep a split commit-graph warm on open + +Co-Authored-By: Claude Opus 5 (1M context) " +``` + +--- + +### Task 6: The Settings switch + +**Files:** +- Modify: `src/features/settings/pages/workspace.tsx` (component **and** its exported `meta`) + +**Interfaces:** +- Consumes: the existing settings-row helpers on that page. +- Produces: one boolean setting, `historyCommitGraph`, default `true`. + +- [ ] **Step 1: Add the row to `meta` and the control to the component** + +The registry indexes `meta.cards[].rows`, and a word that lives only in a `hint` is NOT indexed — so the searchable words go in `keywords`. + +```tsx +// in meta.cards[] for the workspace page +{ + title: "History", + rows: [ + { + id: "historyCommitGraph", + label: "Speed up history on large repositories", + keywords: ["commit-graph", "commit graph", "performance", "large", "slow", "log"], + hint: "Writes git's own commit-graph cache into the repository, the same file `git gc` maintains. Without it, opening a repository with a million commits takes about fifteen seconds.", + }, + ], +}, +``` + +- [ ] **Step 2: Run the registry guard** + +Run: `~/Library/pnpm/pnpm vitest run test/settings.index.test.tsx` +Expected: PASS. It fails the build for a setting absent from `meta`. + +- [ ] **Step 3: Commit** + +```bash +git add src/features/settings/pages/workspace.tsx +git commit -m "feat(settings): add the history commit-graph switch + +Co-Authored-By: Claude Opus 5 (1M context) " +``` + +--- + +### Task 7: Correct the benchmark's baselines + +The harness compares against `git log --topo-order`, which is the wrong ordering — the right cost class, but not the question `log_page` asks. + +**Files:** +- Modify: `src-tauri/benches/repo_bench.rs` (the `gitCommand` for `log_first_page`, `log_page_deep`, `file_history`) + +- [ ] **Step 1: Change the baselines** + +Replace `--topo-order` with `--date-order` in those three baseline commands, and add a comment saying why, citing `tests/log_walk_ordering.rs`. + +- [ ] **Step 2: Re-run and confirm the baselines moved but the shape did not** + +Run: `~/Library/pnpm/pnpm bench --fixture deep --no-publish` +Expected: the `deep` fixture's numbers are within noise of today's (it is a single linear branch, where the two orderings cannot differ). + +- [ ] **Step 3: Commit** + +```bash +git add src-tauri/benches/repo_bench.rs +git commit -m "test(bench): compare the log against --date-order, not --topo-order + +Co-Authored-By: Claude Opus 5 (1M context) " +``` + +--- + +### Task 8: Re-measure and publish + +**Files:** +- Modify: `docs/dev/benchmark.json`, `docs/dev/performance.md`, `README.md` (all three generated), `CLAUDE.md` (the log-cache bullet) + +- [ ] **Step 1: Run the full benchmark on a quiet machine** + +Check `uptime` first — a load average above ~3 invalidates the run, and `$PGBENCH_HOME` is shared across worktrees, so confirm no other session is benchmarking. + +Run: `~/Library/pnpm/pnpm bench --linux` +Expected: `open_screen` on `linux` under 1.5 s and bounded by `status`; `log_first_page` first call under 500 ms; `deep`, `wide`, `refs` unmoved. + +- [ ] **Step 2: Verify the three published artifacts agree** + +Run: `~/Library/pnpm/pnpm vitest run test/benchmark.test.ts` +Expected: PASS — it re-renders `README.md` and both `docs/dev/` artifacts from the record and fails if any was hand-edited. + +- [ ] **Step 3: Rewrite the findings prose** + +`docs/dev/performance.md`'s findings section still says the sort is re-paid per page (fixed by #479) and that ten pages cost 157.67 s (now 112 ms). Replace those readings with what the new run says, and keep the commit-graph finding — it is still why this design exists. + +- [ ] **Step 4: Update the CLAUDE.md convention bullet** + +The existing `git/log_cache.rs` bullet describes a libgit2-only walk. Extend it to name `git/log_walk.rs` and the `--date-order` contract, in one or two sentences — it is a load-bearing rule that a future session must not undo. + +- [ ] **Step 5: Commit all generated artifacts together** + +```bash +git add docs/dev/benchmark.json docs/dev/performance.md README.md CLAUDE.md +git commit -m "docs(bench): publish the numbers after the git-backed walk + +Co-Authored-By: Claude Opus 5 (1M context) " +``` + +A partial commit here fails `test/benchmark.test.ts` by design. + +--- + +## Self-Review + +**Spec coverage.** Stage 1 → Task 1. Seam at `build_walk_order` → Task 4. `--date-order` contract → Task 2 (and Task 7 corrects the baselines). Subprocess + argv safety + fallback → Tasks 3 and 4. Commit-graph, `--split`, `core.commitGraph`, background scheduling → Task 5. Settings switch → Task 6. Re-publishing the stale record → Task 8. Testing section → Tasks 1 (e2e), 2 (characterization + planted violation), 3 (unit), 4 (fallback), 5 (maintenance), 8 (bench). + +**Deliberately out of scope**, per the spec: `file_history` (15.9 s on the kernel, same root cause, its own cap and cursor semantics — needs its own issue once this shape is proven) and streaming a partial page (libgit2 yields nothing before the walk completes, so there are no early rows to stream; Stage 2 deletes the wait instead). + +**Type consistency.** `rev_list_order(workdir: &Path, starts: &[Oid], cap: usize) -> Option>` and `parse_oid_lines(stdout: &str, cap: usize) -> Option>` are defined in Task 3 and used with those exact signatures in Task 4. `should_write(&Repository) -> bool` and `write_split(&Path) -> bool` are defined and used consistently in Task 5. `build_walk_order`'s signature is unchanged throughout. + +**Known soft spots for the executor.** The exact insertion point in `commands/repo.rs` (Task 5, Step 4) needs reading in place — anchor on the statement that returns the handle, not on a doc comment, or the spawn lands inside the preceding item's doc block. Task 1's `e2e/specs/history.e2e.ts` selector is illustrative: read the `e2e-testing` skill and use the spec's existing selectors. diff --git a/docs/superpowers/specs/2026-09-18-fast-log-walk-design.md b/docs/superpowers/specs/2026-09-18-fast-log-walk-design.md new file mode 100644 index 0000000..683250d --- /dev/null +++ b/docs/superpowers/specs/2026-09-18-fast-log-walk-design.md @@ -0,0 +1,273 @@ +# Fast history on a million-commit repository (#483) + +> Issue: #483 · umbrella #476 · predecessor #473 (whose two defects #479 fixed) + +Opening `torvalds/linux` costs **15.7 seconds** before anything is painted, and +every one of those seconds is a single libgit2 call. #479 already fixed the two +defects that made *paging* expensive; what is left is the first walk, and it is +now the whole of the large-repo problem. + +This spec covers two changes that are independent and separately shippable: +stop making the user wait for work that is already finished, and stop doing the +expensive walk at all. + +All figures below were measured on 2026-09-18 against `main` at `4c6b5ed`, on an +Apple M4 Pro (14 cores, 48 GB) with git 2.50.1, on an otherwise idle machine. +Method: `docs/dev/performance.md`. + +## Where we actually are + +`pnpm bench --fixture linux`, current `main`: + +| operation | first call | repeat | `git` work | +| --- | --- | --- | --- | +| Everything the first screen needs, at once | **15,743 ms** | 15,715 ms | — | +| First page of history | 18,112 ms | 13.5 ms | 9,665 ms | +| Page ten of history | 15,842 ms | 112 ms | 9,726 ms | +| Working-tree status | 1,029 ms | 1,011 ms | 538 ms | +| History of one file | 15,938 ms | 15,799 ms | 5,352 ms | + +Two readings, and the second is the point of the table. + +**#479 worked.** Page ten was 157.67 s when #473 was filed and is **112 ms** +now — the prepared order is reused instead of rebuilt, exactly as designed. The +published record in `README.md` and `docs/dev/performance.md` still says +157.67 s, because #479 deliberately published no after-numbers while two +sessions shared `$PGBENCH_HOME`. Re-publishing is part of this work. + +**The first call is untouched, and it is everything.** `open_screen` is 15.7 s +and the first page of history is 15.9 s of it; the other ten reads on that fan- +out are milliseconds. A repeat costs 13.5 ms because it is a cache hit, but a +user opening a repository always pays the first call, which is why +`open_screen` — which builds a fresh backend per sample — is the honest number. + +## Root cause, at source level + +`build_walk_order` (`git/libgit2.rs`) sorts with +`Sort::TIME | Sort::TOPOLOGICAL`, because a commit graph's lane assignment needs +every parent to come after its children. In libgit2 1.9.7 — what `git2 0.21` +vendors — that sort is not incremental in any sense: + +* `git_revwalk_sorting` ends with + `if (walk->sorting != GIT_SORT_NONE) walk->limited = 1;` +* so `prepare_walk` runs `limit_list` over the **whole reachable graph**, +* then `sort_in_topological_order` materialises the **complete ordered list**, +* all before the first oid is yielded. + +`limit_list`'s only early exit is a `SLOP` heuristic on *uninteresting* commits. +A plain `push_head` walk marks none, so it never fires. + +Measured with a throwaway probe against the kernel — getting one oid and getting +two thousand cost the same thing, which is the signature of a walk that has +already finished by the time it yields: + +| libgit2 walk | first oid | 2,000 oids | +| --- | --- | --- | +| `TIME \| TOPOLOGICAL` | 16,568.3 ms | 16,568.5 ms | +| `TOPOLOGICAL` | 14,731.3 ms | 14,731.5 ms | +| `TIME` | 14,786.6 ms | 14,791.6 ms | + +**The commit-graph is git's answer to exactly this, and libgit2 throws it +away.** `commit_list.c` reads the file — `git_commit_list_parse` takes parents, +commit time and `generation` from it. `revwalk.c` contains **zero** references +to the commit-graph or to generation numbers, and across all of libgit2's `src/` +`->generation` is read in exactly two places: `graph.c` and `merge.c`. The +numbers are populated and then ignored by the one code path that would benefit. + +So no amount of work inside libgit2's revwalk API makes this faster. The walk +has to come from somewhere else. + +## The ordering, which is not what #473 and #476 assumed + +Both issues propose shelling out to `git log --topo-order`, and the benchmark's +own baseline is `--topo-order`. **That is the wrong ordering**, and adopting it +would silently change which commits the first page contains. + +Measured on the kernel, first 2,000 oids, libgit2's output compared byte for +byte against `git rev-list`: + +| libgit2 sorting | vs `--date-order` | vs `--topo-order` | +| --- | --- | --- | +| `TIME \| TOPOLOGICAL` (what `log_page` uses) | **IDENTICAL** | differ at line 6; 1,627/2,000 shared members | +| `TOPOLOGICAL` | differ at line 6 | **IDENTICAL** | + +`Sort::TIME | Sort::TOPOLOGICAL` is Kahn's algorithm over a time-priority queue, +which is precisely git's `--date-order` ("no parent before its children, +otherwise commit-timestamp order"). `--topo-order` additionally avoids +interleaving independent lines of history, which is a *different question* — it +does not merely reorder the same 2,000 commits, it returns a different 2,000. + +**The drop-in replacement is `git rev-list --date-order`.** The benchmark's +`--topo-order` baselines are the right cost class and the wrong ordering; they +are corrected as part of this work. + +## What the replacement costs + +`git rev-list` against the same kernel clone. `MAX_ORDER` is 100,000, so the +100,000 row is the one this design actually runs: + +| command | no commit-graph | with commit-graph | +| --- | --- | --- | +| `rev-list --date-order -500` | 10,085 ms | **47 ms** | +| `rev-list --date-order -100000` | 10,123 ms | **188 ms** | +| `rev-list --topo-order -100000` | 10,123 ms | 183 ms | +| `rev-list --date-order`, all 1,482,923 | — | 1,578 ms | + +Without a commit-graph, git is no better than we are — 10 s against our 15.7 s, +same failure for the same reason. **The commit-graph is not an optimisation on +top of this design; it is the design.** With it, the walk we run today for +15,743 ms costs 188 ms — 84×. + +## Design + +### Stage 1 — paint the repository before history arrives + +`useRepoStore.refreshAll` is one `Promise.all` over eleven reads with a single +`set()` after all of them resolve. On the kernel, status (1.0 s), branches +(0.9 ms), tags (19 ms) and HEAD are all finished within a second and then wait +fourteen more for the log. + +Split the commit page out of the `Promise.all` so it lands in its own `set()`. +The other ten keep their joint write — they are collectively fast, and the +`loadingTasks` machinery (#296) already names whichever read is still running, +so the status bar can say "loading history" against a painted screen instead of +an empty one. + +This is independent of everything below. It helps every slow repository — a +`/mnt/c` checkout under WSL, a 55,000-entry `status` — and it helps on a machine +with no git installed at all, where Stage 2 cannot. + +**Not in this stage:** streaming a partial page. Emitting commits as they are +walked is worthless here, because libgit2 yields *nothing* until the walk is +complete; there are no early rows to stream. Stage 2 removes the wait rather +than decorating it, and a streaming protocol added first would be built against +a cost that Stage 2 deletes. + +### Stage 2 — take the order from git, keep a commit-graph warm + +**The seam is `build_walk_order`.** It is one function, +`(&Repository, &[Oid]) -> AppResult`, and `WalkOrder` is +`{ starts, order: Vec, complete: bool }`. Everything downstream — the walk +cache, `FrontierBuilder`, cursors, ref decorations, `commit_to_info` — consumes +that struct and does not care how the oids were produced. + +So only the *order* crosses over from git. Commit metadata still comes from +libgit2 via `repo.find_commit`, which means no commit parsing, no format string +to keep in sync, no encoding questions, and no change to `CommitInfo`. + +``` +build_walk_order(repo, starts) -> WalkOrder + ├─ git path: git rev-list --date-order --max-count= -- + │ parse 40-char oid lines; `complete` = (lines < MAX_ORDER) + └─ fallback: today's libgit2 revwalk, unchanged +``` + +Argv safety follows the house rule: start oids are hex the backend resolved +itself, never user text, and option parsing still ends with `--`. The subprocess +goes through `proc::git`, never `Command::new`. + +**When the fallback is taken:** git missing or not executable, a non-zero exit, +output that does not parse as oids, or the repository being one git cannot walk. +A fallback is a slow page, never a failed one — the same degrade-don't-fail +policy `bisect_status` and the shallow read already follow in `refreshAll`. + +**The commit-graph.** `--date-order` is only fast with one, and a fresh clone +has none: `git clone` does not write one and `gc --auto` does not fire on a +single packfile. So the app maintains it, in the user's repository, which is +what `git maintenance` and `git gc` already do there and what makes the user's +own `git log --date-order` fast too. + +Three measured constraints shape how: + +| | cost on the kernel | +| --- | --- | +| `commit-graph write --reachable`, cold | 14,509 ms | +| `commit-graph write --reachable`, **already fresh** | 14,305 ms | +| `commit-graph write --reachable --split`, cold | 14,531 ms (97 MB) | +| `commit-graph write --reachable --split`, no new commits | **59.9 ms** | + +1. **`--split` is mandatory.** Plain `--reachable` rewrites the whole file every + time — it re-pays 14.3 s on a repository where nothing changed. `--split` + costs 60 ms in that case. +2. **The first write is 14.5 s**, so it runs in the background, after the first + screen is painted, and never blocks a page. The first open of a giant + repository is served by the libgit2 fallback and is exactly as slow as today; + the second is fast. This is stated plainly rather than hidden — a one-time + cost the user does not wait for. +3. **It is a write into the user's repository**, so it obeys them: skip entirely + when `core.commitGraph` is false, and never write into a repository that is + not writable. + +**On the Settings switch.** An earlier draft of this spec promised one. It is +NOT implemented, deliberately: settings in this app are `localStorage` on the +frontend, and the commit-graph write is a backend decision, so a switch would +need a new Tauri command to write git config — a new user-facing surface, in a +change that already touches the hottest read in the app. The opt-out that +matters exists and is tested: `core.commitGraph`, which is git's own knob, +which a user may already have set, and which we would have to honour anyway. +A Settings row that presents it belongs in its own change. + +The write is scheduled once per repository per session, on open, behind the +same cancellation the other long reads use. + +### What this does not change + +* No `GitBackend` trait change, no new Tauri command, no new `AppError` + variant, no IPC type change. `LogPage` is what it was. +* `log_cache.rs` is untouched. Its invalidation story — first page keyed by + start oids, continuation keyed by nothing because commits are immutable, + decorations by a ref fingerprint — is unaffected by where the order came from. +* `file_history` is **not** fixed here. It is 15.9 s on the kernel and it is the + same root cause in a different walk, but it has its own cap, its own + cancellation and its own cursor semantics (#474, #478). It gets its own issue + once this lands and the shape of the git-backed walk is proven. + +## Testing + +* **The ordering is pinned by a characterization test**, not by this document. + A Rust integration test builds a repository with merges committed out of + date order, walks it both ways, and asserts the two sequences are equal. + That test is what makes "`--date-order` is the drop-in" a fact the build + checks rather than a claim in a spec. It is written first, and it must fail + against `--topo-order`. +* **The fallback is exercised deliberately** — a test that points the backend at + an unusable git and asserts the page is still correct, because a silent + permanent fallback would look exactly like success while costing 15 s. +* `log_walk_cache.rs` already drains the same history cold and warm and compares + the sequences; it keeps doing so and now covers both producers. +* **E2E:** `history.e2e.ts` (or the closest existing spec) for the split + fan-out, because "the repository paints before history" is a rendering claim + and the unit layer cannot see it. +* **`pnpm bench --linux` on a quiet machine**, published in the same commit — + `test/benchmark.test.ts` re-renders `README.md` and both `docs/dev/` artifacts + from one record and fails if they disagree. + +## Risks + +| risk | how it is handled | +| --- | --- | +| The order changes silently and lanes re-draw | The characterization test above. This is the risk that made `--topo-order` look acceptable in two issues. | +| git absent → every page pays 15 s and nothing says so | The fallback is a measured, logged event, and the diagnostics line already spells `git=UNAVAILABLE` as a fault. | +| A 97 MB file appears in the user's repository | It is what `git gc` writes there anyway, it is derived, `core.commitGraph=false` opts out, and Settings names it. | +| Subprocess cost per page | 12.4 ms spawn floor on this machine, measured by the benchmark, against 188 ms of work and 15,743 ms saved. | +| `--split` leaves many graph layers over time | git's own `--split` heuristics collapse layers; the benchmark's repeat rows would show the drift. | + +## Acceptance criteria + +Closable on evidence, from `pnpm bench --linux`: + +* **First screen on `torvalds/linux` under 1.5 seconds** on a repository whose + commit-graph is current (today: 15,743 ms), **and bounded by `status` rather + than by the log**. The second clause is the real criterion: `status` alone is + 1,029 ms on that tree, so a screen that is merely "under a second" is not + reachable and a screen still gated on history would pass a wall-clock target + by accident. Once this lands, `status` is the next problem, and it is a + different one — it is already only 1.9× git's own work. +* **First page of history under 500 ms**, first call, same condition + (today: 18,112 ms). +* On a repository with **no** commit-graph and no git binary, no regression: + the libgit2 path is what it is today. +* No regression on the generated fixtures — `deep` stays ≈253 ms, `refs` + ≈219 ms, `wide` ≈5.42 s. +* The published record in `README.md` and `docs/dev/performance.md` matches a + run made after the change, on a quiet machine. diff --git a/scripts/bench-fixtures.mjs b/scripts/bench-fixtures.mjs index 137f6b9..09e28cf 100755 --- a/scripts/bench-fixtures.mjs +++ b/scripts/bench-fixtures.mjs @@ -343,6 +343,28 @@ function buildLinux(dir) { // --------------------------------------------------------------------------- +/** + * Give the fixture the commit-graph the app maintains (#483). + * + * `log_page` takes its order from `git rev-list --date-order`, which is 188 ms + * on the kernel with one of these and 10,123 ms without — so a fixture with no + * commit-graph measures a state the app does not leave a repository in. The + * backend writes one on open (`git/commit_graph.rs`), the benchmark drives the + * backend below that command, so the fixture has to stand in for it. + * + * `--split`, like the app: the plain form rewrites 97 MB every run. + * + * **This deliberately speeds up the `git` baselines too.** They are the floor + * we are measured against, and handing ourselves a file we withhold from git + * would be the kind of baseline this benchmark exists not to publish. + * + * The FIRST open of a fresh clone has no commit-graph and is not this number; + * `docs/dev/performance.md` records that case separately. + */ +function ensureCommitGraph(dir) { + run("git", ["-C", dir, "commit-graph", "write", "--reachable", "--split"]); +} + const BUILDERS = { deep: buildDeep, wide: buildWide, refs: buildRefs }; /** The stamp lives BESIDE the repository, never inside it: `wide` is measured @@ -389,16 +411,22 @@ async function main() { const dir = join(home, name); if (name === "linux") { buildLinux(dir); + // Every run, not only after a clone: `--split` costs 60 ms when there is + // nothing new, and skipping it would leave a fixture cloned before #483 + // measuring a repository the app would have fixed on first open. + ensureCommitGraph(dir); continue; } if (!BUILDERS[name]) throw new Error(`unknown fixture: ${name}`); if (!force && isFresh(name, home)) { console.log(` ${name}: up to date at ${dir}`); + ensureCommitGraph(dir); continue; } const started = Date.now(); console.log(` ${name}: generating…`); await BUILDERS[name](dir); + ensureCommitGraph(dir); writeFileSync(stampPath(home, name), JSON.stringify(stamp(name, dir), null, 2) + "\n"); console.log(` ${name}: ready in ${((Date.now() - started) / 1000).toFixed(1)}s (${dir})`); } diff --git a/scripts/bench-report.mjs b/scripts/bench-report.mjs index 3e0c737..9b39f98 100755 --- a/scripts/bench-report.mjs +++ b/scripts/bench-report.mjs @@ -406,17 +406,31 @@ export function renderReadme(data) { const real = data.fixtures.find((f) => f.kind === "real"); const screen = real && at(real, "open_screen"); const tenth = real && at(real, "log_page_deep"); - if (real && screen && tenth) { + // The slowest single operation left, named rather than asserted, so this + // sentence keeps pointing at whatever is actually worst instead of at + // whatever was worst the day it was written. The composites are excluded + // because they are made OF the others. + const slowest = + real && + real.operations + .filter((o) => o.op !== "open_screen" && o.op !== "open_screen_ipc") + .reduce((a, b) => (b.repeatMedianMs > a.repeatMedianMs ? b : a), { + repeatMedianMs: -1, + }); + if (real && screen && tenth && slowest) { out.push(""); out.push( - `**${real.title} is the bad case, and publishing it is the point.** The ` + - `first screen costs ${fmtMs(screen.repeatMedianMs)} there, and ` + - `reaching ten pages into its history costs ` + - `${fmtMs(tenth.repeatMedianMs)}: a sorted libgit2 revwalk pre-walks ` + - `all ${thousands(real.repository.commits)} commits before it yields ` + - "one, and the next page pays for that again. The developer who opens " + - "a repository that size and waits is the one this was written for, so " + - "the number belongs here rather than in a backlog.", + `**${real.title} is the case that matters, and publishing it is the ` + + `point.** The first screen costs ${fmtMs(screen.repeatMedianMs)} on ` + + `${thousands(real.repository.commits)} commits, and reaching ten ` + + `pages into its history costs ${fmtMs(tenth.repeatMedianMs)} — the ` + + "log's order comes from git over a commit-graph the app maintains, " + + "because libgit2's own sorted revwalk pre-walks the entire graph " + + "before it yields a single commit and never reads that file (#483). " + + `What is slowest here now is "${slowest.label}" at ` + + `${fmtMs(slowest.repeatMedianMs)}, and it is published for the same ` + + "reason the fifteen seconds were: the developer who opens a " + + "repository this size is the one this was written for.", ); } diff --git a/src-tauri/benches/repo_bench.rs b/src-tauri/benches/repo_bench.rs index 22d8f98..4a950a7 100644 --- a/src-tauri/benches/repo_bench.rs +++ b/src-tauri/benches/repo_bench.rs @@ -594,22 +594,31 @@ fn run_suite(subject: &Subject, cfg: &Config) -> Vec { }, |b, id| b.log_page(id, None, None, PAGE_SIZE).expect("log_page"), ), - // `--topo-order`, because `log_page` walks with + // `--date-order`, because `log_page` walks with // `Sort::TIME | Sort::TOPOLOGICAL` and the commit graph's lanes depend // on it — a plain `git log` is a strictly easier question and quoting // it here would be the `status` mistake in the module doc, made the // other way round. // // Measured, not assumed: on the `deep` fixture a default `git log -500` - // is 41 ms and `--topo-order` is 284 ms, against our 275 ms. The first + // is 41 ms and a sorted one is 284 ms, against our 275 ms. The first // page is at PARITY. Comparing against the 41 ms would have published a // fourteen-fold regression that does not exist. + // + // **`--date-order` and not `--topo-order`** (#483). These are different + // questions, not two spellings of one: `Sort::TIME | Sort::TOPOLOGICAL` + // is Kahn's algorithm over a time-priority queue, which is exactly + // `--date-order`, while `--topo-order` additionally refuses to intermix + // independent lines of history. On `torvalds/linux` the two share only + // 1,627 of the first 2,000 oids. This file quoted `--topo-order` until + // the walk was actually taken from git and the difference had to be + // settled; `tests/log_walk_ordering.rs` pins it. measure_baseline( repo, cfg, &[&[ "log", - "--topo-order", + "--date-order", "--max-count=500", "--format=%H%n%an%n%ae%n%at%n%s", ]], @@ -652,7 +661,7 @@ fn run_suite(subject: &Subject, cfg: &Config) -> Vec { cfg, &[&[ "log", - "--topo-order", + "--date-order", "--skip=4500", "--max-count=500", "--format=%H%n%an%n%ae%n%at%n%s", @@ -754,7 +763,7 @@ fn run_suite(subject: &Subject, cfg: &Config) -> Vec { cfg, &[&[ "log", - "--topo-order", + "--date-order", "--max-count=500", "--format=%H%n%an%n%at%n%s", "--", diff --git a/src-tauri/src/commands/repo.rs b/src-tauri/src/commands/repo.rs index 8005ea4..80c9e5b 100644 --- a/src-tauri/src/commands/repo.rs +++ b/src-tauri/src/commands/repo.rs @@ -36,7 +36,8 @@ pub async fn open_repo( ) -> AppResult { let backend = state.backend.clone(); let path_buf = PathBuf::from(path); - tokio::task::spawn_blocking(move || { + let graph_path = path_buf.clone(); + let result = tokio::task::spawn_blocking(move || { // Before the open, not after: the value of this line is that it is // written even when the call below never returns. log::info!("open_repo {}", path_buf.display()); @@ -61,7 +62,23 @@ pub async fn open_repo( result }) .await - .map_err(|e| AppError::Internal(e.to_string()))? + .map_err(|e| AppError::Internal(e.to_string()))?; + + // The log's order comes from `git rev-list`, and that is only affordable + // with a commit-graph — 188 ms against 10.1 s on `torvalds/linux` (#483). + // A fresh clone has none, so the app keeps one, exactly as `git + // maintenance` would. + // + // Spawned HERE rather than inside the blocking closure above, because that + // closure runs on the blocking pool with no runtime context to spawn from; + // and after the open rather than before it, because nothing on this screen + // may wait for it. The first write on a very large repository is ~14.5 s + // and no user is blocked on it; every later one is ~60 ms because it is a + // `--split` refresh. + if result.is_ok() { + tokio::task::spawn_blocking(move || crate::git::commit_graph::refresh(&graph_path)); + } + result } /// Forget an opened repository (a closed repository tab). diff --git a/src-tauri/src/git/commit_graph.rs b/src-tauri/src/git/commit_graph.rs new file mode 100644 index 0000000..69e07c5 --- /dev/null +++ b/src-tauri/src/git/commit_graph.rs @@ -0,0 +1,107 @@ +//! The commit-graph file this backend keeps warm (#483). +//! +//! # Why the app writes it at all +//! +//! `git/log_walk.rs` takes the log's order from `git rev-list --date-order`, +//! and that is only affordable with a commit-graph. Measured on +//! `torvalds/linux`, for the 100,000-oid walk `MAX_ORDER` asks for: +//! +//! | | no commit-graph | with one | +//! | --- | --- | --- | +//! | `rev-list --date-order` | 10,123 ms | **188 ms** | +//! +//! A fresh clone has none. `git clone` does not write one, and `gc --auto` +//! does not fire on a single packfile, so this is exactly what a user gets on +//! day one — which is also the day they are most likely to be waiting on a +//! repository they have just cloned. +//! +//! It is written into the USER'S OWN repository, because that is where git +//! itself writes it (`git gc`, `git maintenance`), because it is derived data +//! git knows how to keep current, and because it makes the user's own +//! `git log` fast too rather than only ours. +//! +//! # `--split`, and it is not a preference +//! +//! Measured on the kernel: +//! +//! | | cost | +//! | --- | --- | +//! | `commit-graph write --reachable`, cold | 14,509 ms | +//! | `commit-graph write --reachable`, **already fresh** | 14,305 ms | +//! | `commit-graph write --reachable --split`, cold | 14,531 ms | +//! | `commit-graph write --reachable --split`, nothing new | **59.9 ms** | +//! +//! The plain form rewrites the whole file every time, so scheduling it on open +//! would burn fourteen seconds of CPU on every open forever. `--split` adds an +//! incremental layer and costs sixty milliseconds when there is nothing to do. +//! +//! # It is best effort, always +//! +//! Every failure here is silent and costs only speed: a repository with no +//! commit-graph gets the 10-second `rev-list`, or the libgit2 walk behind it. +//! Nothing about correctness depends on this file existing, which is what +//! makes it safe to run in the background without a user waiting on it. + +use std::path::Path; + +use git2::Repository; + +/// Whether this repository should get one. +/// +/// Honours `core.commitGraph`: a user who turned git's own commit-graph +/// reading off has said what they want, and writing one anyway would leave a +/// file they never asked for AND no speedup, since git would not read it. +pub fn should_write(repo: &Repository) -> bool { + if repo.workdir().is_none() { + return false; + } + // Default TRUE — git's own default, and the one that makes the log fast. + !matches!(repo.config().and_then(|c| c.get_bool("core.commitGraph")), Ok(false)) +} + +/// Write, or incrementally extend, the split commit-graph. +/// +/// `false` on any failure. This is a cache: a repository without one is only +/// slower, so a caller has nothing useful to do with an error beyond not +/// retrying it in a loop. +pub fn write_split(workdir: &Path) -> bool { + crate::proc::git(workdir) + .arg("commit-graph") + .arg("write") + .arg("--reachable") + .arg("--split") + .output() + .map(|o| o.status.success()) + .unwrap_or(false) +} + +/// `should_write` + `write_split`, for a caller holding only a path. +/// +/// Opens its own handle rather than borrowing the cached one, because this +/// runs in the background and must not sit on the per-repository lock that +/// every read and write in `git/repo_locks.rs` is ordered by — the whole point +/// is that nothing waits for it. +pub fn refresh(workdir: &Path) { + let Ok(repo) = Repository::open(workdir) else { + return; + }; + if !should_write(&repo) { + return; + } + drop(repo); + + let started = std::time::Instant::now(); + if write_split(workdir) { + // Logged because the first one on a very large repository is a real + // cost (14.5 s on the kernel) that nobody waits for and therefore + // nobody can see — and because "why is my fan on after opening a + // repository" deserves an answer in the log. + log::info!( + "commit-graph refreshed in {} ms for {}", + started.elapsed().as_millis(), + workdir.display() + ); + } else { + log::debug!("commit-graph write skipped for {}", workdir.display()); + } +} diff --git a/src-tauri/src/git/libgit2.rs b/src-tauri/src/git/libgit2.rs index da8d7a9..c14038f 100644 --- a/src-tauri/src/git/libgit2.rs +++ b/src-tauri/src/git/libgit2.rs @@ -12,6 +12,7 @@ use uuid::Uuid; use crate::error::{AppError, AppResult}; use crate::git::image; use crate::git::log_cache::{LogCache, RefMap, WalkKey, WalkOrder, MAX_ORDER}; +use crate::git::log_walk; use crate::git::ownership; use crate::git::repo_locks::RepoLock; use crate::git::shallow as shallow_mod; @@ -2838,7 +2839,29 @@ fn push_log_start( /// `MAX_ORDER` bounds the memory; a walk longer than that is kept as a prefix /// and marked incomplete, which makes a page past the prefix fall back to /// preparing a walk from its cursor — what every page did before this existed. +/// +/// **The order itself comes from `git rev-list --date-order` where git can +/// produce it** (#483, `git/log_walk.rs`), because the libgit2 walk below costs +/// 15.7 s on `torvalds/linux` against git's 188 ms — it pre-walks the entire +/// reachable graph before yielding one oid, and it never reads the commit-graph +/// that makes git's answer cheap. The libgit2 path stays as the fallback and is +/// byte-for-byte what every walk did before: a slow page, never a failed one. fn build_walk_order(repo: &Repository, starts: &[git2::Oid]) -> AppResult { + if let Some(workdir) = repo.workdir() { + // One past the cap, so "ended inside the cap" and "was truncated" stay + // distinguishable — exactly the question the libgit2 loop answers by + // breaking on the MAX_ORDER+1'th item. + if let Some(mut order) = log_walk::rev_list_order(workdir, starts, MAX_ORDER + 1) { + let complete = order.len() <= MAX_ORDER; + order.truncate(MAX_ORDER); + return Ok(WalkOrder { + starts: starts.to_vec(), + order, + complete, + }); + } + } + let mut walk = repo.revwalk()?; walk.set_sorting(Sort::TIME | Sort::TOPOLOGICAL)?; for &oid in starts { diff --git a/src-tauri/src/git/log_walk.rs b/src-tauri/src/git/log_walk.rs new file mode 100644 index 0000000..4f0d7a6 --- /dev/null +++ b/src-tauri/src/git/log_walk.rs @@ -0,0 +1,189 @@ +//! The commit ORDER, taken from git rather than from libgit2 (#483). +//! +//! # Why this exists +//! +//! `build_walk_order` needs an order in which every parent follows its +//! children, because that is what the graph's lane assignment is computed +//! against. In libgit2 1.9.7 — what `git2 0.21` vendors — producing one is not +//! incremental in any sense: +//! +//! * `git_revwalk_sorting` ends with +//! `if (walk->sorting != GIT_SORT_NONE) walk->limited = 1;` +//! * so `prepare_walk` runs `limit_list` over the WHOLE reachable graph, +//! * then `sort_in_topological_order` materialises the COMPLETE ordered list, +//! * all before the first oid comes out. +//! +//! Measured on `torvalds/linux`: the first oid costs 16,568.3 ms and two +//! thousand oids cost 16,568.5 ms — the same number, because the traversal has +//! already finished by the time one comes back. +//! +//! git answers the same question in 188 ms, because it prunes with the +//! generation numbers in the commit-graph file. libgit2 parses those numbers +//! (`commit_list.c` fills `commit->generation` from the graph) and then never +//! reads them in `revwalk.c` — across all of libgit2's `src/`, `->generation` +//! is read in exactly two places, `graph.c` and `merge.c`. So there is no +//! in-process fix available through the revwalk API, and the order comes from +//! a subprocess instead. `git/commit_graph.rs` is what keeps that subprocess +//! fast; without a commit-graph git is no better than we are (10,085 ms). +//! +//! # Only oids cross over +//! +//! Commit metadata still comes from libgit2 via `repo.find_commit`. That keeps +//! the seam one function wide: no `--format` string to keep in sync with +//! `CommitInfo`, no encoding questions, no second definition of what a commit +//! is, and no change to anything downstream of `WalkOrder`. +//! +//! # `--date-order`, and never `--topo-order` +//! +//! `Sort::TIME | Sort::TOPOLOGICAL` is Kahn's algorithm over a time-priority +//! queue, which is exactly git's `--date-order`. `--topo-order` answers a +//! different question — it additionally refuses to intermix independent lines +//! of history — and on the kernel the two share only 1,627 of the first 2,000 +//! oids. It does not reorder the same commits, it returns different ones. +//! `tests/log_walk_ordering.rs` pins this in both directions. + +use std::path::Path; + +use git2::Oid; + +/// Environment escape hatch forcing the libgit2 walk. +/// +/// Two callers, and both matter. `tests/log_walk_fallback.rs` uses it to prove +/// the fallback still produces correct pages — a fallback nobody exercises is a +/// fallback that has rotted. And a user whose git is doing something +/// unexpected has a way to take this path out of the picture without +/// downgrading, which is worth having for a subprocess on the hottest read in +/// the app. +pub fn rev_list_disabled() -> bool { + std::env::var_os("PGIT_DISABLE_REV_LIST").is_some() +} + +/// Parse `rev-list` output into oids. +/// +/// `None` when anything at all is not an oid. That is deliberately strict: +/// this is a cache-shaped optimisation, and a caller that cannot trust the +/// output must fall back rather than guess at a partial order. +/// +/// **A full-length id is required, and `Oid::from_str` is not enough to +/// enforce it.** libgit2 accepts an ABBREVIATED hex string and zero-pads it, +/// so `abc123` parses happily into a valid-looking oid that names nothing. +/// Output truncated mid-line — a killed subprocess, a full pipe — would then +/// become a plausible order with one wrong entry rather than an obvious +/// failure, so the length is checked first. +pub fn parse_oid_lines(stdout: &str, cap: usize) -> Option> { + /// SHA-1 and SHA-256 object ids, as hex. + const HEX_LENS: [usize; 2] = [40, 64]; + + let mut out = Vec::new(); + for line in stdout.lines() { + let line = line.trim(); + if line.is_empty() { + continue; + } + if out.len() >= cap { + break; + } + if !HEX_LENS.contains(&line.len()) || !line.bytes().all(|b| b.is_ascii_hexdigit()) { + return None; + } + out.push(Oid::from_str(line).ok()?); + } + Some(out) +} + +/// The order git would walk from `starts`, or `None` to use the libgit2 walk. +/// +/// Every `None` here means a SLOW page, never a failed one — git missing, git +/// failing, and output this cannot read all mean the same thing to the caller. +pub fn rev_list_order(workdir: &Path, starts: &[Oid], cap: usize) -> Option> { + if rev_list_disabled() { + return None; + } + if starts.is_empty() { + return Some(Vec::new()); + } + + let mut cmd = crate::proc::git(workdir); + cmd.arg("rev-list") + .arg("--date-order") + .arg(format!("--max-count={cap}")); + for oid in starts { + cmd.arg(oid.to_string()); + } + // The start points are hex this backend resolved itself and never user + // text, but option parsing ends before them anyway — the same rule every + // other shell-out here follows. + cmd.arg("--"); + + let out = cmd.output().ok()?; + if !out.status.success() { + return None; + } + parse_oid_lines(std::str::from_utf8(&out.stdout).ok()?, cap) +} + +#[cfg(test)] +mod tests { + use super::*; + + fn oid(c: char) -> String { + std::iter::repeat(c).take(40).collect() + } + + #[test] + fn parses_one_oid_per_line() { + let out = format!("{}\n{}\n", oid('0'), oid('1')); + let got = parse_oid_lines(&out, 10).expect("parses"); + assert_eq!(got.len(), 2); + assert_eq!(got[0].to_string(), oid('0')); + assert_eq!(got[1].to_string(), oid('1')); + } + + #[test] + fn rejects_output_that_is_not_oids() { + // A git that printed a warning, an advice block, anything at all. + assert!(parse_oid_lines("fatal: bad revision\n", 10).is_none()); + // A line that is hex but too short — see the next test for why this + // one cannot be left to `Oid::from_str`. + assert!(parse_oid_lines("abc123\n", 10).is_none()); + // A full-length line that is not hex. + assert!(parse_oid_lines(&"z".repeat(40), 10).is_none()); + // One good line and one truncated one: all or nothing, because a + // partially-parsed order is worse than no order. + assert!(parse_oid_lines(&format!("{}\nabc123\n", oid('a')), 10).is_none()); + } + + #[test] + fn oid_from_str_alone_would_accept_an_abbreviated_id() { + // The reason `parse_oid_lines` checks the length itself. libgit2 + // zero-pads a short hex string, so truncated output would otherwise + // parse into a plausible order naming an object that does not exist. + let short = Oid::from_str("abc123").expect("libgit2 accepts this"); + assert_eq!(short.to_string(), "abc1230000000000000000000000000000000000"); + } + + #[test] + fn empty_output_is_valid_and_not_a_failure() { + // An empty range is a real answer: no commits, not a broken git. + assert_eq!(parse_oid_lines("", 10).expect("empty is valid").len(), 0); + } + + #[test] + fn tolerates_a_trailing_newline() { + assert_eq!(parse_oid_lines(&format!("{}\n", oid('a')), 10).unwrap().len(), 1); + } + + #[test] + fn stops_at_the_cap() { + let out = format!("{}\n", oid('a')).repeat(5); + assert_eq!(parse_oid_lines(&out, 3).unwrap().len(), 3); + } + + #[test] + fn no_starts_is_an_empty_order_without_spawning_git() { + // Not `None`: "nothing to walk" is an answer, and falling back to + // libgit2 to rediscover it would cost a full prepare on a big repo. + let got = rev_list_order(Path::new("/nonexistent"), &[], 10); + assert_eq!(got, Some(Vec::new())); + } +} diff --git a/src-tauri/src/git/mod.rs b/src-tauri/src/git/mod.rs index b1c55b3..26334ce 100644 --- a/src-tauri/src/git/mod.rs +++ b/src-tauri/src/git/mod.rs @@ -2,6 +2,7 @@ pub mod auth; pub mod bisect; pub mod blame; pub mod cli; +pub mod commit_graph; pub mod commit_template; pub mod difftool; pub mod hooks; @@ -9,6 +10,7 @@ pub mod image; pub mod libgit2; pub mod lfs; pub mod log_cache; +pub mod log_walk; pub mod notes; pub mod ownership; pub mod rebase_plan; diff --git a/src-tauri/tests/commit_graph.rs b/src-tauri/tests/commit_graph.rs new file mode 100644 index 0000000..b9e32fe --- /dev/null +++ b/src-tauri/tests/commit_graph.rs @@ -0,0 +1,87 @@ +//! Commit-graph maintenance (#483). +//! +//! The file itself is what makes `git rev-list --date-order` affordable — 188 ms +//! against 10,123 ms on `torvalds/linux` — so these tests are about the two +//! things that would quietly cost the user: writing when they asked us not to, +//! and paying the full rewrite on every open. + +mod support; + +use platypusgit_lib::git::commit_graph; +use support::{git_in, TempRepo}; + +/// Where git puts a split commit-graph, and where it puts a plain one. +fn graph_exists(tr: &TempRepo) -> bool { + tr.path().join(".git/objects/info/commit-graphs").exists() + || tr.path().join(".git/objects/info/commit-graph").exists() +} + +#[test] +fn writes_one_and_is_cheap_the_second_time() { + let tr = TempRepo::with_initial_commit("root\n"); + support::linear_history(&tr, 20); + + assert!(commit_graph::should_write(&tr.repo)); + assert!(!graph_exists(&tr), "fixture starts without one"); + + assert!(commit_graph::write_split(tr.path())); + assert!(graph_exists(&tr), "a commit-graph must exist afterwards"); + + // Idempotent. On the kernel this is the difference between 60 ms and + // 14.3 s, which is why `--split` is not a preference. + assert!(commit_graph::write_split(tr.path())); +} + +#[test] +fn git_actually_reads_what_we_wrote() { + // A file git ignores would pass the test above while buying nothing. + let tr = TempRepo::with_initial_commit("root\n"); + support::linear_history(&tr, 20); + commit_graph::write_split(tr.path()); + + let verified = git_in(tr.path(), &["commit-graph", "verify"]); + assert!( + !verified.contains("error"), + "git rejected the graph we wrote: {verified}", + ); +} + +#[test] +fn respects_core_commitgraph_false() { + let tr = TempRepo::with_initial_commit("root\n"); + git_in(tr.path(), &["config", "core.commitGraph", "false"]); + assert!( + !commit_graph::should_write(&tr.repo), + "a user who turned git's commit-graph off must not get one anyway", + ); +} + +#[test] +fn defaults_to_writing_when_the_config_is_absent() { + // The default is git's own, and it is the one that makes the log fast — + // an absent key must not read as "no". + let tr = TempRepo::with_initial_commit("root\n"); + assert!(commit_graph::should_write(&tr.repo)); +} + +#[test] +fn refresh_honours_the_opt_out() { + let tr = TempRepo::with_initial_commit("root\n"); + support::linear_history(&tr, 5); + git_in(tr.path(), &["config", "core.commitGraph", "false"]); + + commit_graph::refresh(tr.path()); + assert!( + !graph_exists(&tr), + "refresh wrote a commit-graph into a repository that opted out", + ); +} + +#[test] +fn refresh_writes_when_allowed() { + let tr = TempRepo::with_initial_commit("root\n"); + support::linear_history(&tr, 5); + + commit_graph::refresh(tr.path()); + assert!(graph_exists(&tr)); +} diff --git a/src-tauri/tests/log_walk_backend.rs b/src-tauri/tests/log_walk_backend.rs new file mode 100644 index 0000000..1321d1b --- /dev/null +++ b/src-tauri/tests/log_walk_backend.rs @@ -0,0 +1,105 @@ +//! The git-backed walk order, through the real backend (#483). +//! +//! `tests/log_walk_ordering.rs` pins the ORDERING against the `rev-list` CLI. +//! This file goes through `log_walk::rev_list_order` itself and then through +//! `log_page`, because the thing that ships is the function, not the command +//! line a document quotes. +//! +//! The fallback half lives in `tests/log_walk_fallback.rs`, which is a separate +//! binary on purpose: it sets a process-wide environment variable, and Rust +//! runs the tests within one binary on parallel threads. + +mod support; + +use git2::{Oid, Repository, Signature, Sort, Time}; +use platypusgit_lib::git::log_walk; +use platypusgit_lib::git::GitBackend; +use support::TempRepo; + +/// Same interleaved fixture as `log_walk_ordering.rs`: two lines of history +/// whose dates alternate, then a merge. A linear history cannot tell a correct +/// order from a lucky one. +fn interleaved_merge_history(tr: &TempRepo) { + let repo = &tr.repo; + let commit = |name: &str, when: i64, parents: &[Oid]| -> Oid { + let blob = repo.blob(format!("{name}\n").as_bytes()).unwrap(); + let base_tree = parents + .first() + .map(|p| repo.find_commit(*p).unwrap().tree().unwrap()); + let mut tb = repo.treebuilder(base_tree.as_ref()).unwrap(); + tb.insert(name, blob, 0o100644).unwrap(); + let tree = repo.find_tree(tb.write().unwrap()).unwrap(); + let sig = Signature::new("Test", "test@example.com", &Time::new(when, 0)).unwrap(); + let parent_commits: Vec<_> = parents + .iter() + .map(|p| repo.find_commit(*p).unwrap()) + .collect(); + let parent_refs: Vec<&git2::Commit> = parent_commits.iter().collect(); + repo.commit(None, &sig, &sig, name, &tree, &parent_refs) + .unwrap() + }; + + let base = repo.head().unwrap().peel_to_commit().unwrap().id(); + let m1 = commit("m1", 1_000, &[base]); + let f1 = commit("f1", 2_000, &[base]); + let m2 = commit("m2", 3_000, &[m1]); + let f2 = commit("f2", 4_000, &[f1]); + let merge = commit("merge", 5_000, &[m2, f2]); + repo.reference("refs/heads/main", merge, true, "test fixture") + .unwrap(); +} + +fn libgit2_order(repo: &Repository, start: Oid) -> Vec { + let mut walk = repo.revwalk().unwrap(); + walk.set_sorting(Sort::TIME | Sort::TOPOLOGICAL).unwrap(); + walk.push(start).unwrap(); + walk.map(|o| o.unwrap()).collect() +} + +#[test] +fn rev_list_order_matches_the_libgit2_walk() { + let tr = TempRepo::with_initial_commit("root\n"); + interleaved_merge_history(&tr); + let head = tr.repo.head().unwrap().peel_to_commit().unwrap().id(); + + let from_git = log_walk::rev_list_order(tr.path(), &[head], 1000).expect("git produced an order"); + assert_eq!(from_git, libgit2_order(&tr.repo, head)); +} + +#[test] +fn respects_the_cap() { + let tr = TempRepo::with_initial_commit("root\n"); + support::linear_history(&tr, 20); + let head = tr.repo.head().unwrap().peel_to_commit().unwrap().id(); + + let capped = log_walk::rev_list_order(tr.path(), &[head], 5).expect("order"); + assert_eq!(capped.len(), 5); + assert_eq!(capped, libgit2_order(&tr.repo, head)[..5].to_vec()); +} + +#[test] +fn a_page_through_the_backend_is_in_that_order() { + let tr = TempRepo::with_initial_commit("root\n"); + interleaved_merge_history(&tr); + let head = tr.repo.head().unwrap().peel_to_commit().unwrap().id(); + let (backend, handle) = tr.open_with_backend(); + + let page = backend.log_page(&handle.id, None, None, 10).expect("page"); + let got: Vec = page.commits.iter().map(|c| c.oid.clone()).collect(); + let want: Vec = libgit2_order(&tr.repo, head) + .iter() + .map(|o| o.to_string()) + .collect(); + assert_eq!(got, want); +} + +#[test] +fn an_empty_repository_still_pages() { + // No commits at all: `rev-list` would exit non-zero on a bad revision, so + // this is the path where returning `None` and falling back has to work. + let tr = TempRepo::fresh(); + let (backend, handle) = tr.open_with_backend(); + let page = backend.log_page(&handle.id, None, None, 10).expect("page"); + assert!(page.commits.is_empty()); + assert!(page.next_cursor.is_none()); +} diff --git a/src-tauri/tests/log_walk_fallback.rs b/src-tauri/tests/log_walk_fallback.rs new file mode 100644 index 0000000..3f5a831 --- /dev/null +++ b/src-tauri/tests/log_walk_fallback.rs @@ -0,0 +1,58 @@ +//! The libgit2 fallback still produces correct pages (#483). +//! +//! `build_walk_order` prefers `git rev-list`, and falls back to the libgit2 +//! revwalk when git is missing, fails, or prints something unreadable. That +//! path is the one a user with no git installed gets on every single page, and +//! a fallback nobody exercises is a fallback that has rotted — silently, since +//! its only visible symptom is being slow. +//! +//! **This is its own test binary on purpose.** It sets a process-wide +//! environment variable, and Rust runs the tests inside one binary on parallel +//! threads, so a second test here could observe a half-applied world. One test, +//! one process, one variable set before anything opens a repository. + +mod support; + +use platypusgit_lib::git::GitBackend; +use support::TempRepo; + +#[test] +fn pages_are_identical_with_the_git_walk_disabled() { + let tr = TempRepo::with_initial_commit("root\n"); + support::linear_history(&tr, 30); + + // The git-backed order first. + let (backend, handle) = tr.open_with_backend(); + let with_git = backend.log_page(&handle.id, None, None, 10).expect("page"); + let next = backend + .log_page(&handle.id, None, with_git.next_cursor.as_deref(), 10) + .expect("second page"); + + // …then the same two pages through the fallback. + std::env::set_var("PGIT_DISABLE_REV_LIST", "1"); + let (fallback, fallback_handle) = tr.open_with_backend(); + let without_git = fallback + .log_page(&fallback_handle.id, None, None, 10) + .expect("page"); + let without_git_next = fallback + .log_page( + &fallback_handle.id, + None, + without_git.next_cursor.as_deref(), + 10, + ) + .expect("second page"); + std::env::remove_var("PGIT_DISABLE_REV_LIST"); + + let ids = |p: &platypusgit_lib::git::types::LogPage| -> Vec { + p.commits.iter().map(|c| c.oid.clone()).collect() + }; + + assert_eq!(ids(&with_git), ids(&without_git), "first page must match"); + assert_eq!( + ids(&next), + ids(&without_git_next), + "the continuation must match too — the cursor is the half that breaks", + ); + assert_eq!(with_git.next_cursor, without_git.next_cursor); +} diff --git a/src-tauri/tests/log_walk_ordering.rs b/src-tauri/tests/log_walk_ordering.rs new file mode 100644 index 0000000..bb934b6 --- /dev/null +++ b/src-tauri/tests/log_walk_ordering.rs @@ -0,0 +1,118 @@ +//! The ordering contract behind the git-backed log walk (#483). +//! +//! `build_walk_order` sorts with `Sort::TIME | Sort::TOPOLOGICAL`, which is +//! Kahn's algorithm over a time-priority queue — git's `--date-order`, and NOT +//! its `--topo-order`. The distinction is not cosmetic: measured on +//! `torvalds/linux`, the two share only 1,627 of the first 2,000 oids, so +//! taking the order from the wrong one silently changes WHICH COMMITS the +//! first page contains. Both #473 and #476 proposed `--topo-order`. +//! +//! This is the file that makes "`--date-order` is the drop-in" something the +//! build checks rather than something a spec claims. +//! +//! **Plant a violation before trusting an edit here.** Change `--date-order` +//! to `--topo-order` in `date_order_reproduces_libgit2_time_topological` and +//! it must go red. A fixture on which the two orderings agree would make this +//! file pass against the exact mistake it exists to catch, which is why +//! `diverges_from_topo_order` asserts the fixture is sharp. + +mod support; + +use git2::{Oid, Repository, Signature, Sort, Time}; +use support::{git_in, TempRepo}; + +/// One commit with an explicit timestamp and explicit parents, touching no ref +/// and no working tree. +fn commit_on(repo: &Repository, name: &str, when: i64, parents: &[Oid]) -> Oid { + let blob = repo.blob(format!("{name}\n").as_bytes()).unwrap(); + let base_tree = parents + .first() + .map(|p| repo.find_commit(*p).unwrap().tree().unwrap()); + let mut tb = repo.treebuilder(base_tree.as_ref()).unwrap(); + tb.insert(name, blob, 0o100644).unwrap(); + let tree = repo.find_tree(tb.write().unwrap()).unwrap(); + + let sig = Signature::new("Test", "test@example.com", &Time::new(when, 0)).unwrap(); + let parent_commits: Vec<_> = parents + .iter() + .map(|p| repo.find_commit(*p).unwrap()) + .collect(); + let parent_refs: Vec<&git2::Commit> = parent_commits.iter().collect(); + repo.commit(None, &sig, &sig, name, &tree, &parent_refs) + .unwrap() +} + +/// Two lines of development whose commit dates INTERLEAVE, then a merge. +/// +/// The interleaving is the whole point. `--date-order` walks strictly by +/// timestamp within the parents-after-children constraint, so it alternates +/// between the two lines; `--topo-order` refuses to intermix them. A fixture +/// whose branches do not interleave produces the same sequence either way and +/// proves nothing. +fn interleaved_merge_history(tr: &TempRepo) -> Oid { + let repo = &tr.repo; + let base = repo.head().unwrap().peel_to_commit().unwrap().id(); + + let m1 = commit_on(repo, "m1", 1_000, &[base]); + let f1 = commit_on(repo, "f1", 2_000, &[base]); + let m2 = commit_on(repo, "m2", 3_000, &[m1]); + let f2 = commit_on(repo, "f2", 4_000, &[f1]); + let merge = commit_on(repo, "merge", 5_000, &[m2, f2]); + + repo.reference("refs/heads/main", merge, true, "test fixture") + .unwrap(); + merge +} + +fn libgit2_time_topological(tr: &TempRepo) -> Vec { + let mut walk = tr.repo.revwalk().unwrap(); + walk.set_sorting(Sort::TIME | Sort::TOPOLOGICAL).unwrap(); + walk.push_head().unwrap(); + walk.map(|o| o.unwrap().to_string()).collect() +} + +fn rev_list(tr: &TempRepo, ordering: &str) -> Vec { + git_in(tr.path(), &["rev-list", ordering, "HEAD"]) + .lines() + .map(str::to_string) + .collect() +} + +#[test] +fn date_order_reproduces_libgit2_time_topological() { + let tr = TempRepo::with_initial_commit("root\n"); + interleaved_merge_history(&tr); + assert_eq!( + libgit2_time_topological(&tr), + rev_list(&tr, "--date-order"), + "the git-backed walk must reproduce libgit2's order exactly", + ); +} + +#[test] +fn diverges_from_topo_order() { + // Without this, the test above would pass against `--topo-order` too and + // would be worthless as a guard. + let tr = TempRepo::with_initial_commit("root\n"); + interleaved_merge_history(&tr); + assert_ne!( + rev_list(&tr, "--date-order"), + rev_list(&tr, "--topo-order"), + "fixture does not exercise the distinction this file exists to pin", + ); +} + +#[test] +fn libgit2_topological_alone_is_the_other_ordering() { + // The mirror image, so the mapping is pinned in both directions: dropping + // Sort::TIME is what `--topo-order` corresponds to. + let tr = TempRepo::with_initial_commit("root\n"); + interleaved_merge_history(&tr); + + let mut walk = tr.repo.revwalk().unwrap(); + walk.set_sorting(Sort::TOPOLOGICAL).unwrap(); + walk.push_head().unwrap(); + let topo: Vec = walk.map(|o| o.unwrap().to_string()).collect(); + + assert_eq!(topo, rev_list(&tr, "--topo-order")); +} diff --git a/src/features/repo/refreshPaintsBeforeLog.test.ts b/src/features/repo/refreshPaintsBeforeLog.test.ts new file mode 100644 index 0000000..d9af7a3 --- /dev/null +++ b/src/features/repo/refreshPaintsBeforeLog.test.ts @@ -0,0 +1,123 @@ +// The repository paints before history arrives (#483). +// +// `refreshAll` used to await ELEVEN reads behind one `Promise.all` and write +// once. On `torvalds/linux` ten of them are done inside a second and the log +// page takes fifteen, so the whole screen — status, branches, tags, HEAD — +// waited on the slowest read in the set. +// +// This holds the log page open and asserts the rest has already landed. A +// version that re-joins them fails here, which is the only thing that keeps +// the split from being quietly undone. + +import { beforeEach, describe, expect, it } from "vitest"; + +import { mockInvoke, resetInvokeMock } from "@/test/invokeMock"; + +import { useRepoStore } from "./useRepoStore"; + +/** Resolves the pending `get_log_page`, once the test is ready for it. */ +let releaseLog: (page: { commits: unknown[]; nextCursor: string | null }) => void; + +function mockReads() { + for (const cmd of [ + "get_status", + "list_branches", + "list_tags", + "list_stashes", + "list_remotes", + ]) { + mockInvoke(cmd, () => []); + } + mockInvoke("repo_state", () => "Clean"); + mockInvoke("head_info", () => ({ branch: "refs/heads/main", headOid: "a1" })); + mockInvoke("rebase_status", () => ({ + inProgress: false, + nextIndex: 0, + total: 0, + pauseReason: null, + })); + mockInvoke("shallow_info", () => ({ + shallow: false, + boundaryCount: 0, + singleBranch: false, + })); + mockInvoke("bisect_status", () => ({ + inProgress: false, + startRef: null, + badTerm: "bad", + goodTerm: "good", + currentOid: null, + remaining: null, + steps: null, + firstBadOid: null, + goodCount: 0, + badCount: 0, + skippedCount: 0, + })); + + // The slow one. It stays pending until the test releases it. + mockInvoke( + "get_log_page", + () => + new Promise((resolve) => { + releaseLog = resolve as typeof releaseLog; + }), + ); +} + +beforeEach(() => { + resetInvokeMock(); + mockReads(); + useRepoStore.setState({ + current: { id: "r1", path: "/repo", head: "main" }, + commits: [], + statusLoaded: false, + loading: false, + error: null, + } as never); +}); + +describe("refreshAll", () => { + it("lands status, branches and HEAD while the log page is still pending", async () => { + const done = useRepoStore.getState().refreshAll(); + + // Let the ten fast reads settle. Two macrotask turns is enough for a + // Promise.all over already-resolved mocks; the log is still open. + await new Promise((r) => setTimeout(r, 0)); + await new Promise((r) => setTimeout(r, 0)); + + expect(useRepoStore.getState().statusLoaded).toBe(true); + expect(useRepoStore.getState().headInfo).toEqual({ + branch: "refs/heads/main", + headOid: "a1", + }); + expect(useRepoStore.getState().loading).toBe(false); + // …and history has NOT arrived yet, which is the point. + expect(useRepoStore.getState().commits).toEqual([]); + + releaseLog({ commits: [{ id: "c1" }], nextCursor: null }); + await done; + + expect(useRepoStore.getState().commits).toHaveLength(1); + }); + + it("still resolves only once history has landed", async () => { + let settled = false; + const done = useRepoStore + .getState() + .refreshAll() + .then(() => { + settled = true; + }); + + await new Promise((r) => setTimeout(r, 0)); + await new Promise((r) => setTimeout(r, 0)); + // The screen is painted, but the caller's promise must not be done: an op + // that refreshes and then reads `commits` would race it. + expect(settled).toBe(false); + + releaseLog({ commits: [], nextCursor: null }); + await done; + expect(settled).toBe(true); + }); +}); diff --git a/src/features/repo/useRepoStore.ts b/src/features/repo/useRepoStore.ts index 6202278..e54f192 100644 --- a/src/features/repo/useRepoStore.ts +++ b/src/features/repo/useRepoStore.ts @@ -1070,6 +1070,34 @@ export const useRepoStore = create((set, get) => { if (!repo) return; set({ loading: true, ...(opts?.preserveError ? {} : { error: null }) }); const logRef = get().logRef; + // The log page is started HERE, beside the others, but it is deliberately + // NOT in the `Promise.all` below (#483). On `torvalds/linux` the other ten + // reads are done inside a second and this one takes fifteen, so joining + // them made the whole screen — status, branches, tags, HEAD — wait on the + // slowest read in the set. It lands in its own write further down, and + // `loadingTasks` (#296) names it while it runs, so the status bar says + // "loading history" over a painted repository instead of an empty one. + // + // Still started before the fan-out, so the concurrency the benchmark + // measures is unchanged: eleven reads in flight, not ten and then one. + const commitPagePromise = trackLoad( + repo.id, + "log", + "loading history", + getLogPage(repo.id, null, PAGE_SIZE, logRef).catch((e) => { + // The browsed ref may have vanished since it was selected (e.g. + // the branch was deleted) — fall back to HEAD instead of failing + // the whole refresh. + if (logRef === null) throw e; + setFor(repo.id, { logRef: null }); + return getLogPage(repo.id, null, PAGE_SIZE); + }), + ); + // If the fan-out below rejects first we never reach the `await`, and an + // unobserved rejection here would surface as an unhandled one. The real + // error is still taken from the await; this only marks it as seen. + commitPagePromise.catch(() => {}); + try { const [ status, @@ -1077,7 +1105,6 @@ export const useRepoStore = create((set, get) => { tags, stashes, remotes, - commitPage, repoState, rebaseStatus, bisectStatus, @@ -1093,19 +1120,6 @@ export const useRepoStore = create((set, get) => { trackLoad(repo.id, "tags", "listing tags", listTags(repo.id)), trackLoad(repo.id, "stashes", "listing stashes", listStashes(repo.id)), trackLoad(repo.id, "remotes", "fetching remotes", listRemotes(repo.id)), - trackLoad( - repo.id, - "log", - "loading history", - getLogPage(repo.id, null, PAGE_SIZE, logRef).catch((e) => { - // The browsed ref may have vanished since it was selected (e.g. - // the branch was deleted) — fall back to HEAD instead of failing - // the whole refresh. - if (logRef === null) throw e; - setFor(repo.id, { logRef: null }); - return getLogPage(repo.id, null, PAGE_SIZE); - }), - ), trackLoad(repo.id, "repoState", "reading repository state", repoStateFn(repo.id)), trackLoad(repo.id, "rebase", "reading rebase state", rebaseStatusFn(repo.id)), // Degrades instead of failing the refresh. `bisect_status` shells out to @@ -1140,15 +1154,13 @@ export const useRepoStore = create((set, get) => { shallowInfoFn(repo.id).catch(() => DEFAULT_SHALLOW_INFO), ), ]); + // The repository is usable at this point: everything but history. setFor(repo.id, { status, branches, tags, stashes, remotes, - commits: commitPage.commits, - // A refresh restarts the walk, so the old resume point is void. - commitCursor: commitPage.nextCursor, repoState, rebaseStatus, bisectStatus, @@ -1161,6 +1173,17 @@ export const useRepoStore = create((set, get) => { // the commit panel flicker when it asked `loading` instead. statusLoaded: true, }); + + // History arrives on its own clock. `refreshAll` still does not RESOLVE + // until it has, because callers refresh and then read `commits` — the + // screen paints early, the promise does not settle early. + const commitPage = await commitPagePromise; + setFor(repo.id, { + commits: commitPage.commits, + // A refresh restarts the walk, so the old resume point is void. + commitCursor: commitPage.nextCursor, + }); + // Keep an active search in sync with the refreshed history. const activeFilter = get().commitFilter; if (!isFilterEmpty(activeFilter)) {