Proposal lists without rendering every diff; each file's lines computed once 3571c519ada6
Listing proposals rendered every open proposal's full diff and mapped each new line to its atom by recomputing the whole file's live lines per line (quadratic). Lists, the API, auto-land and the after-push checks now use load_summary (no rendering), and the proposal page computes each file's lines once. metalgear2's list took 3-4 s per call.
- Author
- Andy McCall @andymccall <[email protected]>
- via
- agent
agent:claude@andymccall <[email protected]> · keyless, under your grant for changes,refs,seal · key 1709a3f4ae01
- When
- · 2026-10-20 04:24 UTC
- Signature
- ◈ sealed by agent:claude@andymccall
- Hash
3571c519ada6813623c872d81fafd38b4c2450e395a10bc5e53c7b6195b2ce03- Depends on
- 3f02e5ea4f2d 46b5e2c59c6b 739a763a558c 86a69b53aff0 9c04c779f032 a7e6e8e6094c ac713df7b38b b5668108487f b64e2537d702 c0fac31d36fd d23464c0ac60 d943160dafeb de505df5cf8d
- Depended on by
- d7e2c2754807 ea55fb62f919
- In bookmarks
- api-unknown-fields diffusion-pin-bookmark draft-ready main proposal-list-fast
- In tags
- v0.6.1 v0.7.0 v0.8.0 v0.9.0 v0.9.1 v0.9.11 v0.9.15 v0.9.16 v0.9.19 v0.9.2 v0.9.20 v0.9.21 v0.9.22 v0.9.23 v0.9.24 v0.9.3 v0.9.4 v0.9.5 v0.9.6 v0.9.7 v0.9.8 v0.9.9
- Edits
- 19 atoms, 0 tree operations, 5 files
Browse the tree at this change
modified CHANGELOG.md +1 −0
| … | 3 unchanged lines |
||
| 4 | 4 | ||
| 5 | 5 | ## Unreleased |
|
| 6 | 6 | ||
| 7 | + | - **Proposal lists and the proposals API are much faster on large files:** listing a repository's proposals rendered every open proposal's full diff and looked up each new line's history separately, recomputing a whole file's lines for every line; lists, the API, auto-land and the checks after a push now skip the rendering, and the proposal page computes each file's lines once. |
|
| 7 | 8 | ## 0.6.0 (2026-10-20) |
|
| 8 | 9 | ||
| 9 | 10 | - **An event bus between the hub's processes (#273):** on PostgreSQL, what one process publishes (a push, a check, a run's jobs, a comment, a waiting webhook delivery, a choice in the conflict resolver) reaches every other process over `LISTEN/NOTIFY`, so live pages on any web process see a push made through another, and a separate worker delivers a push's webhooks at once instead of on its once-a-minute pass. Nothing to configure; on SQLite the bus stays in process. Events say "look now" and are delivered at most once; a process that loses its listening connection listens again on a new one and logs that events may have been missed. A live page that reconnects with an event id its process never gave (another web process's, or one from before a restart) is now told to reload instead of waiting for newer ids. In the conflict resolver, choices and landings cross processes; who is present stays per process. See docs/roles.md. |
|
| … | 252 unchanged lines |
modified crates/hub/src/api/mod.rs +8 −8
| … | 1028 unchanged lines |
||
| 1029 | 1029 | .map(|p| { |
|
| 1030 | 1030 | let (app, repo, owner, name, id) = (app.clone(), repo.clone(), owner.clone(), name.clone(), row.id); |
|
| 1031 | 1031 | async move { |
|
| 1032 | - | let loaded = crate::web::proposals::load(&app, &repo, p).await?; |
|
| 1032 | + | let loaded = crate::web::proposals::load_summary(&app, &repo, p).await?; |
|
| 1033 | 1033 | let v = crate::web::proposals::verdict(&app, &loaded, id).await?; |
|
| 1034 | 1034 | Ok(crate::web::proposals::to_api(&app, &owner, &name, &loaded, &v, None).await) |
|
| 1035 | 1035 | } |
|
| … | 16 unchanged lines |
||
| 1052 | 1052 | Some(id) => db::repos::get_by_id(&app.db, id).await?.map(|r| format!("{}/{}", app.config.base_url(), r.slug())), |
|
| 1053 | 1053 | None => None, |
|
| 1054 | 1054 | }; |
|
| 1055 | - | let loaded = crate::web::proposals::load(&app, &repo, p).await?; |
|
| 1055 | + | let loaded = crate::web::proposals::load_summary(&app, &repo, p).await?; |
|
| 1056 | 1056 | let v = crate::web::proposals::verdict(&app, &loaded, row.id).await?; |
|
| 1057 | 1057 | Ok(Json(crate::web::proposals::to_api(&app, &owner, &name, &loaded, &v, source_url).await)) |
|
| 1058 | 1058 | } |
|
| … | 72 unchanged lines |
||
| 1131 | 1131 | Some(id) => db::repos::get_by_id(&app.db, id).await?.map(|r| format!("{}/{}", app.config.base_url(), r.slug())), |
|
| 1132 | 1132 | None => None, |
|
| 1133 | 1133 | }; |
|
| 1134 | - | let loaded = crate::web::proposals::load(&app, &repo, p).await?; |
|
| 1134 | + | let loaded = crate::web::proposals::load_summary(&app, &repo, p).await?; |
|
| 1135 | 1135 | let v = crate::web::proposals::verdict(&app, &loaded, row.id).await?; |
|
| 1136 | 1136 | Ok(Json(crate::web::proposals::to_api(&app, &owner, &name, &loaded, &v, source_url).await)) |
|
| 1137 | 1137 | } |
|
| … | 98 unchanged lines |
||
| 1236 | 1236 | let c = caller(&app, &headers).await?; |
|
| 1237 | 1237 | let (repo, row, _) = reach(&app, &c, &owner, &name, Access::Read).await?; |
|
| 1238 | 1238 | let p = db::proposals::get(&app.db, row.id, number).await?.ok_or_else(|| not_found("no such proposal"))?; |
|
| 1239 | - | let loaded = crate::web::proposals::load(&app, &repo, p).await?; |
|
| 1239 | + | let loaded = crate::web::proposals::load_summary(&app, &repo, p).await?; |
|
| 1240 | 1240 | let heads = match side.as_str() { |
|
| 1241 | 1241 | "base" => loaded.target_heads, |
|
| 1242 | 1242 | "head" => loaded.heads, |
|
| … | 18 unchanged lines |
||
| 1261 | 1261 | crate::web::proposals::open_proposal(&app, &repo, &row, &me, &body.title, &body.body, &body.target, &body.source, body.draft) |
|
| 1262 | 1262 | .await |
|
| 1263 | 1263 | .map_err(|e| ApiError(StatusCode::BAD_REQUEST, e))?; |
|
| 1264 | - | let loaded = crate::web::proposals::load(&app, &repo, created).await?; |
|
| 1264 | + | let loaded = crate::web::proposals::load_summary(&app, &repo, created).await?; |
|
| 1265 | 1265 | let v = crate::web::proposals::verdict(&app, &loaded, row.id).await?; |
|
| 1266 | 1266 | Ok((StatusCode::CREATED, Json(crate::web::proposals::to_api(&app, &owner, &name, &loaded, &v, None).await))) |
|
| 1267 | 1267 | } |
|
| … | 28 unchanged lines |
||
| 1296 | 1296 | let paths: std::collections::BTreeSet<String> = out.iter().filter(|c| c.anchor.is_some()).filter_map(|c| c.path.clone()).collect(); |
|
| 1297 | 1297 | if !paths.is_empty() { |
|
| 1298 | 1298 | let repo = RepoName::new(&owner, &name).ok_or_else(|| not_found("no such repository"))?; |
|
| 1299 | - | let heads = crate::web::proposals::load(&app, &repo, p).await?.heads; |
|
| 1299 | + | let heads = crate::web::proposals::load_summary(&app, &repo, p).await?.heads; |
|
| 1300 | 1300 | let lines = app |
|
| 1301 | 1301 | .repos |
|
| 1302 | 1302 | .read(&repo, move |r, _| { |
|
| … | 91 unchanged lines |
||
| 1394 | 1394 | let c = caller(&app, &headers).await?; |
|
| 1395 | 1395 | let (repo, row, _) = reach(&app, &c, &owner, &name, Access::Read).await?; |
|
| 1396 | 1396 | let p = db::proposals::get(&app.db, row.id, number).await?.ok_or_else(|| not_found("no such proposal"))?; |
|
| 1397 | - | let loaded = crate::web::proposals::load(&app, &repo, p).await?; |
|
| 1397 | + | let loaded = crate::web::proposals::load_summary(&app, &repo, p).await?; |
|
| 1398 | 1398 | let v = crate::web::proposals::verdict(&app, &loaded, row.id).await?; |
|
| 1399 | 1399 | Ok(Json( |
|
| 1400 | 1400 | v.reviews |
|
| … | 89 unchanged lines |
||
| 1490 | 1490 | let c = caller(&app, &headers).await?; |
|
| 1491 | 1491 | let (repo, row, _) = reach(&app, &c, &owner, &name, Access::Read).await?; |
|
| 1492 | 1492 | let p = db::proposals::get(&app.db, row.id, number).await?.ok_or_else(|| not_found("no such proposal"))?; |
|
| 1493 | - | let loaded = crate::web::proposals::load(&app, &repo, p).await?; |
|
| 1493 | + | let loaded = crate::web::proposals::load_summary(&app, &repo, p).await?; |
|
| 1494 | 1494 | Ok(Json(db::hooks::checks_for_state(&app.db, row.id, &loaded.state).await?.iter().map(check_of).collect())) |
|
| 1495 | 1495 | } |
|
| 1496 | 1496 | ||
| … | 1301 unchanged lines |
modified crates/hub/src/autoland.rs +1 −1
| … | 84 unchanged lines |
||
| 85 | 85 | } |
|
| 86 | 86 | let Some(repo) = RepoName::new(&row.owner, &row.name) else { return }; |
|
| 87 | 87 | let number = p.number; |
|
| 88 | - | let Ok(loaded) = crate::web::proposals::load(app, &repo, p.clone()).await else { return }; |
|
| 88 | + | let Ok(loaded) = crate::web::proposals::load_summary(app, &repo, p.clone()).await else { return }; |
|
| 89 | 89 | let required = db::hooks::required_checks(&app.db, row.id).await.unwrap_or_default(); |
|
| 90 | 90 | let checks = db::hooks::checks_for_state(&app.db, row.id, &loaded.state).await.unwrap_or_default(); |
|
| 91 | 91 | let failed: Vec<&str> = required |
|
| … | 21 unchanged lines |
modified crates/hub/src/repos/proposals.rs +10 −0
| … | 97 unchanged lines |
||
| 98 | 98 | lines.get(new_no.checked_sub(1)?).map(|((h, i), _)| (*h, *i)) |
|
| 99 | 99 | } |
|
| 100 | 100 | ||
| 101 | + | /// Every live line of `path` as its atom, in order: `atom_at` for a whole |
|
| 102 | + | /// file at once (computing them per line is quadratic in the file's length). |
|
| 103 | + | pub fn atoms_of(loaded: &Loaded, path: &str) -> Option<Vec<(ChangeHash, u32)>> { |
|
| 104 | + | let f = loaded.snapshot.files.get(path)?; |
|
| 105 | + | if !matches!(f.content, FileContent::Text { .. }) { |
|
| 106 | + | return None; |
|
| 107 | + | } |
|
| 108 | + | Some(loaded.state.live_lines(&f.node).iter().map(|((h, i), _)| (*h, *i)).collect()) |
|
| 109 | + | } |
|
| 110 | + | ||
| 101 | 111 | /// The current line (1-based) of an atom in `path`, if it is still live. |
|
| 102 | 112 | pub fn line_of(loaded: &Loaded, path: &str, atom: (ChangeHash, u32)) -> Option<usize> { |
|
| 103 | 113 | let f = loaded.snapshot.files.get(path)?; |
|
| … | 106 unchanged lines |
modified crates/hub/src/web/proposals.rs +18 −6
| … | 418 unchanged lines |
||
| 419 | 419 | ||
| 420 | 420 | /// Compute the projection and everything derived from it. |
|
| 421 | 421 | pub async fn load(app: &Shared, repo: &RepoName, row: pdb::ProposalRow) -> Result<Loaded, crate::repos::RepoError> { |
|
| 422 | + | load_with(app, repo, row, true).await |
|
| 423 | + | } |
|
| 424 | + | ||
| 425 | + | /// [`load`] without the rendered diff and the line-to-atom map, which only |
|
| 426 | + | /// the proposal page needs: for lists, the API, landing and the checks that |
|
| 427 | + | /// follow a push. On a repository with large files those two were most of |
|
| 428 | + | /// a list's time (metalgear2's took 3-4 s per call). |
|
| 429 | + | pub async fn load_summary(app: &Shared, repo: &RepoName, row: pdb::ProposalRow) -> Result<Loaded, crate::repos::RepoError> { |
|
| 430 | + | load_with(app, repo, row, false).await |
|
| 431 | + | } |
|
| 432 | + | ||
| 433 | + | async fn load_with(app: &Shared, repo: &RepoName, row: pdb::ProposalRow, render: bool) -> Result<Loaded, crate::repos::RepoError> { |
|
| 422 | 434 | let fork_dir = match (row.source_kind.as_str(), row.source_repo_id) { |
|
| 423 | 435 | ("fork", Some(id)) => { |
|
| 424 | 436 | db::repos::get_by_id(&app.db, id).await.ok().flatten().and_then(|r| RepoName::new(&r.owner, &r.name)).map(|n| app.repos.dir(&n)) |
|
| … | 28 unchanged lines |
||
| 453 | 465 | }; |
|
| 454 | 466 | let p: Projection = pl::project(r, cache, &r2.target, source)?; |
|
| 455 | 467 | let target_loaded = cache.load(r, &p.target_heads)?; |
|
| 456 | - | let files = render_files(&p.files, &p.bases); |
|
| 468 | + | let files = if render { render_files(&p.files, &p.bases) } else { Vec::new() }; |
|
| 457 | 469 | let mut atoms = HashMap::new(); |
|
| 458 | 470 | for f in &files { |
|
| 471 | + | // Each file's live lines once, not once per line. |
|
| 472 | + | let Some(live) = pl::atoms_of(&p.loaded, &f.path) else { continue }; |
|
| 459 | 473 | for l in &f.lines { |
|
| 460 | - | if let Some(n) = l.new_no { |
|
| 461 | - | if let Some((h, i)) = pl::atom_at(&p.loaded, &f.path, n as usize) { |
|
| 462 | - | atoms.insert((f.path.clone(), n as usize), (h.to_hex(), i)); |
|
| 463 | - | } |
|
| 474 | + | if let Some((h, i)) = l.new_no.and_then(|n| live.get((n as usize).checked_sub(1)?)) { |
|
| 475 | + | atoms.insert((f.path.clone(), l.new_no.unwrap_or(0) as usize), (h.to_hex(), *i)); |
|
| 464 | 476 | } |
|
| 465 | 477 | } |
|
| 466 | 478 | } |
|
| … | 73 unchanged lines |
||
| 540 | 552 | for p in open.into_iter().filter(|p| p.target == bookmark) { |
|
| 541 | 553 | let id = p.id; |
|
| 542 | 554 | let number = p.number; |
|
| 543 | - | if let Ok(l) = load(app, &repo, p).await { |
|
| 555 | + | if let Ok(l) = load_summary(app, &repo, p).await { |
|
| 544 | 556 | if l.row.state == "landed" && l.row.landed_by.is_empty() { |
|
| 545 | 557 | pdb::set_landed(&app.db, id, &l.target_state, actor, "push").await.ok(); |
|
| 546 | 558 | } else if l.row.is_open() && !l.hashes.is_empty() && crate::flow::enabled(app, row) { |
|
| … | 1460 unchanged lines |