Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 19 additions & 12 deletions apps/desktop/electron/main.ts
Original file line number Diff line number Diff line change
Expand Up @@ -192,8 +192,9 @@
SshConnection
} from './ssh-connection'
import { createStreamThrottle } from './stream-throttle'
import { applyWindowsOcclusionCommandLineSwitches } from './windows-occlusion-flags'
import { nativeOverlayWidth as computeNativeOverlayWidth, macTitleBarOverlayHeight } from './titlebar-overlay-width'
import { resolveBehindCount, shouldCountCommits } from './update-count'

Check failure on line 197 in apps/desktop/electron/main.ts

View workflow job for this annotation

GitHub Actions / JS & TS checks / apps/desktop / check:lint

Expected "./titlebar-overlay-width" to come before "./windows-occlusion-flags"
import { waitForUpdateClearance } from './update-gate'
import { readLiveUpdateMarker, updateHandoffConflict, writeUpdateMarker } from './update-marker'
import { runRebuildWithRetry } from './update-rebuild'
Expand Down Expand Up @@ -445,19 +446,25 @@
// window is minimized. This switch only affects scheduling priority; it does
// not exempt timers from throttling and costs nothing at idle.
//
// The timer/rAF throttling story is deliberately NOT handled here anymore.
// The old process-wide `disable-background-timer-throttling` /
// `disable-backgrounding-occluded-windows` switches (plus a static
// `backgroundThrottling: false` on every chat window) pinned every renderer's
// `document.visibilityState` to 'visible' forever — which silently turned all
// the renderer's visibility-gated backstop polls and clock ticks into
// always-on timers. A completely idle, minimized Hermes burned ~20% CPU
// around the clock. Throttling is now a runtime dial scoped to streaming:
// see createStreamThrottle() — chat windows are unthrottled while any turn is
// in flight (so a live answer keeps painting while blurred, occluded, or
// minimized, exactly as before) and return to Chromium's default throttling
// once the work settles.
// Timer/rAF throttling is a runtime dial scoped to streaming (see
// createStreamThrottle()) — NOT the old process-wide
// `disable-background-timer-throttling` + static `backgroundThrottling: false`
// combo that pinned every renderer's `document.visibilityState` to 'visible'
// and burned ~20% CPU on an idle minimized Hermes.
//
// Windows still needs the occluded-window opt-out: Chromium's native
// occlusion / backgrounding path can park the browser UI task runner on a
// WaitableEvent with no wake after minimize or full occlusion (#83420).
// That switch set is applied below; it does NOT restore the timer-throttling
// opt-out or a static backgroundThrottling:false.
app.commandLine.appendSwitch('disable-renderer-backgrounding')
applyWindowsOcclusionCommandLineSwitches((name, value) => {
if (value === undefined) {
app.commandLine.appendSwitch(name)
} else {
app.commandLine.appendSwitch(name, value)
}
})

const SOURCE_REPO_ROOT = path.resolve(APP_ROOT, '../..')

Expand Down
65 changes: 61 additions & 4 deletions apps/desktop/electron/stream-throttle.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,18 +34,27 @@

function makeWindow() {
const calls: boolean[] = []
const listeners = new Map<string, () => void>()
const listeners = new Map<string, Array<() => void>>()
let destroyed = false

const win = {
calls,
close() {
destroyed = true
listeners.get('closed')?.()
for (const fn of listeners.get('closed') ?? []) {

Check warning on line 44 in apps/desktop/electron/stream-throttle.test.ts

View workflow job for this annotation

GitHub Actions / JS & TS checks / apps/desktop / check:lint

Expected blank line before this statement
fn()
}
},
emit(event: string) {
for (const fn of listeners.get(event) ?? []) {
fn()
}
},
isDestroyed: () => destroyed,
on(event: string, fn: () => void) {
listeners.set(event, fn)
const list = listeners.get(event) ?? []
list.push(fn)
listeners.set(event, list)
},
webContents: {
isDestroyed: () => destroyed,
Expand All @@ -57,8 +66,7 @@

return win
}

test('registering a window applies the current throttle state immediately', () => {

Check warning on line 69 in apps/desktop/electron/stream-throttle.test.ts

View workflow job for this annotation

GitHub Actions / JS & TS checks / apps/desktop / check:lint

Expected blank line before this statement
const timers = makeTimers()
const throttle = createStreamThrottle(timers)
const idle = makeWindow()
Expand Down Expand Up @@ -150,3 +158,52 @@
// Only the registration-time call landed; nothing after close.
assert.deepEqual(closedWin.calls, [true])
})

test('wake pulses unthrottled then schedules the trailing re-throttle when idle', () => {
const timers = makeTimers()
const throttle = createStreamThrottle(timers)
const win = makeWindow()
throttle.register(win)
win.calls.length = 0

throttle.wake()
assert.deepEqual(win.calls, [false])
assert.equal(throttle.isUnthrottled(), true)
assert.equal(timers.pendingCount, 1)

timers.fire()
assert.deepEqual(win.calls, [false, true])
assert.equal(throttle.isUnthrottled(), false)
})

test('show/restore/focus events wake an idle registered window', () => {
const timers = makeTimers()
const throttle = createStreamThrottle(timers)
const win = makeWindow()
throttle.register(win)
win.calls.length = 0

win.emit('restore')
assert.deepEqual(win.calls, [false])
assert.equal(throttle.isUnthrottled(), true)

win.emit('show')
// Already unthrottled with a trailing timer — no stacked apply.
assert.deepEqual(win.calls, [false])
assert.equal(timers.pendingCount, 1)
})

test('wake during a live turn stays unthrottled without starting a re-throttle', () => {
const timers = makeTimers()
const throttle = createStreamThrottle(timers)
const win = makeWindow()
throttle.register(win)

throttle.update(true)
win.calls.length = 0
throttle.wake()

assert.deepEqual(win.calls, [])
assert.equal(throttle.isUnthrottled(), true)
assert.equal(timers.pendingCount, 0)
})
32 changes: 29 additions & 3 deletions apps/desktop/electron/stream-throttle.ts
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,9 @@
register(win: ThrottleWindowLike & { on?: (event: string, fn: () => void) => void }): void
/** Report whether any turn is in flight across all renderers. */
update(busy: boolean): void
/** Briefly unthrottle after show/restore/focus so a Windows occluded-window
* stall can pump the UI task runner again (#83420). */
wake(): void
}

export function createStreamThrottle(
Expand All @@ -53,6 +56,7 @@
const windows = new Set<ThrottleWindowLike>()
let unthrottled = false
let trailing: unknown = null
let busy = false

function apply(win: ThrottleWindowLike) {
if (win.isDestroyed()) {
Expand Down Expand Up @@ -80,17 +84,25 @@
}
}

return {
const api: StreamThrottle = {
isUnthrottled: () => unthrottled,

register(win) {
windows.add(win)
win.on?.('closed', () => windows.delete(win))
// Defense in depth for #83420: when a chat window returns from
// minimize/occlusion, pulse unthrottled so the browser task runner
// gets a wake even if Chromium's occluded path wedged while hidden.
for (const event of ['show', 'restore', 'focus'] as const) {

Check warning on line 96 in apps/desktop/electron/stream-throttle.ts

View workflow job for this annotation

GitHub Actions / JS & TS checks / apps/desktop / check:lint

Expected blank line before this statement
win.on?.(event, () => api.wake())
}
apply(win)

Check warning on line 99 in apps/desktop/electron/stream-throttle.ts

View workflow job for this annotation

GitHub Actions / JS & TS checks / apps/desktop / check:lint

Expected blank line before this statement
},

update(busy) {
if (busy) {
update(nextBusy) {
busy = nextBusy

if (nextBusy) {
if (trailing !== null) {
timers.clearTimeout(trailing)
trailing = null
Expand All @@ -114,6 +126,20 @@
unthrottled = false
applyAll()
}, delayMs)
},

wake() {
// Force a trailing unthrottle window even when idle. If a turn is
// already in flight, update(true) is enough (and cancels any pending
// re-throttle). Capture busy first — update(true) latches it.
const wasBusy = busy
api.update(true)

if (!wasBusy) {
api.update(false)
}
}
}

return api
}
38 changes: 38 additions & 0 deletions apps/desktop/electron/windows-occlusion-flags.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
import assert from 'node:assert/strict'

import { test } from 'vitest'

import {
applyWindowsOcclusionCommandLineSwitches,
windowsOcclusionCommandLineSwitches
} from './windows-occlusion-flags'

test('windows occlusion switches are empty off Windows', () => {
assert.deepEqual(windowsOcclusionCommandLineSwitches('darwin'), [])
assert.deepEqual(windowsOcclusionCommandLineSwitches('linux'), [])
})

test('windows occlusion switches opt out of occluded-window backgrounding', () => {
const flags = windowsOcclusionCommandLineSwitches('win32')

assert.deepEqual(flags, [
{ switchName: 'disable-backgrounding-occluded-windows' },
{ switchName: 'disable-features', value: 'CalculateNativeWinOcclusion' }
])
})

test('applyWindowsOcclusionCommandLineSwitches appends only on win32', () => {
const calls: Array<[string, string?]> = []
const appendSwitch = (switchName: string, value?: string) => {

Check warning on line 26 in apps/desktop/electron/windows-occlusion-flags.test.ts

View workflow job for this annotation

GitHub Actions / JS & TS checks / apps/desktop / check:lint

Expected blank line before this statement
calls.push(value === undefined ? [switchName] : [switchName, value])
}

applyWindowsOcclusionCommandLineSwitches(appendSwitch, 'darwin')
assert.deepEqual(calls, [])

applyWindowsOcclusionCommandLineSwitches(appendSwitch, 'win32')
assert.deepEqual(calls, [
['disable-backgrounding-occluded-windows'],
['disable-features', 'CalculateNativeWinOcclusion']
])
})
48 changes: 48 additions & 0 deletions apps/desktop/electron/windows-occlusion-flags.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
/**
* Chromium command-line switches that keep the Windows UI task runner from
* stalling when a Hermes window is minimized or fully occluded (#83420).
*
* After stream-scoped throttling landed, idle chat windows return to Chromium's
* default occlusion/backgrounding path. On Windows that path can park the
* browser main thread on a WaitableEvent with no message-pump wake — silent
* freeze, no exception. The perf harness already uses these same flags when
* the window sits behind the IDE; production needs the Windows-only subset.
*
* Intentionally omitted (those pinned visibility forever and burned ~20% CPU
* at idle):
* - disable-background-timer-throttling
* - static webPreferences.backgroundThrottling: false
*
* Stream-throttle still dials setBackgroundThrottling for live turns.
*/

export interface CommandLineSwitch {
switchName: string
value?: string
}

export function windowsOcclusionCommandLineSwitches(
platform: NodeJS.Platform | string = process.platform
): CommandLineSwitch[] {
if (platform !== 'win32') {
return []
}

return [
{ switchName: 'disable-backgrounding-occluded-windows' },
{ switchName: 'disable-features', value: 'CalculateNativeWinOcclusion' }
]
}

export function applyWindowsOcclusionCommandLineSwitches(
appendSwitch: (switchName: string, value?: string) => void,
platform: NodeJS.Platform | string = process.platform
): void {
for (const flag of windowsOcclusionCommandLineSwitches(platform)) {
if (flag.value === undefined) {
appendSwitch(flag.switchName)
} else {
appendSwitch(flag.switchName, flag.value)
}
}
}
Loading