From 5ea99cac46e970a98a905e70807097651b889592 Mon Sep 17 00:00:00 2001 From: Seth Brenith Date: Thu, 16 Jan 2025 08:30:50 -0800 Subject: [PATCH] Fix several problems in heap snapshot Statistics perspective MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 1. The category named "Typed arrays" does include the backing storage of typed arrays, but also includes a bunch of other stuff. This change limits the "Typed arrays" category to just ArrayBuffer storage. 2. The category named "System" represents objects that are unreachable from user roots. However, the word "(system)" is used in the Summary perspective to represent objects of type 'hidden', which are V8 engine internals that haven't been categorized in a more meaningful way. It's confusing to use the same word for two different things, and I've heard from several developers that they don't trust the memory tools when they see values that don't match but seem like they should. This change updates the "System" category to mean the same as "(system)" in the Summary perspective. 3. The current categories in the Statistics perspective don't account for every byte of the snapshot, meaning part of the pie chart is left blank. Developers who see this often think that the tool is broken. This change introduces two new catch-all categories for the rest of the V8 heap and the rest of the Blink/Oilpan/embedder heap. Bug: 40659523, 388997090 Change-Id: Ib234ee1d6cbaae3ad37716823eefdc4a15550c3f Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/6172391 Reviewed-by: Simon Zünd Commit-Queue: Simon Zünd --- .../heap_snapshot_worker/HeapSnapshot.ts | 36 ++++++++++++------- .../heap_snapshot_model/HeapSnapshotModel.ts | 14 +++----- front_end/panels/profiler/HeapSnapshotView.ts | 30 +++++++++++++--- front_end/ui/legacy/themeColors.css | 20 +++++++++++ 4 files changed, 72 insertions(+), 28 deletions(-) diff --git a/front_end/entrypoints/heap_snapshot_worker/HeapSnapshot.ts b/front_end/entrypoints/heap_snapshot_worker/HeapSnapshot.ts index 97acba1915e..fc0bd17e5f6 100644 --- a/front_end/entrypoints/heap_snapshot_worker/HeapSnapshot.ts +++ b/front_end/entrypoints/heap_snapshot_worker/HeapSnapshot.ts @@ -3408,8 +3408,10 @@ export class JSHeapSnapshot extends HeapSnapshot { const nodeCodeType = this.nodeCodeType; const nodeConsStringType = this.nodeConsStringType; const nodeSlicedStringType = this.nodeSlicedStringType; - const distances = this.nodeDistances; + const nodeHiddenType = this.nodeHiddenType; + const nodeStringType = this.nodeStringType; let sizeNative = 0; + let sizeTypedArrays = 0; let sizeCode = 0; let sizeStrings = 0; let sizeJSArrays = 0; @@ -3417,31 +3419,39 @@ export class JSHeapSnapshot extends HeapSnapshot { const node = this.rootNode(); for (let nodeIndex = 0; nodeIndex < nodesLength; nodeIndex += nodeFieldCount) { const nodeSize = nodes.getValue(nodeIndex + nodeSizeOffset); - const ordinal = nodeIndex / nodeFieldCount; - if (distances[ordinal] >= HeapSnapshotModel.HeapSnapshotModel.baseSystemDistance) { + const nodeType = nodes.getValue(nodeIndex + nodeTypeOffset); + if (nodeType === nodeHiddenType) { sizeSystem += nodeSize; continue; } - const nodeType = nodes.getValue(nodeIndex + nodeTypeOffset); node.nodeIndex = nodeIndex; if (nodeType === nodeNativeType) { sizeNative += nodeSize; + if (node.rawName() === 'system / JSArrayBufferData') { + sizeTypedArrays += nodeSize; + } } else if (nodeType === nodeCodeType) { sizeCode += nodeSize; - } else if (nodeType === nodeConsStringType || nodeType === nodeSlicedStringType || node.type() === 'string') { + } else if (nodeType === nodeConsStringType || nodeType === nodeSlicedStringType || nodeType === nodeStringType) { sizeStrings += nodeSize; } else if (node.rawName() === 'Array') { sizeJSArrays += this.calculateArraySize(node); } } - this.#statistics = new HeapSnapshotModel.HeapSnapshotModel.Statistics(); - this.#statistics.total = this.totalSize; - this.#statistics.v8heap = this.totalSize - sizeNative; - this.#statistics.native = sizeNative; - this.#statistics.code = sizeCode; - this.#statistics.jsArrays = sizeJSArrays; - this.#statistics.strings = sizeStrings; - this.#statistics.system = sizeSystem; + this.#statistics = { + total: this.totalSize, + native: { + total: sizeNative, + typedArrays: sizeTypedArrays, + }, + v8heap: { + total: this.totalSize - sizeNative, + code: sizeCode, + jsArrays: sizeJSArrays, + strings: sizeStrings, + system: sizeSystem, + } + }; } private calculateArraySize(node: HeapSnapshotNode): number { diff --git a/front_end/models/heap_snapshot_model/HeapSnapshotModel.ts b/front_end/models/heap_snapshot_model/HeapSnapshotModel.ts index 6b5996d9c1d..ebc99b1127d 100644 --- a/front_end/models/heap_snapshot_model/HeapSnapshotModel.ts +++ b/front_end/models/heap_snapshot_model/HeapSnapshotModel.ts @@ -246,16 +246,10 @@ export class StaticData { } } -export class Statistics { - total!: number; - v8heap!: number; - native!: number; - code!: number; - jsArrays!: number; - strings!: number; - system!: number; - constructor() { - } +export interface Statistics { + total: number; + native: {total: number, typedArrays: number}; + v8heap: {total: number, code: number, jsArrays: number, strings: number, system: number}; } export class NodeFilter { diff --git a/front_end/panels/profiler/HeapSnapshotView.ts b/front_end/panels/profiler/HeapSnapshotView.ts index 5f4ed73fb39..fd8387f4440 100644 --- a/front_end/panels/profiler/HeapSnapshotView.ts +++ b/front_end/panels/profiler/HeapSnapshotView.ts @@ -124,6 +124,14 @@ const UIStrings = { *@description Label on a pie chart in the statistics view for the Heap Snapshot tool */ systemObjects: 'System objects', + /** + *@description Label on a pie chart in the statistics view for the Heap Snapshot tool + */ + otherJSObjects: 'Other JS objects', + /** + *@description Label on a pie chart in the statistics view for the Heap Snapshot tool + */ + otherNonJSObjects: 'Other non-JS objects (such as HTML and CSS)', /** *@description The reported total size used in the selected time frame of the allocation sampling profile *@example {3 MB} PH1 @@ -584,13 +592,25 @@ export class HeapSnapshotView extends UI.View.SimpleView implements DataDisplayD async retrieveStatistics(heapSnapshotProxy: HeapSnapshotProxy): Promise { const statistics = await heapSnapshotProxy.getStatistics(); + const {v8heap, native} = statistics; + const otherJSObjectsSize = v8heap.total - v8heap.code - v8heap.strings - v8heap.jsArrays - v8heap.system; const records = [ - {value: statistics.code, color: '#f77', title: i18nString(UIStrings.code)}, - {value: statistics.strings, color: '#5e5', title: i18nString(UIStrings.strings)}, - {value: statistics.jsArrays, color: '#7af', title: i18nString(UIStrings.jsArrays)}, - {value: statistics.native, color: '#fc5', title: i18nString(UIStrings.typedArrays)}, - {value: statistics.system, color: '#98f', title: i18nString(UIStrings.systemObjects)}, + {value: v8heap.code, color: 'var(--app-color-code)', title: i18nString(UIStrings.code)}, + {value: v8heap.strings, color: 'var(--app-color-strings)', title: i18nString(UIStrings.strings)}, + {value: v8heap.jsArrays, color: 'var(--app-color-js-arrays)', title: i18nString(UIStrings.jsArrays)}, + {value: native.typedArrays, color: 'var(--app-color-typed-arrays)', title: i18nString(UIStrings.typedArrays)}, + {value: v8heap.system, color: 'var(--app-color-system)', title: i18nString(UIStrings.systemObjects)}, + { + value: otherJSObjectsSize, + color: 'var(--app-color-other-js-objects)', + title: i18nString(UIStrings.otherJSObjects) + }, + { + value: native.total - native.typedArrays, + color: 'var(--app-color-other-non-js-objects)', + title: i18nString(UIStrings.otherNonJSObjects) + }, ]; this.statisticsView.setTotalAndRecords(statistics.total, records); return statistics; diff --git a/front_end/ui/legacy/themeColors.css b/front_end/ui/legacy/themeColors.css index 4f3ea9873fc..8fe0b0fc8ef 100644 --- a/front_end/ui/legacy/themeColors.css +++ b/front_end/ui/legacy/themeColors.css @@ -341,6 +341,16 @@ */ --app-color-active-breadcrumb: var(--ref-palette-blue40); + /** + * Colors for the pie chart in the Memory panel. + */ + --app-color-code: var(--ref-palette-cyan70); + --app-color-strings: var(--ref-palette-purple70); + --app-color-js-arrays: var(--ref-palette-green70); + --app-color-typed-arrays: var(--ref-palette-indigo60); + --app-color-other-js-objects: var(--ref-palette-blue70); + --app-color-other-non-js-objects: var(--ref-palette-yellow70); + /** * Gradients */ @@ -749,6 +759,16 @@ */ --app-color-active-breadcrumb: var(--ref-palette-blue40); + /** + * Colors for the pie chart in the Memory panel. + */ + --app-color-code: var(--ref-palette-cyan70); + --app-color-strings: var(--ref-palette-purple60); + --app-color-js-arrays: var(--ref-palette-green70); + --app-color-typed-arrays: var(--ref-palette-indigo60); + --app-color-other-js-objects: var(--ref-palette-blue60); + --app-color-other-non-js-objects: var(--ref-palette-yellow70); + /** * Gradients */