From 835579070912ff7189fb06b867b102f09ccc82a3 Mon Sep 17 00:00:00 2001 From: Peng Lyu Date: Thu, 1 Aug 2024 23:41:08 -0700 Subject: [PATCH 1/7] Enable notebook smoke test and capture memory leaks --- test/automation/src/application.ts | 6 + test/automation/src/notebook.ts | 1 - test/automation/src/playwrightDriver.ts | 43 ++++ test/automation/src/profiler.ts | 224 ++++++++++++++++++ .../smoke/src/areas/notebook/notebook.test.ts | 15 +- 5 files changed, 285 insertions(+), 4 deletions(-) create mode 100644 test/automation/src/profiler.ts diff --git a/test/automation/src/application.ts b/test/automation/src/application.ts index e06eaaf5be4..2a0084e1c1d 100644 --- a/test/automation/src/application.ts +++ b/test/automation/src/application.ts @@ -6,6 +6,7 @@ import { Workbench } from './workbench'; import { Code, launch, LaunchOptions } from './code'; import { Logger, measureAndLog } from './logger'; +import { Profiler } from './profiler'; export const enum Quality { Dev, @@ -63,6 +64,10 @@ export class Application { return this._userDataPath; } + private _profiler: Profiler | undefined; + + get profiler(): Profiler { return this._profiler!; } + async start(): Promise { await this._start(); await this.code.waitForElement('.explorer-folders-view'); @@ -110,6 +115,7 @@ export class Application { }); this._workbench = new Workbench(this._code); + this._profiler = new Profiler(this.code); return code; } diff --git a/test/automation/src/notebook.ts b/test/automation/src/notebook.ts index e2760109a54..dff250027db 100644 --- a/test/automation/src/notebook.ts +++ b/test/automation/src/notebook.ts @@ -23,7 +23,6 @@ export class Notebook { await this.code.waitForElement(activeRowSelector); await this.focusFirstCell(); - await this.waitForActiveCellEditorContents('code()'); } async focusNextCell() { diff --git a/test/automation/src/playwrightDriver.ts b/test/automation/src/playwrightDriver.ts index 0aeb50e2194..89742ef08ac 100644 --- a/test/automation/src/playwrightDriver.ts +++ b/test/automation/src/playwrightDriver.ts @@ -4,6 +4,7 @@ *--------------------------------------------------------------------------------------------*/ import * as playwright from '@playwright/test'; +import type { Protocol } from 'playwright-core/types/protocol'; import { dirname, join } from 'path'; import { promises } from 'fs'; import { IWindowDriver } from './driver'; @@ -83,6 +84,48 @@ export class PlaywrightDriver { await this.whenLoaded; } + private _cdpSession: playwright.CDPSession | undefined; + + async startCDP() { + if (this._cdpSession) { + return; + } + + this._cdpSession = await this.page.context().newCDPSession(this.page); + } + + async evaluate(options: Protocol.Runtime.evaluateParameters): Promise { + if (!this._cdpSession) { + throw new Error('CDP not started'); + } + + return await this._cdpSession.send('Runtime.evaluate', options); + } + + async queryObjects(parameters: Protocol.Runtime.queryObjectsParameters): Promise { + if (!this._cdpSession) { + throw new Error('CDP not started'); + } + + return await this._cdpSession.send('Runtime.queryObjects', parameters); + } + + async callFunctionOn(parameters: Protocol.Runtime.callFunctionOnParameters): Promise { + if (!this._cdpSession) { + throw new Error('CDP not started'); + } + + return await this._cdpSession.send('Runtime.callFunctionOn', parameters); + } + + async getProperties(parameters: Protocol.Runtime.getPropertiesParameters): Promise { + if (!this._cdpSession) { + throw new Error('CDP not started'); + } + + return await this._cdpSession.send('Runtime.getProperties', parameters); + } + private async takeScreenshot(name: string): Promise { try { const persistPath = join(this.options.logsPath, `playwright-screenshot-${PlaywrightDriver.screenShotCounter++}-${name.replace(/\s+/g, '-')}.png`); diff --git a/test/automation/src/profiler.ts b/test/automation/src/profiler.ts new file mode 100644 index 00000000000..dc23e41b310 --- /dev/null +++ b/test/automation/src/profiler.ts @@ -0,0 +1,224 @@ +/*--------------------------------------------------------------------------------------------- + * Copyright (c) Microsoft Corporation. All rights reserved. + * Licensed under the MIT License. See License.txt in the project root for license information. + *--------------------------------------------------------------------------------------------*/ + +import type { Protocol } from 'playwright-core/types/protocol'; +import { Code } from './code'; +import { PlaywrightDriver } from './playwrightDriver'; + +export class Profiler { + constructor(private readonly code: Code) { + } + + async checkLeaks(className: string, fn: () => Promise): Promise { + await this.code.driver.startCDP(); + const instancesBefore = await getInstances(this.code.driver); + const matchedInstances = instancesBefore.find(e => e.name !== undefined && e.name === className); + if (!matchedInstances) { + throw new Error(`${className} not found`); + } + + const countBefore = matchedInstances.count; + + await fn(); + + const instancesAfter = await getInstances(this.code.driver); + const matchedInstancesAfter = instancesAfter.find(e => e.name !== undefined && e.name === className); + if (!matchedInstancesAfter) { + throw new Error(`${className} not found`); + } + + const countAfter = matchedInstancesAfter.count; + if (countAfter !== countBefore) { + throw new Error(`Leaked ${countAfter - countBefore} ${className}`); + } + } +} + +function generateUuid() { + // use `randomValues` if possible + function getRandomValues(bucket: Uint8Array): Uint8Array { + for (let i = 0; i < bucket.length; i++) { + bucket[i] = Math.floor(Math.random() * 256); + } + return bucket; + } + + // prep-work + const _data = new Uint8Array(16); + const _hex: string[] = []; + for (let i = 0; i < 256; i++) { + _hex.push(i.toString(16).padStart(2, '0')); + } + + // get data + getRandomValues(_data); + + // set version bits + _data[6] = (_data[6] & 0x0f) | 0x40; + _data[8] = (_data[8] & 0x3f) | 0x80; + + // print as string + let i = 0; + let result = ''; + result += _hex[_data[i++]]; + result += _hex[_data[i++]]; + result += _hex[_data[i++]]; + result += _hex[_data[i++]]; + result += '-'; + result += _hex[_data[i++]]; + result += _hex[_data[i++]]; + result += '-'; + result += _hex[_data[i++]]; + result += _hex[_data[i++]]; + result += '-'; + result += _hex[_data[i++]]; + result += _hex[_data[i++]]; + result += '-'; + result += _hex[_data[i++]]; + result += _hex[_data[i++]]; + result += _hex[_data[i++]]; + result += _hex[_data[i++]]; + result += _hex[_data[i++]]; + result += _hex[_data[i++]]; + return result; +} + + + +/*--------------------------------------------------------------------------------------------- + * The MIT License (MIT) + * Copyright (c) 2023-present, Simon Siefke + * + * This code is derived from https://github.com/SimonSiefke/vscode-memory-leak-finder + *--------------------------------------------------------------------------------------------*/ + +const getInstances = async (driver: PlaywrightDriver): Promise> => { + const objectGroup = `og:${generateUuid()}`; + const prototypeDescriptor = await driver.evaluate({ + expression: 'Object.prototype', + returnByValue: false, + objectGroup, + }); + const objects = await driver.queryObjects({ + prototypeObjectId: prototypeDescriptor.result.objectId!, + objectGroup, + }); + const fnResult1 = await driver.callFunctionOn({ + functionDeclaration: `function(){ + const objects = this + + const nativeConstructors = [ + Object, + Array, + Function, + Set, + Map, + WeakMap, + WeakSet, + RegExp, + Node, + HTMLScriptElement, + DOMRectReadOnly, + DOMRect, + HTMLHtmlElement, + Node, + DOMTokenList, + HTMLUListElement, + HTMLStyleElement, + HTMLDivElement, + HTMLCollection, + FocusEvent, + Promise, + HTMLLinkElement, + HTMLLIElement, + HTMLAnchorElement, + HTMLSpanElement, + ArrayBuffer, + Uint16Array, + HTMLLabelElement, + TrustedTypePolicy, + Uint8Array, + Uint32Array, + HTMLHeadingElement, + MediaQueryList, + HTMLDocument, + TextDecoder, + TextEncoder, + HTMLInputElement, + HTMLCanvasElement, + HTMLIFrameElement, + Int32Array, + CSSStyleDeclaration + ] + + const isNativeConstructor = object => { + return nativeConstructors.includes(object.constructor) || + object.constructor.name === 'AsyncFunction' || + object.constructor.name === 'GeneratorFunction' || + object.constructor.name === 'AsyncGeneratorFunction' + } + + const isInstance = (object) => { + return object && !isNativeConstructor(object) + } + + const instances = objects.filter(isInstance) + return instances +}`, + objectId: objects.objects.objectId, + returnByValue: false, + objectGroup, + }); + + const fnResult2 = await getInstanceCountMap(driver, objectGroup, fnResult1.result); + const fnResult3 = await getInstanceCountArray(driver, objectGroup, fnResult2.result); + return fnResult3.result.value; +}; + +const getInstanceCountMap = async (driver: PlaywrightDriver, objectGroup: string, objects: Protocol.Runtime.RemoteObject) => { + const fnResult1 = await driver.callFunctionOn({ + functionDeclaration: `function(){ + const instances = this + + const map = new Map() + + for(const instance of instances){ + if(map.has(instance.constructor)){ + map.set(instance.constructor, map.get(instance.constructor) + 1) + } else { + map.set(instance.constructor, 1) + } + } + return map +}`, + objectId: objects.objectId, + returnByValue: false, + objectGroup, + }); + + return fnResult1; +}; + +const getInstanceCountArray = async (driver: PlaywrightDriver, objectGroup: string, map: Protocol.Runtime.RemoteObject) => { + const fnResult1 = await driver.callFunctionOn({ + functionDeclaration: `function(){ + const map = this + const array = [] + + for(const [instanceConstructor, count] of map.entries()){ + array.push({ + name: instanceConstructor.name, + count, + }) + } + + return array +}`, + objectId: map.objectId, + returnByValue: true, + objectGroup, + }); + return fnResult1; +}; diff --git a/test/smoke/src/areas/notebook/notebook.test.ts b/test/smoke/src/areas/notebook/notebook.test.ts index a5251a5f75e..26ed9b14d57 100644 --- a/test/smoke/src/areas/notebook/notebook.test.ts +++ b/test/smoke/src/areas/notebook/notebook.test.ts @@ -8,7 +8,7 @@ import { Application, Logger } from '../../../../automation'; import { installAllHandlers } from '../../utils'; export function setup(logger: Logger) { - describe.skip('Notebooks', () => { // https://github.com/microsoft/vscode/issues/140575 + describe('Notebooks', () => { // https://github.com/microsoft/vscode/issues/140575 // Shared before/after handling installAllHandlers(logger); @@ -25,7 +25,16 @@ export function setup(logger: Logger) { cp.execSync('git reset --hard HEAD --quiet', { cwd: app.workspacePathOrFolder }); }); - it('inserts/edits code cell', async function () { + it('leaks check', async function () { + const app = this.app as Application; + await app.profiler.checkLeaks('NotebookTextModel', async () => { + await app.workbench.notebook.openNotebook(); + await app.workbench.quickaccess.runCommand('workbench.action.files.save'); + await app.workbench.quickaccess.runCommand('workbench.action.closeActiveEditor'); + }); + }); + + it.skip('inserts/edits code cell', async function () { const app = this.app as Application; await app.workbench.notebook.openNotebook(); await app.workbench.notebook.focusNextCell(); @@ -34,7 +43,7 @@ export function setup(logger: Logger) { await app.workbench.notebook.stopEditingCell(); }); - it('inserts/edits markdown cell', async function () { + it.skip('inserts/edits markdown cell', async function () { const app = this.app as Application; await app.workbench.notebook.openNotebook(); await app.workbench.notebook.focusNextCell(); From 4a8f1db241acbf1675e34b2b6c7de05029dd1755 Mon Sep 17 00:00:00 2001 From: Peng Lyu Date: Fri, 2 Aug 2024 11:55:13 -0700 Subject: [PATCH 2/7] compile ipynb for smoke test --- build/azure-pipelines/linux/product-build-linux-test.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/build/azure-pipelines/linux/product-build-linux-test.yml b/build/azure-pipelines/linux/product-build-linux-test.yml index 43673619b16..91cc411dd44 100644 --- a/build/azure-pipelines/linux/product-build-linux-test.yml +++ b/build/azure-pipelines/linux/product-build-linux-test.yml @@ -150,7 +150,7 @@ steps: - script: yarn --cwd test/smoke compile displayName: Compile smoke tests - - script: yarn gulp compile-extension:markdown-language-features compile-extension-media compile-extension:vscode-test-resolver + - script: yarn gulp compile-extension:markdown-language-features compile-extension:ipynb compile-extension-media compile-extension:vscode-test-resolver displayName: Build extensions for smoke tests - script: yarn gulp node From 317a4589c22a8de26088d2badd2e860feaf9d6da Mon Sep 17 00:00:00 2001 From: Peng Lyu Date: Fri, 2 Aug 2024 12:28:02 -0700 Subject: [PATCH 3/7] Support tracking multiple leaks --- test/automation/src/profiler.ts | 37 ++++++++++++------- .../smoke/src/areas/notebook/notebook.test.ts | 2 +- 2 files changed, 25 insertions(+), 14 deletions(-) diff --git a/test/automation/src/profiler.ts b/test/automation/src/profiler.ts index dc23e41b310..a17007a327c 100644 --- a/test/automation/src/profiler.ts +++ b/test/automation/src/profiler.ts @@ -11,27 +11,38 @@ export class Profiler { constructor(private readonly code: Code) { } - async checkLeaks(className: string, fn: () => Promise): Promise { + async checkLeaks(classNames: string | string[], fn: () => Promise): Promise { await this.code.driver.startCDP(); - const instancesBefore = await getInstances(this.code.driver); - const matchedInstances = instancesBefore.find(e => e.name !== undefined && e.name === className); - if (!matchedInstances) { - throw new Error(`${className} not found`); - } - const countBefore = matchedInstances.count; + const countsBefore: { [key: string]: number } = {}; + const instancesBefore = await getInstances(this.code.driver); + const classNamesArray = Array.isArray(classNames) ? classNames : [classNames]; + for (const className of classNamesArray) { + const matchedInstances = instancesBefore.find(e => e.name !== undefined && e.name === className); + if (!matchedInstances) { + throw new Error(`${className} not found`); + } + countsBefore[className] = matchedInstances.count; + } await fn(); const instancesAfter = await getInstances(this.code.driver); - const matchedInstancesAfter = instancesAfter.find(e => e.name !== undefined && e.name === className); - if (!matchedInstancesAfter) { - throw new Error(`${className} not found`); + const leaks: string[] = []; + for (const className of classNamesArray) { + const matchedInstancesAfter = instancesAfter.find(e => e.name !== undefined && e.name === className); + if (!matchedInstancesAfter) { + throw new Error(`${className} not found`); + } + + const countAfter = matchedInstancesAfter.count; + if (countAfter !== countsBefore[className]) { + leaks.push(`Leaked ${countAfter - countsBefore[className]} ${className}`); + } } - const countAfter = matchedInstancesAfter.count; - if (countAfter !== countBefore) { - throw new Error(`Leaked ${countAfter - countBefore} ${className}`); + if (leaks.length > 0) { + throw new Error(leaks.join('\n')); } } } diff --git a/test/smoke/src/areas/notebook/notebook.test.ts b/test/smoke/src/areas/notebook/notebook.test.ts index 26ed9b14d57..da05a98209a 100644 --- a/test/smoke/src/areas/notebook/notebook.test.ts +++ b/test/smoke/src/areas/notebook/notebook.test.ts @@ -27,7 +27,7 @@ export function setup(logger: Logger) { it('leaks check', async function () { const app = this.app as Application; - await app.profiler.checkLeaks('NotebookTextModel', async () => { + await app.profiler.checkLeaks(['NotebookTextModel', 'NotebookCellTextModel'], async () => { await app.workbench.notebook.openNotebook(); await app.workbench.quickaccess.runCommand('workbench.action.files.save'); await app.workbench.quickaccess.runCommand('workbench.action.closeActiveEditor'); From d6349df9681c7849ed01ab96b21f53979a490ed4 Mon Sep 17 00:00:00 2001 From: Peng Lyu Date: Fri, 2 Aug 2024 13:43:33 -0700 Subject: [PATCH 4/7] fix false negative test --- test/smoke/src/areas/notebook/notebook.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/smoke/src/areas/notebook/notebook.test.ts b/test/smoke/src/areas/notebook/notebook.test.ts index da05a98209a..fd67c1174ae 100644 --- a/test/smoke/src/areas/notebook/notebook.test.ts +++ b/test/smoke/src/areas/notebook/notebook.test.ts @@ -27,7 +27,7 @@ export function setup(logger: Logger) { it('leaks check', async function () { const app = this.app as Application; - await app.profiler.checkLeaks(['NotebookTextModel', 'NotebookCellTextModel'], async () => { + await app.profiler.checkLeaks(['NotebookTextModel'], async () => { await app.workbench.notebook.openNotebook(); await app.workbench.quickaccess.runCommand('workbench.action.files.save'); await app.workbench.quickaccess.runCommand('workbench.action.closeActiveEditor'); From af98c564c2a078e9aad49a2df87ffc8cfa518521 Mon Sep 17 00:00:00 2001 From: Peng Lyu Date: Fri, 2 Aug 2024 14:32:46 -0700 Subject: [PATCH 5/7] accurate heap leak checks --- test/automation/src/playwrightDriver.ts | 22 +++++++++++++++++++ test/automation/src/profiler.ts | 22 +++++++++++++++++++ .../smoke/src/areas/notebook/notebook.test.ts | 11 +++++++++- 3 files changed, 54 insertions(+), 1 deletion(-) diff --git a/test/automation/src/playwrightDriver.ts b/test/automation/src/playwrightDriver.ts index 89742ef08ac..59d012879f5 100644 --- a/test/automation/src/playwrightDriver.ts +++ b/test/automation/src/playwrightDriver.ts @@ -94,6 +94,14 @@ export class PlaywrightDriver { this._cdpSession = await this.page.context().newCDPSession(this.page); } + async collectGarbage() { + if (!this._cdpSession) { + throw new Error('CDP not started'); + } + + await this._cdpSession.send('HeapProfiler.collectGarbage'); + } + async evaluate(options: Protocol.Runtime.evaluateParameters): Promise { if (!this._cdpSession) { throw new Error('CDP not started'); @@ -118,6 +126,20 @@ export class PlaywrightDriver { return await this._cdpSession.send('Runtime.callFunctionOn', parameters); } + async takeHeapSnapshot(): Promise { + if (!this._cdpSession) { + throw new Error('CDP not started'); + } + + let snapshot = ''; + this._cdpSession.addListener('HeapProfiler.addHeapSnapshotChunk', ({ chunk }) => { + snapshot += chunk; + }); + + await this._cdpSession.send('HeapProfiler.takeHeapSnapshot'); + return snapshot; + } + async getProperties(parameters: Protocol.Runtime.getPropertiesParameters): Promise { if (!this._cdpSession) { throw new Error('CDP not started'); diff --git a/test/automation/src/profiler.ts b/test/automation/src/profiler.ts index a17007a327c..d6f339c3030 100644 --- a/test/automation/src/profiler.ts +++ b/test/automation/src/profiler.ts @@ -4,6 +4,7 @@ *--------------------------------------------------------------------------------------------*/ import type { Protocol } from 'playwright-core/types/protocol'; +const { decode_bytes } = require('@vscode/v8-heap-parser'); import { Code } from './code'; import { PlaywrightDriver } from './playwrightDriver'; @@ -45,6 +46,26 @@ export class Profiler { throw new Error(leaks.join('\n')); } } + + async checkHeapLeaks(classNames: string | string[], fn: () => Promise): Promise { + await this.code.driver.startCDP(); + await fn(); + + const heapSnapshotAfter = await this.code.driver.takeHeapSnapshot(); + const buff = Buffer.from(heapSnapshotAfter); + const graph = await decode_bytes(buff); + const counts: number[] = Array.from(graph.get_class_counts(classNames)); + const leaks: string[] = []; + for (let i = 0; i < classNames.length; i++) { + if (counts[i] > 0) { + leaks.push(`Leaked ${counts[i]} ${classNames[i]}`); + } + } + + if (leaks.length > 0) { + throw new Error(leaks.join('\n')); + } + } } function generateUuid() { @@ -106,6 +127,7 @@ function generateUuid() { *--------------------------------------------------------------------------------------------*/ const getInstances = async (driver: PlaywrightDriver): Promise> => { + await driver.collectGarbage(); const objectGroup = `og:${generateUuid()}`; const prototypeDescriptor = await driver.evaluate({ expression: 'Object.prototype', diff --git a/test/smoke/src/areas/notebook/notebook.test.ts b/test/smoke/src/areas/notebook/notebook.test.ts index fd67c1174ae..2de6bd77cf6 100644 --- a/test/smoke/src/areas/notebook/notebook.test.ts +++ b/test/smoke/src/areas/notebook/notebook.test.ts @@ -25,7 +25,16 @@ export function setup(logger: Logger) { cp.execSync('git reset --hard HEAD --quiet', { cwd: app.workspacePathOrFolder }); }); - it('leaks check', async function () { + it('check heap leaks', async function () { + const app = this.app as Application; + await app.profiler.checkHeapLeaks(['NotebookTextModel', 'NotebookCellTextModel', 'NotebookEventDispatcher'], async () => { + await app.workbench.notebook.openNotebook(); + await app.workbench.quickaccess.runCommand('workbench.action.files.save'); + await app.workbench.quickaccess.runCommand('workbench.action.closeActiveEditor'); + }); + }); + + it('check object leaks', async function () { const app = this.app as Application; await app.profiler.checkLeaks(['NotebookTextModel'], async () => { await app.workbench.notebook.openNotebook(); From 30330d22a22e47c8aaa51eb208b9f151c5324427 Mon Sep 17 00:00:00 2001 From: Peng Lyu Date: Mon, 5 Aug 2024 13:24:53 -0700 Subject: [PATCH 6/7] release object group --- test/automation/src/playwrightDriver.ts | 18 ++++++++++++++--- test/automation/src/profiler.ts | 20 ++++++++++++++----- .../smoke/src/areas/notebook/notebook.test.ts | 4 ++-- 3 files changed, 32 insertions(+), 10 deletions(-) diff --git a/test/automation/src/playwrightDriver.ts b/test/automation/src/playwrightDriver.ts index 59d012879f5..681ca4ef540 100644 --- a/test/automation/src/playwrightDriver.ts +++ b/test/automation/src/playwrightDriver.ts @@ -110,6 +110,14 @@ export class PlaywrightDriver { return await this._cdpSession.send('Runtime.evaluate', options); } + async releaseObjectGroup(parameters: Protocol.Runtime.releaseObjectGroupParameters): Promise { + if (!this._cdpSession) { + throw new Error('CDP not started'); + } + + await this._cdpSession.send('Runtime.releaseObjectGroup', parameters); + } + async queryObjects(parameters: Protocol.Runtime.queryObjectsParameters): Promise { if (!this._cdpSession) { throw new Error('CDP not started'); @@ -132,11 +140,15 @@ export class PlaywrightDriver { } let snapshot = ''; - this._cdpSession.addListener('HeapProfiler.addHeapSnapshotChunk', ({ chunk }) => { - snapshot += chunk; - }); + const listener = (c: { chunk: string }) => { + snapshot += c.chunk; + }; + + this._cdpSession.addListener('HeapProfiler.addHeapSnapshotChunk', listener); await this._cdpSession.send('HeapProfiler.takeHeapSnapshot'); + + this._cdpSession.removeListener('HeapProfiler.addHeapSnapshotChunk', listener); return snapshot; } diff --git a/test/automation/src/profiler.ts b/test/automation/src/profiler.ts index d6f339c3030..1bf9c0c0bd0 100644 --- a/test/automation/src/profiler.ts +++ b/test/automation/src/profiler.ts @@ -12,26 +12,28 @@ export class Profiler { constructor(private readonly code: Code) { } - async checkLeaks(classNames: string | string[], fn: () => Promise): Promise { + async checkObjectLeaks(classNames: string | string[], fn: () => Promise): Promise { await this.code.driver.startCDP(); const countsBefore: { [key: string]: number } = {}; const instancesBefore = await getInstances(this.code.driver); const classNamesArray = Array.isArray(classNames) ? classNames : [classNames]; for (const className of classNamesArray) { - const matchedInstances = instancesBefore.find(e => e.name !== undefined && e.name === className); + const matchedInstances = instancesBefore.value.find(e => e.name !== undefined && e.name === className); if (!matchedInstances) { throw new Error(`${className} not found`); } countsBefore[className] = matchedInstances.count; } + await instancesBefore.dispose(); + await fn(); const instancesAfter = await getInstances(this.code.driver); const leaks: string[] = []; for (const className of classNamesArray) { - const matchedInstancesAfter = instancesAfter.find(e => e.name !== undefined && e.name === className); + const matchedInstancesAfter = instancesAfter.value.find(e => e.name !== undefined && e.name === className); if (!matchedInstancesAfter) { throw new Error(`${className} not found`); } @@ -42,6 +44,8 @@ export class Profiler { } } + await instancesAfter.dispose(); + if (leaks.length > 0) { throw new Error(leaks.join('\n')); } @@ -126,7 +130,7 @@ function generateUuid() { * This code is derived from https://github.com/SimonSiefke/vscode-memory-leak-finder *--------------------------------------------------------------------------------------------*/ -const getInstances = async (driver: PlaywrightDriver): Promise> => { +const getInstances = async (driver: PlaywrightDriver): Promise<{ value: Array<{ name: string; count: number }>; dispose: () => Promise }> => { await driver.collectGarbage(); const objectGroup = `og:${generateUuid()}`; const prototypeDescriptor = await driver.evaluate({ @@ -207,7 +211,13 @@ const getInstances = async (driver: PlaywrightDriver): Promise { + // release object group + await driver.releaseObjectGroup({ objectGroup: objectGroup }); + } + }; }; const getInstanceCountMap = async (driver: PlaywrightDriver, objectGroup: string, objects: Protocol.Runtime.RemoteObject) => { diff --git a/test/smoke/src/areas/notebook/notebook.test.ts b/test/smoke/src/areas/notebook/notebook.test.ts index 2de6bd77cf6..da4d527fe4f 100644 --- a/test/smoke/src/areas/notebook/notebook.test.ts +++ b/test/smoke/src/areas/notebook/notebook.test.ts @@ -25,7 +25,7 @@ export function setup(logger: Logger) { cp.execSync('git reset --hard HEAD --quiet', { cwd: app.workspacePathOrFolder }); }); - it('check heap leaks', async function () { + it.skip('check heap leaks', async function () { const app = this.app as Application; await app.profiler.checkHeapLeaks(['NotebookTextModel', 'NotebookCellTextModel', 'NotebookEventDispatcher'], async () => { await app.workbench.notebook.openNotebook(); @@ -36,7 +36,7 @@ export function setup(logger: Logger) { it('check object leaks', async function () { const app = this.app as Application; - await app.profiler.checkLeaks(['NotebookTextModel'], async () => { + await app.profiler.checkObjectLeaks(['NotebookTextModel', 'NotebookCellTextModel', 'NotebookEventDispatcher'], async () => { await app.workbench.notebook.openNotebook(); await app.workbench.quickaccess.runCommand('workbench.action.files.save'); await app.workbench.quickaccess.runCommand('workbench.action.closeActiveEditor'); From e4702fa7ced3149396b0953a8273f61a56fe2b74 Mon Sep 17 00:00:00 2001 From: Peng Lyu Date: Mon, 5 Aug 2024 14:14:23 -0700 Subject: [PATCH 7/7] Simplify getInstances. --- test/automation/src/profiler.ts | 103 +++++++------------------------- 1 file changed, 22 insertions(+), 81 deletions(-) diff --git a/test/automation/src/profiler.ts b/test/automation/src/profiler.ts index 1bf9c0c0bd0..9b3a0107d5e 100644 --- a/test/automation/src/profiler.ts +++ b/test/automation/src/profiler.ts @@ -3,7 +3,6 @@ * Licensed under the MIT License. See License.txt in the project root for license information. *--------------------------------------------------------------------------------------------*/ -import type { Protocol } from 'playwright-core/types/protocol'; const { decode_bytes } = require('@vscode/v8-heap-parser'); import { Code } from './code'; import { PlaywrightDriver } from './playwrightDriver'; @@ -15,37 +14,21 @@ export class Profiler { async checkObjectLeaks(classNames: string | string[], fn: () => Promise): Promise { await this.code.driver.startCDP(); - const countsBefore: { [key: string]: number } = {}; - const instancesBefore = await getInstances(this.code.driver); const classNamesArray = Array.isArray(classNames) ? classNames : [classNames]; - for (const className of classNamesArray) { - const matchedInstances = instancesBefore.value.find(e => e.name !== undefined && e.name === className); - if (!matchedInstances) { - throw new Error(`${className} not found`); - } - countsBefore[className] = matchedInstances.count; - } - - await instancesBefore.dispose(); + const countsBefore = await getInstances(this.code.driver, classNamesArray); await fn(); - const instancesAfter = await getInstances(this.code.driver); + const countAfter = await getInstances(this.code.driver, classNamesArray); const leaks: string[] = []; for (const className of classNamesArray) { - const matchedInstancesAfter = instancesAfter.value.find(e => e.name !== undefined && e.name === className); - if (!matchedInstancesAfter) { - throw new Error(`${className} not found`); - } - - const countAfter = matchedInstancesAfter.count; - if (countAfter !== countsBefore[className]) { - leaks.push(`Leaked ${countAfter - countsBefore[className]} ${className}`); + const count = countAfter[className] ?? 0; + const countBefore = countsBefore[className] ?? 0; + if (count !== countBefore) { + leaks.push(`Leaked ${count - countBefore} ${className}`); } } - await instancesAfter.dispose(); - if (leaks.length > 0) { throw new Error(leaks.join('\n')); } @@ -130,7 +113,7 @@ function generateUuid() { * This code is derived from https://github.com/SimonSiefke/vscode-memory-leak-finder *--------------------------------------------------------------------------------------------*/ -const getInstances = async (driver: PlaywrightDriver): Promise<{ value: Array<{ name: string; count: number }>; dispose: () => Promise }> => { +const getInstances = async (driver: PlaywrightDriver, classNames: string[]): Promise<{ [key: string]: number }> => { await driver.collectGarbage(); const objectGroup = `og:${generateUuid()}`; const prototypeDescriptor = await driver.evaluate({ @@ -145,6 +128,7 @@ const getInstances = async (driver: PlaywrightDriver): Promise<{ value: Array<{ const fnResult1 = await driver.callFunctionOn({ functionDeclaration: `function(){ const objects = this + const classNames = ${JSON.stringify(classNames)} const nativeConstructors = [ Object, @@ -202,66 +186,23 @@ const getInstances = async (driver: PlaywrightDriver): Promise<{ value: Array<{ } const instances = objects.filter(isInstance) - return instances + + const counts = Object.create(null) + for(const instance of instances){ + const name=instance.constructor.name + if(classNames.includes(name)){ + counts[name]||= 0 + counts[name]++ + } + } + return counts }`, objectId: objects.objects.objectId, - returnByValue: false, - objectGroup, - }); - - const fnResult2 = await getInstanceCountMap(driver, objectGroup, fnResult1.result); - const fnResult3 = await getInstanceCountArray(driver, objectGroup, fnResult2.result); - return { - value: fnResult3.result.value, - dispose: async () => { - // release object group - await driver.releaseObjectGroup({ objectGroup: objectGroup }); - } - }; -}; - -const getInstanceCountMap = async (driver: PlaywrightDriver, objectGroup: string, objects: Protocol.Runtime.RemoteObject) => { - const fnResult1 = await driver.callFunctionOn({ - functionDeclaration: `function(){ - const instances = this - - const map = new Map() - - for(const instance of instances){ - if(map.has(instance.constructor)){ - map.set(instance.constructor, map.get(instance.constructor) + 1) - } else { - map.set(instance.constructor, 1) - } - } - return map -}`, - objectId: objects.objectId, - returnByValue: false, - objectGroup, - }); - - return fnResult1; -}; - -const getInstanceCountArray = async (driver: PlaywrightDriver, objectGroup: string, map: Protocol.Runtime.RemoteObject) => { - const fnResult1 = await driver.callFunctionOn({ - functionDeclaration: `function(){ - const map = this - const array = [] - - for(const [instanceConstructor, count] of map.entries()){ - array.push({ - name: instanceConstructor.name, - count, - }) - } - - return array -}`, - objectId: map.objectId, returnByValue: true, objectGroup, }); - return fnResult1; + + const returnObject = fnResult1.result.value; + await driver.releaseObjectGroup({ objectGroup: objectGroup }); + return returnObject; };