Skip to content

Commit 2b623fa

Browse files
authored
chat: fix performance slowdowns when closing/reloading the window (microsoft#287186)
- _onDidChangeToolsScheduler.isScheduled is checked to avoid thrashing setTimeout when disposing many tools - WorkspaceExtensionsManagementService was listening to file change events using a debounce. Debounces have overhead because a new timer is scheduled on every single call. For large amount of file changes (during EH shutdown when schemas for tools are deregistered) this caused a notable slowdown. `throttle` should be functionally equivalent. - avoid triggering input updates (w/ downstream editor effects) each time the input gets parsed, which happened every time tool is called - big hammer -- don't bother deregistering MCP tools each time Results: - `216ms` to shut down EH before making these changes - `87ms` in the first three bullets - `54ms` after skipping MCP tool deregistering. (Basically all the overhead there was unregistering the JSON schema for tool inputs.)
1 parent a72f553 commit 2b623fa

8 files changed

Lines changed: 393 additions & 6 deletions

File tree

src/vs/base/common/event.ts

Lines changed: 83 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -345,6 +345,89 @@ export namespace Event {
345345
}, delay, undefined, flushOnListenerRemove ?? true, undefined, disposable);
346346
}
347347

348+
/**
349+
* Throttles an event, ensuring the event is fired at most once during the specified delay period.
350+
* Unlike debounce, throttle will fire immediately on the leading edge and/or after the delay on the trailing edge.
351+
*
352+
* *NOTE* that this function returns an `Event` and it MUST be called with a `DisposableStore` whenever the returned
353+
* event is accessible to "third parties", e.g the event is a public property. Otherwise a leaked listener on the
354+
* returned event causes this utility to leak a listener on the original event.
355+
*
356+
* @param event The event source for the new event.
357+
* @param merge An accumulator function that merges events if multiple occur during the throttle period.
358+
* @param delay The number of milliseconds to throttle.
359+
* @param leading Whether to fire on the leading edge (immediately on first event).
360+
* @param trailing Whether to fire on the trailing edge (after delay with the last value).
361+
* @param leakWarningThreshold See {@link EmitterOptions.leakWarningThreshold}.
362+
* @param disposable A disposable store to register the throttle emitter to.
363+
*/
364+
export function throttle<T>(event: Event<T>, merge: (last: T | undefined, event: T) => T, delay?: number | typeof MicrotaskDelay, leading?: boolean, trailing?: boolean, leakWarningThreshold?: number, disposable?: DisposableStore): Event<T>;
365+
export function throttle<I, O>(event: Event<I>, merge: (last: O | undefined, event: I) => O, delay?: number | typeof MicrotaskDelay, leading?: boolean, trailing?: boolean, leakWarningThreshold?: number, disposable?: DisposableStore): Event<O>;
366+
export function throttle<I, O>(event: Event<I>, merge: (last: O | undefined, event: I) => O, delay: number | typeof MicrotaskDelay = 100, leading = true, trailing = true, leakWarningThreshold?: number, disposable?: DisposableStore): Event<O> {
367+
let subscription: IDisposable;
368+
let output: O | undefined = undefined;
369+
let handle: Timeout | undefined = undefined;
370+
let numThrottledCalls = 0;
371+
372+
const options: EmitterOptions | undefined = {
373+
leakWarningThreshold,
374+
onWillAddFirstListener() {
375+
subscription = event(cur => {
376+
numThrottledCalls++;
377+
output = merge(output, cur);
378+
379+
// If not currently throttling, fire immediately if leading is enabled
380+
if (handle === undefined) {
381+
if (leading) {
382+
emitter.fire(output);
383+
output = undefined;
384+
numThrottledCalls = 0;
385+
}
386+
387+
// Set up the throttle period
388+
if (typeof delay === 'number') {
389+
handle = setTimeout(() => {
390+
// Fire on trailing edge if there were calls during throttle period
391+
if (trailing && numThrottledCalls > 0) {
392+
emitter.fire(output!);
393+
}
394+
output = undefined;
395+
handle = undefined;
396+
numThrottledCalls = 0;
397+
}, delay);
398+
} else {
399+
// Use a special marker to indicate microtask is pending
400+
handle = 0 as unknown as Timeout;
401+
queueMicrotask(() => {
402+
// Fire on trailing edge if there were calls during throttle period
403+
if (trailing && numThrottledCalls > 0) {
404+
emitter.fire(output!);
405+
}
406+
output = undefined;
407+
handle = undefined;
408+
numThrottledCalls = 0;
409+
});
410+
}
411+
}
412+
// If already throttling, just accumulate the value for trailing edge
413+
});
414+
},
415+
onDidRemoveLastListener() {
416+
subscription.dispose();
417+
}
418+
};
419+
420+
if (!disposable) {
421+
_addLeakageTraceLogic(options);
422+
}
423+
424+
const emitter = new Emitter<O>(options);
425+
426+
disposable?.add(emitter);
427+
428+
return emitter.event;
429+
}
430+
348431
/**
349432
* Filters an event such that some condition is _not_ met more than once in a row, effectively ensuring duplicate
350433
* event objects from different sources do not fire the same event object.

src/vs/base/test/common/event.test.ts

Lines changed: 267 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1598,6 +1598,273 @@ suite('Event utils', () => {
15981598
});
15991599
});
16001600

1601+
suite('throttle', () => {
1602+
test('leading only', async function () {
1603+
return runWithFakedTimers({}, async function () {
1604+
const emitter = ds.add(new Emitter<number>());
1605+
const throttled = Event.throttle(emitter.event, (l, e) => l ? l + 1 : 1, 10, /*leading=*/true, /*trailing=*/false);
1606+
1607+
const calls: number[] = [];
1608+
ds.add(throttled((e) => calls.push(e)));
1609+
1610+
// First event fires immediately
1611+
emitter.fire(1);
1612+
assert.deepStrictEqual(calls, [1]);
1613+
1614+
// Subsequent events during throttle period are ignored
1615+
emitter.fire(2);
1616+
emitter.fire(3);
1617+
assert.deepStrictEqual(calls, [1]);
1618+
1619+
// Wait for throttle period to end
1620+
await timeout(15);
1621+
assert.deepStrictEqual(calls, [1], 'no trailing edge fire with trailing=false');
1622+
1623+
// After throttle period, next event fires immediately
1624+
emitter.fire(4);
1625+
assert.deepStrictEqual(calls, [1, 1]);
1626+
});
1627+
});
1628+
1629+
test('trailing only', async function () {
1630+
return runWithFakedTimers({}, async function () {
1631+
const emitter = ds.add(new Emitter<number>());
1632+
const throttled = Event.throttle(emitter.event, (l, e) => l ? l + 1 : 1, 10, /*leading=*/false, /*trailing=*/true);
1633+
1634+
const calls: number[] = [];
1635+
ds.add(throttled((e) => calls.push(e)));
1636+
1637+
// First event does not fire immediately
1638+
emitter.fire(1);
1639+
assert.deepStrictEqual(calls, []);
1640+
1641+
// Multiple events during throttle period
1642+
emitter.fire(2);
1643+
emitter.fire(3);
1644+
assert.deepStrictEqual(calls, []);
1645+
1646+
// Wait for throttle period - should fire with accumulated value
1647+
await timeout(15);
1648+
assert.deepStrictEqual(calls, [3]);
1649+
1650+
// New events start a new throttle period
1651+
emitter.fire(4);
1652+
emitter.fire(5);
1653+
assert.deepStrictEqual(calls, [3]);
1654+
1655+
await timeout(15);
1656+
assert.deepStrictEqual(calls, [3, 2]);
1657+
});
1658+
});
1659+
1660+
test('both leading and trailing', async function () {
1661+
return runWithFakedTimers({}, async function () {
1662+
const emitter = ds.add(new Emitter<number>());
1663+
const throttled = Event.throttle(emitter.event, (l, e) => l ? l + 1 : 1, 10, /*leading=*/true, /*trailing=*/true);
1664+
1665+
const calls: number[] = [];
1666+
ds.add(throttled((e) => calls.push(e)));
1667+
1668+
// First event fires immediately (leading)
1669+
emitter.fire(1);
1670+
assert.deepStrictEqual(calls, [1]);
1671+
1672+
// Events during throttle period are accumulated
1673+
emitter.fire(2);
1674+
emitter.fire(3);
1675+
assert.deepStrictEqual(calls, [1]);
1676+
1677+
// Wait for throttle period - should fire trailing edge with accumulated value
1678+
await timeout(15);
1679+
assert.deepStrictEqual(calls, [1, 2]);
1680+
});
1681+
});
1682+
1683+
test('only leading edge if no subsequent events', async function () {
1684+
return runWithFakedTimers({}, async function () {
1685+
const emitter = ds.add(new Emitter<number>());
1686+
const throttled = Event.throttle(emitter.event, (l, e) => l ? l + 1 : 1, 10, /*leading=*/true, /*trailing=*/true);
1687+
1688+
const calls: number[] = [];
1689+
ds.add(throttled((e) => calls.push(e)));
1690+
1691+
// Single event fires immediately (leading)
1692+
emitter.fire(1);
1693+
assert.deepStrictEqual(calls, [1]);
1694+
1695+
// No more events during throttle period
1696+
await timeout(15);
1697+
// Should not fire trailing edge since there were no more events
1698+
assert.deepStrictEqual(calls, [1]);
1699+
});
1700+
});
1701+
1702+
test('microtask delay', function (done: () => void) {
1703+
const emitter = ds.add(new Emitter<number>());
1704+
const throttled = Event.throttle(emitter.event, (l, e) => l ? l + 1 : 1, MicrotaskDelay);
1705+
1706+
const calls: number[] = [];
1707+
ds.add(throttled((e) => calls.push(e)));
1708+
1709+
// First event fires immediately (leading by default)
1710+
emitter.fire(1);
1711+
assert.deepStrictEqual(calls, [1]);
1712+
1713+
// Events during microtask
1714+
emitter.fire(2);
1715+
emitter.fire(3);
1716+
assert.deepStrictEqual(calls, [1]);
1717+
1718+
// Check after microtask
1719+
queueMicrotask(() => {
1720+
// Should have fired trailing edge
1721+
assert.deepStrictEqual(calls, [1, 2]);
1722+
done();
1723+
});
1724+
});
1725+
1726+
test('merge function accumulates values', async function () {
1727+
return runWithFakedTimers({}, async function () {
1728+
const emitter = ds.add(new Emitter<number>());
1729+
const throttled = Event.throttle(
1730+
emitter.event,
1731+
(last, cur) => (last || 0) + cur,
1732+
10,
1733+
/*leading=*/true,
1734+
/*trailing=*/true
1735+
);
1736+
1737+
const calls: number[] = [];
1738+
ds.add(throttled((e) => calls.push(e)));
1739+
1740+
// First event fires immediately with value 1
1741+
emitter.fire(1);
1742+
assert.deepStrictEqual(calls, [1]);
1743+
1744+
// Accumulate more values: 2 + 3 = 5
1745+
emitter.fire(2);
1746+
emitter.fire(3);
1747+
assert.deepStrictEqual(calls, [1]);
1748+
1749+
await timeout(15);
1750+
// Trailing edge fires with accumulated sum
1751+
assert.deepStrictEqual(calls, [1, 5]);
1752+
});
1753+
});
1754+
1755+
test('rapid consecutive throttle periods', async function () {
1756+
return runWithFakedTimers({}, async function () {
1757+
const emitter = ds.add(new Emitter<number>());
1758+
const throttled = Event.throttle(emitter.event, (l, e) => e, 10, /*leading=*/true, /*trailing=*/true);
1759+
1760+
const calls: number[] = [];
1761+
ds.add(throttled((e) => calls.push(e)));
1762+
1763+
// Period 1
1764+
emitter.fire(1);
1765+
emitter.fire(2);
1766+
assert.deepStrictEqual(calls, [1]);
1767+
1768+
await timeout(15);
1769+
assert.deepStrictEqual(calls, [1, 2]);
1770+
1771+
// Period 2
1772+
emitter.fire(3);
1773+
emitter.fire(4);
1774+
assert.deepStrictEqual(calls, [1, 2, 3]);
1775+
1776+
await timeout(15);
1777+
assert.deepStrictEqual(calls, [1, 2, 3, 4]);
1778+
1779+
// Period 3
1780+
emitter.fire(5);
1781+
assert.deepStrictEqual(calls, [1, 2, 3, 4, 5]);
1782+
1783+
await timeout(15);
1784+
// No trailing fire since only one event
1785+
assert.deepStrictEqual(calls, [1, 2, 3, 4, 5]);
1786+
});
1787+
});
1788+
1789+
test('default parameters', async function () {
1790+
return runWithFakedTimers({}, async function () {
1791+
const emitter = ds.add(new Emitter<number>());
1792+
// Default: delay=100, leading=true, trailing=true
1793+
const throttled = Event.throttle(emitter.event, (l, e) => e);
1794+
1795+
const calls: number[] = [];
1796+
ds.add(throttled((e) => calls.push(e)));
1797+
1798+
emitter.fire(1);
1799+
assert.deepStrictEqual(calls, [1], 'should fire leading edge by default');
1800+
1801+
emitter.fire(2);
1802+
await timeout(110);
1803+
assert.deepStrictEqual(calls, [1, 2], 'should fire trailing edge by default');
1804+
});
1805+
});
1806+
1807+
test('disposal cleans up', async function () {
1808+
return runWithFakedTimers({}, async function () {
1809+
const emitter = ds.add(new Emitter<number>());
1810+
const throttled = Event.throttle(emitter.event, (l, e) => e, 10);
1811+
1812+
const calls: number[] = [];
1813+
const listener = throttled((e) => calls.push(e));
1814+
1815+
emitter.fire(1);
1816+
emitter.fire(2);
1817+
assert.deepStrictEqual(calls, [1]);
1818+
1819+
listener.dispose();
1820+
1821+
// Events after disposal should not fire
1822+
await timeout(15);
1823+
emitter.fire(3);
1824+
assert.deepStrictEqual(calls, [1]);
1825+
});
1826+
});
1827+
1828+
test('no events during throttle with trailing=false', async function () {
1829+
return runWithFakedTimers({}, async function () {
1830+
const emitter = ds.add(new Emitter<number>());
1831+
const throttled = Event.throttle(emitter.event, (l, e) => l ? l + 1 : 1, 10, /*leading=*/true, /*trailing=*/false);
1832+
1833+
const calls: number[] = [];
1834+
ds.add(throttled((e) => calls.push(e)));
1835+
1836+
emitter.fire(1);
1837+
assert.deepStrictEqual(calls, [1]);
1838+
1839+
// No more events
1840+
await timeout(15);
1841+
assert.deepStrictEqual(calls, [1]);
1842+
1843+
// Next event after throttle period
1844+
emitter.fire(2);
1845+
assert.deepStrictEqual(calls, [1, 1]);
1846+
});
1847+
});
1848+
1849+
test('neither leading nor trailing', async function () {
1850+
return runWithFakedTimers({}, async function () {
1851+
const emitter = ds.add(new Emitter<number>());
1852+
const throttled = Event.throttle(emitter.event, (l, e) => e, 10, /*leading=*/false, /*trailing=*/false);
1853+
1854+
const calls: number[] = [];
1855+
ds.add(throttled((e) => calls.push(e)));
1856+
1857+
emitter.fire(1);
1858+
emitter.fire(2);
1859+
emitter.fire(3);
1860+
assert.deepStrictEqual(calls, []);
1861+
1862+
await timeout(15);
1863+
assert.deepStrictEqual(calls, [], 'no events should fire with both leading and trailing false');
1864+
});
1865+
});
1866+
});
1867+
16011868
test('issue #230401', () => {
16021869
let count = 0;
16031870
const emitter = ds.add(new Emitter<void>());

src/vs/editor/common/core/ranges/offsetRange.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,10 @@ export class OffsetRange implements IOffsetRange {
1818
return new OffsetRange(start, endExclusive);
1919
}
2020

21+
public static equals(r1: IOffsetRange, r2: IOffsetRange): boolean {
22+
return r1.start === r2.start && r1.endExclusive === r2.endExclusive;
23+
}
24+
2125
public static addRange(range: OffsetRange, sortedRanges: OffsetRange[]): void {
2226
let i = 0;
2327
while (i < sortedRanges.length && sortedRanges[i].endExclusive < range.start) {

src/vs/workbench/contrib/chat/browser/tools/languageModelToolsService.ts

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -206,7 +206,9 @@ export class LanguageModelToolsService extends Disposable implements ILanguageMo
206206

207207
this._tools.set(toolData.id, { data: toolData });
208208
this._ctxToolsCount.set(this._tools.size);
209-
this._onDidChangeToolsScheduler.schedule();
209+
if (!this._onDidChangeToolsScheduler.isScheduled()) {
210+
this._onDidChangeToolsScheduler.schedule();
211+
}
210212

211213
toolData.when?.keys().forEach(key => this._toolContextKeys.add(key));
212214

@@ -223,7 +225,9 @@ export class LanguageModelToolsService extends Disposable implements ILanguageMo
223225
this._tools.delete(toolData.id);
224226
this._ctxToolsCount.set(this._tools.size);
225227
this._refreshAllToolContextKeys();
226-
this._onDidChangeToolsScheduler.schedule();
228+
if (!this._onDidChangeToolsScheduler.isScheduled()) {
229+
this._onDidChangeToolsScheduler.schedule();
230+
}
227231
});
228232
}
229233

0 commit comments

Comments
 (0)