From 7e785f50e7247abcd99649193e0ec35ba4fad9e0 Mon Sep 17 00:00:00 2001 From: Jonas Aasberg Date: Fri, 18 Sep 2026 13:16:49 +0200 Subject: [PATCH 1/8] docs(perf): spec and plan for the first log walk (#483) Opening torvalds/linux still costs 15.7s on current main, and the whole of it is one libgit2 call: a sorted revwalk materialises the complete ordered list before it yields a single oid, and libgit2 never reads the commit-graph that makes git's answer 188ms. Why: #479 fixed the per-page repayment (page ten went 157.67s -> 112ms) but deliberately left the first walk, which is now the entire large-repo problem. Measured fresh on a quiet machine, against 4c6b5ed. Also records a correction both #473 and #476 need: they propose shelling out to --topo-order, and that is the wrong ordering. libgit2's TIME|TOPOLOGICAL is byte-for-byte --date-order; --topo-order shares only 1,627 of the first 2,000 kernel oids, so adopting it would silently change which commits the first page shows. Co-Authored-By: Claude Opus 5 (1M context) --- .../plans/2026-09-18-fast-log-walk.md | 869 ++++++++++++++++++ .../specs/2026-09-18-fast-log-walk-design.md | 265 ++++++ 2 files changed, 1134 insertions(+) create mode 100644 docs/superpowers/plans/2026-09-18-fast-log-walk.md create mode 100644 docs/superpowers/specs/2026-09-18-fast-log-walk-design.md 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..f29531f --- /dev/null +++ b/docs/superpowers/specs/2026-09-18-fast-log-walk-design.md @@ -0,0 +1,265 @@ +# 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. It joins Settings as one switch, defaulting on, described as + what it is. + +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. From 30b5295129cf5d4cde4ccec6528542c237ece2c4 Mon Sep 17 00:00:00 2001 From: Jonas Aasberg Date: Fri, 18 Sep 2026 14:14:57 +0200 Subject: [PATCH 2/8] perf(repo): paint the repository before history arrives MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit refreshAll awaited 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 seconds 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, so joining them to the slowest read is what turns a one-second screen into a 15.7-second one. The page is still STARTED beside the others, so the eleven-way concurrency open_screen measures is unchanged, and refreshAll still resolves only once history has landed — callers refresh and then read commits. Co-Authored-By: Claude Opus 5 (1M context) --- .../repo/refreshPaintsBeforeLog.test.ts | 123 ++++++++++++++++++ src/features/repo/useRepoStore.ts | 57 +++++--- 2 files changed, 163 insertions(+), 17 deletions(-) create mode 100644 src/features/repo/refreshPaintsBeforeLog.test.ts 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)) { From ded53ace7c357be347c4ae2f026ad3c4313cd35b Mon Sep 17 00:00:00 2001 From: Jonas Aasberg Date: Fri, 18 Sep 2026 14:17:22 +0200 Subject: [PATCH 3/8] test: pin --date-order as libgit2's TIME|TOPOLOGICAL equivalent Co-Authored-By: Claude Opus 5 (1M context) --- src-tauri/tests/log_walk_ordering.rs | 118 +++++++++++++++++++++++++++ 1 file changed, 118 insertions(+) create mode 100644 src-tauri/tests/log_walk_ordering.rs 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")); +} From b7fb2059931ee3d7e4f660e60cccd8c96f3b025a Mon Sep 17 00:00:00 2001 From: Jonas Aasberg Date: Fri, 18 Sep 2026 14:20:08 +0200 Subject: [PATCH 4/8] feat(log): read the commit order from git rev-list --date-order Co-Authored-By: Claude Opus 5 (1M context) --- docs/dev/backend.md | 43 +++++++++ src-tauri/src/git/log_walk.rs | 174 ++++++++++++++++++++++++++++++++++ src-tauri/src/git/mod.rs | 1 + 3 files changed, 218 insertions(+) create mode 100644 src-tauri/src/git/log_walk.rs diff --git a/docs/dev/backend.md b/docs/dev/backend.md index 178e5ae..449a31b 100644 --- a/docs/dev/backend.md +++ b/docs/dev/backend.md @@ -1546,6 +1546,49 @@ 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. + +`git/commit_graph.rs` is what keeps this fast; without a commit-graph git is no +better than we are. + ## 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/src-tauri/src/git/log_walk.rs b/src-tauri/src/git/log_walk.rs new file mode 100644 index 0000000..1d634bb --- /dev/null +++ b/src-tauri/src/git/log_walk.rs @@ -0,0 +1,174 @@ +//! 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; + +/// 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 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..e7129e1 100644 --- a/src-tauri/src/git/mod.rs +++ b/src-tauri/src/git/mod.rs @@ -9,6 +9,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; From b44779419ccadbed205099b89697c410e06629a2 Mon Sep 17 00:00:00 2001 From: Jonas Aasberg Date: Fri, 18 Sep 2026 14:24:17 +0200 Subject: [PATCH 5/8] perf(log): build the walk order from git, falling back to libgit2 Co-Authored-By: Claude Opus 5 (1M context) --- src-tauri/src/git/libgit2.rs | 23 ++++++ src-tauri/src/git/log_walk.rs | 15 ++++ src-tauri/tests/log_walk_backend.rs | 105 +++++++++++++++++++++++++++ src-tauri/tests/log_walk_fallback.rs | 58 +++++++++++++++ 4 files changed, 201 insertions(+) create mode 100644 src-tauri/tests/log_walk_backend.rs create mode 100644 src-tauri/tests/log_walk_fallback.rs 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 index 1d634bb..4f0d7a6 100644 --- a/src-tauri/src/git/log_walk.rs +++ b/src-tauri/src/git/log_walk.rs @@ -46,6 +46,18 @@ 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: @@ -84,6 +96,9 @@ pub fn parse_oid_lines(stdout: &str, cap: usize) -> Option> { /// 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()); } 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); +} From 55eac72a3ba431930b0e5705f3114e29602f07ba Mon Sep 17 00:00:00 2001 From: Jonas Aasberg Date: Fri, 18 Sep 2026 14:28:25 +0200 Subject: [PATCH 6/8] feat(log): keep a split commit-graph warm on open Co-Authored-By: Claude Opus 5 (1M context) --- docs/dev/backend.md | 28 +++++++- src-tauri/src/commands/repo.rs | 21 +++++- src-tauri/src/git/commit_graph.rs | 107 ++++++++++++++++++++++++++++++ src-tauri/src/git/mod.rs | 1 + src-tauri/tests/commit_graph.rs | 87 ++++++++++++++++++++++++ 5 files changed, 240 insertions(+), 4 deletions(-) create mode 100644 src-tauri/src/git/commit_graph.rs create mode 100644 src-tauri/tests/commit_graph.rs diff --git a/docs/dev/backend.md b/docs/dev/backend.md index 449a31b..6e168c1 100644 --- a/docs/dev/backend.md +++ b/docs/dev/backend.md @@ -1586,8 +1586,32 @@ The parser requires a FULL-LENGTH hex id rather than leaving it to output truncated mid-line would otherwise parse into a plausible order naming an object that does not exist. -`git/commit_graph.rs` is what keeps this fast; without a commit-graph git is no -better than we are. +### 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) 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/mod.rs b/src-tauri/src/git/mod.rs index e7129e1..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; 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)); +} From b4a04b621e76a88d6351429b5b969c0651d58a91 Mon Sep 17 00:00:00 2001 From: Jonas Aasberg Date: Fri, 18 Sep 2026 14:30:15 +0200 Subject: [PATCH 7/8] test(bench): compare the log against --date-order, not --topo-order Co-Authored-By: Claude Opus 5 (1M context) --- src-tauri/benches/repo_bench.rs | 19 ++++++++++++++----- 1 file changed, 14 insertions(+), 5 deletions(-) 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", "--", From 2cad379c7c98f3afbf458338d67c8fa3e1273b44 Mon Sep 17 00:00:00 2001 From: Jonas Aasberg Date: Fri, 18 Sep 2026 14:40:18 +0200 Subject: [PATCH 8/8] docs(bench): publish the numbers after the git-backed walk torvalds/linux first screen 15.74s -> 999ms, first page 18.11s -> 222ms, deep 253ms -> 59ms. wide and refs are unmoved, which is right: their cost is status and ref enumeration, neither of which this touched. The fixtures now carry a --split commit-graph, written by bench-fixtures.mjs, because the app writes one on open and a fixture without one measures a state the app does not leave a repository in. The git baselines get the same file, so several ratios look worse than when we withheld it -- that is the correct comparison. The generated headline no longer asserts a cause it cannot see: it names the slowest remaining operation from the data, which is now file_history at 16.9s rather than the log. Co-Authored-By: Claude Opus 5 (1M context) --- CLAUDE.md | 13 + README.md | 12 +- docs/dev/benchmark.json | 508 +++++++++--------- docs/dev/performance.md | 202 ++++--- .../specs/2026-09-18-fast-log-walk-design.md | 12 +- scripts/bench-fixtures.mjs | 28 + scripts/bench-report.mjs | 32 +- 7 files changed, 453 insertions(+), 354 deletions(-) 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/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/specs/2026-09-18-fast-log-walk-design.md b/docs/superpowers/specs/2026-09-18-fast-log-walk-design.md index f29531f..683250d 100644 --- a/docs/superpowers/specs/2026-09-18-fast-log-walk-design.md +++ b/docs/superpowers/specs/2026-09-18-fast-log-walk-design.md @@ -196,8 +196,16 @@ Three measured constraints shape how: 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. It joins Settings as one switch, defaulting on, described as - what it 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. 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.", ); }