From ffc8f243ffd25a0b079124e4ff3bba4da8ab1cf7 Mon Sep 17 00:00:00 2001 From: Seth Brenith Date: Wed, 28 Aug 2024 09:53:57 -0700 Subject: [PATCH] Categorize plain JS objects by interface in heap snapshots MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Heap snapshots often have a very large number of objects with the constructor "Object", since those represent plain JS objects created with object-literal notation. Due to the sheer size of that category, it can be difficult to find patterns of memory usage where large numbers of small matching objects cause substantial memory usage. With this CL, plain JS objects will be separated out by "interface", where an interface is a list of property names, and any object containing those property names matches the interface. Bug: 356183770 Change-Id: I519f2d6593f7aacee162712d8d5b4777734d8373 Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/5817911 Reviewed-by: Simon Zünd Commit-Queue: Seth Brenith --- .../heap_snapshot_worker/HeapSnapshot.ts | 274 +++++++++++++++++- .../panels/profiler/HeapSnapshotDataGrids.ts | 3 +- .../panels/profiler/HeapSnapshotProxy.ts | 8 +- test/e2e/helpers/memory-helpers.ts | 27 +- test/e2e/memory/memory_test.ts | 25 ++ test/e2e/resources/memory/BUILD.gn | 1 + test/e2e/resources/memory/diff.html | 15 + 7 files changed, 334 insertions(+), 19 deletions(-) create mode 100644 test/e2e/resources/memory/diff.html diff --git a/front_end/entrypoints/heap_snapshot_worker/HeapSnapshot.ts b/front_end/entrypoints/heap_snapshot_worker/HeapSnapshot.ts index 333d9f35f6..c22cce6a5e 100644 --- a/front_end/entrypoints/heap_snapshot_worker/HeapSnapshot.ts +++ b/front_end/entrypoints/heap_snapshot_worker/HeapSnapshot.ts @@ -393,7 +393,7 @@ export class HeapSnapshotNode implements HeapSnapshotItem { } rawName(): string { - return this.snapshot.strings[this.nameInternal()]; + return this.snapshot.strings[this.rawNameIndex()]; } isRoot(): boolean { @@ -461,7 +461,7 @@ export class HeapSnapshotNode implements HeapSnapshotItem { this.id(), this.name(), this.distance(), this.nodeIndex, this.retainedSize(), this.selfSize(), this.type()); } - private nameInternal(): number { + rawNameIndex(): number { const snapshot = this.snapshot; return snapshot.nodes.getValue(this.nodeIndex + snapshot.nodeNameOffset); } @@ -638,6 +638,14 @@ export class HeapSnapshotProgress { } } +// An "interface" to be used when classifying plain JS objects in the snapshot. +// An object matches the interface if it contains every listed property (even +// if it also contains extra properties). +interface InterfaceDefinition { + name: string; + properties: string[]; +} + export class HeapSnapshotProblemReport { readonly #errors: string[]; constructor(title: string) { @@ -686,6 +694,23 @@ const BITMASK_FOR_DOM_LINK_STATE = 3; // The class index is stored in the upper 30 bits of the detachedness field. const SHIFT_FOR_CLASS_INDEX = 2; +// The maximum number of results produced by inferInterfaceDefinitions. +const MAX_INTERFACE_COUNT = 1000; + +// After this many properties, inferInterfaceDefinitions can stop adding more +// properties to an interface definition if the name is getting too long. +const MIN_INTERFACE_PROPERTY_COUNT = 1; + +// The maximum length of an interface name produced by inferInterfaceDefinitions. +// This limit can be exceeded if the first MIN_INTERFACE_PROPERTY_COUNT property +// names are long. +const MAX_INTERFACE_NAME_LENGTH = 120; + +// Each interface definition produced by inferInterfaceDefinitions will match at +// least this many objects. There's no point in defining interfaces which match +// only a single object. +const MIN_OBJECT_COUNT_PER_INTERFACE = 2; + export abstract class HeapSnapshot { nodes: Platform.TypedArrayUtilities.BigUint32Array; containmentEdges: Platform.TypedArrayUtilities.BigUint32Array; @@ -702,8 +727,11 @@ export abstract class HeapSnapshot { [x: string]: HeapSnapshotModel.HeapSnapshotModel.Diff, }, }; - #aggregatesForDiffInternal!: { - [x: string]: HeapSnapshotModel.HeapSnapshotModel.AggregateForDiff, + #aggregatesForDiffInternal?: { + interfaceDefinitions: string, + aggregates: { + [x: string]: HeapSnapshotModel.HeapSnapshotModel.AggregateForDiff, + }, }; #aggregates: { [x: string]: { @@ -770,6 +798,8 @@ export abstract class HeapSnapshot { #edgeNamesThatAreNotWeakMaps: Platform.TypedArrayUtilities.BitVector; detachednessAndClassIndexArray?: Uint32Array; #essentialEdges?: Platform.TypedArrayUtilities.BitVector; + #interfaceNames: Map; + #interfaceDefinitions?: InterfaceDefinition[]; constructor(profile: Profile, progress: HeapSnapshotProgress) { this.nodes = profile.nodes; @@ -796,6 +826,7 @@ export abstract class HeapSnapshot { this.#ignoredNodesInRetainersView = new Set(); this.#ignoredEdgesInRetainersView = new Set(); this.#edgeNamesThatAreNotWeakMaps = Platform.TypedArrayUtilities.createBitVector(this.strings.length); + this.#interfaceNames = new Map(); } initialize(): void { @@ -881,6 +912,7 @@ export abstract class HeapSnapshot { this.buildDominatedNodes(); this.#progress.updateStatus('Calculating object names…'); this.calculateObjectNames(); + this.applyInterfaceDefinitions(this.inferInterfaceDefinitions()); this.#progress.updateStatus('Calculating statistics…'); this.calculateStatistics(); this.#progress.updateStatus('Calculating samples…'); @@ -1251,13 +1283,17 @@ export abstract class HeapSnapshot { return this.#allocationProfile.serializeAllocationStack(allocationNodeId); } - aggregatesForDiff(): {[x: string]: HeapSnapshotModel.HeapSnapshotModel.AggregateForDiff} { - if (this.#aggregatesForDiffInternal) { - return this.#aggregatesForDiffInternal; + aggregatesForDiff(interfaceDefinitions: string): {[x: string]: HeapSnapshotModel.HeapSnapshotModel.AggregateForDiff} { + if (this.#aggregatesForDiffInternal?.interfaceDefinitions === interfaceDefinitions) { + return this.#aggregatesForDiffInternal.aggregates; } + // Temporarily apply the interface definitions from the other snapshot. + const originalInterfaceDefinitions = this.#interfaceDefinitions; + this.applyInterfaceDefinitions(JSON.parse(interfaceDefinitions) as InterfaceDefinition[]); const aggregatesByClassName = this.getAggregatesByClassName(true, 'allObjects'); - this.#aggregatesForDiffInternal = {}; + this.applyInterfaceDefinitions(originalInterfaceDefinitions ?? []); + const result: {[x: string]: HeapSnapshotModel.HeapSnapshotModel.AggregateForDiff} = {}; const node = this.createNode(); for (const className in aggregatesByClassName) { @@ -1271,9 +1307,11 @@ export abstract class HeapSnapshot { selfSizes[i] = node.selfSize(); } - this.#aggregatesForDiffInternal[className] = {indexes: indexes, ids: ids, selfSizes: selfSizes}; + result[className] = {indexes: indexes, ids: ids, selfSizes: selfSizes}; } - return this.#aggregatesForDiffInternal; + + this.#aggregatesForDiffInternal = {interfaceDefinitions, aggregates: result}; + return result; } isUserRoot(_node: HeapSnapshotNode): boolean { @@ -2003,6 +2041,222 @@ export abstract class HeapSnapshot { } } + interfaceDefinitions(): string { + return JSON.stringify(this.#interfaceDefinitions ?? []); + } + + private isPlainJSObject(node: HeapSnapshotNode): boolean { + return node.rawType() === this.nodeObjectType && node.rawName() === 'Object'; + } + + private inferInterfaceDefinitions(): InterfaceDefinition[] { + const {edgePropertyType} = this; + + // First, produce a set of candidate definitions by iterating the properties + // on every plain JS Object in the snapshot. + interface InterfaceDefinitionCandidate extends InterfaceDefinition { + // How many objects start with these properties in this order. + count: number; + } + // A map from interface names to their definitions. + const candidates = new Map(); + for (let it = this.allNodes(); it.hasNext(); it.next()) { + const node = it.item(); + if (!this.isPlainJSObject(node)) { + continue; + } + let interfaceName = '{'; + const properties: string[] = []; + for (let edgeIt = node.edges(); edgeIt.hasNext(); edgeIt.next()) { + const edge = edgeIt.item(); + const edgeName = edge.name(); + if (edge.rawType() !== edgePropertyType || edgeName === '__proto__') { + continue; + } + const formattedEdgeName = JSHeapSnapshotNode.formatPropertyName(edgeName); + if (interfaceName.length > MIN_INTERFACE_PROPERTY_COUNT && + interfaceName.length + formattedEdgeName.length > MAX_INTERFACE_NAME_LENGTH) { + break; // The interface name is getting too long. + } + if (interfaceName.length !== 1) { + interfaceName += ', '; + } + interfaceName += formattedEdgeName; + properties.push(edgeName); + } + // The empty interface is not a very meaningful, and can be sort of misleading + // since someone might incorrectly interpret it as objects with no properties. + if (properties.length === 0) { + continue; + } + interfaceName += '}'; + const candidate = candidates.get(interfaceName); + if (candidate) { + ++candidate.count; + } else { + candidates.set(interfaceName, {name: interfaceName, properties, count: 1}); + } + } + + // Next, sort the candidates and select the most popular ones. It's possible that + // some candidates represent the same properties in different orders, but that's + // okay: by sorting here, we ensure that the most popular ordering appears first + // in the result list, and the rules for applying interface definitions will prefer + // the first matching definition if multiple matches contain the same properties. + const sortedCandidates = Array.from(candidates.values()); + sortedCandidates.sort((a, b) => b.count - a.count); + const result: InterfaceDefinition[] = []; + const maxResultSize = Math.min(sortedCandidates.length, MAX_INTERFACE_COUNT); + for (let i = 0; i < maxResultSize; ++i) { + const candidate = sortedCandidates[i]; + if (candidate.count < MIN_OBJECT_COUNT_PER_INTERFACE) { + break; + } + result.push(candidate); + } + + return result; + } + + private applyInterfaceDefinitions(definitions: InterfaceDefinition[]): void { + const {edgePropertyType} = this; + this.#interfaceDefinitions = definitions; + + // Any computed aggregate data will be wrong after recategorization, so clear it. + this.#aggregates = {}; + this.#aggregatesSortedFlags = {}; + + // Information about a named interface. + interface MatchInfo { + name: string; + // The number of properties listed in the interface definition. + propertyCount: number; + // The position of the interface definition in the list of definitions. + index: number; + } + + function selectBetterMatch(a: MatchInfo, b: MatchInfo|null): MatchInfo { + if (!b || a.propertyCount > b.propertyCount) { + return a; + } + if (b.propertyCount > a.propertyCount) { + return b; + } + return a.index <= b.index ? a : b; + } + + // A node in the tree which allows us to search for interfaces matching an object. + // Each edge in this tree represents adding a property, starting from an empty + // object. Properties must be iterated in sorted order. + interface PropertyTreeNode { + // All possible successors from this node. Keys are property names. + next: Map; + // If this node corresponds to a named interface, then matchInfo contains that name. + matchInfo: MatchInfo|null; + // The maximum of all keys in `next`. This helps determine when no further transitions + // are possible from this node. + greatestNext: string|null; + } + + // The root node of the tree. + const propertyTree: PropertyTreeNode = { + next: new Map(), + matchInfo: null, + greatestNext: null, + }; + + // Build up the property tree. + for (let interfaceIndex = 0; interfaceIndex < definitions.length; ++interfaceIndex) { + const definition = definitions[interfaceIndex]; + const properties = definition.properties.toSorted(); + let currentNode = propertyTree; + for (const property of properties) { + const nextMap = currentNode.next; + let nextNode = nextMap.get(property); + if (!nextNode) { + nextNode = { + next: new Map(), + matchInfo: null, + greatestNext: null, + }; + nextMap.set(property, nextNode); + if (currentNode.greatestNext === null || currentNode.greatestNext < property) { + currentNode.greatestNext = property; + } + } + currentNode = nextNode; + } + // Only set matchInfo on this node if it wasn't already set, to ensure that + // interfaces defined earlier in the list have priority. + if (!currentNode.matchInfo) { + currentNode.matchInfo = { + name: definition.name, + propertyCount: properties.length, + index: interfaceIndex, + }; + } + } + + // The fallback match for objects which don't match any defined interface. + const initialMatch: MatchInfo = { + name: 'Object', + propertyCount: 0, + index: Infinity, + }; + + // Iterate all nodes and check whether they match a named interface, using + // the tree constructed above. Then update the class name for each node. + for (let it = this.allNodes(); it.hasNext(); it.next()) { + const node = it.item(); + if (!this.isPlainJSObject(node)) { + continue; + } + + // Collect and sort the properties of this object. + const properties: string[] = []; + for (let edgeIt = node.edges(); edgeIt.hasNext(); edgeIt.next()) { + const edge = edgeIt.item(); + if (edge.rawType() === edgePropertyType) { + properties.push(edge.name()); + } + } + properties.sort(); + + // We may explore multiple possible paths through the tree, so this set tracks + // all states that match with the properties iterated thus far. + const states = new Set(); + states.add(propertyTree); + + // This variable represents the best match found thus far. We start by checking + // whether there is an interface definition for the empty object. + let match = selectBetterMatch(initialMatch, propertyTree.matchInfo); + + // Traverse the tree to find any matches. + for (const property of properties) { + // Iterate only the states that already exist, not the ones added during the loop below. + for (const currentState of Array.from(states.keys())) { + if (currentState.greatestNext === null || property >= currentState.greatestNext) { + // No further transitions are possible from this state. + states.delete(currentState); + } + const nextState = currentState.next.get(property); + if (nextState) { + states.add(nextState); + match = selectBetterMatch(match, nextState.matchInfo); + } + } + } + + // Update the node's class name accordingly. + let classIndex = match === initialMatch ? node.rawNameIndex() : this.#interfaceNames.get(match.name); + if (classIndex === undefined) { + classIndex = this.addString(match.name); + this.#interfaceNames.set(match.name, classIndex); + } + node.setClassIndex(classIndex); + } + } + /** * Iterates children of a node. */ diff --git a/front_end/panels/profiler/HeapSnapshotDataGrids.ts b/front_end/panels/profiler/HeapSnapshotDataGrids.ts index dbd8f3ac7a..3ff2e07fd1 100644 --- a/front_end/panels/profiler/HeapSnapshotDataGrids.ts +++ b/front_end/panels/profiler/HeapSnapshotDataGrids.ts @@ -999,7 +999,8 @@ export class HeapSnapshotDiffDataGrid extends HeapSnapshotViewportDataGrid { // Two snapshots live in different workers isolated from each other. That is why // we first need to collect information about the nodes in the first snapshot and // then pass it to the second snapshot to calclulate the diff. - const aggregatesForDiff = await this.baseSnapshot.aggregatesForDiff(); + const interfaceDefinitions = await this.snapshot.interfaceDefinitions(); + const aggregatesForDiff = await this.baseSnapshot.aggregatesForDiff(interfaceDefinitions); const diffByClassName = await this.snapshot.calculateSnapshotDiff(this.baseSnapshot.uid, aggregatesForDiff); for (const className in diffByClassName) { diff --git a/front_end/panels/profiler/HeapSnapshotProxy.ts b/front_end/panels/profiler/HeapSnapshotProxy.ts index d05a3f47df..403803f9c7 100644 --- a/front_end/panels/profiler/HeapSnapshotProxy.ts +++ b/front_end/panels/profiler/HeapSnapshotProxy.ts @@ -291,16 +291,20 @@ export class HeapSnapshotProxy extends HeapSnapshotProxyObject { return this.callMethodPromise('search', searchConfig, filter); } + interfaceDefinitions(): Promise { + return this.callMethodPromise('interfaceDefinitions'); + } + aggregatesWithFilter(filter: HeapSnapshotModel.HeapSnapshotModel.NodeFilter): Promise<{ [x: string]: HeapSnapshotModel.HeapSnapshotModel.Aggregate, }> { return this.callMethodPromise('aggregatesWithFilter', filter); } - aggregatesForDiff(): Promise<{ + aggregatesForDiff(interfaceDefinitions: string): Promise<{ [x: string]: HeapSnapshotModel.HeapSnapshotModel.AggregateForDiff, }> { - return this.callMethodPromise('aggregatesForDiff'); + return this.callMethodPromise('aggregatesForDiff', interfaceDefinitions); } calculateSnapshotDiff(baseSnapshotId: string, baseSnapshotAggregates: { diff --git a/test/e2e/helpers/memory-helpers.ts b/test/e2e/helpers/memory-helpers.ts index 287c5ef384..9d83232264 100644 --- a/test/e2e/helpers/memory-helpers.ts +++ b/test/e2e/helpers/memory-helpers.ts @@ -66,10 +66,14 @@ export async function takeAllocationTimelineProfile({recordStacks}: {recordStack await waitFor('.heap-snapshot-sidebar-tree-item.selected'); } -export async function takeHeapSnapshot() { +export async function takeHeapSnapshot(name: string = 'Snapshot 1') { await click(NEW_HEAP_SNAPSHOT_BUTTON); await waitForNone('.heap-snapshot-sidebar-tree-item.wait'); - await waitFor('.heap-snapshot-sidebar-tree-item.selected'); + await waitForFunction(async () => { + const selected = await waitFor('.heap-snapshot-sidebar-tree-item.selected'); + const title = await waitFor('span.title', selected); + return (await title.evaluate(e => e.textContent)) === name ? title : undefined; + }); } export async function waitForHeapSnapshotData() { @@ -305,14 +309,18 @@ export async function expandFocusedRow() { await waitFor('.selected.data-grid-data-grid-node.expanded'); } +function parseNumberWithSpaces(number: string): number { + return parseInt(number.replaceAll('\xa0', ''), 10); +} + async function getSizesFromRow(row: puppeteer.ElementHandle) { const numericData = await $$('.numeric-column>.profile-multiple-values>span', row); assert.strictEqual(numericData.length, 4); - function readNumber(e: Element) { - return parseInt((e.textContent as string).replaceAll('\xa0', ''), 10); + function readNumber(e: Element): string { + return e.textContent as string; } - const shallowSize = await numericData[0].evaluate(readNumber); - const retainedSize = await numericData[2].evaluate(readNumber); + const shallowSize = parseNumberWithSpaces(await numericData[0].evaluate(readNumber)); + const retainedSize = parseNumberWithSpaces(await numericData[2].evaluate(readNumber)); assert.isTrue(retainedSize >= shallowSize); return {shallowSize, retainedSize}; } @@ -344,6 +352,13 @@ export async function getCountFromCategoryRow(text: string) { return await countSpan.evaluate(e => parseInt((e.textContent ?? '').substring(1), 10)); } +export async function getAddedCountFromComparisonRow(text: string) { + const row = await getCategoryRow(text); + const addedCountCell = await waitFor('.addedCount-column', row); + const countText = await addedCountCell.evaluate(e => e.textContent ?? ''); + return parseNumberWithSpaces(countText); +} + export async function clickOnContextMenuForRetainer(retainerName: string, menuItem: string) { const retainersPane = await waitFor('.retaining-paths-view'); const element = await waitFor(`//span[text()="${retainerName}"]`, retainersPane, undefined, 'xpath'); diff --git a/test/e2e/memory/memory_test.ts b/test/e2e/memory/memory_test.ts index dd369b19cb..dfd7c975f0 100644 --- a/test/e2e/memory/memory_test.ts +++ b/test/e2e/memory/memory_test.ts @@ -29,6 +29,7 @@ import { expandFocusedRow, findSearchResult, focusTableRow, + getAddedCountFromComparisonRow, getCategoryRow, getCountFromCategoryRow, getDataGridRows, @@ -544,4 +545,28 @@ describe('The Memory Panel', function() { await setSearchFilter('Detached
'); await waitForSearchResultNumber(1); }); + + it('Groups plain JS objects by interface', async () => { + await goToResource('memory/diff.html'); + await navigateToMemoryTab(); + await takeHeapSnapshot(); + await waitForNonEmptyHeapSnapshotData(); + await setClassFilter('{a, b, c, d, '); + // Objects should be grouped by interface if there are at least two matching instances. + assert.strictEqual(2, await getCountFromCategoryRow('{a, b, c, d, p, q, r}')); + assert.isTrue(!(await getCategoryRow('{a, b, c, d, e}', /* wait:*/ false))); + const {frontend, target} = await getBrowserAndPages(); + await target.bringToFront(); + await target.click('button#update'); + await frontend.bringToFront(); + await takeHeapSnapshot('Snapshot 2'); + await waitForNonEmptyHeapSnapshotData(); + await changeViewViaDropdown('Comparison'); + await setClassFilter('{a, b, c, d, '); + // When comparing, the old snapshot is categorized according to the new one's interfaces, + // so the comparison should report only one new object of the following type, not two. + assert.strictEqual(1, await getAddedCountFromComparisonRow('{a, b, c, d, e}')); + // Only one of these objects remains, so it's no longer a category. + assert.isTrue(!(await getCategoryRow('{a, b, c, d, p, q, r}', /* wait:*/ false))); + }); }); diff --git a/test/e2e/resources/memory/BUILD.gn b/test/e2e/resources/memory/BUILD.gn index 2d54113678..62f132d4ca 100644 --- a/test/e2e/resources/memory/BUILD.gn +++ b/test/e2e/resources/memory/BUILD.gn @@ -12,6 +12,7 @@ copy_to_gen("memory") { "detached-dom-tree.html", "detached-iframe.html", "detached-node.html", + "diff.html", "dom-details.html", "dom-objects.html", "event-listeners.html", diff --git a/test/e2e/resources/memory/diff.html b/test/e2e/resources/memory/diff.html new file mode 100644 index 0000000000..6f0bbd0e1e --- /dev/null +++ b/test/e2e/resources/memory/diff.html @@ -0,0 +1,15 @@ + +

Memory Panel (Heap profiler) Test

+ +