From 41fcaac1daeb5254c882fa47a841813b33c1bdbe Mon Sep 17 00:00:00 2001 From: Markus Stange Date: Sat, 2 Dec 2023 13:53:35 -0500 Subject: [PATCH 1/3] Change callNodeChildCount to callNodeHasChildren. We no longer need the child count, because the call tree's getChildren() method now uses nextSibling to find the children, so it doesn't need the child count any more for ending the search early. And the new flame graph implementation has dropped the other use of the child count. Having just one byte per call node rather than four bytes might improve the performance of computeCallTreeCountsAndSummary slightly, because it needs to touch fewer bytes in memory. --- src/profile-logic/call-tree.js | 36 ++++++++++--------- .../__snapshots__/profile-view.test.js.snap | 2 +- src/test/unit/profile-tree.test.js | 2 +- 3 files changed, 22 insertions(+), 18 deletions(-) diff --git a/src/profile-logic/call-tree.js b/src/profile-logic/call-tree.js index 9f2f7876dc..6a52b612b6 100644 --- a/src/profile-logic/call-tree.js +++ b/src/profile-logic/call-tree.js @@ -46,7 +46,7 @@ type CallNodeSummary = { total: Float32Array, }; export type CallTreeCountsAndSummary = { - callNodeChildCount: Uint32Array, + callNodeHasChildren: Uint8Array, callNodeSummary: CallNodeSummary, rootCount: number, rootTotalSummary: number, @@ -74,7 +74,7 @@ export class CallTree { _callNodeInfo: CallNodeInfo; _callNodeTable: CallNodeTable; _callNodeSummary: CallNodeSummary; - _callNodeChildCount: Uint32Array; // A table column matching the callNodeTable + _callNodeHasChildren: Uint8Array; // A table column matching the callNodeTable _thread: Thread; _rootTotalSummary: number; _rootCount: number; @@ -90,7 +90,7 @@ export class CallTree { categories: CategoryList, callNodeInfo: CallNodeInfo, callNodeSummary: CallNodeSummary, - callNodeChildCount: Uint32Array, + callNodeHasChildren: Uint8Array, rootTotalSummary: number, rootCount: number, isHighPrecision: boolean, @@ -100,7 +100,7 @@ export class CallTree { this._callNodeInfo = callNodeInfo; this._callNodeTable = callNodeInfo.callNodeTable; this._callNodeSummary = callNodeSummary; - this._callNodeChildCount = callNodeChildCount; + this._callNodeHasChildren = callNodeHasChildren; this._thread = thread; this._rootTotalSummary = rootTotalSummary; this._rootCount = rootCount; @@ -139,9 +139,9 @@ export class CallTree { ) { const childTotalSummary = this._callNodeSummary.total[childCallNodeIndex]; - const childChildCount = this._callNodeChildCount[childCallNodeIndex]; + const childHasChildren = this._callNodeHasChildren[childCallNodeIndex]; - if (childTotalSummary !== 0 || childChildCount !== 0) { + if (childTotalSummary !== 0 || childHasChildren !== 0) { children.push(childCallNodeIndex); } } @@ -525,7 +525,7 @@ export function computeCallTreeCountsAndSummary( // Compute the following variables: const callNodeTotalSummary = new Float32Array(callNodeTable.length); - const callNodeChildCount = new Uint32Array(callNodeTable.length); + const callNodeHasChildren = new Uint8Array(callNodeTable.length); let rootTotalSummary = 0; let rootCount = 0; @@ -542,7 +542,7 @@ export function computeCallTreeCountsAndSummary( ) { callNodeTotalSummary[callNodeIndex] += callNodeLeaf[callNodeIndex]; rootTotalSummary += abs(callNodeLeaf[callNodeIndex]); - const hasChildren = callNodeChildCount[callNodeIndex] !== 0; + const hasChildren = callNodeHasChildren[callNodeIndex] !== 0; const hasTotalValue = callNodeTotalSummary[callNodeIndex] !== 0; if (!hasChildren && !hasTotalValue) { @@ -555,7 +555,7 @@ export function computeCallTreeCountsAndSummary( } else { callNodeTotalSummary[prefixCallNode] += callNodeTotalSummary[callNodeIndex]; - callNodeChildCount[prefixCallNode]++; + callNodeHasChildren[prefixCallNode] = 1; } } @@ -565,7 +565,7 @@ export function computeCallTreeCountsAndSummary( leaf: callNodeLeaf, total: callNodeTotalSummary, }, - callNodeChildCount, + callNodeHasChildren, rootTotalSummary, rootCount, }; @@ -582,15 +582,19 @@ export function getCallTree( weightType: WeightType ): CallTree { return timeCode('getCallTree', () => { - const { callNodeSummary, callNodeChildCount, rootTotalSummary, rootCount } = - callTreeCountsAndSummary; + const { + callNodeSummary, + callNodeHasChildren, + rootTotalSummary, + rootCount, + } = callTreeCountsAndSummary; return new CallTree( thread, categories, callNodeInfo, callNodeSummary, - callNodeChildCount, + callNodeHasChildren, rootTotalSummary, rootCount, Boolean(thread.isJsTracer), @@ -730,7 +734,7 @@ export function computeTracedTiming( // Compute the following variables: const callNodeTotalSummary = new Float32Array(callNodeTable.length); - const callNodeChildCount = new Uint32Array(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 @@ -741,7 +745,7 @@ export function computeTracedTiming( callNodeIndex-- ) { callNodeTotalSummary[callNodeIndex] += callNodeLeaf[callNodeIndex]; - const hasChildren = callNodeChildCount[callNodeIndex] !== 0; + const hasChildren = callNodeHasChildren[callNodeIndex] !== 0; const hasTotalValue = callNodeTotalSummary[callNodeIndex] !== 0; if (!hasChildren && !hasTotalValue) { @@ -752,7 +756,7 @@ export function computeTracedTiming( if (prefixCallNode !== -1) { callNodeTotalSummary[prefixCallNode] += callNodeTotalSummary[callNodeIndex]; - callNodeChildCount[prefixCallNode]++; + callNodeHasChildren[prefixCallNode] = 1; } } diff --git a/src/test/store/__snapshots__/profile-view.test.js.snap b/src/test/store/__snapshots__/profile-view.test.js.snap index 1810cd7c30..20a37847dc 100644 --- a/src/test/store/__snapshots__/profile-view.test.js.snap +++ b/src/test/store/__snapshots__/profile-view.test.js.snap @@ -2292,7 +2292,7 @@ Object { exports[`snapshots of selectors/profile matches the last stored run of selectedThreadSelector.getCallTree 1`] = ` CallTree { - "_callNodeChildCount": Uint32Array [ + "_callNodeHasChildren": Uint8Array [ 1, 1, 0, diff --git a/src/test/unit/profile-tree.test.js b/src/test/unit/profile-tree.test.js index 825fae3dd9..f749b9101a 100644 --- a/src/test/unit/profile-tree.test.js +++ b/src/test/unit/profile-tree.test.js @@ -83,7 +83,7 @@ describe('unfiltered call tree', function () { ).toEqual({ rootCount: 1, rootTotalSummary: 3, - callNodeChildCount: new Uint32Array([1, 2, 2, 1, 0, 1, 0, 1, 0]), + 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]), From d3e0420861b29e55f239bf907a0885362e7caed1 Mon Sep 17 00:00:00 2001 From: Markus Stange Date: Sat, 2 Dec 2023 15:06:41 -0500 Subject: [PATCH 2/3] Rename CallTreeCountsAndSummary to CallTreeTimings. --- src/profile-logic/call-tree.js | 12 +++---- src/profile-logic/flame-graph.js | 6 ++-- src/selectors/per-thread/stack-sample.js | 41 ++++++++++++------------ src/test/fixtures/utils.js | 6 ++-- src/test/unit/profile-tree.test.js | 16 ++++----- 5 files changed, 40 insertions(+), 41 deletions(-) diff --git a/src/profile-logic/call-tree.js b/src/profile-logic/call-tree.js index 6a52b612b6..7d0e37c3e8 100644 --- a/src/profile-logic/call-tree.js +++ b/src/profile-logic/call-tree.js @@ -45,7 +45,7 @@ type CallNodeSummary = { leaf: Float32Array, total: Float32Array, }; -export type CallTreeCountsAndSummary = { +export type CallTreeTimings = { callNodeHasChildren: Uint8Array, callNodeSummary: CallNodeSummary, rootCount: number, @@ -512,12 +512,12 @@ function _getStackSelf( * what type of weight is in the SamplesLikeTable. For instance, it could be * milliseconds, sample counts, or bytes. */ -export function computeCallTreeCountsAndSummary( +export function computeCallTreeTimings( samples: SamplesLikeTable, sampleIndexToCallNodeIndex: Array, { callNodeTable }: CallNodeInfo, invertCallstack: boolean -): CallTreeCountsAndSummary { +): CallTreeTimings { // Inverted trees need a different method for computing the timing. const { callNodeSelf, callNodeLeaf } = invertCallstack ? _getInvertedStackSelf(samples, callNodeTable, sampleIndexToCallNodeIndex) @@ -578,7 +578,7 @@ export function getCallTree( thread: Thread, callNodeInfo: CallNodeInfo, categories: CategoryList, - callTreeCountsAndSummary: CallTreeCountsAndSummary, + callTreeTimings: CallTreeTimings, weightType: WeightType ): CallTree { return timeCode('getCallTree', () => { @@ -587,7 +587,7 @@ export function getCallTree( callNodeHasChildren, rootTotalSummary, rootCount, - } = callTreeCountsAndSummary; + } = callTreeTimings; return new CallTree( thread, @@ -674,7 +674,7 @@ export function extractSamplesLikeTable( } /** - * This function is extremely similar to computeCallTreeCountsAndSummary, + * This function is extremely similar to computeCallTreeTimings, * 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 diff --git a/src/profile-logic/flame-graph.js b/src/profile-logic/flame-graph.js index 7b26fa12fb..4d85d96774 100644 --- a/src/profile-logic/flame-graph.js +++ b/src/profile-logic/flame-graph.js @@ -10,7 +10,7 @@ import type { IndexIntoCallNodeTable, } from 'firefox-profiler/types'; import type { UniqueStringArray } from 'firefox-profiler/utils/unique-string-array'; -import type { CallTreeCountsAndSummary } from './call-tree'; +import type { CallTreeTimings } from './call-tree'; import { bisectionRightByStrKey } from 'firefox-profiler/utils/bisect'; @@ -229,9 +229,9 @@ export function computeFlameGraphRows( export function getFlameGraphTiming( flameGraphRows: FlameGraphRows, callNodeTable: CallNodeTable, - callTreeCountsAndSummary: CallTreeCountsAndSummary + callTreeTimings: CallTreeTimings ): FlameGraphTiming { - const { callNodeSummary, rootTotalSummary } = callTreeCountsAndSummary; + const { callNodeSummary, rootTotalSummary } = callTreeTimings; const { total, self } = callNodeSummary; const { prefix } = callNodeTable; diff --git a/src/selectors/per-thread/stack-sample.js b/src/selectors/per-thread/stack-sample.js index ddb2e69176..e77712db35 100644 --- a/src/selectors/per-thread/stack-sample.js +++ b/src/selectors/per-thread/stack-sample.js @@ -296,32 +296,31 @@ export function getStackAndSampleSelectorsPerThread( (samples) => samples.weightType || 'samples' ); - const getCallTreeCountsAndSummary: Selector = - createSelector( - threadSelectors.getPreviewFilteredSamplesForCallTree, - getCallNodeInfo, - ProfileSelectors.getProfileInterval, - UrlState.getInvertCallstack, - (samples, callNodeInfo, interval, invertCallStack) => { - const sampleIndexToCallNodeIndex = - ProfileData.getSampleIndexToCallNodeIndex( - samples.stack, - callNodeInfo.stackIndexToCallNodeIndex - ); - return CallTree.computeCallTreeCountsAndSummary( - samples, - sampleIndexToCallNodeIndex, - callNodeInfo, - invertCallStack + const getCallTreeTimings: Selector = createSelector( + threadSelectors.getPreviewFilteredSamplesForCallTree, + getCallNodeInfo, + ProfileSelectors.getProfileInterval, + UrlState.getInvertCallstack, + (samples, callNodeInfo, interval, invertCallStack) => { + const sampleIndexToCallNodeIndex = + ProfileData.getSampleIndexToCallNodeIndex( + samples.stack, + callNodeInfo.stackIndexToCallNodeIndex ); - } - ); + return CallTree.computeCallTreeTimings( + samples, + sampleIndexToCallNodeIndex, + callNodeInfo, + invertCallStack + ); + } + ); const getCallTree: Selector = createSelector( threadSelectors.getPreviewFilteredThread, getCallNodeInfo, ProfileSelectors.getCategories, - getCallTreeCountsAndSummary, + getCallTreeTimings, getWeightTypeForCallTree, CallTree.getCallTree ); @@ -368,7 +367,7 @@ export function getStackAndSampleSelectorsPerThread( createSelector( getFlameGraphRows, (state) => getCallNodeInfo(state).callNodeTable, - getCallTreeCountsAndSummary, + getCallTreeTimings, FlameGraph.getFlameGraphTiming ); diff --git a/src/test/fixtures/utils.js b/src/test/fixtures/utils.js index 1d9120f23c..9a3367ce6d 100644 --- a/src/test/fixtures/utils.js +++ b/src/test/fixtures/utils.js @@ -4,7 +4,7 @@ // @flow import { getCallTree, - computeCallTreeCountsAndSummary, + computeCallTreeTimings, type CallTree, } from 'firefox-profiler/profile-logic/call-tree'; import { getEmptyThread } from 'firefox-profiler/profile-logic/data-structures'; @@ -127,7 +127,7 @@ export function callTreeFromProfile( thread.funcTable, defaultCategory ); - const callTreeCountsAndSummary = computeCallTreeCountsAndSummary( + const callTreeTimings = computeCallTreeTimings( thread.samples, getSampleIndexToCallNodeIndex( thread.samples.stack, @@ -140,7 +140,7 @@ export function callTreeFromProfile( thread, callNodeInfo, categories, - callTreeCountsAndSummary, + callTreeTimings, 'samples' ); } diff --git a/src/test/unit/profile-tree.test.js b/src/test/unit/profile-tree.test.js index f749b9101a..9b33892093 100644 --- a/src/test/unit/profile-tree.test.js +++ b/src/test/unit/profile-tree.test.js @@ -8,7 +8,7 @@ import { } from '../fixtures/profiles/processed-profile'; import { getCallTree, - computeCallTreeCountsAndSummary, + computeCallTreeTimings, } from '../../profile-logic/call-tree'; import { computeFlameGraphRows } from '../../profile-logic/flame-graph'; import { @@ -71,7 +71,7 @@ describe('unfiltered call tree', function () { it('yields expected results', function () { expect( - computeCallTreeCountsAndSummary( + computeCallTreeTimings( thread.samples, getSampleIndexToCallNodeIndex( thread.samples.stack, @@ -436,7 +436,7 @@ describe('inverted call tree', function () { thread.funcTable, defaultCategory ); - const callTreeCountsAndSummary = computeCallTreeCountsAndSummary( + const callTreeTimings = computeCallTreeTimings( thread.samples, getSampleIndexToCallNodeIndex( thread.samples.stack, @@ -449,7 +449,7 @@ describe('inverted call tree', function () { thread, callNodeInfo, categories, - callTreeCountsAndSummary, + callTreeTimings, 'samples' ); @@ -477,7 +477,7 @@ describe('inverted call tree', function () { invertedThread.funcTable, defaultCategory ); - const invertedCallTreeCountsAndSummary = computeCallTreeCountsAndSummary( + const invertedCallTreeTimings = computeCallTreeTimings( invertedThread.samples, getSampleIndexToCallNodeIndex( invertedThread.samples.stack, @@ -490,7 +490,7 @@ describe('inverted call tree', function () { invertedThread, invertedCallNodeInfo, categories, - invertedCallTreeCountsAndSummary, + invertedCallTreeTimings, 'samples' ); @@ -627,7 +627,7 @@ describe('diffing trees', function () { thread.funcTable, defaultCategory ); - const callTreeCountsAndSummary = computeCallTreeCountsAndSummary( + const callTreeTimings = computeCallTreeTimings( thread.samples, getSampleIndexToCallNodeIndex( thread.samples.stack, @@ -636,7 +636,7 @@ describe('diffing trees', function () { callNodeInfo, false ); - expect(callTreeCountsAndSummary.rootTotalSummary).toBe(12); + expect(callTreeTimings.rootTotalSummary).toBe(12); }); }); From 64d6cc86dd066e6e90614cad5897493b49e9e7e5 Mon Sep 17 00:00:00 2001 From: Markus Stange Date: Sat, 2 Dec 2023 15:12:09 -0500 Subject: [PATCH 3/3] Use _callNodeHasChildren in hasChildren(). This avoids calls to getChildren for nodes which are visible but not expanded. The tree view calls hasChildren in order to determine whether to show the disclosure arrow. Avoiding the call to getChildren means that we don't have to allocate the array and sort it. This is especially useful in the inverted tree, which has lots of non-expanded nodes at the root level. --- src/profile-logic/call-tree.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/profile-logic/call-tree.js b/src/profile-logic/call-tree.js index 7d0e37c3e8..af553d6686 100644 --- a/src/profile-logic/call-tree.js +++ b/src/profile-logic/call-tree.js @@ -156,7 +156,7 @@ export class CallTree { } hasChildren(callNodeIndex: IndexIntoCallNodeTable): boolean { - return this.getChildren(callNodeIndex).length !== 0; + return this._callNodeHasChildren[callNodeIndex] !== 0; } _addDescendantsToSet(