Repository navigation
PR discovery rebuilds thread shells and counts unused history #18618
Description
What happens
Branch pull-request discovery builds thread shells even though it only uses thread metadata: project, branch/worktree, saved PR links, and settled/archive state. Those shell reads also count tool history, find run errors and pending work, and walk fork sources.
This happens at startup, on the one-minute sweep, and when a finished run/checkpoint or relevant metadata change requests a one-thread refresh. The single-thread path is narrower but still counts that thread's history.
Source: ThreadPullRequestService.
Proof
Live macOS desktop tracing identifies ThreadPullRequestServiceV2.synchronize as a caller of the expensive shell SQL. One captured shell query took 1.11 seconds during concurrent work. That is whole-query wall time, not a claim that all of it is CPU time or all of it can be eliminated.
On the M4 test host, a controlled fixture using the real projection service, 500 active branch threads, and 50,000 completed command items with 512-byte output strings gave:
| Same 500 discovery threads | Shell read | Metadata read |
|---|---|---|
| Median of 9 warm reads | 13.94 ms | 5.46 ms |
| SELECT statements | 4 | 2 |
| Statements touching run/item/message/provider-thread history | 3 | 0 |
The change removes about 61% of this fixture's read time. This is not an end-to-end GitHub/network timing or whole-app CPU claim.
Proposed fix
Read discovery metadata and the snapshot sequence in one SQLite transaction. Decode the metadata after releasing the transaction. Keep the current sequence guard on thread.pull-request.sync, single-thread scoping, active/archive rules, and startup settled-thread backfill. No wire contract or client behavior needs to change.
This is distinct from #13757, which limits which settled threads receive Git/network discovery after a restart. This issue removes unnecessary history reads for whatever threads discovery legitimately checks.
Found by GPT-6.1-Sol (high), Codex harness in T3 Code.
Activity
juliusmarminge commentedon Oct 12, 2026
Note
Grok responding on behalf of Julius.
This is a performance enhancement, not a bug. Discovery returns the right results, and the measured saving is about 8 ms per 500 threads in a synthetic fixture (13.94 → 5.46 ms). The 1.11 s trace is the whole query under concurrent load, not this caller alone.
I confirmed it on main:
readThreadSnapshotinThreadPullRequestService.tscallsgetShellSnapshotfor sweeps andgetThreadShellfor single-thread refreshes, though discovery only needs metadata.Related work that doesn't cover this:
NodeSqliteClient.readOnly) but still read full shells.I'm keeping this open as an enhancement. A metadata-only read is a reasonable, cheap follow-up that fits alongside #14703.
realhasanshoaib commentedon Oct 12, 2026
The behavior-equivalence check now passes, including legacy PR links. Reproduction scripts: https://gist.github.com/realhasanshoaib/3b3d80b40d7f7b58c899011f0507e240. A repeat run measured 14.04 ms before and 4.87 ms after (median of nine), with the same discovery metadata and zero history SELECTs after.
aryaxo commentedon Oct 22, 2026
+1, with numbers from a large install. On this one, PR discovery is the longest database holder.
statev2.sqlite31.1 GB, about 2,790 threads. About 990 PRs are linked: 762 merged, 14 closed, 213 open.ThreadPullRequestServiceV2.synchronizeran 57 times: median 5.5 s, p90 28.4 s, max 239.7 s.PullRequestSyncReactor.sweep: median 6.0 s, max 315.5 s.syncGroup: 1,621 runs, median 2.9 s, max 161.4 s.GitHubPullRequestApi.getPullRequestSummary: 1,621 calls, about 17 a minute, median 2.7 s.The metadata-only read in the proposed fix looks like exactly what this install needs.
juliusmarminge commentedon Nov 9, 2026
Fixed by #30356 (
0b5600b1c3): PR discovery now reads metadata and the sequence together without reading thread history, including single-thread refreshes. Thanks for the investigation and measurements!