Repository navigation
Metrics: Time site starts from the moment the site shows as running - #5505
Merged
Merged
Conversation
Collaborator
📊 Performance Test ResultsComparing 092ec55 vs trunk app-size
site-editor
site-startup
Results are median values from multiple test runs. Legend: 🟢 Improvement (faster) | 🔴 Regression (slower) | ⚪ No change (<50ms diff) |
This was referenced Oct 28, 2026
chubes4
added a commit
that referenced
this pull request
Oct 28, 2026
…5506) ## Related issues - Follows #5351 and #5505, which trimmed and measured the start path. ## How AI was used Claude (via the Pi coding agent in a Roadie session) traced the start path, wrote the change and its tests, checked the tests by reverting each piece, and counted the `site list` calls in packaged builds. I reviewed the diff and the evidence. ## Proposed Changes Every site event the renderer received made it invalidate the site list. Each refetch spawns a CLI `site list`, which takes about 300 ms. A start or stop sends several events and also awaited one more refetch of its own. Yet each `site-updated` and `site-operations-changed` event already carries the site's full record. - **Renderer (`use-sites.ts`):** writes the event's site into the TanStack Query cache with `setQueryData`, as `useUpdateSitesSortOrder` already does. It keeps the two fields only Studio stores (`sortOrder`, `autoStart`). `satisfies` ties those to `SiteMetadata`, so a new metadata field won't compile until it's carried over. A `site-operations-changed` event keeps the cached `running`, because it doesn't know whether the site runs. - It still refetches for creates, deletes, sites the list doesn't have yet, and events with no site. - It also still refetches when it joins an in-flight fetch, as before. - Start and stop refetch only if the list doesn't already show the result. - **Main process:** a site serving the PHP error page is shown at a URL only `getSites` knows. Its events now go out without the CLI's details, so the renderer refetches for them as before. - **CLI:** `site list` and the events now build the site from one `getSiteDetails`. Until now the events left out `emailPath`, so an applied event would have dropped it. Applying events directly, rather than refetching on each one, exposed two gaps that the refetches had been covering up: - **Auto-start** waited for `isFetchedAfterMount`. That flag also turns true when an event is applied on top of the list persisted from the last launch, which predates the `autoStart` flags the quit set. Auto-start now reads the list itself with `fetchQuery`, which joins the launch fetch already in flight. - **`_events`** read every site's status before subscribing to the process manager. A site that finished starting in between never got an event. It now subscribes first, so the last event for a site reflects its current state. No new cache layer: TanStack Query stays the only one. In packaged builds, I counted `site list` calls through a probe in the CLI entry. A run of the startup metric (one creation plus five starts and stops) made 33 calls on trunk and 10–16 with this change, across three runs each. Calls only remain where a fallback refetch is needed, for example when an operation's release lands after the start has resolved. Local timings were unusable because the machine's load average was 20–30. The Performance Metrics job is the timing check. ## Testing Instructions - `npx vitest run apps/ui/src/data/queries/use-sites.test.tsx apps/studio/src/modules/cli/lib/tests/cli-events-subscriber.test.ts apps/cli/commands/site/tests/list.test.ts` - The new start/stop test fails on trunk. - The auto-start test fails with the old `isFetchedAfterMount` gate. - It also fails if any of these is reverted: the cached `running` for operation events, the kept metadata, or the skipped refetches. - Start and stop a site in the app. Its status, URL and settings update as before. - Break a site's `wp-config.php` and start it. It shows the PHP error page at its URL, as before. ## Pre-merge Checklist - [x] Have you checked for TypeScript, React or other console errors?
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Related issues
site listbefore each start) didn't show insiteStartup, and whysiteStartupis bimodal on trunk (about 2.87 s or 3.39 s).How AI was used in this PR
Claude (via the Pi coding agent in a Roadie session) traced a site start through a packaged build, found the poll interval in Playwright's source, wrote the change, and ran the local measurements below. I reviewed the design and the code.
Proposed Changes
The site-startup metrics stopped the clock when
expect( statusButton ).toHaveAttribute( 'data-state', 'running' )passed. Playwright retries that assertion after 20, 50, 100 and 100 ms, then every 500 ms (retryWithProgressAndBackoffin Playwright 1.64.1). So each time landed on the next 500 ms poll after the site showed as running. Restart times came out in steps of about 0.5 s, and a change smaller than that only shows when it moves a restart down a step.recordRunningTimes( page )puts aMutationObserverin the page. It records the time each element'sdata-statemost recently becamerunning.siteCreationandsiteStartupuse that recorded time for the status button. The assertion still checks that the site is running; it just no longer sets the time.Testing Instructions
Locally, on macOS arm64 with a packaged build, I logged both times for 15 restarts of
tools/metrics/tests/site-startup.test.ts:site listrefetch plus a render.npx eslint tools/metricspasses.To try it, run
npm run package, thennpm run test:metrics.Pre-merge Checklist