Skip to content

perf(launch): hold a loading state back until the work behind it outlasts a grace period - #2606

Merged
datlechin merged 1 commit into
mainfrom
perf/no-loading-states-in-startup-path
Sep 2, 2026
Merged

perf(launch): hold a loading state back until the work behind it outlasts a grace period#2606
datlechin merged 1 commit into
mainfrom
perf/no-loading-states-in-startup-path

Conversation

@datlechin

Copy link
Copy Markdown
Member

Mitchell Hashimoto, watching TablePro open the SQLite sample: "Final frame didn't settle until the icon nearly hit the dock. A full bounce. I recommend getting rid of these three loading states unless the delay exceeds a certain amount. Frame reconciliation on view hierarchies is slow as hell and you do NOT want it in the startup path."

He is right, and instrumenting the launch found a fourth state he could not see.

What the window actually does today

refreshPanes and every phase transition, Debug build, opening the bundled Chinook sample from a restored session. Three runs, within 2ms of each other:

openTab start
 +17ms   refreshPanes  pane = .unavailable(.notConnected)   build 1
 +38ms   phase idle -> connecting
         refreshPanes  pane = .connecting                    build 2
         chrome -> hidden
 +39ms   phase connecting -> connected
         refreshPanes  pane = .content                       build 3
         chrome -> revealed
+110ms   schema loaded, sidebar spinner clears
-----
 205ms   window open to fully loaded

Three pane hierarchies built in the first 103ms, for a connect that takes 39ms. Between builds 2 and 3, applySidebarChromeMode collapses both split items, drops splitView.autosaveName, calls recalculateKeyViewLoop(), and then puts all of it back. That is AppKit layout, not only SwiftUI reconciliation.

The fourth state is build 1: a "Not connected" pane, built 38ms before the connecting card replaces it. It resolves that way because .idle answers both for a window that finished dialling and for one that has not begun, and only the caller knows which.

Root cause

The window's content is a pure function of ConnectionWindowPhase and SchemaState. Neither carries how long the state has lasted, so every state paints the instant it is entered however briefly it lives. One missing input, not five separate spinner bugs.

What the HIG says

  • Launching: "Launch instantly." and "Design a launch screen that's nearly identical to the first screen of your app... If you include elements that look different when launching completes, people may experience an unpleasant flash between the launch screen and your first screen."
  • Loading: "Show something as soon as possible... consider showing placeholder text, graphics, or animations as content loads, replacing these elements as content becomes available." Progress indicators are for "situations where loading takes more than a moment or two".
  • Progress indicators: "Avoid vague terms like loading or authenticating because they seldom add value", which is what "Opening the connection" and "Loading schema…" are.

The change

LoadingRevealPolicy holds both halves of the rule: a 500ms grace before any progress UI appears, and a 500ms minimum dwell once it has. The dwell is the half that gets left out, and DelayedProgressIndicator had left it out: an indicator revealed at 500ms over work that ends at 510ms is a 10ms flash, worse than either state it sits between. The grace reuses the 500ms that component already used rather than inventing a second number.

ConnectionWindowPaneResolver gains .preparing, the sub-grace pane. It draws nothing, and it is the one contentless pane that hidesChrome returns false for. The existing argument that "an object browser and an inspector with nothing to put in them are two empty columns that promise a session the window does not have yet" holds for a wait the user can see and not for one they cannot. On the happy path the window opens in its final shape and stays there.

.idle with an auto-connect pending resolves to .preparing too, which is what removes build 1. The grace expiring is the exit from .preparing in both directions, and .idle reads it for that reason: startActivationConnectIfNeeded returns without dialling when the phase disallows it or the connection record has gone, and a window that never dials has to reach the not-connected pane rather than sit blank.

The grace timer belongs to ConnectionWorkspace, not to the window, for the same reason attemptToken does: a window hosts several connections and each dials on its own clock, so a window-wide flag would let one connection's slow server put a progress screen over another's finished one. Its reveal repaints through syncPanes, so it costs nothing on the path where the connect landed first and the flag never flipped.

SidebarObjectListPresentation gains .preparing on the same input. Past the grace nothing changes: the card, its Cancel button and the spinners are exactly what they are today, which is the case the connecting screen was designed for.

The other two spinners from the screenshots go through the same policy at the view: the toolbar's "Executing…" cluster and the result readout. The toolbar gates the state rather than the content, so a query too fast to report leaves the previous duration standing instead of emptying the item and changing the row's width twice.

After

Same three runs, same build:

openTab start
 +19ms   refreshPanes  pane = .preparing        (draws nothing)
         chrome -> revealed                     (once)
 +82ms   refreshPanes  pane = .content
+194ms   schema loaded

Two pane builds instead of three, and the first draws nothing. One chrome application instead of a collapse and a reveal. The connecting card is never built. The sidebar spinner never appears, because 194ms is inside the grace. Total time is unchanged, as expected: this does not make the connect faster, it stops the window rearranging itself around one.

Not done

  • The toolbar still arrives with the session. installToolbar needs a coordinator and the coordinator needs a session, and moving that is an ownership refactor the measured win does not need. Not collapsing the chrome is what mattered.
  • The tab-content progress views (Loading insights…, Loading users and roles…, Loading dashboard…, Loading schema…) and the sheet-based export, import and backup indicators are untouched. Those are user-initiated operations, not the startup path, and the HIG note about vague labels applies to them separately.

Tests

48 cases across four suites, all passing. The two resolvers are pure and are tested in both directions, including that the grace never delays a settled outcome: a server that refuses in 20ms is an answer, not a wait, and a failed schema read has to keep its message and its Retry.

ConnectionWorkspaceGraceTests covers the timer itself: that a connect landing first leaves no reveal behind for the next one, that re-arming does not restart a wait already running (or a redialling connection would never report anything), that teardown takes a pending reveal with it, and that a window which never dials falls back to the not-connected pane.

No UI test. The behaviour this changes is entirely inside 500ms, and an XCUITest assertion that a card never appeared is a race rather than a check. The logic is in the two pure resolvers and the workspace timer, which is what the unit tests cover.

Also here

Two SwiftLint errors on main that would have made this branch red: a missing thousands separator in SSHPublicKeyFile from #2604, and a blank line before a closing brace in CompareSyncProfileStorage from #2605. Neither file is otherwise touched by this branch.

Verification

  • verify.sh build: PASS
  • verify.sh test over ConnectionWorkspaceGraceTests, ConnectionWindowPaneResolverTests, SidebarObjectListPresentationTests, LoadingRevealPolicyTests: 48 executed, 48 passed, 0 failed
  • verify.sh lint over TablePro and TableProTests: 0 violations. The reported doc-symbol miss (AXCell, CLAUDE.md:220) is on main and untouched here.
  • Before and after both measured three times against an isolated copy of the sample database, with the app launched under TABLEPRO_UI_TEST_SANDBOX so it never read the connections on this machine.

https://claude.ai/code/session_01J6xU4Zx4DRJ5JaxMP437uT

@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

@datlechin
datlechin merged commit 675cd24 into main Sep 2, 2026
8 checks passed
@datlechin
datlechin deleted the perf/no-loading-states-in-startup-path branch September 2, 2026 04:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant