Skip to content

perf(runtime): reduce extension lifecycle and icon churn#558

Open
JustYannicc wants to merge 46 commits into
SuperCmdLabs:mainfrom
JustYannicc:codex/consolidate-runtime-lifecycle-hotpaths
Open

perf(runtime): reduce extension lifecycle and icon churn#558
JustYannicc wants to merge 46 commits into
SuperCmdLabs:mainfrom
JustYannicc:codex/consolidate-runtime-lifecycle-hotpaths

Conversation

@JustYannicc

@JustYannicc JustYannicc commented Jul 3, 2026

Copy link
Copy Markdown
Collaborator

What changed

  • Consolidated the main-process icon IPC cache from perf(icons): cache main icon IPC lookups #536 with bounded LRU maps and in-flight coalescing for file/app icon data URL lookups.
  • Consolidated the extension runtime logging cleanup from perf(extensions): quiet runtime success logs #545 by gating success-path diagnostics behind __SUPERCMD_EXTENSION_RUNTIME_DEBUG while preserving warnings/errors.
  • Consolidated the MenuBarExtra NativeImage cache from perf(menubar): cache native images #547 for tray and menu item icon decode/resize reuse.
  • Consolidated the extension lifecycle sandbox cleanup from perf(extensions): clean sandbox lifecycle handles #550 so each ExtensionView evaluates bundles with scoped window/document objects and cleanup-tracked timers/listeners.
  • Reconciled the lifecycle sandbox with the integration-stack compiled wrapper cache by extending the cached wrapper ABI with scoped window and document parameters.
  • Guarded native helper process lifecycle handoffs for Parakeet, Qwen3, Whisper.cpp, and audio capture so stale child exit/stdout events cannot clear replacement process state or reject replacement requests.

Why

These runtime and extension lifecycle hot paths all reduce repeated native or renderer work that can happen during extension loads, menu-bar refreshes, icon lookups, native helper warmups, and remount/background lifecycles. Keeping them together makes the wrapper/lifecycle ABI merge explicit and preserves the focused performance harness evidence from the smaller fixes.

The native-helper guard closes a lifecycle race where an old helper process could be killed during restart, a replacement process could be spawned, and then the stale old process could still emit exit/stdout events that reset the replacement's readiness, startup promise, or pending request state.

Compatibility impact

  • IPC channel names, arguments, and return shapes are unchanged.
  • Icon and NativeImage caches are bounded; null/error icon results are not permanently cached.
  • Verbose extension success diagnostics remain available via globalThis.__SUPERCMD_EXTENSION_RUNTIME_DEBUG = true; warnings and errors still log by default.
  • Extension bundles still receive the same Raycast shim exports and Node facade behavior, with window, self, global, and globalThis scoped to the current ExtensionView during evaluation.
  • Host renderer globals are not mutated by the lifecycle sandbox.
  • Native helper shutdown still writes the same exit command and kills the selected child; the cleanup path now only clears shared helper state when that selected child is still the active process.

How tested

  • node --test scripts/test-main-icon-ipc-cache.mjs scripts/test-extension-runtime-logging.mjs scripts/test-menubar-native-image-cache.mjs scripts/test-extension-lifecycle-sandbox.mjs scripts/test-extension-wrapper-cache.mjs (26 passing tests)
  • node --test scripts/test-extension-wrapper-cache.mjs scripts/test-extension-sync-facade.mjs scripts/test-background-no-view-dedupe.mjs scripts/test-extension-runtime-logging.mjs scripts/test-extension-lifecycle-sandbox.mjs (19 passing tests)
  • node --test scripts/test-native-helper-process-lifecycle.mjs (3 passing tests)
  • ./node_modules/.bin/tsc -p tsconfig.main.json --noEmit --pretty false
  • pnpm run build:main
  • ./node_modules/.bin/tsc -p tsconfig.renderer.json --noEmit --pretty false
  • ./node_modules/.bin/vite build
  • node /opt/homebrew/lib/node_modules/npm/bin/npm-cli.js run build:main
  • git diff --check codex/perf-consolidated-integration-stack..HEAD
  • git diff --check HEAD^ HEAD
  • Codex LSP diagnostics: no diagnostics for src/main/main.ts after the native-helper lifecycle update.
  • Codex LSP references checked for killParakeetServer, ensureParakeetServer, killQwen3Server, ensureQwen3Server, killWhisperCppServer, ensureWhisperCppServer, killAudioCapturer, and warmAudioCapturer across the edited helper lifecycle call paths.
  • Earlier Codex LSP diagnostics: no errors for src/main/icon-ipc-cache.ts, src/main/menubar-native-image-cache.ts, src/renderer/src/ExtensionView.tsx, and src/renderer/src/utils/extension-wrapper-cache.ts.
  • Earlier Codex LSP references checked for getCompiledExtensionWrapper and createExtensionLifecycleScope across the merged wrapper/lifecycle call paths.

Performance evidence

  • Main icon IPC cache:
    • File icons, 100 concurrent requests: before 100 native calls, after 1.
    • File icons, 100 repeated requests: before 100 native calls, after 1.
    • App icons, 100 concurrent requests: before 100 native calls, after 1.
    • App icons, 100 repeated requests: before 100 native calls, after 1.
  • Extension runtime logging:
    • Baseline success diagnostics: logs=4, exportedToStringCalls=1.
    • Debug helper after fix: defaultDebugCalls=0, enabledDebugCalls=1.
  • MenuBarExtra NativeImage cache:
    • Repeated accepted payloads across 20 updates with 1 tray icon and 36 item/submenu icons: before 1620 native decode/read/resize operations, after 81.
  • Extension lifecycle sandbox:
    • Pre-fix simulation cleanup leaves intervals=1, windowListeners=1, documentListeners=1, total=3.
    • Scoped cleanup starts from the same total=3, registry sees intervals=1 and eventListeners=2, cleanup ends at total=0.
    • Re-evaluation cleanup follows 3 -> 0 -> 3 -> 0 across prior evaluation cleanup and final unmount.
  • Native helper process lifecycle:
    • Legacy reproduction shows a stale old-child exit clears the replacement process and rejects its pending request.
    • Guarded lifecycle keeps the replacement process active, leaves replacement readiness false until its own stdout is seen, preserves the startup promise, and keeps the replacement pending request alive across stale old-child exit/stdout events.

Stack validation

This branch was created from the local codex/perf-consolidated-integration-stack validation stack and then cherry-picked only the assigned fixes from #536, #545, #547, #550, plus the native-helper lifecycle guard commit now included here. The integration-stack head is currently represented upstream by #555; until that stack lands, this PR's GitHub comparison includes those base stack commits plus the consolidation commits here. Renderer typecheck and Vite build were clean on this stack before the native-helper lifecycle follow-up; the follow-up revalidated the focused helper harness, main-process TypeScript no-emit check, main build script, LSP diagnostics/references for the touched main-process lifecycle code, and whitespace checks.

Replaces

…s' into codex/perf-consolidated-integration-stack
…nd-menubar' into codex/perf-consolidated-integration-stack

# Conflicts:
#	src/renderer/src/raycast-api/hooks/use-cached-promise.ts
…emoji' into codex/perf-consolidated-integration-stack

# Conflicts:
#	src/renderer/src/raycast-api/list-runtime-renderers.tsx
#	src/renderer/src/raycast-api/list-runtime.tsx
… into codex/perf-consolidated-integration-stack
…tus' into codex/perf-consolidated-integration-stack
… into codex/perf-consolidated-integration-stack

# Conflicts:
#	src/renderer/src/CameraExtension.tsx
@shobhit99

Copy link
Copy Markdown
Contributor

Reviewed the 5 consolidation commits on top of the base validation stack (93254461, 8ef6b48f, e4061fa3, ede73594, 550165f3) — i.e. the icon IPC cache, quieted runtime logging, MenuBar NativeImage cache, extension lifecycle sandbox, and native-helper lifecycle guard. The ~37 commits below the merge commits belong to the #555 base stack and are out of scope here.

Strengths

  • Icon IPC cache (src/main/icon-ipc-cache.ts): clean bounded LRU with recency reordering, in-flight coalescing, and correct negative-cache avoidance — null/empty/throw results are never stored (resolveFromSource / resolve only cache.set when typeof dataUrl === 'string'), so transient failures retry. Bucket + logical-size derivation matches the old inline handler exactly.
  • MenuBar NativeImage cache (src/main/menubar-native-image-cache.ts): path entries key on path|mtimeMs|bytes|resizeKey, so an edited icon file busts the cache; template state and resize params are baked into the key, so a template/size change is a distinct entry (no stale template bleed-through). The tray template refactor (primaryTemplate for primary, false for fallback) is behavior-equivalent to the old post-selection setTemplateImage.
  • ABI extension verified correct: the 21 positional fn(...) args map 1:1 to EXTENSION_WRAPPER_ARGUMENTS, with window/document inserted between cancelAnimationFrame and navigator in both the wrapper arg list and the call site. global/globalThis (slots 8/9) now point at scopedWindow.
  • Lifecycle sandbox: real leak-prevention win — window.addEventListener/document.addEventListener and window.setInterval/etc. are now tracked and torn down on unmount/re-eval (previously only the bare timer params were). The proxy correctly binds host methods (avoiding illegal-invocation), returns PascalCase/class values unbound so instanceof/new still work, and forces configurable:true in getOwnPropertyDescriptor to stay within Proxy invariants against an empty target. Registry lifecycle wiring (fresh per instance, cleared on unmount and before re-eval) is intact.
  • Native-helper guard (main.ts, all four helpers): the if (process !== child) return guards on exit/stdout/timeout/interval, plus the killXServer(processToKill) early-return that skips shared-state reset (and pending-request rejection) when the killed child isn't the active one, correctly close the stale-child-clobbers-replacement race. Confirmed the full killParakeetServer preserves parakeetPendingRequest when a stale child is killed. fs (main.ts:18) and nativeImage (main.ts:239) are module-level, so the new menubar call sites are in scope.

Concerns / potential bugs

  • (Minor) New process-lifetime staleness for file/app icons. buildFileIconCacheKey/buildAppIconCacheKey (icon-ipc-cache.ts:75-87) key only on resolved path + size — no mtime/content component — and the file/app caches are never clear()ed. On main these IPC handlers always re-resolved, so this introduces a staleness surface: if an app updates its .icns or a file's icon changes mid-session, the launcher serves the old data URL until LRU eviction or restart. The MenuBar cache handles this correctly (mtime+bytes); the file/app caches don't. Acceptable tradeoff for the dedup win (adding a statSync per lookup would partly defeat the purpose), but worth keying on mtime if staleness is ever reported.
  • (Minor / doc accuracy) Sandbox does not isolate writes. The proxy set/defineProperty/deleteProperty traps forward to the host window via Reflect.set(host, ...) (ExtensionView.tsx:3792), so window.x = … / globalThis.x = … from an extension still mutate the host renderer's window. This is not a regression (slots 8/9 were the real host globalThis before, and free window references also hit the host), but the PR description's "Host renderer globals are not mutated by the lifecycle sandbox" overstates it — only reads, timers, and listeners are scoped; writes remain pass-through.

Suggestions

No correctness-blocking bugs found in the reviewable delta.

🟡 Approve with minor comments

🤖 Automated review by Claude Code (Opus 4.8).

@JustYannicc

Copy link
Copy Markdown
Collaborator Author

Addressed the review follow-up on icon cache staleness.

Changes pushed in a89632b:

  • File icon IPC cache keys now include source file metadata (kind, mtimeMs, ctimeMs, size, ino) in addition to normalized path, logical size, and bucket.
  • App icon cache keys now fingerprint the actual icon inputs used by the resolver: Contents/Info.plist plus the resolved .icns path and file metadata, so changing CFBundleIconFile or replacing/modifying the icon file misses the old cached data URL.
  • Added regression coverage for both file-icon and app-icon metadata invalidation while preserving unchanged-source cache hits.

Review triage notes:

  • GraphQL review-thread fetch found no inline review threads and no submitted reviews; the actionable code concern was the top-level note about process-lifetime staleness for file/app icons.
  • The sandbox wording/perf-framing feedback is non-code PR-description framing: the sandbox remains a lifecycle leak/correctness fix and does not claim write isolation in code.

Proof run locally:

  • node --test scripts/test-main-icon-ipc-cache.mjs -> pass (9 tests)
  • node --test scripts/test-extension-lifecycle-sandbox.mjs scripts/test-extension-runtime-logging.mjs scripts/test-menubar-native-image-cache.mjs scripts/test-native-helper-process-lifecycle.mjs scripts/test-icon-runtime-file-icon-cache.mjs scripts/test-icon-runtime-phosphor-cache.mjs -> pass (28 tests)
  • npm run build:main -> pass
  • npm test -> pass (191 tests)

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.

2 participants