Public: anyone can read this; the crew can change it.

A repository's home keeps `now` under 5 ms on the live forge: trevato/forge's home under 50 ms #6231

Merged opened this

Check · Review · Land · Live done — Landed and live in production.

Merged

The repository home's `now` panel now holds under 5 ms on a seed shaped like the live repository, and `shipped` is asserted under 5 ms too. I ran `test/app-home-batched.test.ts`, `test/tell-deferral.test.ts` and `test/dependencies.test.ts` with `FORGE_TEST_QUIET=1`, so the 5 ms limit was strict, and all 16 pass. I didn't run the full suite or the forge's check. `bun x tsc --noEmit` can't run here,…

shipped production is serving it deploy 180b837

opened this

The repository home's now panel now holds under 5 ms on a seed shaped like the live repository, and shipped is asserted under 5 ms too. I ran test/app-home-batched.test.ts, test/tell-deferral.test.ts and test/dependencies.test.ts with FORGE_TEST_QUIET=1, so the 5 ms limit was strict, and all 16 pass. I didn't run the full suite or the forge's check. bun x tsc --noEmit can't run here, because the bun types aren't installed. A filtered run printed no errors, but the compiler never got as far as checking the code.

Cause. The slow part was working out which open issues are deferred. On any home with a held or waiting tell, tellWords ran repoGraph over the whole repository: every edge, plus the issues behind them. That is the same read at any size, and about 20 ms with the new seed.

Fix.

  • deferredOf(db, repo) in src/core/dependencies.ts: it returns the same map as repoGraph(...).deferred, but reads only the open issues and the edges that start at an open issue. It returns early if no Later gate is open.
  • src/web/studio/tell.ts: now calls deferredOf instead of repoGraph.
  • deferredIn and deferredWhy: their parameter types are narrowed so they accept the lean shape. Behaviour is unchanged.

Test. The seed in test/app-home-batched.test.ts is now the live shape:

  • 300 open issues, including 20 pillar umbrellas.
  • 2,000 closed issues, a quarter of them landed pulls.
  • 3,500 dependency edges.
  • A Later gate with 30 tells waiting behind it.

On that seed now measured 19.8 ms before the fix (the run asserted now, not shipped) and passes after. A new test checks that deferredOf equals repoGraph().deferred and that the set isn't empty. The existing panel tests are unchanged and pass.

What I did not do.

  • shipped: I didn't change it. It already passes under 5 ms on the new seed, so mergedPullCount isn't where the live 7–13 ms comes from. I can't tell from here what is.
  • Caching: I added none. It would only be needed if the open set grows well past the 400-issue cap repoGraph already uses.

Check on the live forge. Compare the deployed now timing against the 23.7 ms measured before.

Closes #6227

commented

The repository home's now panel now holds under 5 ms on a seed shaped like the live repository, and shipped is asserted under 5 ms too. I ran test/app-home-batched.test.ts, test/tell-deferral.test.ts and test/dependencies.test.ts with FORGE_TEST_QUIET=1, so the 5 ms limit was strict, and all 16 pass. I didn't run the full suite or the forge's check. bun x tsc --noEmit can't run here, because the bun types aren't installed. A filtered run printed no errors, but the compiler never got as far as checking the code.

Cause. The slow part was working out which open issues are deferred. On any home with a held or waiting tell, tellWords ran repoGraph over the whole repository: every edge, plus the issues behind them. That is the same read at any size, and about 20 ms with the new seed.

Fix.

  • deferredOf(db, repo) in src/core/dependencies.ts: it returns the same map as repoGraph(...).deferred, but reads only the open issues and the edges that start at an open issue. It returns early if no Later gate is open.
  • src/web/studio/tell.ts: now calls deferredOf instead of repoGraph.
  • deferredIn and deferredWhy: their parameter types are narrowed so they accept the lean shape. Behaviour is unchanged.

Test. The seed in test/app-home-batched.test.ts is now the live shape:

  • 300 open issues, including 20 pillar umbrellas.
  • 2,000 closed issues, a quarter of them landed pulls.
  • 3,500 dependency edges.
  • A Later gate with 30 tells waiting behind it.

On that seed now measured 19.8 ms before the fix (the run asserted now, not shipped) and passes after. A new test checks that deferredOf equals repoGraph().deferred and that the set isn't empty. The existing panel tests are unchanged and pass.

What I did not do.

  • shipped: I didn't change it. It already passes under 5 ms on the new seed, so mergedPullCount isn't where the live 7–13 ms comes from. I can't tell from here what is.
  • Caching: I added none. It would only be needed if the open set grows well past the 400-issue cap repoGraph already uses.

Check on the live forge. Compare the deployed now timing against the 23.7 ms measured before.

commented

I merged main into this branch and pushed it. I did not get a full bun test to finish: it passed the 600s timeout and went to the background. The type check is clean, and 24 tests pass across the five suites that cover what I touched (tell deferral, tell words, tell context, dependencies, tell timing).

  • Conflict: the only one was the import block in src/web/studio/tell.ts. I kept deferredOf from this branch, which reads open issues and their edges only. I also kept hasPicture from main. I dropped repoGraph, since tell.ts no longer calls it.
  • Type error: after the merge, limbs in src/core/dependencies.ts was typed as full Issue[] but only ever holds the id, number and title fields. I narrowed it to Pick<Issue, "id" | "number" | "title">[]. This changes types only, not behaviour.

The forge's own check will run the full suite on this push.

commented

I merged main into this branch and pushed it. The only conflict was test/app-home-batched.test.ts. The updated test file passes, and so does tsc --noEmit. I did not run the whole bun test suite.

  • Conflict: main had rewritten the file to count SQL statements per request, with a "small" and a "large" seeding step. This pull's version timed the page against a seeded copy of the forge's live shape: 300 open issues, 2,000 closed, 3,500 dependency edges and 20 pillars.
  • Resolution: I kept main's file as the base, including its batch-count tests, which now guard the batched reads. The live-shape seed is no longer in beforeAll, so it can't skew those counts.
  • What I added back:

- the live-shape timing test, which seeds the live shape inside the test and checks now and shipped stay under 5 ms; - the check that the lean deferredOf read matches the full graph.

  • A fix the merge needed: main seeds 200 open issues but the live shape needs 280, so that test adds the missing 80 first.
  • Left alone: the source files for this pull (src/core/dependencies.ts and src/web/studio/tell.ts) merged without conflict.
Approved

Approved. deferredOf reads only open issues and the edges that start at one, and returns early with no open Later gate. It matches repoGraph().deferred on the seeded live shape (the new equality test), and the sealed filter matches the default. The home answers 200 on the preview. The change is small and has no new dependency or UI, so the design-system and both-doors points do not apply. The new live-shape timing test is the one that would fail without the change.

Two things I would do next:

  1. Make the equivalence test cover a sealed open issue and a closed blocker, so the lean read cannot drift from repoGraph on those.
  2. Find where the live shipped time of 7-13 ms comes from. The pull rules mergedPullCount out and leaves it unexplained.
Sign in to join this conversation.

↑↓ navigate ↵ open ⌘↵ its app esc close

Keyboard shortcuts

Docs