From 48ef450057c4f5406aef3bdd9b3f07f903d8ab2f Mon Sep 17 00:00:00 2001 From: Markus Stange Date: Fri, 19 Jan 2024 15:04:05 -0500 Subject: [PATCH 1/8] Remove unused rootCount field. This became unused in https://github.com/firefox-devtools/profiler/commit/e0edc96eb5592805810132ba71234bcde2ec8831 , part of #4807. --- src/profile-logic/call-tree.js | 19 +++---------------- .../__snapshots__/profile-view.test.js.snap | 1 - src/test/unit/profile-tree.test.js | 1 - 3 files changed, 3 insertions(+), 18 deletions(-) diff --git a/src/profile-logic/call-tree.js b/src/profile-logic/call-tree.js index 6ac2174eac..8d04a4c037 100644 --- a/src/profile-logic/call-tree.js +++ b/src/profile-logic/call-tree.js @@ -47,7 +47,6 @@ type CallNodeSummary = { export type CallTreeTimings = { callNodeHasChildren: Uint8Array, callNodeSummary: CallNodeSummary, - rootCount: number, rootTotalSummary: number, }; @@ -76,7 +75,6 @@ export class CallTree { _callNodeHasChildren: Uint8Array; // A table column matching the callNodeTable _thread: Thread; _rootTotalSummary: number; - _rootCount: number; _displayDataByIndex: Map; // _children is indexed by IndexIntoCallNodeTable. Since they are // integers, using an array directly is faster than going through a Map. @@ -91,7 +89,6 @@ export class CallTree { callNodeSummary: CallNodeSummary, callNodeHasChildren: Uint8Array, rootTotalSummary: number, - rootCount: number, isHighPrecision: boolean, weightType: WeightType ) { @@ -102,7 +99,6 @@ export class CallTree { this._callNodeHasChildren = callNodeHasChildren; this._thread = thread; this._rootTotalSummary = rootTotalSummary; - this._rootCount = rootCount; this._displayDataByIndex = new Map(); this._children = []; this._isHighPrecision = isHighPrecision; @@ -528,7 +524,6 @@ export function computeCallTreeTimings( const callNodeTotalSummary = new Float32Array(callNodeTable.length); const callNodeHasChildren = new Uint8Array(callNodeTable.length); let rootTotalSummary = 0; - let rootCount = 0; // Workaround for https://bugzilla.mozilla.org/show_bug.cgi?id=1858310 const abs = Math.abs; @@ -551,9 +546,7 @@ export function computeCallTreeTimings( } const prefixCallNode = callNodeTable.prefix[callNodeIndex]; - if (prefixCallNode === -1) { - rootCount++; - } else { + if (prefixCallNode !== -1) { callNodeTotalSummary[prefixCallNode] += callNodeTotalSummary[callNodeIndex]; callNodeHasChildren[prefixCallNode] = 1; @@ -568,7 +561,6 @@ export function computeCallTreeTimings( }, callNodeHasChildren, rootTotalSummary, - rootCount, }; } @@ -583,12 +575,8 @@ export function getCallTree( weightType: WeightType ): CallTree { return timeCode('getCallTree', () => { - const { - callNodeSummary, - callNodeHasChildren, - rootTotalSummary, - rootCount, - } = callTreeTimings; + const { callNodeSummary, callNodeHasChildren, rootTotalSummary } = + callTreeTimings; return new CallTree( thread, @@ -597,7 +585,6 @@ export function getCallTree( callNodeSummary, callNodeHasChildren, rootTotalSummary, - rootCount, Boolean(thread.isJsTracer), weightType ); diff --git a/src/test/store/__snapshots__/profile-view.test.js.snap b/src/test/store/__snapshots__/profile-view.test.js.snap index cfd72bf73c..b112486454 100644 --- a/src/test/store/__snapshots__/profile-view.test.js.snap +++ b/src/test/store/__snapshots__/profile-view.test.js.snap @@ -2632,7 +2632,6 @@ CallTree { "_children": Array [], "_displayDataByIndex": Map {}, "_isHighPrecision": false, - "_rootCount": 1, "_rootTotalSummary": 2, "_thread": Object { "frameTable": Object { diff --git a/src/test/unit/profile-tree.test.js b/src/test/unit/profile-tree.test.js index 2c14121fa1..d0d6770716 100644 --- a/src/test/unit/profile-tree.test.js +++ b/src/test/unit/profile-tree.test.js @@ -80,7 +80,6 @@ describe('unfiltered call tree', function () { false ) ).toEqual({ - rootCount: 1, rootTotalSummary: 3, callNodeHasChildren: new Uint8Array([1, 1, 1, 1, 0, 1, 0, 1, 0]), callNodeSummary: { From 6f4c71ab5871c2dca0dad9560044c5e780b94cfc Mon Sep 17 00:00:00 2001 From: Markus Stange Date: Mon, 15 Jan 2024 18:52:54 -0500 Subject: [PATCH 2/8] Only pass the traced self/total for a single node to the CallTreeSidebar. With the fast inverted tree implementation, getting the running/total for an inverted call node will require a function call and not just a lookup in a big array. This is a step towards that world. --- src/components/sidebar/CallTreeSidebar.js | 19 +++--- src/selectors/per-thread/stack-sample.js | 16 ++++++ src/test/store/profile-view.test.js | 70 ++++++++--------------- src/types/profile-derived.js | 10 ++++ 4 files changed, 61 insertions(+), 54 deletions(-) diff --git a/src/components/sidebar/CallTreeSidebar.js b/src/components/sidebar/CallTreeSidebar.js index 56fa52d85b..f36eb9a310 100644 --- a/src/components/sidebar/CallTreeSidebar.js +++ b/src/components/sidebar/CallTreeSidebar.js @@ -30,7 +30,7 @@ import type { ThreadsKey, CategoryList, IndexIntoCallNodeTable, - TracedTiming, + SelfAndTotal, Milliseconds, WeightType, IndexIntoCategoryList, @@ -291,7 +291,7 @@ type StateProps = {| +timings: TimingsForPath, +categoryList: CategoryList, +weightType: WeightType, - +tracedTiming: TracedTiming | null, + +selectedNodeTracedSelfAndTotal: SelfAndTotal | null, |}; type Props = ConnectedProps<{||}, StateProps, {||}>; @@ -352,7 +352,7 @@ class CallTreeSidebarImpl extends React.PureComponent { timings, categoryList, weightType, - tracedTiming, + selectedNodeTracedSelfAndTotal, } = this.props; const { forPath: { selfTime, totalTime }, @@ -402,24 +402,24 @@ class CallTreeSidebarImpl extends React.PureComponent {

Call node details

- {tracedTiming ? ( + {selectedNodeTracedSelfAndTotal ? ( ) : null} - {tracedTiming ? ( + {selectedNodeTracedSelfAndTotal ? ( ({ timings: selectedNodeSelectors.getTimingsForSidebar(state), categoryList: getCategories(state), weightType: selectedThreadSelectors.getWeightTypeForCallTree(state), - tracedTiming: selectedThreadSelectors.getTracedTiming(state), + selectedNodeTracedSelfAndTotal: + selectedThreadSelectors.getTracedSelfAndTotalForSelectedCallNode(state), }), component: CallTreeSidebarImpl, }); diff --git a/src/selectors/per-thread/stack-sample.js b/src/selectors/per-thread/stack-sample.js index 0bac1067a5..7bd029ccf5 100644 --- a/src/selectors/per-thread/stack-sample.js +++ b/src/selectors/per-thread/stack-sample.js @@ -44,6 +44,7 @@ import type { $ReturnType, TracedTiming, ThreadsKey, + SelfAndTotal, } from 'firefox-profiler/types'; import type { ThreadSelectorsPerThread } from './thread'; @@ -350,6 +351,20 @@ export function getStackAndSampleSelectorsPerThread( CallTree.computeTracedTiming ); + const getTracedSelfAndTotalForSelectedCallNode: Selector = + createSelector( + getSelectedCallNodeIndex, + getTracedTiming, + (selectedCallNodeIndex, tracedTiming) => { + if (selectedCallNodeIndex === null || tracedTiming === null) { + return null; + } + const total = tracedTiming.running[selectedCallNodeIndex]; + const self = tracedTiming.self[selectedCallNodeIndex]; + return { total, self }; + } + ); + const getStackTimingByDepth: Selector = createSelector( threadSelectors.getFilteredSamplesForCallTree, @@ -412,6 +427,7 @@ export function getStackAndSampleSelectorsPerThread( getSourceViewLineTimings, getAssemblyViewAddressTimings, getTracedTiming, + getTracedSelfAndTotalForSelectedCallNode, getStackTimingByDepth, getFilteredCallNodeMaxDepthPlusOne, getPreviewFilteredCallNodeMaxDepthPlusOne, diff --git a/src/test/store/profile-view.test.js b/src/test/store/profile-view.test.js index f2b6bd6e76..7578e34ab8 100644 --- a/src/test/store/profile-view.test.js +++ b/src/test/store/profile-view.test.js @@ -3294,10 +3294,12 @@ describe('traced timing', function () { return { funcNames: funcNamesDictPerThread[0], - getCallNode: (...callNodePath) => - ensureExists(callNodeInfo.getCallNodeIndexFromPath(callNodePath)), - running, - self, + getSelfAndTotal: (...callNodePath) => { + const callNodeIndex = ensureExists( + callNodeInfo.getCallNodeIndexFromPath(callNodePath) + ); + return { self: self[callNodeIndex], total: running[callNodeIndex] }; + }, profile, }; } @@ -3305,9 +3307,7 @@ describe('traced timing', function () { it('computes traced timing', function () { const { funcNames: { A, B, C }, - getCallNode, - running, - self, + getSelfAndTotal, profile, } = setup( { inverted: false }, @@ -3318,24 +3318,18 @@ describe('traced timing', function () { ` ); - expect(running[getCallNode(A)]).toBe(6); - expect(self[getCallNode(A)]).toBe(2); - - expect(running[getCallNode(A, B)]).toBe(4); - expect(self[getCallNode(A, B)]).toBe(4); + expect(getSelfAndTotal(A)).toEqual({ self: 2, total: 6 }); + expect(getSelfAndTotal(A, B)).toEqual({ self: 4, total: 4 }); // This is the last sample, which is deduced to be the interval length. - expect(running[getCallNode(C)]).toBe(profile.meta.interval); - expect(self[getCallNode(C)]).toBe(profile.meta.interval); + const interval = profile.meta.interval; + expect(getSelfAndTotal(C)).toEqual({ self: interval, total: interval }); }); it('computes traced timing for an inverted tree', function () { const { funcNames: { A, B, C }, - getCallNode, - running, - // Rename self to make the assertions more readable. - self: self___, + getSelfAndTotal, } = setup( { inverted: true }, ` @@ -3356,23 +3350,12 @@ describe('traced timing', function () { // Running: [ 1, 4, 4, 1.5, 1, 1 ] // Self: [ 1, 4, 0, 1.5, 0, 0 ] - expect(running[getCallNode(A)]).toBe(1); - expect(self___[getCallNode(A)]).toBe(1); - - expect(running[getCallNode(B)]).toBe(4); - expect(self___[getCallNode(B)]).toBe(4); - - expect(running[getCallNode(B, A)]).toBe(4); - expect(self___[getCallNode(B, A)]).toBe(0); - - expect(running[getCallNode(C)]).toBe(1.5); - expect(self___[getCallNode(C)]).toBe(1.5); - - expect(running[getCallNode(C, B)]).toBe(1); - expect(self___[getCallNode(C, B)]).toBe(0); - - expect(running[getCallNode(C, B, A)]).toBe(1); - expect(self___[getCallNode(C, B, A)]).toBe(0); + expect(getSelfAndTotal(A)).toEqual({ self: 1, total: 1 }); + expect(getSelfAndTotal(B)).toEqual({ self: 4, total: 4 }); + expect(getSelfAndTotal(B, A)).toEqual({ self: 0, total: 4 }); + expect(getSelfAndTotal(C)).toEqual({ self: 1.5, total: 1.5 }); + expect(getSelfAndTotal(C, B)).toEqual({ self: 0, total: 1 }); + expect(getSelfAndTotal(C, B, A)).toEqual({ self: 0, total: 1 }); }); it('does not compute traced timing for other types', function () { @@ -3393,9 +3376,7 @@ describe('traced timing', function () { it('computes traced timing based on the preview selection', function () { const { funcNames: { A, B, C }, - getCallNode, - running, - self, + getSelfAndTotal, profile, } = setup( { inverted: false, previewSelection: { start: 1, end: 5.5 } }, @@ -3411,16 +3392,15 @@ describe('traced timing', function () { // the second sample will have a "traced duration" of the interval, because // it's the last sample in the range. - expect(running[getCallNode(A)]).toBe(4 + profile.meta.interval); - expect(self[getCallNode(A)]).toBe(profile.meta.interval); - - expect(running[getCallNode(A, B)]).toBe(4); - expect(self[getCallNode(A, B)]).toBe(4); + expect(getSelfAndTotal(A)).toEqual({ + self: profile.meta.interval, + total: 4 + profile.meta.interval, + }); + expect(getSelfAndTotal(A, B)).toEqual({ self: 4, total: 4 }); // Call node [C] is fully outside the preview range, so we should have no // traced duration for it. - expect(running[getCallNode(C)]).toBe(0); - expect(self[getCallNode(C)]).toBe(0); + expect(getSelfAndTotal(C)).toEqual({ self: 0, total: 0 }); }); }); diff --git a/src/types/profile-derived.js b/src/types/profile-derived.js index 841af62a39..d5e67b4791 100644 --- a/src/types/profile-derived.js +++ b/src/types/profile-derived.js @@ -654,6 +654,16 @@ export type ProfileFilterPageData = {| favicon: string | null, |}; +/** + * The self and total time, usually for a single call node. + * As with most places where the terms "self" and "total" are used, the meaning + * of the numbers depends on the context: + * - When used for "traced" timing, the values are Milliseconds. + * - Otherwise, the values are in the same unit as the sample weight type. For + * example, they could be sample counts, weights, or bytes. + */ +export type SelfAndTotal = {| self: number, total: number |}; + /** * This struct contains the traced timing for each call node. The arrays are indexed * by the CallNodeIndex, and the values in the Float32Arrays are Milliseconds. The From 5400136bffbe12b793c764791f9eeab6e00dcdb7 Mon Sep 17 00:00:00 2001 From: Markus Stange Date: Wed, 17 Jan 2024 15:08:19 -0500 Subject: [PATCH 3/8] Break _getStackSelf into two functions. --- src/profile-logic/call-tree.js | 116 +++++++++++++++------------------ src/types/profile-derived.js | 9 +++ 2 files changed, 61 insertions(+), 64 deletions(-) diff --git a/src/profile-logic/call-tree.js b/src/profile-logic/call-tree.js index 8d04a4c037..97fc59c644 100644 --- a/src/profile-logic/call-tree.js +++ b/src/profile-logic/call-tree.js @@ -30,6 +30,7 @@ import type { SamplesTable, ExtraBadgeInfo, BottomBoxInfo, + CallNodeLeafAndSummary, } from 'firefox-profiler/types'; import ExtensionIcon from '../../res/img/svg/extension.svg'; @@ -414,22 +415,18 @@ export class CallTree { } } -function _getInvertedStackSelf( - // The samples could either be a SamplesTable, or a JsAllocationsTable. - samples: SamplesLikeTable, - callNodeTable: CallNodeTable, - sampleIndexToCallNodeIndex: Array -): { - // In an inverted profile, all the amount of self unit (time, bytes, count, etc.) is - // accounted to the root nodes. So `callNodeSelf` will be 0 for all non-root nodes. - callNodeSelf: Float32Array, - // This property stores the amount of unit (time, bytes, count, etc.) spent in the - // stacks' leaf nodes. Later these values will make it possible to compute the - // total for all nodes by summing up the values up the tree. +// In an inverted profile, all the amount of self unit (time, bytes, count, etc.) is +// accounted to the root nodes. So `callNodeSelf` will be 0 for all non-root nodes. +function _getInvertedCallNodeSelf( callNodeLeaf: Float32Array, -} { + callNodeTable: CallNodeTable +): Float32Array { // Compute an array that maps the callNodeIndex to its root. const callNodeToRoot = new Int32Array(callNodeTable.length); + + // Compute the self time during the same loop. + const callNodeSelf = new Float32Array(callNodeTable.length); + for ( let callNodeIndex = 0; callNodeIndex < callNodeTable.length; @@ -449,41 +446,22 @@ function _getInvertedStackSelf( // recursively is the value we're looking for. callNodeToRoot[callNodeIndex] = callNodeToRoot[prefixCallNode]; } + callNodeSelf[callNodeToRoot[callNodeIndex]] += callNodeLeaf[callNodeIndex]; } - // Calculate the timing information by going through each sample. - const callNodeSelf = new Float32Array(callNodeTable.length); - const callNodeLeaf = new Float32Array(callNodeTable.length); - for ( - let sampleIndex = 0; - sampleIndex < sampleIndexToCallNodeIndex.length; - sampleIndex++ - ) { - const callNodeIndex = sampleIndexToCallNodeIndex[sampleIndex]; - if (callNodeIndex !== null) { - const rootIndex = callNodeToRoot[callNodeIndex]; - const weight = samples.weight ? samples.weight[sampleIndex] : 1; - callNodeSelf[rootIndex] += weight; - callNodeLeaf[callNodeIndex] += weight; - } - } - - return { callNodeSelf, callNodeLeaf }; + return callNodeSelf; } /** - * This is a helper function to get the stack timings for un-inverted call trees. + * Compute the leaf time for each call node, and the sum of the absolute leaf + * values. */ -function _getStackSelf( +function _getCallNodeLeafAndSummary( samples: SamplesLikeTable, callNodeTable: CallNodeTable, sampleIndexToCallNodeIndex: Array -): { - callNodeSelf: Float32Array, // Milliseconds[] - callNodeLeaf: Float32Array, // Milliseconds[] -} { - const callNodeSelf = new Float32Array(callNodeTable.length); - +): CallNodeLeafAndSummary { + const callNodeLeaf = new Float32Array(callNodeTable.length); for ( let sampleIndex = 0; sampleIndex < sampleIndexToCallNodeIndex.length; @@ -492,11 +470,23 @@ function _getStackSelf( const callNodeIndex = sampleIndexToCallNodeIndex[sampleIndex]; if (callNodeIndex !== null) { const weight = samples.weight ? samples.weight[sampleIndex] : 1; - callNodeSelf[callNodeIndex] += weight; + callNodeLeaf[callNodeIndex] += weight; } } - return { callNodeSelf, callNodeLeaf: callNodeSelf }; + // Workaround for https://bugzilla.mozilla.org/show_bug.cgi?id=1858310 + const abs = Math.abs; + + let rootTotalSummary = 0; + for ( + let callNodeIndex = 0; + callNodeIndex < callNodeTable.length; + callNodeIndex++ + ) { + rootTotalSummary += abs(callNodeLeaf[callNodeIndex]); + } + + return { callNodeLeaf, rootTotalSummary }; } /** @@ -511,22 +501,25 @@ export function computeCallTreeTimings( samples: SamplesLikeTable, sampleIndexToCallNodeIndex: Array, callNodeInfo: CallNodeInfo, - invertCallstack: boolean + _invertCallstack: boolean ): CallTreeTimings { const callNodeTable = callNodeInfo.getCallNodeTable(); - // Inverted trees need a different method for computing the timing. - const { callNodeSelf, callNodeLeaf } = invertCallstack - ? _getInvertedStackSelf(samples, callNodeTable, sampleIndexToCallNodeIndex) - : _getStackSelf(samples, callNodeTable, sampleIndexToCallNodeIndex); + const { callNodeLeaf, rootTotalSummary } = _getCallNodeLeafAndSummary( + samples, + callNodeTable, + sampleIndexToCallNodeIndex + ); + + // The self values depend on whether the call tree is inverted: In an inverted + // tree, all the self time is in the roots. + const callNodeSelf = callNodeInfo.isInverted() + ? _getInvertedCallNodeSelf(callNodeLeaf, callNodeTable) + : callNodeLeaf; // Compute the following variables: const callNodeTotalSummary = new Float32Array(callNodeTable.length); const callNodeHasChildren = new Uint8Array(callNodeTable.length); - let rootTotalSummary = 0; - - // Workaround for https://bugzilla.mozilla.org/show_bug.cgi?id=1858310 - const abs = Math.abs; // We loop the call node table in reverse, so that we find the children // before their parents, and the total is known at the time we reach a @@ -537,7 +530,6 @@ export function computeCallTreeTimings( callNodeIndex-- ) { callNodeTotalSummary[callNodeIndex] += callNodeLeaf[callNodeIndex]; - rootTotalSummary += abs(callNodeLeaf[callNodeIndex]); const hasChildren = callNodeHasChildren[callNodeIndex] !== 0; const hasTotalValue = callNodeTotalSummary[callNodeIndex] !== 0; @@ -676,7 +668,7 @@ export function computeTracedTiming( samples: SamplesLikeTable, callNodeInfo: CallNodeInfo, interval: Milliseconds, - invertCallstack: boolean + _invertCallstack: boolean ): TracedTiming | null { const callNodeTable = callNodeInfo.getCallNodeTable(); const stackIndexToCallNodeIndex = callNodeInfo.getStackIndexToCallNodeIndex(); @@ -710,18 +702,14 @@ export function computeTracedTiming( samples.stack, stackIndexToCallNodeIndex ); - // Inverted trees need a different method for computing the timing. - const { callNodeSelf, callNodeLeaf } = invertCallstack - ? _getInvertedStackSelf( - samplesWithWeight, - callNodeTable, - sampleIndexToCallNodeIndex - ) - : _getStackSelf( - samplesWithWeight, - callNodeTable, - sampleIndexToCallNodeIndex - ); + const { callNodeLeaf } = _getCallNodeLeafAndSummary( + samplesWithWeight, + callNodeTable, + sampleIndexToCallNodeIndex + ); + const callNodeSelf = callNodeInfo.isInverted() + ? _getInvertedCallNodeSelf(callNodeLeaf, callNodeTable) + : callNodeLeaf; // Compute the following variables: const callNodeTotalSummary = new Float32Array(callNodeTable.length); diff --git a/src/types/profile-derived.js b/src/types/profile-derived.js index d5e67b4791..b524d1162d 100644 --- a/src/types/profile-derived.js +++ b/src/types/profile-derived.js @@ -654,6 +654,15 @@ export type ProfileFilterPageData = {| favicon: string | null, |}; +export type CallNodeLeafAndSummary = {| + // This property stores the amount of unit (time, bytes, count, etc.) spent in the + // stacks' leaf nodes. + callNodeLeaf: Float32Array, + // The sum of absolute values in callNodeLeaf. + // This is used for computing the percentages displayed in the call tree. + rootTotalSummary: number, +|}; + /** * The self and total time, usually for a single call node. * As with most places where the terms "self" and "total" are used, the meaning From e6a7b6eb343c39d22379947406529fc8103e3ac5 Mon Sep 17 00:00:00 2001 From: Markus Stange Date: Wed, 17 Jan 2024 15:13:51 -0500 Subject: [PATCH 4/8] Fold CallNodeSummary into CallTreeTimings. --- src/profile-logic/call-tree.js | 47 ++++------- src/profile-logic/flame-graph.js | 3 +- .../__snapshots__/profile-view.test.js.snap | 82 +++++++++++-------- src/test/unit/profile-tree.test.js | 8 +- 4 files changed, 69 insertions(+), 71 deletions(-) diff --git a/src/profile-logic/call-tree.js b/src/profile-logic/call-tree.js index 97fc59c644..0cc2749e3a 100644 --- a/src/profile-logic/call-tree.js +++ b/src/profile-logic/call-tree.js @@ -40,14 +40,12 @@ import * as ProfileData from './profile-data'; import type { CallTreeSummaryStrategy } from '../types/actions'; type CallNodeChildren = IndexIntoCallNodeTable[]; -type CallNodeSummary = { + +export type CallTreeTimings = { + callNodeHasChildren: Uint8Array, self: Float32Array, leaf: Float32Array, total: Float32Array, -}; -export type CallTreeTimings = { - callNodeHasChildren: Uint8Array, - callNodeSummary: CallNodeSummary, rootTotalSummary: number, }; @@ -72,7 +70,7 @@ export class CallTree { _categories: CategoryList; _callNodeInfo: CallNodeInfo; _callNodeTable: CallNodeTable; - _callNodeSummary: CallNodeSummary; + _callTreeTimings: CallTreeTimings; _callNodeHasChildren: Uint8Array; // A table column matching the callNodeTable _thread: Thread; _rootTotalSummary: number; @@ -87,19 +85,17 @@ export class CallTree { thread: Thread, categories: CategoryList, callNodeInfo: CallNodeInfo, - callNodeSummary: CallNodeSummary, - callNodeHasChildren: Uint8Array, - rootTotalSummary: number, + callTreeTimings: CallTreeTimings, isHighPrecision: boolean, weightType: WeightType ) { this._categories = categories; this._callNodeInfo = callNodeInfo; this._callNodeTable = callNodeInfo.getCallNodeTable(); - this._callNodeSummary = callNodeSummary; - this._callNodeHasChildren = callNodeHasChildren; + this._callTreeTimings = callTreeTimings; + this._callNodeHasChildren = callTreeTimings.callNodeHasChildren; this._thread = thread; - this._rootTotalSummary = rootTotalSummary; + this._rootTotalSummary = callTreeTimings.rootTotalSummary; this._displayDataByIndex = new Map(); this._children = []; this._isHighPrecision = isHighPrecision; @@ -134,7 +130,7 @@ export class CallTree { childCallNodeIndex = this._callNodeTable.nextSibling[childCallNodeIndex] ) { const childTotalSummary = - this._callNodeSummary.total[childCallNodeIndex]; + this._callTreeTimings.total[childCallNodeIndex]; const childHasChildren = this._callNodeHasChildren[childCallNodeIndex]; if (childTotalSummary !== 0 || childHasChildren !== 0) { @@ -143,8 +139,8 @@ export class CallTree { } children.sort( (a, b) => - Math.abs(this._callNodeSummary.total[b]) - - Math.abs(this._callNodeSummary.total[a]) + Math.abs(this._callTreeTimings.total[b]) - + Math.abs(this._callTreeTimings.total[a]) ); this._children[callNodeIndex] = children; } @@ -188,9 +184,9 @@ export class CallTree { const funcName = this._thread.stringTable.getString( this._thread.funcTable.name[funcIndex] ); - const total = this._callNodeSummary.total[callNodeIndex]; + const total = this._callTreeTimings.total[callNodeIndex]; const totalRelative = total / this._rootTotalSummary; - const self = this._callNodeSummary.self[callNodeIndex]; + const self = this._callTreeTimings.self[callNodeIndex]; const selfRelative = self / this._rootTotalSummary; return { @@ -404,7 +400,7 @@ export class CallTree { let maxNode = -1; let maxAbs = 0; for (let nodeIndex = callNodeIndex; nodeIndex < rangeEnd; nodeIndex++) { - const nodeLeaf = Math.abs(this._callNodeSummary.leaf[nodeIndex]); + const nodeLeaf = Math.abs(this._callTreeTimings.leaf[nodeIndex]); if (maxNode === -1 || nodeLeaf > maxAbs) { maxNode = nodeIndex; maxAbs = nodeLeaf; @@ -546,11 +542,9 @@ export function computeCallTreeTimings( } return { - callNodeSummary: { - self: callNodeSelf, - leaf: callNodeLeaf, - total: callNodeTotalSummary, - }, + self: callNodeSelf, + leaf: callNodeLeaf, + total: callNodeTotalSummary, callNodeHasChildren, rootTotalSummary, }; @@ -567,16 +561,11 @@ export function getCallTree( weightType: WeightType ): CallTree { return timeCode('getCallTree', () => { - const { callNodeSummary, callNodeHasChildren, rootTotalSummary } = - callTreeTimings; - return new CallTree( thread, categories, callNodeInfo, - callNodeSummary, - callNodeHasChildren, - rootTotalSummary, + callTreeTimings, Boolean(thread.isJsTracer), weightType ); diff --git a/src/profile-logic/flame-graph.js b/src/profile-logic/flame-graph.js index 4d85d96774..fe74e4e25a 100644 --- a/src/profile-logic/flame-graph.js +++ b/src/profile-logic/flame-graph.js @@ -231,8 +231,7 @@ export function getFlameGraphTiming( callNodeTable: CallNodeTable, callTreeTimings: CallTreeTimings ): FlameGraphTiming { - const { callNodeSummary, rootTotalSummary } = callTreeTimings; - const { total, self } = callNodeSummary; + const { total, self, rootTotalSummary } = callTreeTimings; const { prefix } = callNodeTable; // This is where we build up the return value, one row at a time. diff --git a/src/test/store/__snapshots__/profile-view.test.js.snap b/src/test/store/__snapshots__/profile-view.test.js.snap index b112486454..ba34b58ba5 100644 --- a/src/test/store/__snapshots__/profile-view.test.js.snap +++ b/src/test/store/__snapshots__/profile-view.test.js.snap @@ -2433,41 +2433,6 @@ CallTree { 8, ], }, - "_callNodeSummary": Object { - "leaf": Float32Array [ - 0, - 0, - 0, - 0, - 0, - 0, - 2, - 0, - 0, - ], - "self": Float32Array [ - 0, - 0, - 0, - 0, - 0, - 0, - 2, - 0, - 0, - ], - "total": Float32Array [ - 2, - 2, - 0, - 0, - 0, - 2, - 2, - 0, - 0, - ], - }, "_callNodeTable": Object { "category": Int32Array [ 0, @@ -2571,6 +2536,53 @@ CallTree { 9, ], }, + "_callTreeTimings": Object { + "callNodeHasChildren": Uint8Array [ + 1, + 1, + 0, + 0, + 0, + 1, + 0, + 0, + 0, + ], + "leaf": Float32Array [ + 0, + 0, + 0, + 0, + 0, + 0, + 2, + 0, + 0, + ], + "rootTotalSummary": 2, + "self": Float32Array [ + 0, + 0, + 0, + 0, + 0, + 0, + 2, + 0, + 0, + ], + "total": Float32Array [ + 2, + 2, + 0, + 0, + 0, + 2, + 2, + 0, + 0, + ], + }, "_categories": Array [ Object { "color": "grey", diff --git a/src/test/unit/profile-tree.test.js b/src/test/unit/profile-tree.test.js index d0d6770716..108aa16f53 100644 --- a/src/test/unit/profile-tree.test.js +++ b/src/test/unit/profile-tree.test.js @@ -82,11 +82,9 @@ describe('unfiltered call tree', function () { ).toEqual({ rootTotalSummary: 3, callNodeHasChildren: new Uint8Array([1, 1, 1, 1, 0, 1, 0, 1, 0]), - callNodeSummary: { - self: new Float32Array([0, 0, 0, 0, 1, 0, 1, 0, 1]), - leaf: new Float32Array([0, 0, 0, 0, 1, 0, 1, 0, 1]), - total: new Float32Array([3, 3, 2, 1, 1, 1, 1, 1, 1]), - }, + self: new Float32Array([0, 0, 0, 0, 1, 0, 1, 0, 1]), + leaf: new Float32Array([0, 0, 0, 0, 1, 0, 1, 0, 1]), + total: new Float32Array([3, 3, 2, 1, 1, 1, 1, 1, 1]), }); }); }); From f8152a7df269f01e30c85c5f737a2da0d6035b5b Mon Sep 17 00:00:00 2001 From: Markus Stange Date: Wed, 17 Jan 2024 15:29:12 -0500 Subject: [PATCH 5/8] Feed CallNodeLeafAndSummary into computeCallTreeTimings. --- src/profile-logic/call-tree.js | 35 +++++---------- src/selectors/per-thread/stack-sample.js | 11 ++--- src/test/fixtures/utils.js | 15 ++++--- src/test/unit/profile-tree.test.js | 54 ++++++++++++++---------- 4 files changed, 56 insertions(+), 59 deletions(-) diff --git a/src/profile-logic/call-tree.js b/src/profile-logic/call-tree.js index 0cc2749e3a..7522b2f3a2 100644 --- a/src/profile-logic/call-tree.js +++ b/src/profile-logic/call-tree.js @@ -452,12 +452,12 @@ function _getInvertedCallNodeSelf( * Compute the leaf time for each call node, and the sum of the absolute leaf * values. */ -function _getCallNodeLeafAndSummary( +export function computeCallNodeLeafAndSummary( samples: SamplesLikeTable, - callNodeTable: CallNodeTable, - sampleIndexToCallNodeIndex: Array + sampleIndexToCallNodeIndex: Array, + callNodeCount: number ): CallNodeLeafAndSummary { - const callNodeLeaf = new Float32Array(callNodeTable.length); + const callNodeLeaf = new Float32Array(callNodeCount); for ( let sampleIndex = 0; sampleIndex < sampleIndexToCallNodeIndex.length; @@ -474,11 +474,7 @@ function _getCallNodeLeafAndSummary( const abs = Math.abs; let rootTotalSummary = 0; - for ( - let callNodeIndex = 0; - callNodeIndex < callNodeTable.length; - callNodeIndex++ - ) { + for (let callNodeIndex = 0; callNodeIndex < callNodeCount; callNodeIndex++) { rootTotalSummary += abs(callNodeLeaf[callNodeIndex]); } @@ -488,24 +484,13 @@ function _getCallNodeLeafAndSummary( /** * This computes all of the count and timing information displayed in the calltree. * It takes into account both the normal tree, and the inverted tree. - * - * Note: The "timionmgs" could have a number of different meanings based on the - * what type of weight is in the SamplesLikeTable. For instance, it could be - * milliseconds, sample counts, or bytes. */ export function computeCallTreeTimings( - samples: SamplesLikeTable, - sampleIndexToCallNodeIndex: Array, callNodeInfo: CallNodeInfo, - _invertCallstack: boolean + callNodeLeafAndSummary: CallNodeLeafAndSummary ): CallTreeTimings { const callNodeTable = callNodeInfo.getCallNodeTable(); - - const { callNodeLeaf, rootTotalSummary } = _getCallNodeLeafAndSummary( - samples, - callNodeTable, - sampleIndexToCallNodeIndex - ); + const { callNodeLeaf, rootTotalSummary } = callNodeLeafAndSummary; // The self values depend on whether the call tree is inverted: In an inverted // tree, all the self time is in the roots. @@ -691,10 +676,10 @@ export function computeTracedTiming( samples.stack, stackIndexToCallNodeIndex ); - const { callNodeLeaf } = _getCallNodeLeafAndSummary( + const { callNodeLeaf } = computeCallNodeLeafAndSummary( samplesWithWeight, - callNodeTable, - sampleIndexToCallNodeIndex + sampleIndexToCallNodeIndex, + callNodeTable.length ); const callNodeSelf = callNodeInfo.isInverted() ? _getInvertedCallNodeSelf(callNodeLeaf, callNodeTable) diff --git a/src/selectors/per-thread/stack-sample.js b/src/selectors/per-thread/stack-sample.js index 7bd029ccf5..1811f62b95 100644 --- a/src/selectors/per-thread/stack-sample.js +++ b/src/selectors/per-thread/stack-sample.js @@ -304,19 +304,20 @@ export function getStackAndSampleSelectorsPerThread( const getCallTreeTimings: Selector = createSelector( threadSelectors.getPreviewFilteredSamplesForCallTree, getCallNodeInfo, - ProfileSelectors.getProfileInterval, - UrlState.getInvertCallstack, - (samples, callNodeInfo, interval, invertCallStack) => { + (samples, callNodeInfo) => { const sampleIndexToCallNodeIndex = ProfileData.getSampleIndexToCallNodeIndex( samples.stack, callNodeInfo.getStackIndexToCallNodeIndex() ); - return CallTree.computeCallTreeTimings( + const callNodeLeafAndSummary = CallTree.computeCallNodeLeafAndSummary( samples, sampleIndexToCallNodeIndex, + callNodeInfo.getCallNodeTable().length + ); + return CallTree.computeCallTreeTimings( callNodeInfo, - invertCallStack + callNodeLeafAndSummary ); } ); diff --git a/src/test/fixtures/utils.js b/src/test/fixtures/utils.js index 8b329e7ad4..de1db874b2 100644 --- a/src/test/fixtures/utils.js +++ b/src/test/fixtures/utils.js @@ -4,6 +4,7 @@ // @flow import { getCallTree, + computeCallNodeLeafAndSummary, computeCallTreeTimings, type CallTree, } from 'firefox-profiler/profile-logic/call-tree'; @@ -128,13 +129,15 @@ export function callTreeFromProfile( defaultCategory ); const callTreeTimings = computeCallTreeTimings( - thread.samples, - getSampleIndexToCallNodeIndex( - thread.samples.stack, - callNodeInfo.getStackIndexToCallNodeIndex() - ), callNodeInfo, - false + computeCallNodeLeafAndSummary( + thread.samples, + getSampleIndexToCallNodeIndex( + thread.samples.stack, + callNodeInfo.getStackIndexToCallNodeIndex() + ), + callNodeInfo.getCallNodeTable().length + ) ); return getCallTree( thread, diff --git a/src/test/unit/profile-tree.test.js b/src/test/unit/profile-tree.test.js index 108aa16f53..eb81d70663 100644 --- a/src/test/unit/profile-tree.test.js +++ b/src/test/unit/profile-tree.test.js @@ -8,6 +8,7 @@ import { } from '../fixtures/profiles/processed-profile'; import { getCallTree, + computeCallNodeLeafAndSummary, computeCallTreeTimings, } from '../../profile-logic/call-tree'; import { computeFlameGraphRows } from '../../profile-logic/flame-graph'; @@ -69,17 +70,18 @@ describe('unfiltered call tree', function () { ); it('yields expected results', function () { - expect( - computeCallTreeTimings( + const callTreeTimings = computeCallTreeTimings( + callNodeInfo, + computeCallNodeLeafAndSummary( thread.samples, getSampleIndexToCallNodeIndex( thread.samples.stack, callNodeInfo.getStackIndexToCallNodeIndex() ), - callNodeInfo, - false + callNodeInfo.getCallNodeTable().length ) - ).toEqual({ + ); + expect(callTreeTimings).toEqual({ rootTotalSummary: 3, callNodeHasChildren: new Uint8Array([1, 1, 1, 1, 0, 1, 0, 1, 0]), self: new Float32Array([0, 0, 0, 0, 1, 0, 1, 0, 1]), @@ -431,13 +433,15 @@ describe('inverted call tree', function () { defaultCategory ); const callTreeTimings = computeCallTreeTimings( - thread.samples, - getSampleIndexToCallNodeIndex( - thread.samples.stack, - callNodeInfo.getStackIndexToCallNodeIndex() - ), callNodeInfo, - false + computeCallNodeLeafAndSummary( + thread.samples, + getSampleIndexToCallNodeIndex( + thread.samples.stack, + callNodeInfo.getStackIndexToCallNodeIndex() + ), + callNodeInfo.getCallNodeTable().length + ) ); const callTree = getCallTree( thread, @@ -469,13 +473,15 @@ describe('inverted call tree', function () { defaultCategory ); const invertedCallTreeTimings = computeCallTreeTimings( - thread.samples, - getSampleIndexToCallNodeIndex( - thread.samples.stack, - invertedCallNodeInfo.getStackIndexToCallNodeIndex() - ), invertedCallNodeInfo, - true + computeCallNodeLeafAndSummary( + thread.samples, + getSampleIndexToCallNodeIndex( + thread.samples.stack, + invertedCallNodeInfo.getStackIndexToCallNodeIndex() + ), + invertedCallNodeInfo.getCallNodeTable().length + ) ); const invertedCallTree = getCallTree( thread, @@ -619,13 +625,15 @@ describe('diffing trees', function () { defaultCategory ); const callTreeTimings = computeCallTreeTimings( - thread.samples, - getSampleIndexToCallNodeIndex( - thread.samples.stack, - callNodeInfo.getStackIndexToCallNodeIndex() - ), callNodeInfo, - false + computeCallNodeLeafAndSummary( + thread.samples, + getSampleIndexToCallNodeIndex( + thread.samples.stack, + callNodeInfo.getStackIndexToCallNodeIndex() + ), + callNodeInfo.getCallNodeTable().length + ) ); expect(callTreeTimings.rootTotalSummary).toBe(12); }); From b13edbb3f8f4a39913d110b63318a84379407fa1 Mon Sep 17 00:00:00 2001 From: Markus Stange Date: Wed, 17 Jan 2024 15:53:28 -0500 Subject: [PATCH 6/8] Use computeCallTreeTimings for traced timing computation. --- src/components/flame-graph/Canvas.js | 10 +-- src/components/flame-graph/FlameGraph.js | 8 ++- src/profile-logic/call-tree.js | 89 +++++++----------------- src/selectors/per-thread/stack-sample.js | 37 +++++++--- src/test/store/profile-view.test.js | 8 +-- src/types/profile-derived.js | 11 --- 6 files changed, 67 insertions(+), 96 deletions(-) diff --git a/src/components/flame-graph/Canvas.js b/src/components/flame-graph/Canvas.js index 22b1c54226..d730d71313 100644 --- a/src/components/flame-graph/Canvas.js +++ b/src/components/flame-graph/Canvas.js @@ -33,7 +33,6 @@ import type { CallTreeSummaryStrategy, WeightType, SamplesLikeTable, - TracedTiming, InnerWindowID, Page, } from 'firefox-profiler/types'; @@ -49,7 +48,10 @@ import type { ChartCanvasHoverInfo, } from '../shared/chart/Canvas'; -import type { CallTree } from 'firefox-profiler/profile-logic/call-tree'; +import type { + CallTree, + CallTreeTimings, +} from 'firefox-profiler/profile-logic/call-tree'; export type OwnProps = {| +thread: Thread, @@ -75,7 +77,7 @@ export type OwnProps = {| +callTreeSummaryStrategy: CallTreeSummaryStrategy, +samples: SamplesLikeTable, +unfilteredSamples: SamplesLikeTable, - +tracedTiming: TracedTiming | null, + +tracedTiming: CallTreeTimings | null, +displayImplementation: boolean, +displayStackType: boolean, |}; @@ -391,7 +393,7 @@ class FlameGraphCanvasImpl extends React.PureComponent { const time = formatCallNodeNumberWithUnit( 'tracing-ms', false, - tracedTiming.running[callNodeIndex] + tracedTiming.total[callNodeIndex] ); percentage = `${time} (${percentage})`; } diff --git a/src/components/flame-graph/FlameGraph.js b/src/components/flame-graph/FlameGraph.js index 35769154e3..f80531e8fd 100644 --- a/src/components/flame-graph/FlameGraph.js +++ b/src/components/flame-graph/FlameGraph.js @@ -42,7 +42,6 @@ import type { CallTreeSummaryStrategy, CallNodeInfo, IndexIntoCallNodeTable, - TracedTiming, ThreadsKey, InnerWindowID, Page, @@ -50,7 +49,10 @@ import type { import type { FlameGraphTiming } from 'firefox-profiler/profile-logic/flame-graph'; -import type { CallTree } from 'firefox-profiler/profile-logic/call-tree'; +import type { + CallTree, + CallTreeTimings, +} from 'firefox-profiler/profile-logic/call-tree'; import type { ConnectedProps } from 'firefox-profiler/utils/connect'; @@ -87,7 +89,7 @@ type StateProps = {| +callTreeSummaryStrategy: CallTreeSummaryStrategy, +samples: SamplesLikeTable, +unfilteredSamples: SamplesLikeTable, - +tracedTiming: TracedTiming | null, + +tracedTiming: CallTreeTimings | null, +displayImplementation: boolean, +displayStackType: boolean, |}; diff --git a/src/profile-logic/call-tree.js b/src/profile-logic/call-tree.js index 7522b2f3a2..0f80f8c284 100644 --- a/src/profile-logic/call-tree.js +++ b/src/profile-logic/call-tree.js @@ -6,7 +6,6 @@ import { oneLine } from 'common-tags'; import { timeCode } from '../utils/time-code'; import { - getSampleIndexToCallNodeIndex, getOriginAnnotationForFunc, getCategoryPairLabel, getBottomBoxInfoForCallNode, @@ -26,8 +25,6 @@ import type { CallNodeData, CallNodeDisplayData, Milliseconds, - TracedTiming, - SamplesTable, ExtraBadgeInfo, BottomBoxInfo, CallNodeLeafAndSummary, @@ -628,7 +625,7 @@ export function extractSamplesLikeTable( } /** - * This function is extremely similar to computeCallTreeTimings, + * This function is extremely similar to computeCallNodeLeafAndSummary, * but is specialized for converting sample counts into traced timing. Samples * don't have duration information associated with them, it's mostly how long they * were observed to be running. This function computes the timing the exact same @@ -638,15 +635,12 @@ export function extractSamplesLikeTable( * did not agree. In order to remove confusion, we can show the sample counts, * plus the traced timing, which is a compromise between correctness, and consistency. */ -export function computeTracedTiming( +export function computeCallNodeTracedLeafAndSummary( samples: SamplesLikeTable, - callNodeInfo: CallNodeInfo, - interval: Milliseconds, - _invertCallstack: boolean -): TracedTiming | null { - const callNodeTable = callNodeInfo.getCallNodeTable(); - const stackIndexToCallNodeIndex = callNodeInfo.getStackIndexToCallNodeIndex(); - + sampleIndexToCallNodeIndex: Array, + callNodeCount: number, + interval: Milliseconds +): CallNodeLeafAndSummary | null { if (samples.weightType !== 'samples' || samples.weight) { // Only compute for the samples weight types that have no weights. If a samples // table has weights then it's a diff profile. Currently, we aren't calculating @@ -658,63 +652,28 @@ export function computeTracedTiming( return null; } - // Compute the timing duration, which is the time between this sample and the next. - const weight = []; - for (let sampleIndex = 0; sampleIndex < samples.length - 1; sampleIndex++) { - weight.push(samples.time[sampleIndex + 1] - samples.time[sampleIndex]); - } - if (samples.length > 0) { - // Use the sampling interval for the last sample. - weight.push(interval); - } - const samplesWithWeight: SamplesTable = { - ...samples, - weight, - }; - - const sampleIndexToCallNodeIndex = getSampleIndexToCallNodeIndex( - samples.stack, - stackIndexToCallNodeIndex - ); - const { callNodeLeaf } = computeCallNodeLeafAndSummary( - samplesWithWeight, - sampleIndexToCallNodeIndex, - callNodeTable.length - ); - const callNodeSelf = callNodeInfo.isInverted() - ? _getInvertedCallNodeSelf(callNodeLeaf, callNodeTable) - : callNodeLeaf; - - // Compute the following variables: - const callNodeTotalSummary = new Float32Array(callNodeTable.length); - const callNodeHasChildren = new Uint8Array(callNodeTable.length); - - // We loop the call node table in reverse, so that we find the children - // before their parents, and the total time is known at the time we reach a - // node. - for ( - let callNodeIndex = callNodeTable.length - 1; - callNodeIndex >= 0; - callNodeIndex-- - ) { - callNodeTotalSummary[callNodeIndex] += callNodeLeaf[callNodeIndex]; - const hasChildren = callNodeHasChildren[callNodeIndex] !== 0; - const hasTotalValue = callNodeTotalSummary[callNodeIndex] !== 0; + const callNodeLeaf = new Float32Array(callNodeCount); + let rootTotalSummary = 0; - if (!hasChildren && !hasTotalValue) { - continue; + for (let sampleIndex = 0; sampleIndex < samples.length - 1; sampleIndex++) { + const callNodeIndex = sampleIndexToCallNodeIndex[sampleIndex]; + if (callNodeIndex !== null) { + const sampleTracedTime = + samples.time[sampleIndex + 1] - samples.time[sampleIndex]; + callNodeLeaf[callNodeIndex] += sampleTracedTime; + rootTotalSummary += sampleTracedTime; } + } - const prefixCallNode = callNodeTable.prefix[callNodeIndex]; - if (prefixCallNode !== -1) { - callNodeTotalSummary[prefixCallNode] += - callNodeTotalSummary[callNodeIndex]; - callNodeHasChildren[prefixCallNode] = 1; + if (samples.length > 0) { + const callNodeIndex = sampleIndexToCallNodeIndex[samples.length - 1]; + if (callNodeIndex !== null) { + // Use the sampling interval for the last sample. + const sampleTracedTime = interval; + callNodeLeaf[callNodeIndex] += sampleTracedTime; + rootTotalSummary += sampleTracedTime; } } - return { - self: callNodeSelf, - running: callNodeTotalSummary, - }; + return { callNodeLeaf, rootTotalSummary }; } diff --git a/src/selectors/per-thread/stack-sample.js b/src/selectors/per-thread/stack-sample.js index 1811f62b95..1852b81fef 100644 --- a/src/selectors/per-thread/stack-sample.js +++ b/src/selectors/per-thread/stack-sample.js @@ -42,7 +42,6 @@ import type { StartEndRange, Selector, $ReturnType, - TracedTiming, ThreadsKey, SelfAndTotal, } from 'firefox-profiler/types'; @@ -344,13 +343,33 @@ export function getStackAndSampleSelectorsPerThread( getAddressTimings ); - const getTracedTiming: Selector = createSelector( - threadSelectors.getPreviewFilteredSamplesForCallTree, - getCallNodeInfo, - ProfileSelectors.getProfileInterval, - UrlState.getInvertCallstack, - CallTree.computeTracedTiming - ); + const getTracedTiming: Selector = + createSelector( + threadSelectors.getPreviewFilteredSamplesForCallTree, + getCallNodeInfo, + ProfileSelectors.getProfileInterval, + (samples, callNodeInfo, interval) => { + const sampleIndexToCallNodeIndex = + ProfileData.getSampleIndexToCallNodeIndex( + samples.stack, + callNodeInfo.getStackIndexToCallNodeIndex() + ); + const callNodeLeafAndSummary = + CallTree.computeCallNodeTracedLeafAndSummary( + samples, + sampleIndexToCallNodeIndex, + callNodeInfo.getCallNodeTable().length, + interval + ); + if (callNodeLeafAndSummary === null) { + return null; + } + return CallTree.computeCallTreeTimings( + callNodeInfo, + callNodeLeafAndSummary + ); + } + ); const getTracedSelfAndTotalForSelectedCallNode: Selector = createSelector( @@ -360,7 +379,7 @@ export function getStackAndSampleSelectorsPerThread( if (selectedCallNodeIndex === null || tracedTiming === null) { return null; } - const total = tracedTiming.running[selectedCallNodeIndex]; + const total = tracedTiming.total[selectedCallNodeIndex]; const self = tracedTiming.self[selectedCallNodeIndex]; return { total, self }; } diff --git a/src/test/store/profile-view.test.js b/src/test/store/profile-view.test.js index 7578e34ab8..edaaa07eb8 100644 --- a/src/test/store/profile-view.test.js +++ b/src/test/store/profile-view.test.js @@ -3287,7 +3287,7 @@ describe('traced timing', function () { const callNodeInfo = selectedThreadSelectors.getCallNodeInfo(getState()); - const { running, self } = ensureExists( + const { total, self } = ensureExists( selectedThreadSelectors.getTracedTiming(getState()), 'Expected to get a traced timing.' ); @@ -3298,7 +3298,7 @@ describe('traced timing', function () { const callNodeIndex = ensureExists( callNodeInfo.getCallNodeIndexFromPath(callNodePath) ); - return { self: self[callNodeIndex], total: running[callNodeIndex] }; + return { self: self[callNodeIndex], total: total[callNodeIndex] }; }, profile, }; @@ -3347,8 +3347,8 @@ describe('traced timing', function () { ); // This test is a bit hard to assert in a really readable fasshion. - // Running: [ 1, 4, 4, 1.5, 1, 1 ] - // Self: [ 1, 4, 0, 1.5, 0, 0 ] + // total: [ 1, 4, 4, 1.5, 1, 1 ] + // Self: [ 1, 4, 0, 1.5, 0, 0 ] expect(getSelfAndTotal(A)).toEqual({ self: 1, total: 1 }); expect(getSelfAndTotal(B)).toEqual({ self: 4, total: 4 }); diff --git a/src/types/profile-derived.js b/src/types/profile-derived.js index b524d1162d..956ad7a456 100644 --- a/src/types/profile-derived.js +++ b/src/types/profile-derived.js @@ -673,17 +673,6 @@ export type CallNodeLeafAndSummary = {| */ export type SelfAndTotal = {| self: number, total: number |}; -/** - * This struct contains the traced timing for each call node. The arrays are indexed - * by the CallNodeIndex, and the values in the Float32Arrays are Milliseconds. The - * traced timing is computed by summing the distance between samples for a given call - * node. See the `computeTracedTiming` for more details. - */ -export type TracedTiming = {| - +self: Float32Array, - +running: Float32Array, -|}; - /* * Event delay table that holds the pre-processed event delay values and other * statistics about it. From cfed73b86bf3a5a2be08fd1d399806ef5cfbc63b Mon Sep 17 00:00:00 2001 From: Markus Stange Date: Wed, 17 Jan 2024 17:16:05 -0500 Subject: [PATCH 7/8] Use getSampleIndexToCallNodeIndexForPreviewFilteredThread. --- src/selectors/per-thread/stack-sample.js | 25 ++++++++++++------------ 1 file changed, 13 insertions(+), 12 deletions(-) diff --git a/src/selectors/per-thread/stack-sample.js b/src/selectors/per-thread/stack-sample.js index 1852b81fef..d10525c03e 100644 --- a/src/selectors/per-thread/stack-sample.js +++ b/src/selectors/per-thread/stack-sample.js @@ -234,6 +234,15 @@ export function getStackAndSampleSelectorsPerThread( ) ); + const getSampleIndexToCallNodeIndexForPreviewFilteredThread: Selector< + Array, + > = createSelector( + (state) => + threadSelectors.getPreviewFilteredSamplesForCallTree(state).stack, + (state) => getCallNodeInfo(state).getStackIndexToCallNodeIndex(), + ProfileData.getSampleIndexToCallNodeIndex + ); + const getSampleIndexToCallNodeIndexForTabFilteredThread: Selector< Array, > = createSelector( @@ -302,13 +311,9 @@ export function getStackAndSampleSelectorsPerThread( const getCallTreeTimings: Selector = createSelector( threadSelectors.getPreviewFilteredSamplesForCallTree, + getSampleIndexToCallNodeIndexForPreviewFilteredThread, getCallNodeInfo, - (samples, callNodeInfo) => { - const sampleIndexToCallNodeIndex = - ProfileData.getSampleIndexToCallNodeIndex( - samples.stack, - callNodeInfo.getStackIndexToCallNodeIndex() - ); + (samples, sampleIndexToCallNodeIndex, callNodeInfo) => { const callNodeLeafAndSummary = CallTree.computeCallNodeLeafAndSummary( samples, sampleIndexToCallNodeIndex, @@ -346,14 +351,10 @@ export function getStackAndSampleSelectorsPerThread( const getTracedTiming: Selector = createSelector( threadSelectors.getPreviewFilteredSamplesForCallTree, + getSampleIndexToCallNodeIndexForPreviewFilteredThread, getCallNodeInfo, ProfileSelectors.getProfileInterval, - (samples, callNodeInfo, interval) => { - const sampleIndexToCallNodeIndex = - ProfileData.getSampleIndexToCallNodeIndex( - samples.stack, - callNodeInfo.getStackIndexToCallNodeIndex() - ); + (samples, sampleIndexToCallNodeIndex, callNodeInfo, interval) => { const callNodeLeafAndSummary = CallTree.computeCallNodeTracedLeafAndSummary( samples, From 10074e36281976924708cf77a6c06e49bcb07fe4 Mon Sep 17 00:00:00 2001 From: Markus Stange Date: Thu, 18 Jan 2024 18:08:43 -0500 Subject: [PATCH 8/8] Split out part of CallTree into CallTreeInternal. This will let us add a second implementation of CallTreeInternal for the inverted tree. What's still in CallTree will be used for both the inverted and the non-inverted tree. --- src/profile-logic/call-tree.js | 174 +++++---- .../__snapshots__/profile-view.test.js.snap | 342 +++++++++++++++--- 2 files changed, 394 insertions(+), 122 deletions(-) diff --git a/src/profile-logic/call-tree.js b/src/profile-logic/call-tree.js index 0f80f8c284..b8e01e0525 100644 --- a/src/profile-logic/call-tree.js +++ b/src/profile-logic/call-tree.js @@ -28,6 +28,7 @@ import type { ExtraBadgeInfo, BottomBoxInfo, CallNodeLeafAndSummary, + SelfAndTotal, } from 'firefox-profiler/types'; import ExtensionIcon from '../../res/img/svg/extension.svg'; @@ -63,18 +64,111 @@ function extractFaviconFromLibname(libname: string): string | null { } } -export class CallTree { - _categories: CategoryList; +interface CallTreeInternal { + hasChildren(callNodeIndex: IndexIntoCallNodeTable): boolean; + createChildren(nodeIndex: IndexIntoCallNodeTable): CallNodeChildren; + createRoots(): CallNodeChildren; + getSelfAndTotal(nodeIndex: IndexIntoCallNodeTable): SelfAndTotal; + findHeaviestPathInSubtree( + callNodeIndex: IndexIntoCallNodeTable + ): CallNodePath; +} + +export class CallTreeInternalImpl implements CallTreeInternal { _callNodeInfo: CallNodeInfo; _callNodeTable: CallNodeTable; _callTreeTimings: CallTreeTimings; _callNodeHasChildren: Uint8Array; // A table column matching the callNodeTable + + constructor(callNodeInfo: CallNodeInfo, callTreeTimings: CallTreeTimings) { + this._callNodeInfo = callNodeInfo; + this._callNodeTable = callNodeInfo.getCallNodeTable(); + this._callTreeTimings = callTreeTimings; + this._callNodeHasChildren = callTreeTimings.callNodeHasChildren; + } + + _getFirstChildIndex( + callNodeIndex: IndexIntoCallNodeTable | -1 + ): IndexIntoCallNodeTable | -1 { + if (callNodeIndex === -1) { + return this._callNodeTable.length !== 0 ? 0 : -1; + } + const subtreeRangeEnd = this._callNodeTable.subtreeRangeEnd[callNodeIndex]; + if (subtreeRangeEnd !== callNodeIndex + 1) { + return callNodeIndex + 1; + } + return -1; + } + + createRoots() { + return this.createChildren(-1); + } + + createChildren(callNodeIndex: IndexIntoCallNodeTable): CallNodeChildren { + const firstChild = this._getFirstChildIndex(callNodeIndex); + const children = []; + for ( + let childCallNodeIndex = firstChild; + childCallNodeIndex !== -1; + childCallNodeIndex = this._callNodeTable.nextSibling[childCallNodeIndex] + ) { + const childTotalSummary = this._callTreeTimings.total[childCallNodeIndex]; + const childHasChildren = this._callNodeHasChildren[childCallNodeIndex]; + + if (childTotalSummary !== 0 || childHasChildren !== 0) { + children.push(childCallNodeIndex); + } + } + children.sort( + (a, b) => + Math.abs(this._callTreeTimings.total[b]) - + Math.abs(this._callTreeTimings.total[a]) + ); + return children; + } + + hasChildren(callNodeIndex: IndexIntoCallNodeTable): boolean { + return this._callNodeHasChildren[callNodeIndex] !== 0; + } + + getSelfAndTotal(callNodeIndex: IndexIntoCallNodeTable): SelfAndTotal { + const self = this._callTreeTimings.self[callNodeIndex]; + const total = this._callTreeTimings.total[callNodeIndex]; + return { self, total }; + } + + findHeaviestPathInSubtree( + callNodeIndex: IndexIntoCallNodeTable + ): CallNodePath { + const rangeEnd = this._callNodeTable.subtreeRangeEnd[callNodeIndex]; + + // Find the call node with the highest leaf time. + let maxNode = -1; + let maxAbs = 0; + for (let nodeIndex = callNodeIndex; nodeIndex < rangeEnd; nodeIndex++) { + const nodeLeaf = Math.abs(this._callTreeTimings.leaf[nodeIndex]); + if (maxNode === -1 || nodeLeaf > maxAbs) { + maxNode = nodeIndex; + maxAbs = nodeLeaf; + } + } + + return this._callNodeInfo.getCallNodePathFromIndex(maxNode); + } +} + +export class CallTree { + _categories: CategoryList; + _internal: CallTreeInternal; + _callNodeInfo: CallNodeInfo; + _callNodeTable: CallNodeTable; _thread: Thread; _rootTotalSummary: number; _displayDataByIndex: Map; // _children is indexed by IndexIntoCallNodeTable. Since they are // integers, using an array directly is faster than going through a Map. _children: Array; + _roots: IndexIntoCallNodeTable[]; _isHighPrecision: boolean; _weightType: WeightType; @@ -82,70 +176,39 @@ export class CallTree { thread: Thread, categories: CategoryList, callNodeInfo: CallNodeInfo, - callTreeTimings: CallTreeTimings, + internal: CallTreeInternal, + rootTotalSummary: number, isHighPrecision: boolean, weightType: WeightType ) { this._categories = categories; + this._internal = internal; this._callNodeInfo = callNodeInfo; this._callNodeTable = callNodeInfo.getCallNodeTable(); - this._callTreeTimings = callTreeTimings; - this._callNodeHasChildren = callTreeTimings.callNodeHasChildren; this._thread = thread; - this._rootTotalSummary = callTreeTimings.rootTotalSummary; + this._rootTotalSummary = rootTotalSummary; this._displayDataByIndex = new Map(); this._children = []; + this._roots = internal.createRoots(); this._isHighPrecision = isHighPrecision; this._weightType = weightType; } - _getFirstChildIndex( - callNodeIndex: IndexIntoCallNodeTable | -1 - ): IndexIntoCallNodeTable | -1 { - if (callNodeIndex === -1) { - return this._callNodeTable.length !== 0 ? 0 : -1; - } - const subtreeRangeEnd = this._callNodeTable.subtreeRangeEnd[callNodeIndex]; - if (subtreeRangeEnd !== callNodeIndex + 1) { - return callNodeIndex + 1; - } - return -1; - } - getRoots() { - return this.getChildren(-1); + return this._roots; } getChildren(callNodeIndex: IndexIntoCallNodeTable): CallNodeChildren { let children = this._children[callNodeIndex]; if (children === undefined) { - children = []; - const firstChild = this._getFirstChildIndex(callNodeIndex); - for ( - let childCallNodeIndex = firstChild; - childCallNodeIndex !== -1; - childCallNodeIndex = this._callNodeTable.nextSibling[childCallNodeIndex] - ) { - const childTotalSummary = - this._callTreeTimings.total[childCallNodeIndex]; - const childHasChildren = this._callNodeHasChildren[childCallNodeIndex]; - - if (childTotalSummary !== 0 || childHasChildren !== 0) { - children.push(childCallNodeIndex); - } - } - children.sort( - (a, b) => - Math.abs(this._callTreeTimings.total[b]) - - Math.abs(this._callTreeTimings.total[a]) - ); + children = this._internal.createChildren(callNodeIndex); this._children[callNodeIndex] = children; } return children; } hasChildren(callNodeIndex: IndexIntoCallNodeTable): boolean { - return this._callNodeHasChildren[callNodeIndex] !== 0; + return this._internal.hasChildren(callNodeIndex); } _addDescendantsToSet( @@ -181,9 +244,9 @@ export class CallTree { const funcName = this._thread.stringTable.getString( this._thread.funcTable.name[funcIndex] ); - const total = this._callTreeTimings.total[callNodeIndex]; + + const { self, total } = this._internal.getSelfAndTotal(callNodeIndex); const totalRelative = total / this._rootTotalSummary; - const self = this._callTreeTimings.self[callNodeIndex]; const selfRelative = self / this._rootTotalSummary; return { @@ -382,30 +445,12 @@ export class CallTree { if (callNodeIndex === null) { return []; } - const heaviestPath = this.findHeaviestPathInSubtree(callNodeIndex); + const heaviestPath = + this._internal.findHeaviestPathInSubtree(callNodeIndex); const startingDepth = this._callNodeTable.depth[callNodeIndex]; const partialPath = heaviestPath.slice(startingDepth); return partialPath.reverse(); } - - findHeaviestPathInSubtree( - callNodeIndex: IndexIntoCallNodeTable - ): CallNodePath { - const rangeEnd = this._callNodeTable.subtreeRangeEnd[callNodeIndex]; - - // Find the call node with the highest leaf time. - let maxNode = -1; - let maxAbs = 0; - for (let nodeIndex = callNodeIndex; nodeIndex < rangeEnd; nodeIndex++) { - const nodeLeaf = Math.abs(this._callTreeTimings.leaf[nodeIndex]); - if (maxNode === -1 || nodeLeaf > maxAbs) { - maxNode = nodeIndex; - maxAbs = nodeLeaf; - } - } - - return this._callNodeInfo.getCallNodePathFromIndex(maxNode); - } } // In an inverted profile, all the amount of self unit (time, bytes, count, etc.) is @@ -547,7 +592,8 @@ export function getCallTree( thread, categories, callNodeInfo, - callTreeTimings, + new CallTreeInternalImpl(callNodeInfo, callTreeTimings), + callTreeTimings.rootTotalSummary, Boolean(thread.isJsTracer), weightType ); diff --git a/src/test/store/__snapshots__/profile-view.test.js.snap b/src/test/store/__snapshots__/profile-view.test.js.snap index ba34b58ba5..a2a9958e8e 100644 --- a/src/test/store/__snapshots__/profile-view.test.js.snap +++ b/src/test/store/__snapshots__/profile-view.test.js.snap @@ -2304,17 +2304,6 @@ CallNodeInfoImpl { exports[`snapshots of selectors/profile matches the last stored run of selectedThreadSelector.getCallTree 1`] = ` CallTree { - "_callNodeHasChildren": Uint8Array [ - 1, - 1, - 0, - 0, - 0, - 1, - 0, - 0, - 0, - ], "_callNodeInfo": CallNodeInfoImpl { "_cache": Map {}, "_callNodeTable": Object { @@ -2536,53 +2525,6 @@ CallTree { 9, ], }, - "_callTreeTimings": Object { - "callNodeHasChildren": Uint8Array [ - 1, - 1, - 0, - 0, - 0, - 1, - 0, - 0, - 0, - ], - "leaf": Float32Array [ - 0, - 0, - 0, - 0, - 0, - 0, - 2, - 0, - 0, - ], - "rootTotalSummary": 2, - "self": Float32Array [ - 0, - 0, - 0, - 0, - 0, - 0, - 2, - 0, - 0, - ], - "total": Float32Array [ - 2, - 2, - 0, - 0, - 0, - 2, - 2, - 0, - 0, - ], - }, "_categories": Array [ Object { "color": "grey", @@ -2643,8 +2585,292 @@ CallTree { ], "_children": Array [], "_displayDataByIndex": Map {}, + "_internal": CallTreeInternalImpl { + "_callNodeHasChildren": Uint8Array [ + 1, + 1, + 0, + 0, + 0, + 1, + 0, + 0, + 0, + ], + "_callNodeInfo": CallNodeInfoImpl { + "_cache": Map {}, + "_callNodeTable": Object { + "category": Int32Array [ + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + ], + "depth": Array [ + 0, + 1, + 2, + 2, + 3, + 2, + 3, + 2, + 3, + ], + "func": Int32Array [ + 0, + 1, + 2, + 3, + 4, + 5, + 6, + 7, + 8, + ], + "innerWindowID": Float64Array [ + 0, + 0, + 0, + 0, + 0, + 0, + 2, + 0, + 0, + ], + "length": 9, + "maxDepth": 3, + "nextSibling": Int32Array [ + -1, + -1, + 3, + 5, + -1, + 7, + -1, + -1, + -1, + ], + "prefix": Int32Array [ + -1, + 0, + 1, + 1, + 3, + 1, + 5, + 1, + 7, + ], + "sourceFramesInlinedIntoSymbol": Array [ + null, + null, + null, + null, + null, + null, + null, + null, + null, + ], + "subcategory": Int32Array [ + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + ], + "subtreeRangeEnd": Uint32Array [ + 9, + 9, + 3, + 5, + 5, + 7, + 7, + 9, + 9, + ], + }, + "_isInverted": false, + "_stackIndexToCallNodeIndex": Int32Array [ + 0, + 1, + 2, + 3, + 4, + 5, + 6, + 7, + 8, + ], + }, + "_callNodeTable": Object { + "category": Int32Array [ + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + ], + "depth": Array [ + 0, + 1, + 2, + 2, + 3, + 2, + 3, + 2, + 3, + ], + "func": Int32Array [ + 0, + 1, + 2, + 3, + 4, + 5, + 6, + 7, + 8, + ], + "innerWindowID": Float64Array [ + 0, + 0, + 0, + 0, + 0, + 0, + 2, + 0, + 0, + ], + "length": 9, + "maxDepth": 3, + "nextSibling": Int32Array [ + -1, + -1, + 3, + 5, + -1, + 7, + -1, + -1, + -1, + ], + "prefix": Int32Array [ + -1, + 0, + 1, + 1, + 3, + 1, + 5, + 1, + 7, + ], + "sourceFramesInlinedIntoSymbol": Array [ + null, + null, + null, + null, + null, + null, + null, + null, + null, + ], + "subcategory": Int32Array [ + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + ], + "subtreeRangeEnd": Uint32Array [ + 9, + 9, + 3, + 5, + 5, + 7, + 7, + 9, + 9, + ], + }, + "_callTreeTimings": Object { + "callNodeHasChildren": Uint8Array [ + 1, + 1, + 0, + 0, + 0, + 1, + 0, + 0, + 0, + ], + "leaf": Float32Array [ + 0, + 0, + 0, + 0, + 0, + 0, + 2, + 0, + 0, + ], + "rootTotalSummary": 2, + "self": Float32Array [ + 0, + 0, + 0, + 0, + 0, + 0, + 2, + 0, + 0, + ], + "total": Float32Array [ + 2, + 2, + 0, + 0, + 0, + 2, + 2, + 0, + 0, + ], + }, + }, "_isHighPrecision": false, "_rootTotalSummary": 2, + "_roots": Array [ + 0, + ], "_thread": Object { "frameTable": Object { "address": Array [