Skip to content

Commit

Permalink
Fix several problems in heap snapshot Statistics perspective
Browse files Browse the repository at this point in the history
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 <[email protected]>
Commit-Queue: Simon Zünd <[email protected]>
  • Loading branch information
sethbrenith authored and Devtools-frontend LUCI CQ committed Jan 17, 2025
1 parent 5627f32 commit 5ea99ca
Show file tree
Hide file tree
Showing 4 changed files with 72 additions and 28 deletions.
36 changes: 23 additions & 13 deletions front_end/entrypoints/heap_snapshot_worker/HeapSnapshot.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3408,40 +3408,50 @@ 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;
let sizeSystem = 0;
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 {
Expand Down
14 changes: 4 additions & 10 deletions front_end/models/heap_snapshot_model/HeapSnapshotModel.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
30 changes: 25 additions & 5 deletions front_end/panels/profiler/HeapSnapshotView.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -584,13 +592,25 @@ export class HeapSnapshotView extends UI.View.SimpleView implements DataDisplayD
async retrieveStatistics(heapSnapshotProxy: HeapSnapshotProxy):
Promise<HeapSnapshotModel.HeapSnapshotModel.Statistics> {
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;
Expand Down
20 changes: 20 additions & 0 deletions front_end/ui/legacy/themeColors.css
Original file line number Diff line number Diff line change
Expand Up @@ -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
*/
Expand Down Expand Up @@ -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
*/
Expand Down

0 comments on commit 5ea99ca

Please sign in to comment.