fix(dispatchers): separate gc bucket for main frame, increased limit (#28629)

- Keep main frames in a separate bucket, so that page methods that
redirect to the main frame continue to work.
- Increase default dispatchers limit to `10_000`.
- Increase dispatchers limit for `JSHandle`/`ElementHandle` to
`100_000`.

Fixes #28320, #28503.
This commit is contained in:
Dmitry Gozman 2023-12-14 10:42:08 -08:00 committed by GitHub
parent 3f3f332060
commit 35e8c440c2
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
3 changed files with 52 additions and 26 deletions

View file

@ -34,9 +34,15 @@ export function existingDispatcher<DispatcherType>(object: any): DispatcherType
return object[dispatcherSymbol]; return object[dispatcherSymbol];
} }
let maxDispatchers = 1000; let maxDispatchersOverride: number | undefined;
export function setMaxDispatchersForTest(value: number | undefined) { export function setMaxDispatchersForTest(value: number | undefined) {
maxDispatchers = value || 1000; maxDispatchersOverride = value;
}
function maxDispatchersForBucket(gcBucket: string) {
return maxDispatchersOverride ?? {
'JSHandle': 100000,
'ElementHandle': 100000,
}[gcBucket] ?? 10000;
} }
export class Dispatcher<Type extends { guid: string }, ChannelType, ParentScopeType extends DispatcherScope> extends EventEmitter implements channels.Channel { export class Dispatcher<Type extends { guid: string }, ChannelType, ParentScopeType extends DispatcherScope> extends EventEmitter implements channels.Channel {
@ -50,10 +56,11 @@ export class Dispatcher<Type extends { guid: string }, ChannelType, ParentScopeT
readonly _guid: string; readonly _guid: string;
readonly _type: string; readonly _type: string;
readonly _gcBucket: string;
_object: Type; _object: Type;
private _openScope = new LongStandingScope(); private _openScope = new LongStandingScope();
constructor(parent: ParentScopeType | DispatcherConnection, object: Type, type: string, initializer: channels.InitializerTraits<Type>) { constructor(parent: ParentScopeType | DispatcherConnection, object: Type, type: string, initializer: channels.InitializerTraits<Type>, gcBucket?: string) {
super(); super();
this._connection = parent instanceof DispatcherConnection ? parent : parent._connection; this._connection = parent instanceof DispatcherConnection ? parent : parent._connection;
@ -63,6 +70,7 @@ export class Dispatcher<Type extends { guid: string }, ChannelType, ParentScopeT
this._guid = guid; this._guid = guid;
this._type = type; this._type = type;
this._object = object; this._object = object;
this._gcBucket = gcBucket ?? type;
(object as any)[dispatcherSymbol] = this; (object as any)[dispatcherSymbol] = this;
@ -74,7 +82,7 @@ export class Dispatcher<Type extends { guid: string }, ChannelType, ParentScopeT
if (this._parent) if (this._parent)
this._connection.sendCreate(this._parent, type, guid, initializer, this._parent._object); this._connection.sendCreate(this._parent, type, guid, initializer, this._parent._object);
this._connection.maybeDisposeStaleDispatchers(type); this._connection.maybeDisposeStaleDispatchers(this._gcBucket);
} }
parentScope(): ParentScopeType { parentScope(): ParentScopeType {
@ -133,7 +141,7 @@ export class Dispatcher<Type extends { guid: string }, ChannelType, ParentScopeT
// Clean up from parent and connection. // Clean up from parent and connection.
this._parent?._dispatchers.delete(this._guid); this._parent?._dispatchers.delete(this._guid);
const list = this._connection._dispatchersByType.get(this._type); const list = this._connection._dispatchersByBucket.get(this._gcBucket);
list?.delete(this._guid); list?.delete(this._guid);
this._connection._dispatchers.delete(this._guid); this._connection._dispatchers.delete(this._guid);
@ -178,8 +186,7 @@ export class RootDispatcher extends Dispatcher<{ guid: '' }, any, any> {
export class DispatcherConnection { export class DispatcherConnection {
readonly _dispatchers = new Map<string, DispatcherScope>(); readonly _dispatchers = new Map<string, DispatcherScope>();
// Collect stale dispatchers by type. readonly _dispatchersByBucket = new Map<string, Set<string>>();
readonly _dispatchersByType = new Map<string, Set<string>>();
onmessage = (message: object) => {}; onmessage = (message: object) => {};
private _waitOperations = new Map<string, CallMetadata>(); private _waitOperations = new Map<string, CallMetadata>();
private _isLocal: boolean; private _isLocal: boolean;
@ -248,26 +255,23 @@ export class DispatcherConnection {
registerDispatcher(dispatcher: DispatcherScope) { registerDispatcher(dispatcher: DispatcherScope) {
assert(!this._dispatchers.has(dispatcher._guid)); assert(!this._dispatchers.has(dispatcher._guid));
this._dispatchers.set(dispatcher._guid, dispatcher); this._dispatchers.set(dispatcher._guid, dispatcher);
const type = dispatcher._type; let list = this._dispatchersByBucket.get(dispatcher._gcBucket);
let list = this._dispatchersByType.get(type);
if (!list) { if (!list) {
list = new Set(); list = new Set();
this._dispatchersByType.set(type, list); this._dispatchersByBucket.set(dispatcher._gcBucket, list);
} }
list.add(dispatcher._guid); list.add(dispatcher._guid);
} }
maybeDisposeStaleDispatchers(type: string) { maybeDisposeStaleDispatchers(gcBucket: string) {
const list = this._dispatchersByType.get(type); const maxDispatchers = maxDispatchersForBucket(gcBucket);
if (list && list.size > maxDispatchers) const list = this._dispatchersByBucket.get(gcBucket);
this._disposeStaleDispatchers(type, list); if (!list || list.size <= maxDispatchers)
} return;
const dispatchersArray = [...list];
private _disposeStaleDispatchers(type: string, dispatchers: Set<string>) { const disposeCount = (maxDispatchers / 10) | 0;
const dispatchersArray = [...dispatchers]; this._dispatchersByBucket.set(gcBucket, new Set(dispatchersArray.slice(disposeCount)));
this._dispatchersByType.set(type, new Set(dispatchersArray.slice(maxDispatchers / 10))); for (let i = 0; i < disposeCount; ++i) {
for (let i = 0; i < maxDispatchers / 10; ++i) {
const d = this._dispatchers.get(dispatchersArray[i]); const d = this._dispatchers.get(dispatchersArray[i]);
if (!d) if (!d)
continue; continue;

View file

@ -25,6 +25,7 @@ import { RequestDispatcher } from './networkDispatchers';
import type { CallMetadata } from '../instrumentation'; import type { CallMetadata } from '../instrumentation';
import type { BrowserContextDispatcher } from './browserContextDispatcher'; import type { BrowserContextDispatcher } from './browserContextDispatcher';
import type { PageDispatcher } from './pageDispatcher'; import type { PageDispatcher } from './pageDispatcher';
import { debugAssert } from '../../utils';
export class FrameDispatcher extends Dispatcher<Frame, channels.FrameChannel, BrowserContextDispatcher | PageDispatcher> implements channels.FrameChannel { export class FrameDispatcher extends Dispatcher<Frame, channels.FrameChannel, BrowserContextDispatcher | PageDispatcher> implements channels.FrameChannel {
_type_Frame = true; _type_Frame = true;
@ -43,13 +44,18 @@ export class FrameDispatcher extends Dispatcher<Frame, channels.FrameChannel, Br
} }
private constructor(scope: BrowserContextDispatcher, frame: Frame) { private constructor(scope: BrowserContextDispatcher, frame: Frame) {
// Main frames are gc'ed separately from any other frames, so that
// methods on Page that redirect to the main frame remain operational.
// Note: we cannot check parentFrame() here because it may be null after the frame has been detached.
debugAssert(frame._page.mainFrame(), 'Cannot determine whether the frame is a main frame');
const gcBucket = frame._page.mainFrame() === frame ? 'MainFrame' : 'Frame';
const pageDispatcher = existingDispatcher<PageDispatcher>(frame._page); const pageDispatcher = existingDispatcher<PageDispatcher>(frame._page);
super(pageDispatcher || scope, frame, 'Frame', { super(pageDispatcher || scope, frame, 'Frame', {
url: frame.url(), url: frame.url(),
name: frame.name(), name: frame.name(),
parentFrame: FrameDispatcher.fromNullable(scope, frame.parentFrame()), parentFrame: FrameDispatcher.fromNullable(scope, frame.parentFrame()),
loadStates: Array.from(frame._firedLifecycleEvents), loadStates: Array.from(frame._firedLifecycleEvents),
}); }, gcBucket);
this._browserContextDispatcher = scope; this._browserContextDispatcher = scope;
this._frame = frame; this._frame = frame;
this.addObjectListener(Frame.Events.AddLifecycle, lifecycleEvent => { this.addObjectListener(Frame.Events.AddLifecycle, lifecycleEvent => {

View file

@ -14,18 +14,22 @@
* limitations under the License. * limitations under the License.
*/ */
import { contextTest as test } from '../config/browserTest'; import { contextTest as test, expect } from '../config/browserTest';
test.slow(); test.slow();
test('cycle frames', async ({ page, server }) => { test('cycle frames', async ({ page, server }) => {
const kFrameCount = 1200; require('../../packages/playwright-core/lib/server/dispatchers/dispatcher').setMaxDispatchersForTest(100);
const kFrameCount = 310;
await page.goto(server.EMPTY_PAGE); await page.goto(server.EMPTY_PAGE);
let cb; let cb;
const promise = new Promise(f => cb = f); const promise = new Promise(f => cb = f);
let counter = 0; let counter = 0;
page.on('frameattached', () => { page.on('frameattached', async () => {
// Make sure we can access page.
await page.title();
if (++counter === kFrameCount) if (++counter === kFrameCount)
cb(); cb();
}); });
@ -40,5 +44,17 @@ test('cycle frames', async ({ page, server }) => {
} }
}, { url: server.PREFIX + '/one-style.html', count: kFrameCount }).catch(() => {}); }, { url: server.PREFIX + '/one-style.html', count: kFrameCount }).catch(() => {});
await promise; await promise;
await new Promise(f => setTimeout(f, 500)); await page.waitForTimeout(500);
require('../../packages/playwright-core/lib/server/dispatchers/dispatcher').setMaxDispatchersForTest(null);
});
test('cycle handles', async ({ page, server }) => {
await page.goto(server.EMPTY_PAGE);
await page.setContent(`<div><span>hi</span></div>`.repeat(2000));
const divs = await page.$$('div');
for (const div of divs) {
const span = await div.$('span');
expect(await span.textContent()).toBe('hi');
}
}); });