-
Notifications
You must be signed in to change notification settings - Fork 3.2k
Reduce background Git ref and port polling #4187
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
3897689
e1bf7a2
72c361c
66bdee8
7944bea
5b816e5
8f2fdbb
d46e13c
152b294
7ef36da
bfdd40a
a188c8e
365fca6
97457be
c58f8ec
9b39a60
32a480b
594bc91
3b8c967
62fa974
50fe890
bb1446c
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,33 @@ | ||
| # Background Git Ref And Port Polling | ||
|
|
||
| Frequently mounted Git-ref and preview-discovery surfaces stay fresh without continuously repeating their most expensive subprocess work. | ||
|
|
||
| Expected behavior: | ||
|
|
||
| - Git ref lists revalidate their first page every 20 seconds instead of every five seconds. Loaded cursor pages are one-shot snapshots, and inactive ref atoms expire after 30 seconds. | ||
| - The composer branch selector's open-triggered ref refresh originated upstream. This branch preserves that behavior while routing it through the same branch-owned open-only refresh helper used by the Diff panel comparison-ref menu, which explicitly refreshes both local and remote refs on open. | ||
| - Closing either menu resets its query state without triggering another ref refresh, so user interaction stays fresh without adding close-triggered work. | ||
| - Preview port discovery performs one immediate scan when the first subscriber retains it. Subscriptions replay the latest snapshot instead of initiating a duplicate scan. | ||
| - Subscription replay and concurrent snapshot broadcasts are serialized so a stale replay cannot arrive after a newer scan result. | ||
| - Managed terminal process-set changes trigger an immediate port scan; unchanged registrations and redundant removals do not. | ||
| - The broad `lsof` safety-net scan runs every 20 seconds when no server is known and every 10 seconds while a listener is present. This preserves discovery for servers started outside T3-managed terminals without a permanent three-second system-wide process sweep. | ||
|
|
||
| Primary files: | ||
|
|
||
| - `packages/client-runtime/src/state/vcs.ts` | ||
| - `apps/web/src/components/BranchToolbarBranchSelector.tsx` | ||
| - `apps/web/src/components/DiffPanel.tsx` | ||
| - `apps/web/src/components/vcsRefMenuRefresh.ts` | ||
| - `apps/server/src/preview/PortScanner.ts` | ||
| - `apps/server/src/ws.ts` | ||
|
|
||
| Focused regression coverage: | ||
|
|
||
| - `packages/client-runtime/src/state/vcs.test.ts` covers the 20-second first-page revalidation interval. | ||
| - `apps/web/src/components/vcsRefMenuRefresh.test.ts` covers the shared helper's one-callback and multiple-callback open paths plus its no-refresh-on-close path; component wiring remains visible in the two primary component files above. | ||
| - `apps/server/src/preview/PortScanner.test.ts` covers snapshot replay without rescanning, ordered replay during concurrent broadcasts, and unchanged terminal registrations avoiding redundant probes. | ||
|
|
||
| ## Development Ports | ||
|
|
||
| - Web: `5740` | ||
| - Server/WebSocket: `13780` |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -21,7 +21,7 @@ import * as Duration from "effect/Duration"; | |
| import * as Effect from "effect/Effect"; | ||
| import * as Layer from "effect/Layer"; | ||
| import * as Ref from "effect/Ref"; | ||
| import * as Schedule from "effect/Schedule"; | ||
| import * as Semaphore from "effect/Semaphore"; | ||
| import * as Scope from "effect/Scope"; | ||
|
|
||
| import * as ProcessRunner from "../processRunner.ts"; | ||
|
|
@@ -50,7 +50,8 @@ export const COMMON_DEV_PORTS: ReadonlyArray<number> = Object.freeze([ | |
| 3000, 3001, 3333, 4173, 4200, 4321, 5000, 5173, 5174, 5175, 5500, 8000, 8080, 8081, 8888, 9000, | ||
| ]); | ||
|
|
||
| const POLL_INTERVAL = Duration.seconds(3); | ||
| const ACTIVE_POLL_INTERVAL = Duration.seconds(10); | ||
| const IDLE_POLL_INTERVAL = Duration.seconds(20); | ||
| const LSOF_TIMEOUT_MS = 5_000; | ||
| const WINDOWS_LISTENER_TIMEOUT_MS = 5_000; | ||
|
|
||
|
|
@@ -79,6 +80,9 @@ const terminalOwnerKey = (owner: { | |
| readonly terminalId: string; | ||
| }): string => `${owner.threadId}\u0000${owner.terminalId}`; | ||
|
|
||
| const processIdsEqual = (left: ReadonlySet<number>, right: ReadonlySet<number>): boolean => | ||
| left.size === right.size && [...left].every((processId) => right.has(processId)); | ||
|
|
||
| const parseLsofOutput = ( | ||
| raw: string, | ||
| terminalByProcessId: ReadonlyMap<number, TerminalProcessOwner> = new Map(), | ||
|
|
@@ -190,6 +194,7 @@ export const make = Effect.gen(function* PortDiscoveryMake() { | |
| const net = yield* Net.NetService; | ||
| const processRunner = yield* ProcessRunner.ProcessRunner; | ||
| const hostPlatform = yield* HostProcessPlatform; | ||
| const notificationLock = yield* Semaphore.make(1); | ||
| const stateRef = yield* Ref.make<ScannerState>({ | ||
| lastSnapshot: [], | ||
| listeners: new Set(), | ||
|
|
@@ -292,25 +297,49 @@ export const make = Effect.gen(function* PortDiscoveryMake() { | |
| yield* Effect.forEach(listeners, (listener) => listener(servers), { discard: true }); | ||
| }); | ||
|
|
||
| const publishSnapshot = Effect.fn("PortDiscovery.publishSnapshot")(function* ( | ||
| next: ReadonlyArray<DiscoveredLocalServer>, | ||
| ) { | ||
| yield* notificationLock.withPermit( | ||
| Effect.gen(function* () { | ||
| const changed = yield* Ref.modify(stateRef, (state) => | ||
| serversEqual(state.lastSnapshot, next) | ||
| ? [false, state] | ||
| : [true, { ...state, lastSnapshot: next }], | ||
| ); | ||
| if (changed) yield* broadcast(next); | ||
| }), | ||
| ); | ||
| }); | ||
|
|
||
| const pollTick = Effect.fn("PortDiscovery.pollTick")( | ||
| function* () { | ||
| if ((yield* Ref.get(stateRef)).retainCount <= 0) return; | ||
| const next = yield* scanOnce(); | ||
| const changed = yield* Ref.modify(stateRef, (state) => | ||
| serversEqual(state.lastSnapshot, next) | ||
| ? [false, state] | ||
| : [true, { ...state, lastSnapshot: next }], | ||
| ); | ||
| if (changed) yield* broadcast(next); | ||
| yield* publishSnapshot(next); | ||
| }, | ||
| Effect.catchCause((cause: Cause.Cause<never>) => | ||
| Effect.logWarning("preview port scan failed", Cause.pretty(cause)), | ||
| ), | ||
| ); | ||
|
|
||
| // Single layer-scoped polling fiber. Ticks are no-ops when no client is | ||
| // currently retained, so the cost is one Ref.get every POLL_INTERVAL. | ||
| yield* Effect.forkScoped(pollTick().pipe(Effect.repeat(Schedule.spaced(POLL_INTERVAL)))); | ||
| // Keep broad listener discovery as a fallback, but avoid a system-wide lsof | ||
| // process every three seconds while the app is otherwise idle. Terminal PID | ||
| // changes trigger immediate scans below; the periodic loop is only the | ||
| // safety net for listeners started outside a managed terminal. | ||
| yield* Effect.forkScoped( | ||
| Effect.gen(function* () { | ||
| while (true) { | ||
| const state = yield* Ref.get(stateRef); | ||
| yield* Effect.sleep( | ||
| state.retainCount > 0 && state.lastSnapshot.length > 0 | ||
| ? ACTIVE_POLL_INTERVAL | ||
| : IDLE_POLL_INTERVAL, | ||
| ); | ||
| yield* pollTick(); | ||
| } | ||
| }), | ||
| ); | ||
|
|
||
| const acquireRetention = Effect.fn("PortDiscovery.retain")(function* () { | ||
| const wasIdle = yield* Ref.modify(stateRef, (state) => [ | ||
|
|
@@ -334,16 +363,23 @@ export const make = Effect.gen(function* PortDiscoveryMake() { | |
| const subscribe: PortDiscovery["Service"]["subscribe"] = Effect.fn("PortDiscovery.subscribe")( | ||
| (listener) => | ||
| Effect.acquireRelease( | ||
| Ref.update(stateRef, (state) => ({ | ||
| ...state, | ||
| listeners: new Set([...state.listeners, listener]), | ||
| })), | ||
| notificationLock.withPermit( | ||
| Ref.modify(stateRef, (state) => [ | ||
| state.lastSnapshot, | ||
| { | ||
| ...state, | ||
| listeners: new Set([...state.listeners, listener]), | ||
| }, | ||
| ]).pipe(Effect.tap(listener)), | ||
| ), | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Leaked listener breaks port broadcastsMedium Severity
Reviewed by Cursor Bugbot for commit bb1446c. Configure here. |
||
| () => | ||
| Ref.update(stateRef, (state) => { | ||
| const listeners = new Set(state.listeners); | ||
| listeners.delete(listener); | ||
| return { ...state, listeners }; | ||
| }), | ||
| notificationLock.withPermit( | ||
| Ref.update(stateRef, (state) => { | ||
| const listeners = new Set(state.listeners); | ||
| listeners.delete(listener); | ||
| return { ...state, listeners }; | ||
| }), | ||
| ), | ||
| ), | ||
| ); | ||
|
|
||
|
|
@@ -356,26 +392,33 @@ export const make = Effect.gen(function* PortDiscoveryMake() { | |
| const processIds = new Set( | ||
| input.processIds.filter((processId) => Number.isInteger(processId) && processId > 0), | ||
| ); | ||
| yield* Ref.update(stateRef, (state) => { | ||
| const changed = yield* Ref.modify(stateRef, (state) => { | ||
| const terminalProcesses = new Map(state.terminalProcesses); | ||
| const key = terminalOwnerKey(owner); | ||
| const existing = terminalProcesses.get(key); | ||
| if (existing && processIdsEqual(existing.processIds, processIds)) { | ||
| return [false, state] as const; | ||
| } | ||
| if (processIds.size === 0) { | ||
| if (!existing) return [false, state] as const; | ||
| terminalProcesses.delete(key); | ||
| } else { | ||
| terminalProcesses.set(key, { owner, processIds }); | ||
| } | ||
| return { ...state, terminalProcesses }; | ||
| return [true, { ...state, terminalProcesses }] as const; | ||
| }); | ||
| if (changed) yield* pollTick(); | ||
| }); | ||
|
|
||
| const unregisterTerminal: PortDiscovery["Service"]["unregisterTerminal"] = Effect.fn( | ||
| "PortDiscovery.unregisterTerminal", | ||
| )(function* (input) { | ||
| yield* Ref.update(stateRef, (state) => { | ||
| const changed = yield* Ref.modify(stateRef, (state) => { | ||
| const terminalProcesses = new Map(state.terminalProcesses); | ||
| terminalProcesses.delete(terminalOwnerKey(input)); | ||
| return { ...state, terminalProcesses }; | ||
| const removed = terminalProcesses.delete(terminalOwnerKey(input)); | ||
| return [removed, removed ? { ...state, terminalProcesses } : state] as const; | ||
| }); | ||
| if (changed) yield* pollTick(); | ||
| }); | ||
|
|
||
| return PortDiscovery.of({ | ||
|
|
||


Uh oh!
There was an error while loading. Please reload this page.