Cloonar/deckle

Fork 0

perf(history): parse each blob once and grep main's tip once per request #289

New issue

Closed

opened 2026-10-14 18:38:54 +02:00 by dominik.polakovics · 0 comments

dominik.polakovics commented

2026-10-14 18:38:54 +02:00

Owner

This was generated by AI during triage.

Summary

GET /api/element/history parses every blob once per version instead of once per blob, and greps main's tip twice on a first-page request. Remove both redundancies without changing the response. Follow-up to #283 / PR #287, which named them as deliberately deferred.

Current behavior

One default-window request runs:

gitrepo.NeedleFiles(tip, …) — git grep -F -l at the tip plus one cat-file --batch — for the element's version on main.
gitrepo.NeedleHistory — which, when before is empty, runs grepHolding(start, …) again for the same tip (start == q.Tip), then the pickaxe, one git log, and its own cat-file --batch. The tip's blobs are read a second time.
A loop over versions calling locateElement(v.Before, …) and locateElement(v.After, …). locateElement calls content.Parse on every FileBlob it is handed, every time. Neighbouring versions share most of their blobs (NeedleHistory keeps one cur state and only swaps the touched files), and the tip's blobs from NeedleFiles are parsed again for the pending row. editAuthors (merge publishes) and pendingAuthors (branch mode) locate through the same function and repeat the parse the same way.

Baseline measured during triage on the opt-in cost test (300 pages × 15 elements, 2,000 publishes touching 3 pages each; go 1.26.8, git 2.47.3):

Phase	Time
Median request (5 runs)	192 ms (bound 250 ms)
`MainTip`	2.8 ms
`NeedleFiles` — the duplicate grep + cat-file	21.5 ms
`NeedleHistory`, 200 versions	127 ms
Rest of the request (working-tree walk, parse, diff, encode)	41 ms

The duplicate grep accounts for most of the 21.5 ms; the parse savings come out of the 41 ms. PR #287 estimated the two changes together at roughly 30–40 ms.

Desired behavior

Each distinct blob is parsed at most once per request, wherever it occurs: a version's Before or After, a merge's Edits, the tip's files, the draft tip's files.
A first-page request (no before) runs one git grep at main's tip and reads the tip's blobs once. Continuation pages (before set) are unchanged in shape: their window starts below the tip, so the tip's files still come from one NeedleFiles call.
The JSON response is byte-identical to today's for every request, including the pending row and authors.

Decisions

The parse cache is keyed by blob identity, not by location. A file untouched across several publishes is one blob under many commit:path names, and the same blob reaches the handler from NeedleFiles and from NeedleHistory in separate byte slices. Keying by commit:path or by slice pointer would miss both. git cat-file --batch already prints each object's id in its header, which readBlobs currently discards; carrying it on FileBlob gives a free, exact key. Working-tree files have no object id and are parsed once anyway.
The cache lives for one request. Blobs are immutable, so a longer-lived cache would be correct, but one request is where the repetition is and a process-wide cache would need a bound and eviction. Keep it request-scoped.
"Parse this blob and find the element by uid with role element" keeps one owner. Today that is locateElement in internal/server/history.go. The cache extends it (or the function moves to elemops together with the cache); editAuthors, pendingRow, pendingAuthors and historyWorkingTree keep calling the one function. No second locate helper (CLAUDE.md "Every rule has exactly one owner").
"Which kept files hold the needle at a commit" keeps one owner. That is holdingFiles/grepHolding in internal/gitrepo. The grep dedupe has NeedleHistory hand the handler the window-start state it already computes (the kept files holding the needle at the window's start, with content) so the handler calls NeedleFiles only when the window does not start at the tip. The handler must not pass pre-fetched blobs into gitrepo or grep on its own.
One issue, one PR. Both changes are internal to the handler and internal/gitrepo, small, and measured by the same benchmark; splitting them buys nothing.

Key interfaces

gitrepo.FileBlob — gains the blob's git object id, filled by readBlobs from the cat-file --batch header. Every producer (blobsAt, holdingNeedle, historyEdits) carries it through.
gitrepo.NeedleHistory(HistoryQuery) — its answer additionally names the kept files holding the needle at the window's start with content (the resident set it greps and reads already). Shape is the agent's call: a field on a returned struct, or an extra return value.
handleElementHistory — calls NeedleFiles only on continuation pages; on the first page the tip's version comes from the NeedleHistory answer. The order of the two reads may change; both are pinned to tip, so the response cannot.
locateElement(blobs, uid, schemas) — becomes a method on, or takes, a per-request document cache; every call site in history.go goes through it.
Starting hints: internal/server/history.go, internal/gitrepo/history.go, internal/server/history_bench_test.go.

Acceptance criteria

handleElementHistory parses each distinct blob (by git object id) at most once per request, including blobs reached through HistoryVersion.Edits and DraftEdits. A test counts parses — for example by instrumenting the cache — on a fixture where neighbouring versions share blobs.
A first-page request runs exactly one git grep at main's tip and reads the tip's blobs with one cat-file --batch. A continuation page (before set) still resolves the tip's files through NeedleFiles.
go test ./internal/server and go test ./internal/gitrepo pass with their existing history tests unchanged; the response is byte-identical for the existing fixtures (a golden comparison of a first page and a continuation page before and after is acceptable evidence).
BenchmarkElementHistory and DECKLE_HISTORY_PERF=1 … TestElementHistoryCostBound show the gain; the PR reports before/after medians on the same machine.
locateElement remains the only place that parses a blob and finds the element by uid; grepHolding remains the only git grep for "files holding the needle at a commit". No "same as" comments.
go vet ./... and gofmt -l are clean.

How to verify

go test ./internal/gitrepo ./internal/server
go test ./internal/server -run '^$' -bench BenchmarkElementHistory -benchtime 10x
DECKLE_HISTORY_PERF=1 go test ./internal/server -run TestElementHistoryCostBound -v

Compare the git phases log line of the cost test before and after: NeedleFiles should drop to zero on the measured request path (or be skipped), and the "rest of the median request" should shrink by the parse savings.
Pick one commit from the perf fixture as before and confirm the continuation page still answers with the same entries as today.

Assumptions to check

git cat-file --batch prints <oid> SP <type> SP <size> and readBlobs can keep the oid without a second git call (it parses that header today and drops fields[0]).
NeedleHistory's initial cur state at start, filtered by holdingNeedle, equals what NeedleFiles(start, …) returns: same files, same order (both sorted by path), same Keep filter. If anything differs, the response would change and the gap must be closed before the dedupe lands.
The 30–40 ms estimate from PR #287 was never measured separately; the benchmark decides what the gain is.

Out of scope

Any wire change to GET /api/element/history; docs/api.md and ADR 0023 stay as they are.
Caching parsed documents across requests or across elements.
Speeding up the pickaxe, the window size, or the working-tree walk.
Deduplicating the draft-tip grep in DraftEdits (branch mode) — a different commit than main's tip, so not a repeat.
If docs/architecture.md or ADR 0023 quote a figure the change moves (ADR 0023 quotes the pickaxe at about 60 ms and "well under a quarter of a second"), update only that sentence; nothing else in the docs changes.

Blocked by

None - can start immediately

Original report

Follow-up to #283 / PR #287 (GET /api/element/history). The PR description and its review both name two speed-ups that were deliberately left out of the first cut. Filing them so they can be triaged separately from the feature.

What

One default-window history request today does more work than it needs to on two counts:

Every blob is parsed once per version, not once per blob. internal/server/history.go (handleElementHistory) walks versions and calls locateElement on v.Before and v.After for each. Neighbouring versions share most of their blobs (a file untouched between two commits is the same blob on both sides), yet locateElement runs content.Parse on each FileBlob afresh every time it is asked. main's tip blobs from NeedleFiles are parsed a further time for the pending row.
main's tip is grepped twice. The handler calls gitrepo.NeedleFiles(tip, …) for the element's current version, and then gitrepo.NeedleHistory runs grepHolding(start, …) again for the same tip when before is empty (start == q.Tip). Each is a separate git grep -F -l over the site directory plus its own cat-file --batch.

The PR's measurement (DECKLE_HISTORY_PERF=1 go test ./internal/server -run TestElementHistoryCostBound -v, 300 pages × 15 elements, 2,000 publishes touching 3 pages each) puts one request at a median of about 190 ms against the 250 ms bound. The PR estimates the two changes together are worth about 30–40 ms of that.

Possible shape

Parse each distinct blob once per request and key the parsed document by blob content (or by commit:path as readBlobs already keys it), so locateElement on neighbouring versions and on the tip is a lookup, not a reparse. gitrepo.HistoryVersion already carries the blobs; the cache can live in the handler or in elemops, wherever the one owner of "parse this file and find the element" ends up.
Let the first page's NeedleHistory reuse the tip's grep and blobs that NeedleFiles already produced (or have NeedleHistory return the tip's FileBlobs so the handler calls NeedleFiles only on cursor pages).

Both are internal to the handler and internal/gitrepo; no wire shape changes.

Acceptance

handleElementHistory parses each distinct blob at most once per request.
A first-page request runs one git grep at main's tip, not two.
The history response is byte-identical before and after (existing tests in internal/server/history_test.go and internal/gitrepo/history_test.go pass unchanged).
BenchmarkElementHistory and the opt-in TestElementHistoryCostBound show the gain; report the before/after medians in the PR.
Rule ownership: the change must not add a second place that decides "which file holds the element" or "parse and locate" — it extends locateElement / gitrepo rather than siblings (CLAUDE.md "Every rule has exactly one owner").

Docs

Nothing user-facing changes. docs/api.md and ADR 0023 stay as they are. If the cost note in docs/architecture.md (history section) quotes the measured figures, update them.

> *This was generated by AI during triage.* ## Summary `GET /api/element/history` parses every blob once per version instead of once per blob, and greps `main`'s tip twice on a first-page request. Remove both redundancies without changing the response. Follow-up to #283 / PR #287, which named them as deliberately deferred. ## Current behavior One default-window request runs: - `gitrepo.NeedleFiles(tip, …)` — `git grep -F -l` at the tip plus one `cat-file --batch` — for the element's version on `main`. - `gitrepo.NeedleHistory` — which, when `before` is empty, runs `grepHolding(start, …)` again for the same tip (`start == q.Tip`), then the pickaxe, one `git log`, and its own `cat-file --batch`. The tip's blobs are read a second time. - A loop over `versions` calling `locateElement(v.Before, …)` and `locateElement(v.After, …)`. `locateElement` calls `content.Parse` on every `FileBlob` it is handed, every time. Neighbouring versions share most of their blobs (`NeedleHistory` keeps one `cur` state and only swaps the touched files), and the tip's blobs from `NeedleFiles` are parsed again for the pending row. `editAuthors` (merge publishes) and `pendingAuthors` (branch mode) locate through the same function and repeat the parse the same way. Baseline measured during triage on the opt-in cost test (300 pages × 15 elements, 2,000 publishes touching 3 pages each; go 1.26.8, git 2.47.3): | Phase | Time | | --- | --- | | Median request (5 runs) | 192 ms (bound 250 ms) | | `MainTip` | 2.8 ms | | `NeedleFiles` — the duplicate grep + cat-file | 21.5 ms | | `NeedleHistory`, 200 versions | 127 ms | | Rest of the request (working-tree walk, parse, diff, encode) | 41 ms | The duplicate grep accounts for most of the 21.5 ms; the parse savings come out of the 41 ms. PR #287 estimated the two changes together at roughly 30–40 ms. ## Desired behavior - Each distinct blob is parsed at most once per request, wherever it occurs: a version's `Before` or `After`, a merge's `Edits`, the tip's files, the draft tip's files. - A first-page request (no `before`) runs one `git grep` at `main`'s tip and reads the tip's blobs once. Continuation pages (`before` set) are unchanged in shape: their window starts below the tip, so the tip's files still come from one `NeedleFiles` call. - The JSON response is byte-identical to today's for every request, including the pending row and authors. ## Decisions - **The parse cache is keyed by blob identity, not by location.** A file untouched across several publishes is one blob under many `commit:path` names, and the same blob reaches the handler from `NeedleFiles` and from `NeedleHistory` in separate byte slices. Keying by `commit:path` or by slice pointer would miss both. `git cat-file --batch` already prints each object's id in its header, which `readBlobs` currently discards; carrying it on `FileBlob` gives a free, exact key. Working-tree files have no object id and are parsed once anyway. - **The cache lives for one request.** Blobs are immutable, so a longer-lived cache would be correct, but one request is where the repetition is and a process-wide cache would need a bound and eviction. Keep it request-scoped. - **"Parse this blob and find the element by uid with role element" keeps one owner.** Today that is `locateElement` in `internal/server/history.go`. The cache extends it (or the function moves to `elemops` together with the cache); `editAuthors`, `pendingRow`, `pendingAuthors` and `historyWorkingTree` keep calling the one function. No second locate helper (CLAUDE.md "Every rule has exactly one owner"). - **"Which kept files hold the needle at a commit" keeps one owner.** That is `holdingFiles`/`grepHolding` in `internal/gitrepo`. The grep dedupe has `NeedleHistory` hand the handler the window-start state it already computes (the kept files holding the needle at the window's start, with content) so the handler calls `NeedleFiles` only when the window does not start at the tip. The handler must not pass pre-fetched blobs into gitrepo or grep on its own. - **One issue, one PR.** Both changes are internal to the handler and `internal/gitrepo`, small, and measured by the same benchmark; splitting them buys nothing. ## Key interfaces - `gitrepo.FileBlob` — gains the blob's git object id, filled by `readBlobs` from the `cat-file --batch` header. Every producer (`blobsAt`, `holdingNeedle`, `historyEdits`) carries it through. - `gitrepo.NeedleHistory(HistoryQuery)` — its answer additionally names the kept files holding the needle at the window's start with content (the `resident` set it greps and reads already). Shape is the agent's call: a field on a returned struct, or an extra return value. - `handleElementHistory` — calls `NeedleFiles` only on continuation pages; on the first page the tip's version comes from the `NeedleHistory` answer. The order of the two reads may change; both are pinned to `tip`, so the response cannot. - `locateElement(blobs, uid, schemas)` — becomes a method on, or takes, a per-request document cache; every call site in `history.go` goes through it. - Starting hints: `internal/server/history.go`, `internal/gitrepo/history.go`, `internal/server/history_bench_test.go`. ## Acceptance criteria - [ ] `handleElementHistory` parses each distinct blob (by git object id) at most once per request, including blobs reached through `HistoryVersion.Edits` and `DraftEdits`. A test counts parses — for example by instrumenting the cache — on a fixture where neighbouring versions share blobs. - [ ] A first-page request runs exactly one `git grep` at `main`'s tip and reads the tip's blobs with one `cat-file --batch`. A continuation page (`before` set) still resolves the tip's files through `NeedleFiles`. - [ ] `go test ./internal/server` and `go test ./internal/gitrepo` pass with their existing history tests unchanged; the response is byte-identical for the existing fixtures (a golden comparison of a first page and a continuation page before and after is acceptable evidence). - [ ] `BenchmarkElementHistory` and `DECKLE_HISTORY_PERF=1 … TestElementHistoryCostBound` show the gain; the PR reports before/after medians on the same machine. - [ ] `locateElement` remains the only place that parses a blob and finds the element by uid; `grepHolding` remains the only `git grep` for "files holding the needle at a commit". No "same as" comments. - [ ] `go vet ./...` and `gofmt -l` are clean. ## How to verify ``` go test ./internal/gitrepo ./internal/server go test ./internal/server -run '^$' -bench BenchmarkElementHistory -benchtime 10x DECKLE_HISTORY_PERF=1 go test ./internal/server -run TestElementHistoryCostBound -v ``` - Compare the `git phases` log line of the cost test before and after: `NeedleFiles` should drop to zero on the measured request path (or be skipped), and the "rest of the median request" should shrink by the parse savings. - Pick one commit from the perf fixture as `before` and confirm the continuation page still answers with the same entries as today. ## Assumptions to check - `git cat-file --batch` prints `<oid> SP <type> SP <size>` and `readBlobs` can keep the oid without a second git call (it parses that header today and drops `fields[0]`). - `NeedleHistory`'s initial `cur` state at `start`, filtered by `holdingNeedle`, equals what `NeedleFiles(start, …)` returns: same files, same order (both sorted by path), same `Keep` filter. If anything differs, the response would change and the gap must be closed before the dedupe lands. - The `30–40 ms` estimate from PR #287 was never measured separately; the benchmark decides what the gain is. ## Out of scope - Any wire change to `GET /api/element/history`; `docs/api.md` and ADR 0023 stay as they are. - Caching parsed documents across requests or across elements. - Speeding up the pickaxe, the window size, or the working-tree walk. - Deduplicating the draft-tip grep in `DraftEdits` (branch mode) — a different commit than `main`'s tip, so not a repeat. - If `docs/architecture.md` or ADR 0023 quote a figure the change moves (ADR 0023 quotes the pickaxe at about 60 ms and "well under a quarter of a second"), update only that sentence; nothing else in the docs changes. ## Blocked by None - can start immediately ## Original report Follow-up to #283 / PR #287 (`GET /api/element/history`). The PR description and its review both name two speed-ups that were deliberately left out of the first cut. Filing them so they can be triaged separately from the feature. ## What One default-window history request today does more work than it needs to on two counts: 1. **Every blob is parsed once per version, not once per blob.** `internal/server/history.go` (`handleElementHistory`) walks `versions` and calls `locateElement` on `v.Before` and `v.After` for each. Neighbouring versions share most of their blobs (a file untouched between two commits is the same blob on both sides), yet `locateElement` runs `content.Parse` on each `FileBlob` afresh every time it is asked. `main`'s tip blobs from `NeedleFiles` are parsed a further time for the pending row. 2. **`main`'s tip is grepped twice.** The handler calls `gitrepo.NeedleFiles(tip, …)` for the element's current version, and then `gitrepo.NeedleHistory` runs `grepHolding(start, …)` again for the same tip when `before` is empty (`start == q.Tip`). Each is a separate `git grep -F -l` over the site directory plus its own `cat-file --batch`. The PR's measurement (`DECKLE_HISTORY_PERF=1 go test ./internal/server -run TestElementHistoryCostBound -v`, 300 pages × 15 elements, 2,000 publishes touching 3 pages each) puts one request at a median of about 190 ms against the 250 ms bound. The PR estimates the two changes together are worth about 30–40 ms of that. ## Possible shape - Parse each distinct blob once per request and key the parsed document by blob content (or by `commit:path` as `readBlobs` already keys it), so `locateElement` on neighbouring versions and on the tip is a lookup, not a reparse. `gitrepo.HistoryVersion` already carries the blobs; the cache can live in the handler or in `elemops`, wherever the one owner of "parse this file and find the element" ends up. - Let the first page's `NeedleHistory` reuse the tip's grep and blobs that `NeedleFiles` already produced (or have `NeedleHistory` return the tip's `FileBlob`s so the handler calls `NeedleFiles` only on cursor pages). Both are internal to the handler and `internal/gitrepo`; no wire shape changes. ## Acceptance - `handleElementHistory` parses each distinct blob at most once per request. - A first-page request runs one `git grep` at `main`'s tip, not two. - The history response is byte-identical before and after (existing tests in `internal/server/history_test.go` and `internal/gitrepo/history_test.go` pass unchanged). - `BenchmarkElementHistory` and the opt-in `TestElementHistoryCostBound` show the gain; report the before/after medians in the PR. - Rule ownership: the change must not add a second place that decides "which file holds the element" or "parse and locate" — it extends `locateElement` / `gitrepo` rather than siblings (CLAUDE.md "Every rule has exactly one owner"). ## Docs Nothing user-facing changes. `docs/api.md` and ADR 0023 stay as they are. If the cost note in `docs/architecture.md` (history section) quotes the measured figures, update them.

dominik.polakovics added the

needs-triage

enhancement

labels

2026-10-14 18:38:54 +02:00

dominik.polakovics referenced this issue

2026-10-14 22:45:39 +02:00

Attribute the element history to the editor who made each change (pseudonymous authorship, merge publish, host mailmap) #295

dominik.polakovics added

ready-for-agent

and removed

needs-triage