CPLAT-12472: move the session rescan off the UI loop - #172
Merged
Merged
Conversation
|
| Commit | Scanned at | New | Resolved | Net |
|---|---|---|---|---|
96fa04a |
2026-09-30 04:32 UTC | 0 | 0 | 0 |
20bdb92 < |
2026-09-30 06:21 UTC | 0 | 0 | 0 |
Last scanned: 20bdb92 · 2026-09-30 06:21 UTC
|
| Commit | Scanned at | New | Resolved | Net |
|---|---|---|---|---|
96fa04a |
2026-09-30 04:33 UTC | 0 | 0 | 0 |
96fa04a |
2026-09-30 04:32 UTC | — | — | — |
20bdb92 < |
2026-09-30 06:22 UTC | 0 | 0 | 0 |
Last scanned: 20bdb92 · 2026-09-30 06:22 UTC
Kairo-Kim
approved these changes
Sep 30, 2026
Kairo-Kim
added
the
auto-review/approved
Reported as "list navigation locks up sometimes". Navigation itself was never the problem. Profiled against a real 277-session directory, a keypress costs 11-23µs and the debounced preview update another ~20µs. The stall was doRefresh, which ran two expensive calls inline on the UI loop: session.ScanSessions ~308ms tmux.MarkLiveSessions ~127ms (shells out to tmux) os.Stat x279 0.5ms So ~420ms of frozen UI per refresh — on R, on the post-spawn delayedRefreshMsg, and with live updates on, every 3s tick. The comment calling that whole fallback "lightweight" was wrong about which part cost what: the stat pass really is cheap, the tmux call is not. scanSessionsCmd now runs the scan AND the live-state marking off the loop and returns refreshScannedMsg; the handler does only the merge (carryOverRefState, injectRemoteSessions, rebuild), measured at 1.9ms p50 / 8.8ms max — inside a frame. doRefresh keeps just the mtime pass inline so ordering stays current between scans, and dedups concurrent scans behind refreshScanInFlight: the tick is 3s but a large scan takes longer, so without it the walks stack up and compete for the same disk. The startup path (sessionsScannedMsg) had the same 127ms MarkLiveSessions call on the loop; it moved into that command too. 420ms -> 0.26ms p50, 3.2ms max. The trade is that LIVE/BUSY badges refresh when the scan lands rather than synchronously, so between scans a badge can lag by up to one tick. TestDoRefreshRebuildsFilteredSessionItemsWhenLiveStateChanges still holds — it just has to run the dispatched command first, which is what the runtime does. Two guards, each verified by reverting the fix and confirming they fail. They assert shape (a command is dispatched; the session slice does not change until its result is handled) rather than wall-clock time, so they cannot go flaky on a loaded machine.
gavin-jeong
force-pushed
the
CPLAT-12472-async-rescan
branch
from
September 30, 2026 06:21
96fa04a to
20bdb92
Compare
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
4 participants
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.
JIRA: https://sendbird.atlassian.net/browse/CPLAT-12472
Reported as "list navigation locks up sometimes".
Navigation was never the problem
Profiled against a real 277-session directory. A keypress costs 11-23µs; the debounced preview update another ~20µs. Both are far inside a frame.
The stall is
doRefresh, which ran two expensive calls inline on the UI loop:session.ScanSessionstmux.MarkLiveSessions(shells out to tmux)os.Stat×279 — the rest of the "lightweight" passThat is ~420ms of frozen UI per refresh, and
doRefreshruns onR, on the post-spawndelayedRefreshMsg, and — with live updates on — every 3s tick.The comment calling that whole fallback "lightweight" was wrong about which part cost what: the stat pass really is cheap, the tmux call is not.
Fix
scanSessionsCmdruns the scan and the live-state marking off the loop, returningrefreshScannedMsg. The handler does only the merge (carryOverRefState,injectRemoteSessions, rebuild) — 1.9ms p50 / 8.8ms max, inside a frame.doRefreshkeeps just the mtime pass inline so ordering stays current between scans, and dedups concurrent scans behindrefreshScanInFlight: the tick is 3s but a large scan takes longer, so without it the walks stack up and compete for the same disk.The startup path (
sessionsScannedMsg) had the same 127msMarkLiveSessionscall on the loop; it moved into that command too.420ms → 0.26ms p50, 3.2ms max.
The trade — worth a look
LIVE/BUSYbadges now refresh when the scan lands rather than synchronously, so between scans a badge can lag by up to one tick. I think that is clearly the right trade against freezing the UI on every refresh, but it is a behaviour change and I would rather have it looked at than buried.TestDoRefreshRebuildsFilteredSessionItemsWhenLiveStateChangesstill holds — it just has to run the dispatched command first, which is what the runtime does.Test plan
go build ./... && go vet ./... && go test ./...greenTestDoRefreshDoesNotScanOnTheUILoop— a session written to disk must not appear until the dispatched scan's result is handled; if the scan runs inline it shows up before the call returnsTestDoRefreshDedupsConcurrentScans— a second refresh while one is in flight must not start another walkNote
Branched from
master, not from #171 — the two are independent and I did not want the reviews entangled.Security checklist
0.0.0.0/0inbound