diff --git a/CLAUDE.md b/CLAUDE.md index cdef78e..17eb69b 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -264,6 +264,16 @@ Each rule's full story (why, traps, tests that pin it) is in the named doc. - **The log is paged** — `s.commits` is a prefix of history, never the answer to "does X exist / is X an ancestor"; ask the backend. (`docs/dev/frontend.md`) +- **…and the backend prepares ONE walk for all those pages** (`git/log_cache.rs`, + #473). A sorted libgit2 revwalk materialises the whole ordered list before it + yields anything, so a walk per page cost the same per page — 157 s at page ten + of the kernel. Everything cached there is derivable from disk, so it must stay + a PURE ACCELERATOR: a lock failure is a miss and never a failed page, and the + proof is `log_walk_cache.rs` draining the same history warm and cold and + comparing. A first page is invalidated by its start oids, a continuation by + 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`) - **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/docs/dev/architecture.md b/docs/dev/architecture.md index 0490213..0438616 100644 --- a/docs/dev/architecture.md +++ b/docs/dev/architecture.md @@ -274,6 +274,25 @@ git/ │ write still excludes everything. Carries its own unit tests │ with a fake handle, proving overlap by barrier rather than by │ wall clock. See backend.md +├── log_cache.rs What `log_page` used to rebuild on every page (#473): the +│ prepared WALK and the REF MAP. libgit2's topological sort is +│ not incremental — setting a sort order sets `walk->limited`, so +│ `prepare_walk` traverses the whole reachable graph and +│ materialises the complete ordered list before yielding one oid, +│ which is why 500 commits and 1.5 M cost the same and why a walk +│ per page paid that price per page (page 10 of the kernel: 157 +│ s). So the walk is prepared once, kept as a `Vec` capped at +│ MAX_ORDER, and later pages are slices. Two different +│ invalidations on purpose: a FIRST page is keyed by (refspec, +│ start oids) so a moved ref misses, a CONTINUATION is keyed by +│ the frontier it was emitted with and consults no refs at all +│ (the set a frontier reaches is commits, and commits are +│ immutable). The ref map is keyed by a fingerprint of every ref +│ name + target, so a `git tag` in a terminal invalidates it like +│ one made in the app. PURE ACCELERATOR: a poisoned mutex is a +│ miss, never a failed page, and `close` drops the lot. Carries +│ its own unit tests; `tests/log_walk_cache.rs` holds the +│ transparency property end to end. See backend.md, performance.md ├── update_refs.rs Stacked branches (#240) — git's `rebase --update-refs`, │ IMPLEMENTED not passed through, because our rebase is our own │ libgit2 replay with no `git rebase` process to hand a flag to. diff --git a/docs/dev/backend.md b/docs/dev/backend.md index 4324648..178e5ae 100644 --- a/docs/dev/backend.md +++ b/docs/dev/backend.md @@ -1544,6 +1544,113 @@ walk a `Vec` per commit — `{ name, kind }`, where `kind` is - `src-tauri/tests/log_decoration.rs` pins all of it, including a tag and a branch of the same name staying two distinct decorations. +`collect_ref_map` no longer runs per page — see below. + +## The paged log prepares ONE walk (#473) + +`log_page` used to build a fresh revwalk per page. That reads like an obvious +design until you look at what libgit2 does with a sort order: + +- `git_revwalk_sorting` sets `walk->limited` for ANY sorting but `NONE`, so + `prepare_walk` runs `limit_list` over the entire reachable graph; +- `sort_in_topological_order` then materialises the **complete** ordered list, + and only afterwards does `git_revwalk_next` start popping it. + +So asking that walk for 500 commits and asking it for all 1,482,923 of them +cost the same thing, and one walk per page paid that price per page. The +benchmark measured it as a per-page cost *flat in depth*: on `torvalds/linux` +page one was 15.95 s and page ten 157.67 s — ten times one page. git pays for +the same sort once and then `--skip`s. + +`git/log_cache.rs` keeps the finished sort instead. A walk is prepared once, +its output kept as a `Vec` capped at `MAX_ORDER` (100,000 — two hundred +pages, 2 MB), and every later page is a slice of it. The cursor contract is +unchanged: `next_cursor` is still the frontier, still self-describing, and a +cursor the cache cannot place still prepares its own walk. + +**There are two invalidation stories and they are deliberately different.** + +- A **first page** is keyed by `(refspec, start oids)`. Any ref that moves + changes an oid in the key, so the next first page misses and rebuilds. That + is the whole mechanism — there is no write hook to forget to call. +- A **continuation** is keyed by the frontier it was emitted with, and consults + no refs at all. It does not need to: resuming from a cursor has always + ignored `refspec` (`push_page_start`), and the set a frontier reaches is made + of commits, which are immutable. New history arrives as CHILDREN of what is + there, never inside that set. So a walk prepared ten minutes ago has the same + tail today. + +A cursor is placed by **exact match against the frontier we emitted**, never by +"find an order containing these oids". Two walks over one repository share +commits, so containment would happily resolve an `--all` cursor inside a +single-branch walk and continue the wrong history. + +**The ref map goes the same way**, and it was worth as much: `collect_ref_map` +enumerated and peeled every ref on every page to decorate 500 rows — 16× git's +own work on a repository with 7,001 refs and 2,000 commits. It is now cached +against a `ref_fingerprint`: one enumeration of every ref's name and target +with **no peeling**, combined commutatively so enumeration order cannot change +it. A fingerprint rather than a write hook, because a `git tag` typed in a +terminal has to invalidate it exactly like one made in the app; and rather than +an mtime, because loose refs live in nested directories that no single +timestamp covers. `commits_since` and `commits_between` read the same cache. + +**It is a pure accelerator, and that is a rule, not a description.** Everything +held is derivable from disk, a poisoned mutex degrades to a miss instead of +failing the page, and `close` drops the repository's entries. `tests/ +log_walk_cache.rs` holds it down by draining the same history twice — through +one backend where the cache is hot, and through a fresh backend per page where +it can never hit — and comparing the two. Those tests pass just as well against +a cache that never hits, which is why the file also asserts on +`log_cache_stats()`: eleven pages must prepare exactly one walk and build +exactly one ref map, and a commit must make the next first page prepare a +second. All four failure modes were planted and watched go red. + +**One behaviour is now visible that was always there.** When two commits share +a *second*, which lane comes out first is not something either walk promises: +libgit2 orders the topological queue by time through a `git_pqueue` whose +comparator is not stable, so the answer depends on insertion order — and a walk +restarted from a cursor inserts differently from one that ran straight through. +The walk-per-page version had the same freedom and used it at every page +boundary; one prepared walk is merely self-consistent where ten were not. What +holds either way is the actual contract — every commit exactly once, no parent +before its child — and that is what the same-second fixture asserts. + +**The filtered walk reads the same order.** `log_filtered_page` is commit +search, and it had both defects for the same reason and in the same two lines. +It is also where they hurt most: a search that matches nothing recent walks a +long way before it fills a page, and the next page threw that walk away and +sorted all of history again. + +It cannot go through `page_plan`, and the reason is worth keeping. A filtered +page visits far more commits than it returns, so it can run off the end of an +order capped at `MAX_ORDER` in the middle of a page — and a short page there is +indistinguishable, to the user and to the frontend, from "no more matches +exist". So `cached_filtered_plan` asks `WalkOrder::serves` with `usize::MAX`, +which admits any offset into a COMPLETE order and no offset at all into a capped +one; a miss falls back to the revwalk-per-page the filtered log always had. A +history past the cap is therefore no worse off than before #473. Filling the +page from the cached prefix and then continuing into a live walk is the version +that would help there too, and it needs a frontier the builder can hand back +mid-page. + +It must also not PREPARE a walk to find that out: preparing one and then +discovering it is capped pays for the topological sort twice, on exactly the +repositories where that sort is most expensive. `LogCache::has_walk` answers +"has this walk been prepared before" without counting a hit or moving anything, +which is a different question from `first_page`'s "can this serve the page in +hand". The one time a filtered page does prepare and file a walk is when nothing +is filed for that key at all — the page was going to pay for a walk anyway, and +a search should not depend on the log having been read first to be fast, even +though in the app it always has been. + +Both sources run one `matches` closure, through the generic `filtered_page`, +because a search that returned different matches depending on whether a walk +happened to be cached is the one bug this change must not have. The frontier it +emits is filed against an offset ONLY when the page came out of a cached order: +a page that walked for itself ends somewhere no order has an index for, and +filing that offset would put the next page at the wrong depth. + ## Reading the log (#274) Where it is — `tauri_plugin_log`'s `LogDir` target, i.e. Tauri's `app_log_dir`: diff --git a/docs/dev/performance.md b/docs/dev/performance.md index 18dcf76..9537178 100644 --- a/docs/dev/performance.md +++ b/docs/dev/performance.md @@ -166,7 +166,7 @@ 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. -### 2. The log walk is not slow — the topological SORT is, and it is re-paid per page +### 2. The log walk is not slow — the topological SORT is, and it was re-paid per page The obvious reading of "our 500-commit page takes 252 ms and `git log -500` takes 41 ms" is that the walk is six times too slow. It is wrong, and it nearly @@ -179,8 +179,8 @@ not sort that way. Asked the same question, `git log --topo-order -500` costs this benchmark are `--topo-order` for exactly that reason, and the near miss is written up in the spec. -What survives is sharper. git pays for that sort **once and then skips**; we pay -it per page: +What survived was sharper. git pays for that sort **once and then skips**; we +paid it again on every page: | | `deep` (50k commits) | `torvalds/linux` (1.5M commits) | | --- | --- | --- | @@ -188,15 +188,52 @@ it per page: | our tenth page | 2.40 s | 157.67 s | | `git log --topo-order --skip=4500 -500` | 193 ms | 9.68 s | -The per-page cost is flat in depth — ten pages cost ten times one page — which -is the signature of restarting the walk rather than continuing it. The cursor -already carries the frontier, so the walk logically continues; it is the sort -that is rebuilt. +The per-page cost was flat in depth — ten pages cost ten times one page — which +is the signature of restarting the walk rather than continuing one. -### 3. The ref map is rebuilt on every page, too +#### The measurement the fix rests on -`log_page` calls `collect_ref_map(repo)` per call, enumerating and peeling every -ref so the page can decorate its 500 commits. Two fixtures isolate it, and the +Fixed in #473 by preparing the walk once and paging out of what it produced. +The reason that is affordable is one measurement, and it is worth recording +because it is the opposite of what "just cache it" usually costs. Draining a +revwalk that has **already been prepared** is very nearly free, because libgit2 +does the ordering during preparation and then hands commits out of a list it +already has: + +| | prepare + take(500) | drain the whole rest of the order | +| --- | --- | --- | +| `deep` (50,000 commits) | 560.2 ms | **1.9 ms** (49,500 more) | +| `torvalds/linux` (1,482,923 commits) | 31.8 s | **63.4 ms** (1,482,423 more) | + +0.2% more buys the entire order, which is why the cache holds a `Vec` +rather than the prepared `Revwalk` — holding the walk would mean holding a +`git2::Repository` alive between IPC calls, outliving the lock acquisition +`git/repo_locks.rs` orders every access by, to save that 0.2%. The source-level +reason it comes out this way is in `src-tauri/src/git/log_cache.rs`: +`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 is yielded. + +**The tables below have not been re-measured for that change**, and the numbers +in this section are the "before" they always were. Two sessions were benchmarking +this repository at once, and `$PGBENCH_HOME` is one shared directory: control +rows moved 25–30% on operations neither change touches (`wide` status 5.38 s → +3.92 s, `diff_commit` 1.53 s → 1.12 s), which is not a result, it is two +programs sharing a machine. Re-measure on a quiet machine, with one session +running, before publishing an after. + +And re-measure **all four fixtures**, or knowingly keep the rest. The renderer +publishes every result it finds in `$PGBENCH_HOME/results`, which is deliberate +— it is what stops `pnpm bench --fixture deep` from silently deleting +`torvalds/linux` from the record — but it also means a one-fixture run +publishes fresh numbers for that fixture and whatever happens to be sitting +beside it. `test/benchmark.test.ts` cannot catch that: it checks the markdown +against the JSON, and both would be wrong together. + +### 3. The ref map was rebuilt on every page, too + +`log_page` called `collect_ref_map(repo)` per call, enumerating and peeling every +ref so the page could decorate its 500 commits. Two fixtures isolate it, and the variable between them is refs rather than history: | fixture | history | refs | ours | `git` work | ratio | @@ -204,9 +241,31 @@ variable between them is refs rather than history: | `deep` | 50,000 commits | 1 | 252 ms | 194 ms | 1.3× | | `refs` | 2,000 commits | 7,001 | 135 ms | 8.5 ms | **16×** | -Twenty-five times *less* history, and still most of the cost. This one looks -much cheaper to fix than the sort: cache the map per repository and invalidate -it on ref writes. +Twenty-five times *less* history, and still most of the cost. + +Fixed in #473 alongside the walk, and two things found on the way are worth +keeping, because both are counter-intuitive: + +* **The peeling is not the expense.** This issue was written as "enumerating and + peeling every ref"; measured on `refs`, one enumeration costs 113 ms and the + peel inside it costs 10 ms. So the map is validated on a FIRST page and reused + by the continuations behind it — revalidating per scroll cost 113 ms of a + 117 ms page, for a question that cannot have changed. +* **Do not compute a fingerprint before building the map on a cold cache.** It + enumerates every ref twice for an answer that cannot match anything, and it + made a cold eleven-read fan-out on `refs` *slower* than the code it replaced + (219 ms → 364 ms). It hides, too: `open_screen` builds a fresh backend per + sample, so that path is always the cold one. `collect_ref_map` returns the + fingerprint from its own pass for this reason. + +The same two defects were in `log_filtered_page`, which is commit search — the +surface where they hurt most, because a search that matches nothing recent walks +a long way before it fills a page, and the next page threw that walk away. It +reads the same prepared order now. One limit is deliberate and written down in +`cached_filtered_plan`: a search visits far more commits than it returns, so +only an order that is ALL of history can serve one, and a repository past +`MAX_ORDER` falls back to the walk-per-page it always had rather than to a short +page that would look like "no more matches exist". ### What is already good, and worth not breaking diff --git a/src-tauri/src/git/libgit2.rs b/src-tauri/src/git/libgit2.rs index 7698121..da8d7a9 100644 --- a/src-tauri/src/git/libgit2.rs +++ b/src-tauri/src/git/libgit2.rs @@ -11,6 +11,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::ownership; use crate::git::repo_locks::RepoLock; use crate::git::shallow as shallow_mod; @@ -82,6 +83,9 @@ pub struct RebaseState { pub struct Libgit2Backend { repos: Mutex>>>, rebases: Mutex>, + /// What the paged log used to rebuild on every page (#473) — the prepared + /// walk and the ref decorations. Pure accelerator: see `git/log_cache.rs`. + log_cache: LogCache, /// Serializes the guard → write → cleanup window in `init` to prevent two /// concurrent calls on the same path from interfering. Without this lock, call /// A's cleanup could delete `.git` that call B just created, since the @@ -94,10 +98,16 @@ impl Libgit2Backend { Self { repos: Mutex::new(HashMap::new()), rebases: Mutex::new(HashMap::new()), + log_cache: LogCache::new(), init_lock: Mutex::new(()), } } + /// What the paged log's cache has done so far — see `CacheStats`. + pub fn log_cache_stats(&self) -> crate::git::log_cache::CacheStats { + self.log_cache.stats() + } + /// Clone the repo's own lock out of the map and RELEASE the map lock /// before running `f`. The map guard used to stay alive for the whole /// operation, so every git op in the process — any repository, any window — @@ -181,6 +191,227 @@ impl Libgit2Backend { self.repo_cell(repo_id)?.shared_mut(f) } + /// The ref decorations a log page stamps on its rows, rebuilt only when a + /// ref has actually moved (#473). + /// + /// `collect_ref_map` used to run on every page: on a repository with 7,001 + /// refs it was 16× git's own work for the same question, and most of a + /// page's cost on a repository whose history is short. The fingerprint is + /// one enumeration with no peeling; the map behind it is shared by `Arc`, + /// so several concurrent pages read one copy. + /// + /// `validate` is false for a CONTINUATION page, which reuses whatever the + /// walk's first page saw without asking again. That is measured, not a + /// guess: on the `refs` fixture, validating costs 113 ms of a 117 ms page, + /// because enumerating 7,001 loose refs is the whole expense and peeling + /// them is 10 ms of it. Paying that once per REFRESH is proportionate — + /// `branches` and `tags` beside it in the same fan-out pay 187 ms and 149 + /// ms for the same enumeration — and once per SCROLL is not. So a + /// continuation decorates with the refs its first page saw: a coherent + /// snapshot rather than a stale one, since the walk it pages is already a + /// snapshot and every refresh starts at a first page. + /// + /// Note the shape of the cold arm. Nothing cached means going straight to + /// `collect_ref_map`, whose own pass yields the fingerprint — asking for + /// one first would enumerate every ref twice for an answer that cannot + /// match anything, and that made a cold fan-out on `refs` slower than the + /// version this replaced. + fn ref_map(&self, repo_id: &RepoId, repo: &Repository, validate: bool) -> Arc { + if let Some((had, map)) = self.log_cache.ref_map(repo_id) { + if !validate || ref_fingerprint(repo) == had { + return map; + } + } + let (map, fingerprint) = collect_ref_map(repo); + let map = Arc::new(map); + self.log_cache + .put_ref_map(repo_id, fingerprint, Arc::clone(&map)); + map + } + + /// Which prepared walk answers this page, and where in it the page starts. + /// + /// `Ok(None)` is "nothing to walk" — an unborn HEAD, or a cursor whose + /// every oid is missing from a shallow clone — and the caller returns an + /// empty page for it, exactly as the walk-per-page version did. + /// The prepared walk a FILTERED page can be served from, if one is already + /// cached. Never prepares one. + /// + /// Separate from `page_plan` because a filtered page differs from a plain + /// one in two ways that both point here. + /// + /// **Only a COMPLETE order can answer it.** A search visits far more + /// commits than it returns, so it can run off the end of an order capped at + /// `MAX_ORDER` in the middle of a page — and a short page there is + /// indistinguishable, to the user and to the frontend, from "no more + /// matches exist". `usize::MAX` is how that is asked: `WalkOrder::serves` + /// admits any offset into a complete order and no offset at all into a + /// capped one, which is exactly the question. + /// + /// **It must not PREPARE one to find out.** Preparing a walk and then + /// discovering it is capped would pay for the topological sort twice, on + /// precisely the repositories where that sort is most expensive. A miss + /// here falls back to the revwalk-per-page the filtered log always had, so + /// a repository past the cap is no worse off than before #473 — filling the + /// page from the cached prefix and then continuing into a live walk is the + /// version that would help there too, and it needs a frontier the builder + /// can hand back mid-page. + fn cached_filtered_plan( + &self, + repo_id: &RepoId, + repo: &Repository, + refspec: Option<&str>, + cursor: Option<&[String]>, + ) -> AppResult> { + // A continuation. `refspec` is ignored here, as it always has been: the + // frontier already encodes the walk this page continues. + if let Some(frontier) = cursor.filter(|c| !c.is_empty()) { + let mut want = Vec::with_capacity(frontier.len()); + for raw in frontier { + want.push( + git2::Oid::from_str(raw).map_err(|_| AppError::InvalidRef(raw.clone()))?, + ); + } + let mut sorted = want.clone(); + sorted.sort(); + let Some((key, order, offset)) = self.log_cache.resume(repo_id, &sorted, usize::MAX) + else { + return Ok(None); + }; + // Seeded with the cursor rather than the walk's own start points: a + // lane this page stops short of has to survive into the NEXT + // cursor, and at this depth the live lanes are the ones the caller + // just handed back. + return Ok(Some(PagePlan { + order, + offset, + seed: live_commits(repo, &want), + key, + })); + } + + let starts = resolve_log_starts(repo, refspec)?; + if starts.is_empty() { + return Ok(None); + } + let key = WalkKey::new(refspec, &starts); + if let Some(order) = self.log_cache.first_page(repo_id, &key, usize::MAX) { + return Ok(Some(PagePlan { + seed: order.starts.clone(), + order, + offset: 0, + key, + })); + } + + // Nothing usable filed. If a walk for this key has been prepared + // before, the only reason it cannot serve is the cap, and preparing a + // second one would rediscover that at full price — so search on past + // the cap, exactly as this function did before #473. + if self.log_cache.has_walk(repo_id, &key) { + return Ok(None); + } + // Otherwise this is the first anyone has asked, and the page was going + // to pay for a walk regardless. Prepare it through the same path the + // plain log uses and file it, so the pages behind this one are slices + // rather than walks — a search must not depend on the log having been + // read first to be fast, even though in the app it always has been. + let order = Arc::new(self.prepare_walk(repo, &starts)?); + self.log_cache.insert(repo_id, key.clone(), Arc::clone(&order)); + if !order.complete { + return Ok(None); + } + Ok(Some(PagePlan { + seed: order.starts.clone(), + order, + offset: 0, + key, + })) + } + + + fn page_plan( + &self, + repo_id: &RepoId, + repo: &Repository, + refspec: Option<&str>, + cursor: Option<&[String]>, + limit: usize, + ) -> AppResult> { + // A continuation. `refspec` is ignored here, as it always has been: + // the frontier already encodes the walk this page continues. + if let Some(frontier) = cursor.filter(|c| !c.is_empty()) { + let mut want = Vec::with_capacity(frontier.len()); + for raw in frontier { + want.push( + git2::Oid::from_str(raw).map_err(|_| AppError::InvalidRef(raw.clone()))?, + ); + } + let mut sorted = want.clone(); + sorted.sort(); + if let Some((key, order, offset)) = self.log_cache.resume(repo_id, &sorted, limit) { + // Seeded with the cursor rather than the walk's own start + // points: a lane this page stops short of has to survive into + // the NEXT cursor, and at this depth the live lanes are the + // ones the caller just handed back. + let seed = live_commits(repo, &want); + return Ok(Some(PagePlan { + order, + offset, + seed, + key, + })); + } + // A frontier oid can be missing in a shallow clone; skip it rather + // than failing the whole page. + let seed = live_commits(repo, &want); + if seed.is_empty() { + return Ok(None); + } + let key = WalkKey::new(None, &seed); + let order = Arc::new(self.prepare_walk(repo, &seed)?); + self.log_cache.insert(repo_id, key.clone(), Arc::clone(&order)); + return Ok(Some(PagePlan { + order, + offset: 0, + seed, + key, + })); + } + + // A first page. The refs name the walk, which is also what invalidates + // it: a ref that moved is a different set of start oids, so a different + // key, so a miss. + let starts = resolve_log_starts(repo, refspec)?; + if starts.is_empty() { + return Ok(None); + } + let key = WalkKey::new(refspec, &starts); + if let Some(order) = self.log_cache.first_page(repo_id, &key, limit) { + return Ok(Some(PagePlan { + seed: order.starts.clone(), + order, + offset: 0, + key, + })); + } + let order = Arc::new(self.prepare_walk(repo, &starts)?); + self.log_cache.insert(repo_id, key.clone(), Arc::clone(&order)); + Ok(Some(PagePlan { + order, + offset: 0, + seed: starts, + key, + })) + } + + /// `build_walk_order`, counted — the count is what proves the cache above + /// it is doing anything. + fn prepare_walk(&self, repo: &Repository, starts: &[git2::Oid]) -> AppResult { + self.log_cache.record_walk_prepared(); + build_walk_order(repo, starts) + } + /// The in-process blame: libgit2, HEAD, no subprocess. /// /// This is what a repository with no `blame.ignoreRevsFile` gets, and what @@ -2520,28 +2751,21 @@ fn resolve_commit<'a>(repo: &'a Repository, revspec: &str) -> AppResult, -) -> AppResult> { +/// Separate from pushing them onto a walk because the answer is also the +/// walk's IDENTITY: `log_cache` keys a prepared walk by these oids, so a ref +/// that moved is a different walk and misses (#473). Returned in resolution +/// order, deduplicated — the order is observable as the cursor's lane order. +fn resolve_log_starts(repo: &Repository, refspec: Option<&str>) -> AppResult> { match refspec { None => match repo.head() { - Ok(h) => { - let oid = h.peel_to_commit()?.id(); - walk.push(oid)?; - Ok(vec![oid]) - } + Ok(h) => Ok(vec![h.peel_to_commit()?.id()]), // Fresh repo, HEAD points at a branch with no commits yet → nothing // to walk. This is the only "empty log, not an error" case. Err(e) if e.code() == git2::ErrorCode::UnbornBranch => Ok(Vec::new()), @@ -2555,18 +2779,15 @@ fn push_log_start( // would hide the commits the user is actually sitting on). Tags are // deliberately out: this scope is about branches. Some(spec) if spec == REFSPEC_ALL => { - let mut starts = Vec::new(); - let mut push = |oid: git2::Oid, walk: &mut git2::Revwalk| -> AppResult<()> { - if starts.contains(&oid) { - return Ok(()); + let mut starts: Vec = Vec::new(); + let push = |oid: git2::Oid, starts: &mut Vec| { + if !starts.contains(&oid) { + starts.push(oid); } - walk.push(oid)?; - starts.push(oid); - Ok(()) }; if let Ok(head) = repo.head() { if let Ok(commit) = head.peel_to_commit() { - push(commit.id(), walk)?; + push(commit.id(), &mut starts); } } for glob in ["refs/heads/*", "refs/remotes/*/*"] { @@ -2575,22 +2796,128 @@ fn push_log_start( // A head's or remote head's target IS the commit oid — // no peel (an object read per ref, per page) needed. if let Some(oid) = r.target() { - push(oid, walk)?; + push(oid, &mut starts); } else if let Ok(commit) = r.peel_to_commit() { // A remote's symbolic HEAD (refs/remotes/origin/HEAD) // peels to a tip already pushed — dedup handles it. - push(commit.id(), walk)?; + push(commit.id(), &mut starts); } } } } Ok(starts) } - Some(spec) => { - let commit = resolve_commit(repo, spec)?; - walk.push(commit.id())?; - Ok(vec![commit.id()]) + Some(spec) => Ok(vec![resolve_commit(repo, spec)?.id()]), + } +} + +/// `resolve_log_starts`, pushed onto `walk`. Returns the oids actually pushed; +/// empty means "nothing to walk". The caller needs the oids, not just a flag: a +/// pushed start point that the page's limit stops short of has to survive into +/// the next cursor. +fn push_log_start( + repo: &Repository, + walk: &mut git2::Revwalk, + refspec: Option<&str>, +) -> AppResult> { + let starts = resolve_log_starts(repo, refspec)?; + for &oid in &starts { + walk.push(oid)?; + } + Ok(starts) +} + +/// Prepare one walk and keep what it produces (#473). +/// +/// This is the expensive call in the whole file on a large repository, and it +/// costs the same whether the caller wants 500 commits or every one of them: +/// libgit2's topological sort materialises the complete ordered list inside +/// `git_revwalk_next`'s first call. So it is drained rather than sipped, and +/// the result is what every page of this walk is then sliced out of. +/// +/// `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. +fn build_walk_order(repo: &Repository, starts: &[git2::Oid]) -> AppResult { + 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, + }) +} + +/// A fingerprint of the whole ref database: every ref's name and what it points +/// at, combined so that enumeration order cannot change the answer. +/// +/// This is what decides whether the cached ref decorations are still the truth +/// (#473). It reads names and targets and peels NOTHING, which is the entire +/// saving — peeling 500 annotated tags is an object read each, and that is the +/// work `collect_ref_map` does that this skips on a hit. +/// +/// A fingerprint rather than a write hook, because a `git tag` typed in a +/// terminal has to invalidate it exactly like one made in the app; and rather +/// than an mtime, because loose refs live in nested directories where no single +/// timestamp covers them all. +fn ref_fingerprint(repo: &Repository) -> u64 { + let Ok(refs) = repo.references() else { + // Unreadable refdb: a fingerprint nobody can match, so the map is + // rebuilt rather than a stale one being served. + return u64::MAX; + }; + let mut fp = RefFingerprint::default(); + for r in refs.flatten() { + fp.add(&r); + } + fp.finish() +} + +/// The running fingerprint, so `ref_fingerprint` and `collect_ref_map` cannot +/// drift apart. +/// +/// One definition on purpose: the two have to agree exactly or the map built +/// by one is rejected by the other on the very next call, and nothing about +/// that failure is visible except the cache silently never hitting. +/// `the_two_fingerprint_paths_agree` pins it. +#[derive(Default)] +struct RefFingerprint { + total: u64, + count: u64, +} + +impl RefFingerprint { + fn add(&mut self, r: &git2::Reference<'_>) { + use std::hash::{Hash, Hasher}; + let mut h = std::collections::hash_map::DefaultHasher::new(); + r.name_bytes().hash(&mut h); + match r.target() { + Some(oid) => oid.as_bytes().hash(&mut h), + // A symbolic ref (`refs/remotes/origin/HEAD`) has no direct target; + // what it names is what can change under it. + None => r.symbolic_target_bytes().hash(&mut h), + } + // Commutative, so `references()` may hand them over in any order it + // likes; the count is mixed in separately so that a ref whose hash is + // zero still moves the answer when it appears or disappears. + self.total = self.total.wrapping_add(h.finish()); + self.count += 1; + } + + fn finish(self) -> u64 { + self.total.wrapping_mul(31).wrapping_add(self.count) } } @@ -2647,7 +2974,12 @@ impl FrontierBuilder { /// `None` ⟺ end of history: any parent we saw but did not walk IS more /// history, so an empty frontier means there is nothing left. - fn finish(self, repo: &Repository) -> Option> { + /// + /// Oids rather than strings because the cursor has a second reader now: + /// `log_cache` files the frontier it emitted against the position it + /// resumes at, and that lookup has to be exact (#473). The strings are made + /// at the edge, in `LogPage`. + fn finish(self, repo: &Repository) -> Option> { let mut out = Vec::new(); for p in self.candidates { if self.visited.contains(&p) { @@ -2660,7 +2992,7 @@ impl FrontierBuilder { .map(|odb| odb.exists(p)) .unwrap_or_else(|_| repo.find_commit(p).is_ok()); if exists { - out.push(p.to_string()); + out.push(p); } } if out.is_empty() { @@ -2671,6 +3003,90 @@ impl FrontierBuilder { } } +/// The frontier as it crosses IPC. +fn cursor_strings(frontier: &[git2::Oid]) -> Vec { + frontier.iter().map(git2::Oid::to_string).collect() +} + +/// The oids of `want` this repository actually has, in the order given. +/// +/// A cursor can name a commit a shallow clone does not carry; such a lane is +/// skipped rather than failing the page, which is what `push_page_start` has +/// always done. +fn live_commits(repo: &Repository, want: &[git2::Oid]) -> Vec { + want.iter() + .copied() + .filter(|oid| repo.find_commit(*oid).is_ok()) + .collect() +} + +/// Which prepared walk a page is a slice of, and where the slice starts. +/// One page of a FILTERED log, read from whatever is handing out commits. +/// +/// Generic over the source because there are two and they must behave +/// identically: a slice of a prepared walk's order, and a revwalk this page +/// built for itself. Returns the rows, the frontier to resume from, and HOW +/// MANY commits were read — which is what the caller files the frontier +/// against, and it is not `commits.len()`, because a search reads far more +/// than it returns. +/// +/// Take from `rows` ONLY while there is room for another match. +/// `for oid in rows { if out.len() >= limit { break } … }` yields an oid first +/// and discards it on the break, without recording it in the frontier — and a +/// cursor start-point lost that way is in neither `visited` nor `candidates`, +/// so `finish` omits it and every commit reachable only through that lane +/// disappears from the log for good. The plain log sidesteps this by taking +/// exactly `limit`; a search cannot, because it decides per commit whether one +/// counts. +fn filtered_page( + repo: &Repository, + rows: I, + ref_map: &RefMap, + seed: &[git2::Oid], + limit: usize, + mut matches: impl FnMut(&Repository, &git2::Commit<'_>) -> AppResult, +) -> AppResult<(Vec, Option>, usize)> +where + I: Iterator>, +{ + let mut rows = rows; + let mut out = Vec::new(); + // The frontier tracks every VISITED commit, not just the matches: resuming + // from a match's parents would skip the non-matching commits between them + // and lose their ancestors entirely. + let mut frontier = FrontierBuilder::new(limit.min(4096)); + frontier.seed(seed); + let mut read = 0usize; + + while out.len() < limit { + let Some(oid) = rows.next() else { break }; + let oid = oid?; + let commit = repo.find_commit(oid)?; + frontier.visit(oid, commit.parent_ids()); + read += 1; + if !matches(repo, &commit)? { + continue; + } + let mut info = commit_to_info(&commit); + info.refs = ref_map.get(&oid).cloned().unwrap_or_default(); + out.push(info); + } + + Ok((out, frontier.finish(repo), read)) +} + + +struct PagePlan { + order: Arc, + offset: usize, + /// The live lanes at `offset` — what the frontier builder seeds with, so a + /// lane this page stops short of survives into the next cursor. + seed: Vec, + /// Where the order is filed, so the frontier this page emits can be filed + /// beside it. + key: WalkKey, +} + /// Seed a revwalk for a page: from the cursor frontier when resuming, /// otherwise from `refspec`/HEAD. An empty result means "nothing to walk". /// @@ -2701,13 +3117,29 @@ fn push_page_start( } } -/// Map git2's per-ref lookup by target OID. Scans once per log call. -fn collect_ref_map(repo: &Repository) -> HashMap> { +/// Map git2's per-ref lookup by target OID, and the fingerprint of the ref +/// database it was built from. +/// +/// The fingerprint rides along rather than being asked for separately because +/// this scan is the expensive thing (#473): on a repository with 7,001 loose +/// refs, one enumeration is 113 ms and the peeling inside it is 10 ms of that. +/// A caller that computed a fingerprint first and then called this would +/// enumerate everything twice. +/// +/// No longer once per log call — see `Libgit2Backend::ref_map`. +fn collect_ref_map(repo: &Repository) -> (HashMap>, u64) { // A map, not a list: every log walk decorates each of its rows from this, // and a linear scan per row was O(refs x rows) with a clone per hit. let mut out: HashMap> = HashMap::new(); - if let Ok(refs) = repo.references() { + let mut fp = RefFingerprint::default(); + let Ok(refs) = repo.references() else { + // Unreadable refdb: a fingerprint nobody can match, so this is rebuilt + // next time rather than a stale map being served forever. + return (out, u64::MAX); + }; + { for r in refs.flatten() { + fp.add(&r); let name = match r.shorthand() { Ok(n) => n.to_string(), Err(_) => continue, @@ -2750,7 +3182,7 @@ fn collect_ref_map(repo: &Repository) -> HashMap> { } } } - out + (out, fp.finish()) } fn parse_reflog_op(raw_message: &str) -> (ReflogOp, String) { @@ -3342,6 +3774,10 @@ impl GitBackend for Libgit2Backend { // file handles once any in-flight op's clone finishes. An absent id is // success on purpose (see the trait doc). map.remove(repo_id); + // The log cache IS dropped, unlike `rebases` below: every entry in it + // is a megabytes-sized snapshot of a repository nobody has open any + // more, and all of it is derivable again from disk (#473). + self.log_cache.forget(repo_id); // `rebases` is deliberately left alone: its entries are bytes, not // handles, they are keyed by an id a re-open can never mint again, and // an in-progress rebase rehydrates from `.git/platypusgit-rebase.json` @@ -3589,22 +4025,21 @@ impl GitBackend for Libgit2Backend { limit: usize, ) -> AppResult { self.with_repo_read(repo_id, |repo| { - let ref_map = collect_ref_map(repo); - let mut walk = repo.revwalk()?; - walk.set_sorting(Sort::TIME | Sort::TOPOLOGICAL)?; - let starts = push_page_start(repo, &mut walk, refspec, cursor)?; - if starts.is_empty() { + // A first page validates the decorations; a continuation reuses what + // its first page saw (#473). + let ref_map = self.ref_map(repo_id, repo, cursor.is_none()); + let Some(plan) = self.page_plan(repo_id, repo, refspec, cursor, limit)? else { return Ok(LogPage { commits: Vec::new(), next_cursor: None, }); - } + }; + let rows = plan.order.order[plan.offset..].iter().take(limit); let mut out = Vec::with_capacity(limit.min(4096)); let mut frontier = FrontierBuilder::new(limit.min(4096)); - frontier.seed(&starts); - for oid in walk.by_ref().take(limit) { - let oid = oid?; + frontier.seed(&plan.seed); + for &oid in rows { let commit = repo.find_commit(oid)?; let refs: Vec = ref_map.get(&oid).cloned().unwrap_or_default(); let mut info = commit_to_info(&commit); @@ -3613,8 +4048,21 @@ impl GitBackend for Libgit2Backend { out.push(info); } + let next = frontier.finish(repo); + // File the frontier against where it continues, so the page after + // this one is a slice of the same prepared walk rather than a + // second one (#473). + if let Some(f) = &next { + self.log_cache.remember_cursor( + repo_id, + &plan.key, + f.clone(), + plan.offset + out.len(), + ); + } + Ok(LogPage { - next_cursor: frontier.finish(repo), + next_cursor: next.as_deref().map(cursor_strings), commits: out, }) }) @@ -3689,45 +4137,29 @@ impl GitBackend for Libgit2Backend { }; self.with_repo(repo_id, |repo| { - let ref_map = collect_ref_map(repo); - let mut walk = repo.revwalk()?; - walk.set_sorting(Sort::TIME | Sort::TOPOLOGICAL)?; - let starts = push_page_start(repo, &mut walk, refspec, cursor)?; - if starts.is_empty() { - return Ok(LogPage { - commits: Vec::new(), - next_cursor: None, - }); - } - - let mut out = Vec::new(); - // The frontier tracks every VISITED commit, not just the matches: - // resuming from a match's parents would skip the non-matching - // commits between them and lose their ancestors entirely. - let mut frontier = FrontierBuilder::new(limit.min(4096)); - frontier.seed(&starts); - // Pull from the walk ONLY while there is room for another match. - // `for oid in walk { if out.len() >= limit { break } … }` yields an oid - // first and discards it on the break, without recording it in the - // frontier — and a cursor start-point lost that way is in neither - // `visited` nor `candidates`, so `finish` omits it and every commit - // reachable only through that lane disappears from the log for good. - // `log_page` sidesteps this with `walk.by_ref().take(limit)`; the - // filtered walk cannot, because it decides per commit whether one - // counts towards the limit. - let mut walk = walk; - while out.len() < limit { - let Some(oid) = walk.next() else { break }; - let oid = oid?; - let commit = repo.find_commit(oid)?; - frontier.visit(oid, commit.parent_ids()); + // A first page validates the decorations; a continuation reuses + // what its first page saw (#473). + let ref_map = self.ref_map(repo_id, repo, cursor.is_none()); + + // Which commits this page reads, and in what sequence: a prepared + // walk the log already paid for when one can answer the question, + // and otherwise a revwalk of this page's own — which is what EVERY + // filtered page did before #473. + let plan = self.cached_filtered_plan(repo_id, repo, refspec, cursor)?; + + // Whether a commit counts towards `limit`. Written once and handed + // to whichever source is reading, so the two cannot drift: a search + // returning different matches depending on whether a walk happened + // to be cached is the one bug this change must not have. + let matches = |repo: &Repository, commit: &git2::Commit<'_>| -> AppResult { + let oid = commit.id(); // sha prefix — cheap, check first. Compared nibble-by-nibble // off the raw bytes: `oid.to_string()` allocated 40 hex chars // for EVERY commit the walk visits, matching or not. if let Some(ref q) = sha_q { if !oid_has_hex_prefix(&oid, q) { - continue; + return Ok(false); } } @@ -3735,12 +4167,12 @@ impl GitBackend for Libgit2Backend { let ts = commit.time().seconds(); if let Some(since) = filter.since { if ts < since { - continue; + return Ok(false); } } if let Some(until) = filter.until { if ts > until { - continue; + return Ok(false); } } @@ -3750,7 +4182,7 @@ impl GitBackend for Libgit2Backend { let name = author.name().unwrap_or("").to_lowercase(); let email = author.email().unwrap_or("").to_lowercase(); if !name.contains(q.as_str()) && !email.contains(q.as_str()) { - continue; + return Ok(false); } } @@ -3758,14 +4190,14 @@ impl GitBackend for Libgit2Backend { if let Some(ref q) = message_q { let msg = commit.message().unwrap_or("").to_lowercase(); if !msg.contains(q.as_str()) { - continue; + return Ok(false); } } // path — expensive, check late. if let Some(ref p) = path_q { - if !commit_touches_path(repo, &commit, p)? { - continue; + if !commit_touches_path(repo, commit, p)? { + return Ok(false); } } @@ -3773,26 +4205,58 @@ impl GitBackend for Libgit2Backend { // commit, so it runs LAST: an author- or path-scoped search // only diffs the commits every cheaper filter already accepted. if let Some(ref m) = content_m { - if !commit_diff_matches_content(repo, &commit, m, path_q.as_deref())? { - continue; + if !commit_diff_matches_content(repo, commit, m, path_q.as_deref())? { + return Ok(false); } } - let refs: Vec = ref_map.get(&oid).cloned().unwrap_or_default(); - let mut info = commit_to_info(&commit); - info.refs = refs; - out.push(info); + Ok(true) + }; + + let (commits, next, at, filed) = match &plan { + Some(p) => { + let rows = p.order.order[p.offset..].iter().copied().map(Ok); + let (commits, next, read) = + filtered_page(repo, rows, &ref_map, &p.seed, limit, matches)?; + (commits, next, p.offset + read, Some(p.key.clone())) + } + None => { + let mut walk = repo.revwalk()?; + walk.set_sorting(Sort::TIME | Sort::TOPOLOGICAL)?; + let starts = push_page_start(repo, &mut walk, refspec, cursor)?; + if starts.is_empty() { + return Ok(LogPage { + commits: Vec::new(), + next_cursor: None, + }); + } + self.log_cache.record_walk_prepared(); + let rows = walk.map(|r| r.map_err(AppError::from)); + let (commits, next, read) = + filtered_page(repo, rows, &ref_map, &starts, limit, matches)?; + (commits, next, read, None) + } + }; + + // File the frontier against where it continues, so the page after + // this one slices the same prepared walk. ONLY when this page came + // out of a cached order: a page that walked for itself ends + // somewhere no order has an index for, and filing that offset + // against one would put the next page at the wrong depth. + if let (Some(f), Some(key)) = (&next, filed) { + self.log_cache.remember_cursor(repo_id, &key, f.clone(), at); } + Ok(LogPage { - next_cursor: frontier.finish(repo), - commits: out, + next_cursor: next.as_deref().map(cursor_strings), + commits, }) }) } fn commits_since(&self, repo_id: &RepoId, base: &str) -> AppResult> { self.with_repo(repo_id, |repo| { - let ref_map = collect_ref_map(repo); + let ref_map = self.ref_map(repo_id, repo, true); let head = match repo.head() { Ok(h) => h.peel_to_commit()?.id(), @@ -3842,7 +4306,7 @@ impl GitBackend for Libgit2Backend { limit: usize, ) -> AppResult> { self.with_repo(repo_id, |repo| { - let ref_map = collect_ref_map(repo); + let ref_map = self.ref_map(repo_id, repo, true); // `resolve_commit` maps a failure to InvalidRef with the offending // spec, so the UI can name the side the user typed wrong. let base_oid = resolve_commit(repo, base)?.id(); diff --git a/src-tauri/src/git/log_cache.rs b/src-tauri/src/git/log_cache.rs new file mode 100644 index 0000000..634731c --- /dev/null +++ b/src-tauri/src/git/log_cache.rs @@ -0,0 +1,544 @@ +//! What `log_page` used to rebuild on every single page (#473). +//! +//! Two things, and the benchmark in `docs/dev/performance.md` measured both. +//! +//! **The walk.** `log_page` walks with `Sort::TIME | Sort::TOPOLOGICAL`, +//! because the commit graph's lane assignment needs a parent to come after +//! every one of its children. In libgit2 1.9.7 that sort is not incremental in +//! any sense: `git_revwalk_sorting` sets `walk->limited`, so `prepare_walk` +//! runs `limit_list` over the whole reachable graph and then +//! `sort_in_topological_order` materialises the COMPLETE ordered list — +//! before the first oid comes out. Asking that walk for 500 commits and asking +//! it for all 1.5 million of them therefore cost the same thing, and building +//! one walk per page means paying that identical price per page. It shows up +//! as a per-page cost that is flat in depth: on `torvalds/linux`, page one is +//! 15.95 s and page ten is 157.67 s — ten times one page, not one page plus a +//! little. git pays it once and then skips. +//! +//! So the fix is not to make the walk cheaper — nothing here can, see the +//! commit-graph finding in `docs/dev/performance.md` — it is to stop throwing +//! the finished sort away. A walk is prepared once, its output is kept as a +//! `Vec`, and every later page is a slice of it. +//! +//! **The ref map.** `collect_ref_map` enumerated and peeled every ref on every +//! page, to decorate 500 rows. On a repository with 7,001 refs and 2,000 +//! commits that was 16× git's own work for the same question, with history a +//! twenty-fifth of the size of the fixture beside it. It is cached against a +//! FINGERPRINT of the ref database rather than a timer or a write hook, so a +//! `git tag` typed in a terminal invalidates it exactly like one made in the +//! app. +//! +//! ## This cache is an accelerator and nothing else +//! +//! Every entry here is derivable from the repository on disk, so dropping all +//! of it must change nothing but the clock. That is the property to preserve +//! when editing this file, it is what `log_walk_cache.rs` asserts by draining +//! the same history cold and warm and comparing the two sequences, and it is +//! why a poisoned mutex here degrades to a miss instead of failing the page. +//! +//! ## Why a continuation may be served from a stale walk +//! +//! A first page is keyed by (refspec, start oids), so a ref that moves changes +//! the key and the next first page rebuilds. A CONTINUATION is not keyed that +//! way and does not need to be: resuming from a cursor ignores the refs +//! entirely (`push_page_start` does too), the frontier's reachable set is made +//! of commits, and a commit is immutable. New history can only be added as +//! CHILDREN of what is already there, never inside the set an existing +//! frontier reaches. So the tail of a walk prepared ten minutes ago is the +//! same tail it would have today. + +use std::collections::HashMap; +use std::sync::atomic::{AtomicU64, Ordering}; +use std::sync::{Arc, Mutex}; + +use git2::Oid; + +use crate::git::types::{RefInfo, RepoId}; + +/// How much of one walk is kept, in commits. +/// +/// A bound, not a guess at what fits: the kernel's 1.48 M oids would be 30 MB +/// for one walk of one repository, and several tabs each hold their own. At +/// 100,000 an entry is 2 MB and covers two hundred 500-commit pages — further +/// than any human scrolls, and past it a page falls back to preparing a walk +/// from the cursor, which is what EVERY page did before this file existed. +pub const MAX_ORDER: usize = 100_000; + +/// How many distinct walks one repository remembers. +/// +/// Two, so that flipping the History scope between "All" and one branch — the +/// one alternation a user actually performs — does not evict on every switch. +const MAX_WALKS: usize = 2; + +/// How many emitted cursors one walk remembers, so a continuation can be +/// matched to the exact position it continues from. +/// +/// Unreachable in practice: `MAX_ORDER / 500` is 200 pages, so a walk runs out +/// of order long before it runs out of cursor slots. It exists so that a +/// pathological caller (`limit = 1`, scrolling forever) cannot grow the map +/// without bound. +const MAX_CURSORS: usize = 512; + +/// A commit's ref decorations, by the commit they point at. +pub type RefMap = HashMap>; + +/// Which walk this is: the question that was asked, and the commits it was +/// asked from. +/// +/// `starts` is SORTED, so two enumerations of the same refs in a different +/// order are the same key. It is the whole invalidation story for a first +/// page: any ref that moves changes an oid in here, which makes a new key, +/// which misses. +#[derive(Clone, PartialEq, Eq, Hash, Debug)] +pub struct WalkKey { + pub refspec: Option, + pub starts: Vec, +} + +impl WalkKey { + pub fn new(refspec: Option<&str>, starts: &[Oid]) -> Self { + let mut sorted = starts.to_vec(); + sorted.sort(); + Self { + refspec: refspec.map(str::to_string), + starts: sorted, + } + } +} + +/// One prepared walk's output, in the order the walk produced it. +#[derive(Debug)] +pub struct WalkOrder { + /// The start points that were pushed, in push order — what a first page + /// seeds its frontier with. Not `WalkKey::starts`, which is sorted for + /// hashing; the cursor's lane order is observable and should not shuffle. + pub starts: Vec, + /// Every oid the walk yielded, capped at `MAX_ORDER`. + pub order: Vec, + /// True when the walk ENDED inside the cap, so `order` is all of history + /// from `starts` and running off its end means the end of history. + pub complete: bool, +} + +impl WalkOrder { + /// Whether a page of `limit` starting at `offset` can be answered from + /// here. + /// + /// A short slice is only an answer when the walk is complete — then it IS + /// the end of history. An incomplete order that runs out mid-page would + /// hand back a page that stops for no reason the caller can see, so that + /// case rebuilds instead. + pub fn serves(&self, offset: usize, limit: usize) -> bool { + if self.complete { + offset <= self.order.len() + } else { + offset.saturating_add(limit) <= self.order.len() + } + } +} + +struct Walk { + key: WalkKey, + order: Arc, + /// A frontier this walk emitted → the index it resumes at. + /// + /// Keyed by the frontier itself, sorted, because that is what the caller + /// hands back. Matching by "the first oid of this frontier that appears in + /// some cached order" would be cheaper and is WRONG: two walks over the + /// same repository share commits, so a cursor from the "All" walk would + /// happily resolve against a single-branch walk and continue the wrong + /// history. A cursor we emitted is the only one we can place exactly. + cursors: HashMap, usize>, +} + +#[derive(Default)] +struct RepoEntry { + /// Most recently used first. + walks: Vec, + /// Ref-database fingerprint → the decorations built from it. + refs: Option<(u64, Arc)>, +} + +/// What the cache did, for anyone who has to prove it did it. +/// +/// A cache that never hits is invisible from the outside: pagination returns +/// exactly the same commits either way, which is the property this file is +/// built for and also the reason a functional test cannot tell the two apart. +/// The alternative — asserting on a clock — is a flake. So the counters are +/// public, `log_walk_cache.rs` reads them, and so does the benchmark. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub struct CacheStats { + /// Pages served from an already-prepared walk. + pub hits: u64, + /// Walks prepared — the expensive thing this file exists to avoid. + pub walks_prepared: u64, + /// Ref maps built — every one of them enumerated and peeled every ref. + pub ref_maps_built: u64, +} + +/// Per-repository derived state for the paged log. +#[derive(Default)] +pub struct LogCache { + repos: Mutex>, + hits: AtomicU64, + walks_prepared: AtomicU64, + ref_maps_built: AtomicU64, +} + +impl LogCache { + pub fn new() -> Self { + Self::default() + } + + pub fn stats(&self) -> CacheStats { + CacheStats { + hits: self.hits.load(Ordering::Relaxed), + walks_prepared: self.walks_prepared.load(Ordering::Relaxed), + ref_maps_built: self.ref_maps_built.load(Ordering::Relaxed), + } + } + + /// Count a walk that had to be prepared. Called by the backend, which is + /// the only place that can know a build actually happened. + pub fn record_walk_prepared(&self) { + self.walks_prepared.fetch_add(1, Ordering::Relaxed); + } + + /// The order for a FIRST page of `key`, if one is cached and can answer it. + pub fn first_page(&self, repo: &RepoId, key: &WalkKey, limit: usize) -> Option> { + let mut repos = self.repos.lock().ok()?; + let entry = repos.get_mut(repo)?; + let at = entry.walks.iter().position(|w| &w.key == key)?; + if !entry.walks[at].order.serves(0, limit) { + return None; + } + let walk = entry.walks.remove(at); + let order = Arc::clone(&walk.order); + entry.walks.insert(0, walk); + self.hits.fetch_add(1, Ordering::Relaxed); + Some(order) + } + + /// The walk a cursor continues, and where in it the next page begins. + /// + /// `cursor` must be sorted — the caller sorts what it receives, and + /// `remember_cursor` sorts what it emits, so the two agree. + pub fn resume( + &self, + repo: &RepoId, + cursor: &[Oid], + limit: usize, + ) -> Option<(WalkKey, Arc, usize)> { + let mut repos = self.repos.lock().ok()?; + let entry = repos.get_mut(repo)?; + let at = entry + .walks + .iter() + .position(|w| w.cursors.contains_key(cursor))?; + let offset = *entry.walks[at].cursors.get(cursor)?; + if !entry.walks[at].order.serves(offset, limit) { + return None; + } + let walk = entry.walks.remove(at); + let out = (walk.key.clone(), Arc::clone(&walk.order), offset); + entry.walks.insert(0, walk); + self.hits.fetch_add(1, Ordering::Relaxed); + Some(out) + } + + /// File a freshly prepared walk, evicting the least recently used one. + /// Whether a walk for `key` is already filed, whatever it can answer. + /// + /// Not the same question as `first_page`, and the difference is the point. + /// `first_page` says "can this serve the page in hand", and a CAPPED order + /// says no to a filtered page every time (see `cached_filtered_plan`). This + /// says "has this walk been prepared before", so a search does not prepare + /// a second one to rediscover that the history is longer than `MAX_ORDER` — + /// which would pay for the topological sort twice on every search, on + /// exactly the repositories where it is most expensive. + /// + /// Deliberately counts no hit and moves nothing: it is a question about the + /// cache, not a read from it. + pub fn has_walk(&self, repo: &RepoId, key: &WalkKey) -> bool { + self.repos + .lock() + .map(|repos| { + repos + .get(repo) + .is_some_and(|e| e.walks.iter().any(|w| &w.key == key)) + }) + .unwrap_or(false) + } + + pub fn insert(&self, repo: &RepoId, key: WalkKey, order: Arc) { + let Ok(mut repos) = self.repos.lock() else { + return; + }; + let entry = repos.entry(repo.clone()).or_default(); + entry.walks.retain(|w| w.key != key); + entry.walks.insert( + 0, + Walk { + key, + order, + cursors: HashMap::new(), + }, + ); + entry.walks.truncate(MAX_WALKS); + } + + /// Record that `cursor` continues `key` at `next`, so the page after it + /// can be a slice instead of a walk. + pub fn remember_cursor(&self, repo: &RepoId, key: &WalkKey, mut cursor: Vec, next: usize) { + let Ok(mut repos) = self.repos.lock() else { + return; + }; + let Some(entry) = repos.get_mut(repo) else { + return; + }; + let Some(walk) = entry.walks.iter_mut().find(|w| &w.key == key) else { + return; + }; + cursor.sort(); + // Cleared rather than evicted one by one: keeping an insertion order + // beside the map would double what the keys cost, to defend a bound + // that a 500-commit page cannot reach in the first place. + if walk.cursors.len() >= MAX_CURSORS { + walk.cursors.clear(); + } + walk.cursors.insert(cursor, next); + } + + /// The ref decorations this repository last built, and the fingerprint of + /// the ref database they were built from. + /// + /// The caller decides what to do with the pair, and that is the point: a + /// cold call must NOT enumerate the refs to produce a fingerprint it is + /// about to throw away — doing exactly that made a cold eleven-read + /// fan-out on the `refs` fixture slower than the version this replaced + /// (219 ms → 364 ms), because it enumerated 7,001 loose refs twice. + pub fn ref_map(&self, repo: &RepoId) -> Option<(u64, Arc)> { + let repos = self.repos.lock().ok()?; + let (had, map) = repos.get(repo)?.refs.as_ref()?; + Some((*had, Arc::clone(map))) + } + + /// File a freshly built ref map. Called only after `collect_ref_map` ran, + /// which is what makes the counter here mean "a map was built". + pub fn put_ref_map(&self, repo: &RepoId, fingerprint: u64, map: Arc) { + self.ref_maps_built.fetch_add(1, Ordering::Relaxed); + let Ok(mut repos) = self.repos.lock() else { + return; + }; + repos.entry(repo.clone()).or_default().refs = Some((fingerprint, map)); + } + + /// Drop everything held for one repository — `close` calls this, so the + /// memory goes away with the tab rather than with the process. + pub fn forget(&self, repo: &RepoId) { + if let Ok(mut repos) = self.repos.lock() { + repos.remove(repo); + } + } +} + +#[cfg(test)] +mod tests { + use super::*; + + fn oid(n: u8) -> Oid { + let mut raw = [0u8; 20]; + raw[19] = n; + Oid::from_bytes(&raw).unwrap() + } + + fn order(n: usize, complete: bool) -> Arc { + Arc::new(WalkOrder { + starts: vec![oid(0)], + order: (0..n).map(|i| oid(i as u8)).collect(), + complete, + }) + } + + fn repo() -> RepoId { + RepoId("r".into()) + } + + #[test] + fn a_first_page_hits_the_walk_it_was_filed_under() { + let c = LogCache::new(); + let key = WalkKey::new(None, &[oid(1)]); + c.insert(&repo(), key.clone(), order(10, true)); + + assert!(c.first_page(&repo(), &key, 5).is_some()); + assert_eq!(c.stats().hits, 1); + } + + /// The whole invalidation story for a first page: a moved ref is a + /// different start oid, which is a different key, which is a miss. + #[test] + fn a_moved_start_point_is_a_different_walk() { + let c = LogCache::new(); + c.insert(&repo(), WalkKey::new(None, &[oid(1)]), order(10, true)); + + let moved = WalkKey::new(None, &[oid(2)]); + assert!(c.first_page(&repo(), &moved, 5).is_none()); + } + + /// …and the enumeration order of the refs is not part of that story. + #[test] + fn the_start_points_are_a_set_not_a_list() { + let c = LogCache::new(); + c.insert( + &repo(), + WalkKey::new(None, &[oid(1), oid(2), oid(3)]), + order(10, true), + ); + + let shuffled = WalkKey::new(None, &[oid(3), oid(1), oid(2)]); + assert!(c.first_page(&repo(), &shuffled, 5).is_some()); + } + + #[test] + fn a_different_refspec_is_a_different_walk() { + let c = LogCache::new(); + c.insert(&repo(), WalkKey::new(None, &[oid(1)]), order(10, true)); + + let scoped = WalkKey::new(Some("refs/heads/main"), &[oid(1)]); + assert!(c.first_page(&repo(), &scoped, 5).is_none()); + } + + #[test] + fn a_remembered_cursor_resumes_where_it_left_off() { + let c = LogCache::new(); + let key = WalkKey::new(None, &[oid(1)]); + c.insert(&repo(), key.clone(), order(10, true)); + c.remember_cursor(&repo(), &key, vec![oid(5), oid(6)], 5); + + // Sorted the other way round: the caller hands back whatever order the + // frontier travelled in, and it must still match. + let (found, _, at) = c.resume(&repo(), &[oid(5), oid(6)], 2).expect("resume"); + assert_eq!(found, key); + assert_eq!(at, 5); + } + + /// A cursor nobody emitted is a miss, not a guess. Two walks over one + /// repository share commits, so "find an order containing these oids" + /// would place an "All" cursor inside a single-branch walk. + #[test] + fn an_unknown_cursor_does_not_resolve() { + let c = LogCache::new(); + let key = WalkKey::new(None, &[oid(1)]); + c.insert(&repo(), key, order(10, true)); + + assert!(c.resume(&repo(), &[oid(5)], 2).is_none()); + } + + /// An incomplete order cannot answer a page that runs off its end — that + /// page would stop early for a reason the caller cannot see. + #[test] + fn an_incomplete_order_refuses_a_page_it_cannot_fill() { + let c = LogCache::new(); + let key = WalkKey::new(None, &[oid(1)]); + c.insert(&repo(), key.clone(), order(10, false)); + c.remember_cursor(&repo(), &key, vec![oid(9)], 9); + + assert!(c.resume(&repo(), &[oid(9)], 5).is_none(), "9 + 5 > 10"); + assert!(c.resume(&repo(), &[oid(9)], 1).is_some(), "9 + 1 == 10"); + } + + /// A COMPLETE order answers the same page happily: running out is the end + /// of history, which is a real answer. + #[test] + fn a_complete_order_answers_past_its_end() { + let c = LogCache::new(); + let key = WalkKey::new(None, &[oid(1)]); + c.insert(&repo(), key.clone(), order(10, true)); + c.remember_cursor(&repo(), &key, vec![oid(9)], 9); + + assert!(c.resume(&repo(), &[oid(9)], 500).is_some()); + } + + #[test] + fn a_third_walk_evicts_the_least_recently_used_one() { + let c = LogCache::new(); + let a = WalkKey::new(None, &[oid(1)]); + let b = WalkKey::new(None, &[oid(2)]); + let d = WalkKey::new(None, &[oid(3)]); + c.insert(&repo(), a.clone(), order(10, true)); + c.insert(&repo(), b.clone(), order(10, true)); + // Touch `a` so `b` becomes the coldest. + c.first_page(&repo(), &a, 1).expect("a is cached"); + c.insert(&repo(), d.clone(), order(10, true)); + + assert!(c.first_page(&repo(), &a, 1).is_some(), "a was used last"); + assert!(c.first_page(&repo(), &d, 1).is_some(), "d is newest"); + assert!(c.first_page(&repo(), &b, 1).is_none(), "b was coldest"); + } + + /// Re-filing a key replaces it rather than growing a second copy, or two + /// pages of the same walk would evict everything else between them. + #[test] + fn re_inserting_a_key_replaces_it() { + let c = LogCache::new(); + let a = WalkKey::new(None, &[oid(1)]); + let b = WalkKey::new(None, &[oid(2)]); + c.insert(&repo(), a.clone(), order(10, true)); + c.insert(&repo(), a.clone(), order(10, true)); + c.insert(&repo(), b.clone(), order(10, true)); + + assert!(c.first_page(&repo(), &a, 1).is_some(), "a must survive"); + assert!(c.first_page(&repo(), &b, 1).is_some(), "b must survive"); + } + + #[test] + fn a_ref_map_survives_only_its_own_fingerprint() { + let c = LogCache::new(); + c.put_ref_map(&repo(), 7, Arc::new(RefMap::new())); + + let (had, _) = c.ref_map(&repo()).expect("cached"); + assert_eq!(had, 7, "the fingerprint travels back with the map"); + } + + #[test] + fn closing_a_repository_forgets_everything() { + let c = LogCache::new(); + let key = WalkKey::new(None, &[oid(1)]); + c.insert(&repo(), key.clone(), order(10, true)); + c.put_ref_map(&repo(), 7, Arc::new(RefMap::new())); + + c.forget(&repo()); + + assert!(c.first_page(&repo(), &key, 1).is_none()); + assert!(c.ref_map(&repo()).is_none()); + } + + /// Two repositories share this cache and must not share entries — the tab + /// next door is a different history under the same refspec. + #[test] + fn repositories_do_not_share_entries() { + let c = LogCache::new(); + let key = WalkKey::new(None, &[oid(1)]); + c.insert(&RepoId("a".into()), key.clone(), order(10, true)); + + assert!(c.first_page(&RepoId("b".into()), &key, 1).is_none()); + } + + /// The cursor map is bounded, and a full one drops back to a miss rather + /// than growing. + #[test] + fn the_cursor_map_is_bounded() { + let c = LogCache::new(); + let key = WalkKey::new(None, &[oid(1)]); + c.insert(&repo(), key.clone(), order(MAX_ORDER, true)); + for i in 0..MAX_CURSORS + 1 { + c.remember_cursor(&repo(), &key, vec![oid(1), oid((i % 251) as u8)], i); + } + + let Ok(repos) = c.repos.lock() else { + panic!("poisoned") + }; + let held = repos[&repo()].walks[0].cursors.len(); + assert!(held <= MAX_CURSORS, "held {held} cursors"); + } +} diff --git a/src-tauri/src/git/mod.rs b/src-tauri/src/git/mod.rs index ba96e31..b1c55b3 100644 --- a/src-tauri/src/git/mod.rs +++ b/src-tauri/src/git/mod.rs @@ -8,6 +8,7 @@ pub mod hooks; pub mod image; pub mod libgit2; pub mod lfs; +pub mod log_cache; pub mod notes; pub mod ownership; pub mod rebase_plan; diff --git a/src-tauri/tests/log_filtered_cache.rs b/src-tauri/tests/log_filtered_cache.rs new file mode 100644 index 0000000..2a4d92e --- /dev/null +++ b/src-tauri/tests/log_filtered_cache.rs @@ -0,0 +1,193 @@ +//! The FILTERED log pages out of the same prepared walk (#473). +//! +//! `log_filtered_page` had both of the defects `log_page` had, for the same +//! reason and in the same two lines: a revwalk built per page, and +//! `collect_ref_map` called per page. Commit search is the surface where that +//! hurts most on a large repository, because a search that matches nothing +//! recent walks a long way before it fills a page — and then the next page +//! threw that walk away and started the topological sort again. +//! +//! Same split as `log_walk_cache.rs`, and for the same reason. **Transparency** +//! drains one search warm and cold and compares, which is the pre-change +//! behaviour held against the new one. **Effect** asserts on the counters, +//! because every transparency test here passes just as well against a cache +//! that never hits. + +mod support; + +use platypusgit_lib::git::libgit2::Libgit2Backend; +use platypusgit_lib::git::types::{LogFilter, LogPage, RepoId}; +use platypusgit_lib::git::GitBackend; +use support::{linear_history, TempRepo}; + +/// Matches `commit 1`, `commit 10`…`commit 19` — 11 of 30, spread across the +/// history rather than bunched at one end, so a page boundary falls inside the +/// matches and the walk has to carry on past commits that do not match. +fn filter() -> LogFilter { + LogFilter { + message: Some("commit 1".into()), + ..Default::default() + } +} + +fn summaries(page: &LogPage) -> Vec { + page.commits.iter().map(|c| c.summary.clone()).collect() +} + +/// Page a search through ONE backend — the cache is hot from page two. +fn search_warm(be: &Libgit2Backend, id: &RepoId, page: usize) -> Vec { + let mut out = Vec::new(); + let mut cursor: Option> = None; + loop { + let p = be + .log_filtered_page(id, &filter(), None, cursor.as_deref(), page) + .unwrap(); + out.extend(summaries(&p)); + match p.next_cursor { + Some(c) => cursor = Some(c), + None => break, + } + assert!(out.len() < 10_000, "pagination did not terminate"); + } + out +} + +/// The same search, on a backend that has never seen this repository — so +/// every page pays for its own walk, exactly as every page used to. +fn search_cold(tr: &TempRepo, page: usize) -> Vec { + let mut out = Vec::new(); + let mut cursor: Option> = None; + loop { + let (be, handle) = tr.open_with_backend(); + let p = be + .log_filtered_page(&handle.id, &filter(), None, cursor.as_deref(), page) + .unwrap(); + out.extend(summaries(&p)); + match p.next_cursor { + Some(c) => cursor = Some(c), + None => break, + } + assert!(out.len() < 10_000, "pagination did not terminate"); + } + out +} + +fn history(n: usize) -> TempRepo { + let tr = TempRepo::with_initial_commit("hi\n"); + linear_history(&tr, n); + tr +} + +/// Transparency: the cache changes the clock and nothing else. +#[test] +fn a_paged_search_returns_the_same_commits_warm_and_cold() { + let tr = history(30); + let (be, handle) = tr.open_with_backend(); + + let warm = search_warm(&be, &handle.id, 4); + let cold = search_cold(&tr, 4); + + assert_eq!(warm, cold, "a cached search must return what a cold one does"); + assert_eq!(warm.len(), 11, "got {warm:?}"); +} + +/// Transparency, at a page size that puts a boundary between two matches. +#[test] +fn a_paged_search_agrees_with_an_unpaged_one() { + let tr = history(30); + let (be, handle) = tr.open_with_backend(); + + let paged = search_warm(&be, &handle.id, 3); + let whole: Vec = be + .log_filtered(&handle.id, &filter(), None, 100) + .unwrap() + .iter() + .map(|c| c.summary.clone()) + .collect(); + + assert_eq!(paged, whole, "paging a search must not drop or reorder a match"); +} + +/// Effect: the walk. THE test for the filtered half of #473 — planting the +/// violation (build a revwalk per filtered page, as it did) makes this one +/// walk per page instead. +#[test] +fn paging_a_search_prepares_one_walk() { + let tr = history(30); + let (be, handle) = tr.open_with_backend(); + + let found = search_warm(&be, &handle.id, 3); + + assert_eq!(found.len(), 11, "got {found:?}"); + assert_eq!( + be.log_cache_stats().walks_prepared, + 1, + "a search paged four deep must prepare ONE walk, not one per page", + ); +} + +/// Effect: the ref map. It decorated 500 rows by enumerating and peeling every +/// ref, once per page. +#[test] +fn paging_a_search_builds_one_ref_map() { + let tr = history(30); + let (be, handle) = tr.open_with_backend(); + + search_warm(&be, &handle.id, 3); + + assert_eq!( + be.log_cache_stats().ref_maps_built, + 1, + "a continuation must decorate with the refs its first page saw", + ); +} + +/// A search asks the same question of the same graph as the log beside it, so +/// it reads the walk the log already prepared rather than preparing a second. +#[test] +fn a_search_reuses_the_walk_the_log_prepared() { + let tr = history(30); + let (be, handle) = tr.open_with_backend(); + + be.log_page(&handle.id, None, None, 5).unwrap(); + assert_eq!(be.log_cache_stats().walks_prepared, 1); + + let hits = be + .log_filtered_page(&handle.id, &filter(), None, None, 100) + .unwrap(); + + assert_eq!(summaries(&hits).len(), 11); + assert_eq!( + be.log_cache_stats().walks_prepared, + 1, + "a search must not re-prepare a walk the log already materialised", + ); +} + +/// A commit moves HEAD, so the search's walk is rebuilt rather than served +/// from an order that no longer describes this repository. +#[test] +fn a_moved_head_re_prepares_the_search_walk() { + let tr = history(30); + let (be, handle) = tr.open_with_backend(); + + be.log_filtered_page(&handle.id, &filter(), None, None, 5) + .unwrap(); + assert_eq!(be.log_cache_stats().walks_prepared, 1); + + tr.add_commit("late.txt", "late\n", "commit 100"); + + let after = be + .log_filtered_page(&handle.id, &filter(), None, None, 5) + .unwrap(); + + assert_eq!( + after.commits[0].summary, "commit 100", + "a moved HEAD must not be searched from the cached order", + ); + assert_eq!( + be.log_cache_stats().walks_prepared, + 2, + "the walk must be re-prepared once HEAD has moved", + ); +} diff --git a/src-tauri/tests/log_walk_cache.rs b/src-tauri/tests/log_walk_cache.rs new file mode 100644 index 0000000..a01af26 --- /dev/null +++ b/src-tauri/tests/log_walk_cache.rs @@ -0,0 +1,604 @@ +//! The paged log's walk cache (#473). +//! +//! `log_page` used to prepare a fresh revwalk per page, and in libgit2 a +//! topologically sorted walk materialises its COMPLETE ordered list before it +//! yields anything — so every page paid for sorting all of history. Ten pages +//! into `torvalds/linux` cost two minutes and thirty-eight seconds, ten times +//! what one page cost. The walk is now prepared once and paged from. +//! +//! Two kinds of test here, and the split is the point. +//! +//! **Transparency.** Everything the cache holds is derivable from the +//! repository, so turning it off must change nothing but the clock. Each of +//! these drains the same history twice — once through one backend, where the +//! cache is hot, and once through a fresh backend per page, where it can never +//! hit — and compares the two. That is the pre-change behaviour, byte for +//! byte, held against the new one. +//! +//! **Effect.** Every transparency test above passes just as well against a +//! cache that never hits, which would make them worthless on their own. So the +//! rest assert on `log_cache_stats`: how many walks a drain prepared, and that +//! a moved ref makes the next one prepare another. Those are the tests that go +//! red when the cache stops working — and the ones to plant a violation +//! against before trusting an edit here. + +mod support; + +use platypusgit_lib::git::libgit2::Libgit2Backend; +use platypusgit_lib::git::types::{RefKind, RepoId}; +use platypusgit_lib::git::GitBackend; +use support::{linear_history, TempRepo}; + +/// One row as the UI sees it: the commit, and the pills stamped on it. +type Row = (String, Vec<(String, RefKind)>); + +fn rows(page: &platypusgit_lib::git::types::LogPage) -> Vec { + page.commits + .iter() + .map(|c| { + ( + c.oid.clone(), + c.refs.iter().map(|r| (r.name.clone(), r.kind)).collect(), + ) + }) + .collect() +} + +/// Walk all of history through ONE backend — the cache is hot from page two. +fn drain_warm(be: &Libgit2Backend, id: &RepoId, refspec: Option<&str>, page: usize) -> Vec { + let mut out = Vec::new(); + let mut cursor: Option> = None; + loop { + let p = be.log_page(id, refspec, cursor.as_deref(), page).unwrap(); + out.extend(rows(&p)); + match p.next_cursor { + Some(c) => cursor = Some(c), + None => break, + } + assert!(out.len() < 10_000, "pagination did not terminate"); + } + out +} + +/// Walk all of history through a FRESH backend per page, so no page is ever +/// served from a cache. This is what the code did before #473. +fn drain_cold(tr: &TempRepo, refspec: Option<&str>, page: usize) -> Vec { + let mut out = Vec::new(); + let mut cursor: Option> = None; + loop { + let (be, handle) = tr.open_with_backend(); + let p = be + .log_page(&handle.id, refspec, cursor.as_deref(), page) + .unwrap(); + out.extend(rows(&p)); + match p.next_cursor { + Some(c) => cursor = Some(c), + None => break, + } + assert!(out.len() < 10_000, "pagination did not terminate"); + } + out +} + +fn checkout(tr: &TempRepo, branch: &str) { + tr.repo.set_head(&format!("refs/heads/{branch}")).unwrap(); + let mut co = git2::build::CheckoutBuilder::new(); + co.force(); + tr.repo.checkout_head(Some(&mut co)).unwrap(); +} + +/// A commit on HEAD at an EXACT time, because the walk's order depends on it. +/// +/// libgit2 sorts the topological queue by commit time, and `git_pqueue` is a +/// binary heap whose comparator is not stable — so among commits sharing a +/// second, which one comes out first depends on the order they were inserted. +/// A fixture built with `Signature::now` makes every commit in the same second +/// and leaves the order of two live lanes genuinely ambiguous, which no test +/// can then pin. Every multi-lane fixture below therefore spaces its commits +/// out — `merged_history(3600)` — and the one case that deliberately does not, +/// `merged_history(0)`, asserts only what a tie can still promise. +fn commit_at(tr: &TempRepo, file: &str, message: &str, when: i64) -> git2::Oid { + support::fs::write_file(tr.path(), file, message); + let mut index = tr.repo.index().unwrap(); + index.add_path(std::path::Path::new(file)).unwrap(); + index.write().unwrap(); + let tree_oid = index.write_tree().unwrap(); + let tree = tr.repo.find_tree(tree_oid).unwrap(); + let sig = + git2::Signature::new("Test", "test@example.com", &git2::Time::new(when, 0)).unwrap(); + let head = tr.repo.head().ok().and_then(|h| h.peel_to_commit().ok()); + let parents: Vec<&git2::Commit> = head.iter().collect(); + tr.repo + .commit(Some("HEAD"), &sig, &sig, message, &tree, &parents) + .unwrap() +} + +/// initial → (main 1, main 2) and (feat 1, feat 2) → a real merge commit, so +/// the walk has two live lanes and a page boundary can land between them. +fn merged_history(spacing: i64) -> TempRepo { + let tr = TempRepo::fresh(); + commit_at(&tr, "README.md", "initial", 1_700_000_000); + { + let base = tr.repo.head().unwrap().peel_to_commit().unwrap(); + tr.repo.branch("feature", &base, false).unwrap(); + } + commit_at(&tr, "m1.txt", "main 1", 1_700_000_000 + spacing); + commit_at(&tr, "m2.txt", "main 2", 1_700_000_000 + 2 * spacing); + checkout(&tr, "feature"); + commit_at(&tr, "f1.txt", "feat 1", 1_700_000_000 + 3 * spacing); + commit_at(&tr, "f2.txt", "feat 2", 1_700_000_000 + 4 * spacing); + checkout(&tr, "main"); + { + let main_tip = tr.repo.head().unwrap().peel_to_commit().unwrap(); + let feat_tip = tr + .repo + .find_branch("feature", git2::BranchType::Local) + .unwrap() + .get() + .peel_to_commit() + .unwrap(); + let tree = main_tip.tree().unwrap(); + let sig = git2::Signature::new( + "Test", + "test@example.com", + &git2::Time::new(1_700_000_000 + 5 * spacing, 0), + ) + .unwrap(); + tr.repo + .commit( + Some("HEAD"), + &sig, + &sig, + "merge feature", + &tree, + &[&main_tip, &feat_tip], + ) + .unwrap(); + } + tr +} + +/// Every parent that appears in `rows` appears AFTER its child — the promise +/// the whole topological sort exists to make, and the one that survives a tie. +fn is_topological(tr: &TempRepo, rows: &[Row]) -> Result<(), String> { + let at: std::collections::HashMap<&str, usize> = rows + .iter() + .enumerate() + .map(|(i, (oid, _))| (oid.as_str(), i)) + .collect(); + for (i, (oid, _)) in rows.iter().enumerate() { + let commit = tr + .repo + .find_commit(git2::Oid::from_str(oid).unwrap()) + .unwrap(); + for parent in commit.parent_ids() { + if let Some(&j) = at.get(parent.to_string().as_str()) { + if j <= i { + return Err(format!("{oid} at {i} has parent {parent} at {j}")); + } + } + } + } + Ok(()) +} + +// ---------------------------------------------------------------- transparency + +/// The property everything else rests on: cached and uncached produce the same +/// history, page size by page size. Sizes that do and do not divide the 21 +/// commits, so a page boundary lands everywhere it can. +#[test] +fn a_cached_walk_pages_exactly_like_an_uncached_one() { + let tr = TempRepo::with_initial_commit("hi\n"); + linear_history(&tr, 20); + let (be, handle) = tr.open_with_backend(); + + for page in [1, 2, 3, 5, 7, 21, 500] { + let warm = drain_warm(&be, &handle.id, None, page); + let cold = drain_cold(&tr, None, page); + assert_eq!(warm, cold, "page size {page}"); + assert_eq!(warm.len(), 21, "page size {page}"); + } +} + +/// The same, where a page boundary can fall between two live lanes — the case +/// the frontier cursor exists for, and the one an off-by-one in the resume +/// offset would corrupt without changing the commit COUNT. +#[test] +fn a_merge_pages_the_same_cached_and_uncached() { + let tr = merged_history(3600); + let (be, handle) = tr.open_with_backend(); + + for page in [1, 2, 3, 6, 500] { + let warm = drain_warm(&be, &handle.id, None, page); + let cold = drain_cold(&tr, None, page); + assert_eq!(warm, cold, "page size {page}"); + assert_eq!(warm.len(), 6, "page size {page}"); + } +} + +/// Every branch tip is a start point, so `--all` is the walk with the most +/// lanes and the most seeding to get wrong. +#[test] +fn the_all_scope_pages_the_same_cached_and_uncached() { + let tr = merged_history(3600); + let (be, handle) = tr.open_with_backend(); + + for page in [1, 2, 4, 500] { + let warm = drain_warm(&be, &handle.id, Some("--all"), page); + let cold = drain_cold(&tr, Some("--all"), page); + assert_eq!(warm, cold, "page size {page}"); + } +} + +/// When two lanes share a commit SECOND, which of them comes first is not +/// something either walk promises: libgit2 orders the topological queue by +/// time through a binary heap with an unstable comparator, so the answer +/// depends on insertion order — and a walk restarted from a cursor inserts in +/// a different order than one that ran straight through. That was true of the +/// walk-per-page version too; it is simply now visible, because one prepared +/// walk is self-consistent where ten of them were not. +/// +/// What must hold regardless, and is the actual contract of a paged log: every +/// commit exactly once, and no parent before its child. +#[test] +fn a_tie_in_commit_time_still_pages_every_commit_exactly_once() { + let tr = merged_history(0); + let (be, handle) = tr.open_with_backend(); + + for page in [1, 2, 3, 6, 500] { + let warm = drain_warm(&be, &handle.id, None, page); + let cold = drain_cold(&tr, None, page); + + let seen: std::collections::BTreeSet<&String> = warm.iter().map(|(o, _)| o).collect(); + assert_eq!(seen.len(), warm.len(), "page size {page}: a commit repeated"); + assert_eq!( + seen, + cold.iter().map(|(o, _)| o).collect(), + "page size {page}: a different set of commits", + ); + is_topological(&tr, &warm).unwrap_or_else(|e| panic!("page size {page}: {e}")); + } +} + +/// Ref decorations travel with the row, and they come from the cached ref map. +/// A map that went stale would drop the tag from the second read. +#[test] +fn a_tag_made_after_the_first_page_shows_up_on_the_next_one() { + let tr = TempRepo::with_initial_commit("hi\n"); + linear_history(&tr, 3); + let (be, handle) = tr.open_with_backend(); + + let before = be.log_page(&handle.id, None, None, 10).unwrap(); + assert!( + before.commits[0].refs.iter().all(|r| r.name != "v1"), + "the tag does not exist yet", + ); + + let head = tr.repo.head().unwrap().peel_to_commit().unwrap(); + tr.repo + .tag_lightweight("v1", head.as_object(), false) + .unwrap(); + + let after = be.log_page(&handle.id, None, None, 10).unwrap(); + let pills: Vec<&str> = after.commits[0] + .refs + .iter() + .map(|r| r.name.as_str()) + .collect(); + assert!(pills.contains(&"v1"), "got {pills:?}"); +} + +/// An annotated tag is the expensive half of `collect_ref_map` — it is the one +/// that has to be peeled — so it is the one most worth proving still arrives +/// after the map has been cached once. +#[test] +fn an_annotated_tag_made_later_shows_up_too() { + let tr = TempRepo::with_initial_commit("hi\n"); + linear_history(&tr, 3); + let (be, handle) = tr.open_with_backend(); + be.log_page(&handle.id, None, None, 10).unwrap(); + + let head = tr.repo.head().unwrap().peel_to_commit().unwrap(); + let sig = git2::Signature::now("Test", "test@example.com").unwrap(); + tr.repo + .tag("v2", head.as_object(), &sig, "release two", false) + .unwrap(); + + let after = be.log_page(&handle.id, None, None, 10).unwrap(); + let tagged = after.commits[0] + .refs + .iter() + .any(|r| r.name == "v2" && r.kind == RefKind::Tag); + assert!(tagged, "got {:?}", after.commits[0].refs); +} + +/// A continuation decorates with the refs its FIRST page saw, and the next +/// first page picks the new one up. +/// +/// This is the one place the cache is visible from outside, and it is a +/// deliberate trade measured on the `refs` fixture: validating the +/// decorations costs 113 ms of a 117 ms page there, because enumerating 7,001 +/// loose refs is the whole expense. Once per refresh is proportionate — the +/// `branches` and `tags` reads beside it in the same fan-out pay 187 ms and +/// 149 ms for the same enumeration — and once per scroll is not. A scroll +/// therefore shows one coherent snapshot of the decorations rather than a +/// different one per page. +#[test] +fn a_continuation_keeps_the_decorations_its_first_page_saw() { + let tr = TempRepo::with_initial_commit("hi\n"); + linear_history(&tr, 5); + let (be, handle) = tr.open_with_backend(); + + let first = be.log_page(&handle.id, None, None, 2).unwrap(); + let cursor = first.next_cursor.clone().expect("more history"); + + // Tag a commit the NEXT page will contain. + let target = tr + .repo + .find_commit(git2::Oid::from_str(&cursor[0]).unwrap()) + .unwrap(); + tr.repo + .tag_lightweight("mid-scroll", target.as_object(), false) + .unwrap(); + + let next = be + .log_page(&handle.id, None, Some(&cursor), 2) + .unwrap(); + assert!( + next.commits[0].refs.iter().all(|r| r.name != "mid-scroll"), + "a continuation does not re-read the refs: {:?}", + next.commits[0].refs, + ); + + // …and a refresh — which is always a first page — does pick it up. + let refreshed = be.log_page(&handle.id, None, None, 500).unwrap(); + let tagged = refreshed + .commits + .iter() + .any(|c| c.refs.iter().any(|r| r.name == "mid-scroll")); + assert!(tagged, "a first page must validate the decorations"); +} + +/// A commit made after a page was cached must appear in the next first page — +/// the invalidation that matters most, because it happens every time the user +/// commits. +#[test] +fn a_new_commit_appears_in_the_next_first_page() { + let tr = TempRepo::with_initial_commit("hi\n"); + linear_history(&tr, 3); + let (be, handle) = tr.open_with_backend(); + + let before = be.log_page(&handle.id, None, None, 10).unwrap(); + assert_eq!(before.commits.len(), 4); + + tr.add_commit("new.txt", "new\n", "brand new"); + + let after = be.log_page(&handle.id, None, None, 10).unwrap(); + assert_eq!(after.commits.len(), 5, "the new commit is missing"); + assert_eq!(after.commits[0].summary, "brand new"); +} + +/// Two scopes over one repository are two different walks. Asking for one +/// right after the other must not hand back the other one's history. +#[test] +fn two_scopes_do_not_share_a_walk() { + let tr = merged_history(3600); + let (be, handle) = tr.open_with_backend(); + + let head_only = be.log_page(&handle.id, None, None, 500).unwrap(); + let feature = be + .log_page(&handle.id, Some("feature"), None, 500) + .unwrap(); + let head_again = be.log_page(&handle.id, None, None, 500).unwrap(); + + assert_eq!(head_only.commits.len(), 6, "HEAD sees the merge"); + assert_eq!(feature.commits.len(), 3, "feature does not"); + assert_eq!( + head_only.commits.iter().map(|c| &c.oid).collect::>(), + head_again.commits.iter().map(|c| &c.oid).collect::>(), + "the second read of HEAD came back as something else", + ); +} + +// ---------------------------------------------------------------------- effect + +/// The point of the whole change: N pages, one walk. +/// +/// This is the test that fails if `page_plan` stops consulting the cache, and +/// the one the numbers in `docs/dev/performance.md` are downstream of. Twenty +/// one commits in pages of two is eleven pages. +#[test] +fn paging_a_whole_history_prepares_exactly_one_walk() { + let tr = TempRepo::with_initial_commit("hi\n"); + linear_history(&tr, 20); + let (be, handle) = tr.open_with_backend(); + + let all = drain_warm(&be, &handle.id, None, 2); + assert_eq!(all.len(), 21); + + let stats = be.log_cache_stats(); + assert_eq!( + stats.walks_prepared, 1, + "eleven pages prepared {} walks", + stats.walks_prepared, + ); + assert_eq!(stats.hits, 10, "ten of the eleven pages should be slices"); +} + +/// …and the ref map is built once for those eleven pages, not eleven times. +#[test] +fn paging_a_whole_history_builds_exactly_one_ref_map() { + let tr = TempRepo::with_initial_commit("hi\n"); + linear_history(&tr, 20); + let (be, handle) = tr.open_with_backend(); + + drain_warm(&be, &handle.id, None, 2); + + assert_eq!(be.log_cache_stats().ref_maps_built, 1); +} + +/// A repeated first page — what `refreshAll` issues on every refresh — must +/// not prepare a second walk. On the kernel that one fact is 15.9 seconds per +/// refresh. +#[test] +fn a_repeated_first_page_prepares_no_second_walk() { + let tr = TempRepo::with_initial_commit("hi\n"); + linear_history(&tr, 10); + let (be, handle) = tr.open_with_backend(); + + for _ in 0..5 { + be.log_page(&handle.id, None, None, 500).unwrap(); + } + + assert_eq!(be.log_cache_stats().walks_prepared, 1); +} + +/// A commit invalidates the walk, because the start oid it is keyed by moved. +/// Without this the cache would be a correctness bug rather than a speed-up, +/// so it is asserted as a COUNT as well as by the commit showing up. +#[test] +fn a_commit_makes_the_next_first_page_prepare_a_new_walk() { + let tr = TempRepo::with_initial_commit("hi\n"); + linear_history(&tr, 3); + let (be, handle) = tr.open_with_backend(); + + be.log_page(&handle.id, None, None, 500).unwrap(); + assert_eq!(be.log_cache_stats().walks_prepared, 1); + + tr.add_commit("new.txt", "new\n", "brand new"); + be.log_page(&handle.id, None, None, 500).unwrap(); + + assert_eq!(be.log_cache_stats().walks_prepared, 2); +} + +/// A tag moves the ref fingerprint even though it moves no start point, so the +/// ref map is rebuilt and the walk is not. Both halves matter: rebuilding the +/// walk for a tag would throw away the expensive thing for the cheap reason. +#[test] +fn a_tag_rebuilds_the_ref_map_and_not_the_walk() { + let tr = TempRepo::with_initial_commit("hi\n"); + linear_history(&tr, 3); + let (be, handle) = tr.open_with_backend(); + + be.log_page(&handle.id, None, None, 500).unwrap(); + let before = be.log_cache_stats(); + + let head = tr.repo.head().unwrap().peel_to_commit().unwrap(); + tr.repo + .tag_lightweight("v1", head.as_object(), false) + .unwrap(); + be.log_page(&handle.id, None, None, 500).unwrap(); + let after = be.log_cache_stats(); + + assert_eq!(after.ref_maps_built, before.ref_maps_built + 1); + assert_eq!( + after.walks_prepared, before.walks_prepared, + "a tag is not a new walk", + ); +} + +/// The two places a ref fingerprint is computed must agree. +/// +/// `collect_ref_map` produces one from its own pass and `ref_fingerprint` +/// computes one to validate it, and if the two ever disagree the map built by +/// one is rejected by the other on the very next call. Nothing about that is +/// visible from outside — pagination stays correct, the decorations stay +/// correct, and the cache simply never hits again. So it is asserted through +/// the only door there is: a repository with every shape of ref in it, read +/// twice, must rebuild nothing the second time. +#[test] +fn the_two_fingerprint_paths_agree() { + let tr = TempRepo::with_initial_commit("hi\n"); + linear_history(&tr, 3); + { + let head = tr.repo.head().unwrap().peel_to_commit().unwrap(); + let sig = git2::Signature::now("Test", "test@example.com").unwrap(); + tr.repo.branch("topic", &head, false).unwrap(); + tr.repo + .tag_lightweight("light", head.as_object(), false) + .unwrap(); + tr.repo + .tag("heavy", head.as_object(), &sig, "annotated", false) + .unwrap(); + // A symbolic ref has no direct target, so it takes the other arm of the + // fingerprint — and of the map. + tr.repo + .reference_symbolic("refs/remotes/origin/HEAD", "refs/heads/main", true, "probe") + .unwrap(); + } + let (be, handle) = tr.open_with_backend(); + + be.log_page(&handle.id, Some("--all"), None, 500).unwrap(); + let first = be.log_cache_stats(); + be.log_page(&handle.id, Some("--all"), None, 500).unwrap(); + + assert_eq!( + be.log_cache_stats().ref_maps_built, + first.ref_maps_built, + "the validating fingerprint disagrees with the one the map was built \ + with, so the cache can never hit", + ); +} + +/// Nothing changed, so nothing is rebuilt — the case that is true on almost +/// every call, and the one a fingerprint that is not stable would silently +/// ruin. +#[test] +fn an_unchanged_repository_rebuilds_nothing() { + let tr = TempRepo::with_initial_commit("hi\n"); + linear_history(&tr, 3); + { + let head = tr.repo.head().unwrap().peel_to_commit().unwrap(); + let sig = git2::Signature::now("Test", "test@example.com").unwrap(); + tr.repo.branch("other", &head, false).unwrap(); + tr.repo + .tag_lightweight("light", head.as_object(), false) + .unwrap(); + tr.repo + .tag("heavy", head.as_object(), &sig, "annotated", false) + .unwrap(); + } + let (be, handle) = tr.open_with_backend(); + + be.log_page(&handle.id, Some("--all"), None, 500).unwrap(); + let first = be.log_cache_stats(); + for _ in 0..4 { + be.log_page(&handle.id, Some("--all"), None, 500).unwrap(); + } + let after = be.log_cache_stats(); + + assert_eq!(after.walks_prepared, first.walks_prepared, "walk rebuilt"); + assert_eq!(after.ref_maps_built, first.ref_maps_built, "ref map rebuilt"); + assert_eq!(after.hits, first.hits + 4, "four pages, four slices"); +} + +/// A cursor from one backend handed to another has nothing to resume against, +/// and must still page correctly — the fallback every page took before #473, +/// and the one a cache eviction drops back to. +#[test] +fn a_cursor_with_no_cache_behind_it_still_pages() { + let tr = TempRepo::with_initial_commit("hi\n"); + linear_history(&tr, 5); + let (first, handle) = tr.open_with_backend(); + + let page = first.log_page(&handle.id, None, None, 2).unwrap(); + let cursor = page.next_cursor.expect("more history"); + + let (second, other) = tr.open_with_backend(); + let resumed = second + .log_page(&other.id, None, Some(&cursor), 2) + .unwrap(); + + assert_eq!(resumed.commits.len(), 2); + assert_eq!( + second.log_cache_stats().walks_prepared, + 1, + "an unknown cursor has to prepare its own walk", + ); + let expected = drain_cold(&tr, None, 2); + assert_eq!( + resumed.commits.iter().map(|c| &c.oid).collect::>(), + expected[2..4].iter().map(|(o, _)| o).collect::>(), + ); +}