From f597380b20f0a5196ffdfe2c191f4560eb5f6595 Mon Sep 17 00:00:00 2001 From: Markus Stange Date: Fri, 19 Jan 2024 23:45:31 -0500 Subject: [PATCH 01/18] Add an inverted tree test for getSamplesSelectedStates and getTreeOrderComparator. --- src/test/unit/profile-data.test.js | 360 +++++++++++++++++++++-------- 1 file changed, 266 insertions(+), 94 deletions(-) diff --git a/src/test/unit/profile-data.test.js b/src/test/unit/profile-data.test.js index 5eff9f333f..0f16c6368f 100644 --- a/src/test/unit/profile-data.test.js +++ b/src/test/unit/profile-data.test.js @@ -859,102 +859,274 @@ describe('convertStackToCallNodeAndCategoryPath', function () { }); describe('getSamplesSelectedStates', function () { - const { - profile, - funcNamesDictPerThread: [{ A, B, D, E, F }], - } = getProfileFromTextSamples(` - A A A A A - B D B D D - C E F G - `); - const thread = profile.threads[0]; - const callNodeInfo = getCallNodeInfo( - thread.stackTable, - thread.frameTable, - thread.funcTable, - 0 - ); - const stackIndexToCallNodeIndex = - callNodeInfo.getStackIndexToNonInvertedCallNodeIndex(); - const sampleCallNodes = getSampleIndexToCallNodeIndex( - thread.samples.stack, - stackIndexToCallNodeIndex - ); - - const A_B = callNodeInfo.getCallNodeIndexFromPath([A, B]); - const A_B_F = callNodeInfo.getCallNodeIndexFromPath([A, B, F]); - const A_D = callNodeInfo.getCallNodeIndexFromPath([A, D]); - const A_D_E = callNodeInfo.getCallNodeIndexFromPath([A, D, E]); - - it('determines the selection status of all the samples', function () { - expect( - getSamplesSelectedStates( - callNodeInfo, - sampleCallNodes, - sampleCallNodes, - A_B - ) - ).toEqual([ - 'SELECTED', - 'UNSELECTED_ORDERED_AFTER_SELECTED', - 'SELECTED', - 'UNSELECTED_ORDERED_AFTER_SELECTED', - 'UNSELECTED_ORDERED_AFTER_SELECTED', - ]); - expect( - getSamplesSelectedStates( - callNodeInfo, - sampleCallNodes, - sampleCallNodes, - A_D - ) - ).toEqual([ - 'UNSELECTED_ORDERED_BEFORE_SELECTED', - 'SELECTED', - 'UNSELECTED_ORDERED_BEFORE_SELECTED', - 'SELECTED', - 'SELECTED', - ]); - expect( - getSamplesSelectedStates( - callNodeInfo, - sampleCallNodes, - sampleCallNodes, - A_B_F - ) - ).toEqual([ - 'UNSELECTED_ORDERED_BEFORE_SELECTED', - 'UNSELECTED_ORDERED_AFTER_SELECTED', - 'SELECTED', - 'UNSELECTED_ORDERED_AFTER_SELECTED', - 'UNSELECTED_ORDERED_AFTER_SELECTED', - ]); - expect( - getSamplesSelectedStates( - callNodeInfo, - sampleCallNodes, - sampleCallNodes, - A_D_E - ) - ).toEqual([ - 'UNSELECTED_ORDERED_BEFORE_SELECTED', - 'SELECTED', - 'UNSELECTED_ORDERED_BEFORE_SELECTED', - 'UNSELECTED_ORDERED_AFTER_SELECTED', - 'UNSELECTED_ORDERED_BEFORE_SELECTED', - ]); + function setup(textSamples) { + const { + profile, + funcNamesDictPerThread: [funcNamesDict], + } = getProfileFromTextSamples(textSamples); + const thread = profile.threads[0]; + const callNodeInfo = getCallNodeInfo( + thread.stackTable, + thread.frameTable, + thread.funcTable, + 0 + ); + const stackIndexToCallNodeIndex = + callNodeInfo.getStackIndexToNonInvertedCallNodeIndex(); + const sampleCallNodes = getSampleIndexToCallNodeIndex( + thread.samples.stack, + stackIndexToCallNodeIndex + ); + + const categories = ensureExists( + profile.meta.categories, + 'Expected to find categories' + ); + const defaultCategory = categories.findIndex((c) => c.name === 'Other'); + const callNodeInfoInverted = getInvertedCallNodeInfo( + thread, + callNodeInfo.getNonInvertedCallNodeTable(), + stackIndexToCallNodeIndex, + defaultCategory + ); + const stackIndexToInvertedCallNodeIndex = + callNodeInfoInverted.getStackIndexToCallNodeIndex(); + const sampleInvertedCallNodes = getSampleIndexToCallNodeIndex( + thread.samples.stack, + stackIndexToInvertedCallNodeIndex + ); + + return { + callNodeInfo, + callNodeInfoInverted, + sampleInvertedCallNodes, + sampleCallNodes, + funcNamesDict, + }; + } + + describe('non-inverted', function () { + const { + callNodeInfo, + sampleCallNodes, + funcNamesDict: { A, B, D, E, F }, + } = setup(` + A A A A A + B D B D D + C E F G + `); + + const A_B = callNodeInfo.getCallNodeIndexFromPath([A, B]); + const A_B_F = callNodeInfo.getCallNodeIndexFromPath([A, B, F]); + const A_D = callNodeInfo.getCallNodeIndexFromPath([A, D]); + const A_D_E = callNodeInfo.getCallNodeIndexFromPath([A, D, E]); + + it('determines the selection status of all the samples', function () { + expect( + getSamplesSelectedStates( + callNodeInfo, + sampleCallNodes, + sampleCallNodes, + A_B + ) + ).toEqual([ + 'SELECTED', + 'UNSELECTED_ORDERED_AFTER_SELECTED', + 'SELECTED', + 'UNSELECTED_ORDERED_AFTER_SELECTED', + 'UNSELECTED_ORDERED_AFTER_SELECTED', + ]); + expect( + getSamplesSelectedStates( + callNodeInfo, + sampleCallNodes, + sampleCallNodes, + A_D + ) + ).toEqual([ + 'UNSELECTED_ORDERED_BEFORE_SELECTED', + 'SELECTED', + 'UNSELECTED_ORDERED_BEFORE_SELECTED', + 'SELECTED', + 'SELECTED', + ]); + expect( + getSamplesSelectedStates( + callNodeInfo, + sampleCallNodes, + sampleCallNodes, + A_B_F + ) + ).toEqual([ + 'UNSELECTED_ORDERED_BEFORE_SELECTED', + 'UNSELECTED_ORDERED_AFTER_SELECTED', + 'SELECTED', + 'UNSELECTED_ORDERED_AFTER_SELECTED', + 'UNSELECTED_ORDERED_AFTER_SELECTED', + ]); + expect( + getSamplesSelectedStates( + callNodeInfo, + sampleCallNodes, + sampleCallNodes, + A_D_E + ) + ).toEqual([ + 'UNSELECTED_ORDERED_BEFORE_SELECTED', + 'SELECTED', + 'UNSELECTED_ORDERED_BEFORE_SELECTED', + 'UNSELECTED_ORDERED_AFTER_SELECTED', + 'UNSELECTED_ORDERED_BEFORE_SELECTED', + ]); + }); + + it('can sort the samples based on their selection status', function () { + const comparator = getTreeOrderComparator(sampleCallNodes); + const samples = [4, 1, 3, 0, 2]; // some random order + samples.sort(comparator); + expect(samples).toEqual([0, 2, 4, 1, 3]); + expect(comparator(0, 0)).toBe(0); + expect(comparator(1, 1)).toBe(0); + expect(comparator(4, 4)).toBe(0); + expect(comparator(0, 2)).toBeLessThan(0); + expect(comparator(2, 0)).toBeGreaterThan(0); + }); }); - it('can sort the samples based on their selection status', function () { - const comparator = getTreeOrderComparator(sampleCallNodes); - const samples = [4, 1, 3, 0, 2]; // some random order - samples.sort(comparator); - expect(samples).toEqual([0, 2, 4, 1, 3]); - expect(comparator(0, 0)).toBe(0); - expect(comparator(1, 1)).toBe(0); - expect(comparator(4, 4)).toBe(0); - expect(comparator(0, 2)).toBeLessThan(0); - expect(comparator(2, 0)).toBeGreaterThan(0); + describe('inverted', function () { + /** + * - [cn0] A = A + * - [cn1] B = A -> B + * - [cn2] A = A -> B -> A + * - [cn3] C = A -> B -> C + * - [cn4] A = A -> A + * - [cn5] B = A -> A -> B + * - [cn6] C = A -> C + * + * + * - [in0] A + * - [in1] A + * - [in2] B + * - [in3] A + * - [in4] B + * - [in5] A + * - [in6] A + * - [in7] C + * - [in8] A + * - [in9] B + * - [in10] A + * */ + const { + callNodeInfoInverted, + sampleInvertedCallNodes, + funcNamesDict: { A, B, C }, + } = setup(` + A A A A A A A + A B B C B A + A C B + `); + + const inBA = callNodeInfoInverted.getCallNodeIndexFromPath([B, A]); + const inCBA = callNodeInfoInverted.getCallNodeIndexFromPath([C, B, A]); + const inB = callNodeInfoInverted.getCallNodeIndexFromPath([B]); + + // 0 1 2 3 4 5 6 + // A A A A A A A + // A B B C B A + // A C B + + it('determines the selection status of all the samples', function () { + // Test B <- A <- ... + // Only samples 2 and 6 have stacks ending in ... -> A -> B + expect( + getSamplesSelectedStates( + callNodeInfoInverted, + sampleInvertedCallNodes, + sampleInvertedCallNodes, + inBA + ) + ).toEqual([ + 'UNSELECTED_ORDERED_BEFORE_SELECTED', + 'UNSELECTED_ORDERED_BEFORE_SELECTED', + 'SELECTED', + 'UNSELECTED_ORDERED_BEFORE_SELECTED', + 'UNSELECTED_ORDERED_AFTER_SELECTED', + 'UNSELECTED_ORDERED_AFTER_SELECTED', + 'SELECTED', + ]); + // Test C <- B <- A <- ... + // Only sample 5 has a stack ending in ... -> A -> B -> C + expect( + getSamplesSelectedStates( + callNodeInfoInverted, + sampleInvertedCallNodes, + sampleInvertedCallNodes, + inCBA + ) + ).toEqual([ + 'UNSELECTED_ORDERED_BEFORE_SELECTED', + 'UNSELECTED_ORDERED_BEFORE_SELECTED', + 'UNSELECTED_ORDERED_BEFORE_SELECTED', + 'UNSELECTED_ORDERED_BEFORE_SELECTED', + 'UNSELECTED_ORDERED_BEFORE_SELECTED', + 'SELECTED', + 'UNSELECTED_ORDERED_BEFORE_SELECTED', + ]); + // Test B <- ... + // Only samples 2 and 6 have stacks ending in ... -> B + expect( + getSamplesSelectedStates( + callNodeInfoInverted, + sampleInvertedCallNodes, + sampleInvertedCallNodes, + inB + ) + ).toEqual([ + 'UNSELECTED_ORDERED_BEFORE_SELECTED', + 'UNSELECTED_ORDERED_BEFORE_SELECTED', + 'SELECTED', + 'UNSELECTED_ORDERED_BEFORE_SELECTED', + 'UNSELECTED_ORDERED_AFTER_SELECTED', + 'UNSELECTED_ORDERED_AFTER_SELECTED', + 'SELECTED', + ]); + }); + + it('can sort the samples based on their selection status', function () { + const comparator = getTreeOrderComparator(sampleInvertedCallNodes); + + /** + * original order (non-inverted): + * 0 1 2 3 4 5 6 + * A A A A A A A + * A B B C B A + * A C B + * + * sorted order ("inverted" if you read from bottom to top): + * A A A + * A B A A A B + * A A A B B C C + * 0 1 3 2 6 4 5 + */ + + // A should come before A <- B. + expect(comparator(0, 2)).toBeLessThan(0); + expect(comparator(2, 0)).toBeGreaterThan(0); + + // A <- A should come before A <- B. + expect(comparator(1, 2)).toBeLessThan(0); + expect(comparator(2, 1)).toBeGreaterThan(0); + + // Sort a random order of samples, make sure it's correct. + const samples = [5, 6, 0, 1, 2, 3, 4]; + samples.sort(comparator); + expect(samples).toEqual([0, 1, 3, 2, 6, 4, 5]); + + // The same sample index should always compare equally. + expect(comparator(0, 0)).toBe(0); + expect(comparator(1, 1)).toBe(0); + expect(comparator(4, 4)).toBe(0); + }); }); }); From 3a97c4582a95564de95b263b604130ed9a843b12 Mon Sep 17 00:00:00 2001 From: Markus Stange Date: Tue, 23 Jan 2024 11:45:42 -0500 Subject: [PATCH 02/18] Add CallNodeInfoInverted. This just adds a new interface that we can hang functionality off of which is specific to the inverted tree. No functional changes. --- src/profile-logic/call-node-info.js | 50 +++++++++++++++---- src/profile-logic/profile-data.js | 13 +++-- .../__snapshots__/profile-view.test.js.snap | 3 -- src/types/profile-derived.js | 13 +++++ 4 files changed, 58 insertions(+), 21 deletions(-) diff --git a/src/profile-logic/call-node-info.js b/src/profile-logic/call-node-info.js index cf28cd6747..3b42803169 100644 --- a/src/profile-logic/call-node-info.js +++ b/src/profile-logic/call-node-info.js @@ -9,6 +9,7 @@ import { hashPath } from 'firefox-profiler/utils/path'; import type { IndexIntoFuncTable, CallNodeInfo, + CallNodeInfoInverted, CallNodeTable, CallNodePath, IndexIntoCallNodeTable, @@ -16,26 +17,27 @@ import type { /** * The implementation of the CallNodeInfo interface. + * + * CallNodeInfoInvertedImpl inherits from this class and shares this implementation. + * By the end of this commit stack, it will no longer inherit from this class and + * will have its own implementation. */ export class CallNodeInfoImpl implements CallNodeInfo { - // If true, call node indexes describe nodes in the inverted call tree. - _isInverted: boolean; - // The call node table. This is either the inverted or the non-inverted call - // node table, depending on _isInverted. + // node table, depending on isInverted(). _callNodeTable: CallNodeTable; - // The non-inverted call node table, regardless of _isInverted. + // The non-inverted call node table, regardless of isInverted(). _nonInvertedCallNodeTable: CallNodeTable; // The mapping of stack index to corresponding call node index. This maps to // either the inverted or the non-inverted call node table, depending on - // _isInverted. + // isInverted(). _stackIndexToCallNodeIndex: Int32Array; // The mapping of stack index to corresponding non-inverted call node index. // This always maps to the non-inverted call node table, regardless of - // _isInverted. + // isInverted(). _stackIndexToNonInvertedCallNodeIndex: Int32Array; // This is a Map. This map speeds up @@ -47,19 +49,23 @@ export class CallNodeInfoImpl implements CallNodeInfo { callNodeTable: CallNodeTable, nonInvertedCallNodeTable: CallNodeTable, stackIndexToCallNodeIndex: Int32Array, - stackIndexToNonInvertedCallNodeIndex: Int32Array, - isInverted: boolean + stackIndexToNonInvertedCallNodeIndex: Int32Array ) { this._callNodeTable = callNodeTable; this._nonInvertedCallNodeTable = nonInvertedCallNodeTable; this._stackIndexToCallNodeIndex = stackIndexToCallNodeIndex; this._stackIndexToNonInvertedCallNodeIndex = stackIndexToNonInvertedCallNodeIndex; - this._isInverted = isInverted; } isInverted(): boolean { - return this._isInverted; + // Overridden in subclass + return false; + } + + asInverted(): CallNodeInfoInverted | null { + // Overridden in subclass + return null; } getCallNodeTable(): CallNodeTable { @@ -202,3 +208,25 @@ export class CallNodeInfoImpl implements CallNodeInfo { return null; } } + +/** + * A subclass of CallNodeInfoImpl for "invert call stack" mode. + * + * This currently shares its implementation with CallNodeInfoImpl; + * this._callNodeTable is the inverted call node table. + * + * By the end of this commit stack, we will no longer have an inverted call node + * table and this class will stop inheriting from CallNodeInfoImpl. + */ +export class CallNodeInfoInvertedImpl + extends CallNodeInfoImpl + implements CallNodeInfoInverted +{ + isInverted(): boolean { + return true; + } + + asInverted(): CallNodeInfoInverted | null { + return this; + } +} diff --git a/src/profile-logic/profile-data.js b/src/profile-logic/profile-data.js index 94f22830fa..3254a85081 100644 --- a/src/profile-logic/profile-data.js +++ b/src/profile-logic/profile-data.js @@ -16,7 +16,7 @@ import { shallowCloneFrameTable, shallowCloneFuncTable, } from './data-structures'; -import { CallNodeInfoImpl } from './call-node-info'; +import { CallNodeInfoImpl, CallNodeInfoInvertedImpl } from './call-node-info'; import { INSTANT, INTERVAL, @@ -59,6 +59,7 @@ import type { IndexIntoFrameTable, PageList, CallNodeInfo, + CallNodeInfoInverted, CallNodeTable, CallNodePath, CallNodeAndCategoryPath, @@ -109,8 +110,7 @@ export function getCallNodeInfo( callNodeTable, callNodeTable, stackIndexToCallNodeIndex, - stackIndexToCallNodeIndex, - false + stackIndexToCallNodeIndex ); } @@ -432,7 +432,7 @@ export function getInvertedCallNodeInfo( nonInvertedCallNodeTable: CallNodeTable, stackIndexToNonInvertedCallNodeIndex: Int32Array, defaultCategory: IndexIntoCategoryList -): CallNodeInfo { +): CallNodeInfoInverted { // We compute an inverted stack table, but we don't let it escape this function. const { invertedThread, @@ -474,12 +474,11 @@ export function getInvertedCallNodeInfo( invertedStackIndexToCallNodeIndex[invertedStackIndex]; } } - return new CallNodeInfoImpl( + return new CallNodeInfoInvertedImpl( callNodeTable, nonInvertedCallNodeTable, nonInvertedStackIndexToCallNodeIndex, - stackIndexToNonInvertedCallNodeIndex, - true + stackIndexToNonInvertedCallNodeIndex ); } diff --git a/src/test/store/__snapshots__/profile-view.test.js.snap b/src/test/store/__snapshots__/profile-view.test.js.snap index ff3b49e82c..41a98d367e 100644 --- a/src/test/store/__snapshots__/profile-view.test.js.snap +++ b/src/test/store/__snapshots__/profile-view.test.js.snap @@ -2321,7 +2321,6 @@ CallNodeInfoImpl { 9, ], }, - "_isInverted": false, "_nonInvertedCallNodeTable": Object { "category": Int32Array [ 0, @@ -2557,7 +2556,6 @@ CallTree { 9, ], }, - "_isInverted": false, "_nonInvertedCallNodeTable": Object { "category": Int32Array [ 0, @@ -2964,7 +2962,6 @@ CallTree { 9, ], }, - "_isInverted": false, "_nonInvertedCallNodeTable": Object { "category": Int32Array [ 0, diff --git a/src/types/profile-derived.js b/src/types/profile-derived.js index 03cc077102..af6090351d 100644 --- a/src/types/profile-derived.js +++ b/src/types/profile-derived.js @@ -128,6 +128,9 @@ export interface CallNodeInfo { // If true, call node indexes describe nodes in the inverted call tree. isInverted(): boolean; + // Returns this object as CallNodeInfoInverted if isInverted(), otherwise null. + asInverted(): CallNodeInfoInverted | null; + // Returns the call node table. If isInverted() is true, this is an inverted // call node table, otherwise this is the non-inverted call node table. getCallNodeTable(): CallNodeTable; @@ -178,6 +181,16 @@ export interface CallNodeInfo { ): IndexIntoCallNodeTable | null; } +// An index into SuffixOrderedCallNodes. +export type SuffixOrderIndex = number; + +/** + * A sub-interface of CallNodeInfo with additional functionality for the inverted + * call tree. + */ +export interface CallNodeInfoInverted extends CallNodeInfo { +} + export type LineNumber = number; // Stores the line numbers which are hit by each stack, for one specific source From 92b0120e3d1bff1aef32c7a6dceb22536a18e14f Mon Sep 17 00:00:00 2001 From: Markus Stange Date: Tue, 6 Aug 2024 14:58:34 -0400 Subject: [PATCH 03/18] Implement the "suffix order". This is the main new concept in this PR that allows us to make the inverted tree fast. See the comment above CallNodeInfoInverted in src/types/profile-derived.js for details. The PR is structured as follows: - Implement the suffix order in a brute force manner (this commit). - Use the suffix order to re-implement everything that was using the inverted call node table. - Once nothing is using the inverted call node table directly anymore, make it fast. We make it fast by rewriting the computation of the inverted call node table and of the suffix order so that we only materialize inverted call nodes that are displayed in the call tree, and not for every sample. And we only compute the suffix order to the level of precision needed to have correct ranges for all materialized inverted call nodes. --- src/profile-logic/call-node-info.js | 69 +++++++++++++++ src/profile-logic/profile-data.js | 62 ++++++++++++- src/test/unit/profile-data.test.js | 105 ++++++++++++++++++++++ src/types/profile-derived.js | 133 ++++++++++++++++++++++++++++ src/utils/bisect.js | 129 +++++++++++++++++++++++++++ 5 files changed, 497 insertions(+), 1 deletion(-) diff --git a/src/profile-logic/call-node-info.js b/src/profile-logic/call-node-info.js index 3b42803169..54fa8ba432 100644 --- a/src/profile-logic/call-node-info.js +++ b/src/profile-logic/call-node-info.js @@ -5,6 +5,7 @@ // @flow import { hashPath } from 'firefox-profiler/utils/path'; +import { bisectEqualRange } from 'firefox-profiler/utils/bisect'; import type { IndexIntoFuncTable, @@ -13,6 +14,7 @@ import type { CallNodeTable, CallNodePath, IndexIntoCallNodeTable, + SuffixOrderIndex, } from 'firefox-profiler/types'; /** @@ -222,6 +224,32 @@ export class CallNodeInfoInvertedImpl extends CallNodeInfoImpl implements CallNodeInfoInverted { + // This is a Map. + // It lists the non-inverted call nodes in "suffix order", i.e. ordered by + // comparing their call paths from back to front. + _suffixOrderedCallNodes: Uint32Array; + // This is the inverse of _suffixOrderedCallNodes; i.e. it is a + // Map. + _suffixOrderIndexes: Uint32Array; + + constructor( + callNodeTable: CallNodeTable, + nonInvertedCallNodeTable: CallNodeTable, + stackIndexToCallNodeIndex: Int32Array, + stackIndexToNonInvertedCallNodeIndex: Int32Array, + suffixOrderedCallNodes: Uint32Array, + suffixOrderIndexes: Uint32Array + ) { + super( + callNodeTable, + nonInvertedCallNodeTable, + stackIndexToCallNodeIndex, + stackIndexToNonInvertedCallNodeIndex + ); + this._suffixOrderedCallNodes = suffixOrderedCallNodes; + this._suffixOrderIndexes = suffixOrderIndexes; + } + isInverted(): boolean { return true; } @@ -229,4 +257,45 @@ export class CallNodeInfoInvertedImpl asInverted(): CallNodeInfoInverted | null { return this; } + + getSuffixOrderedCallNodes(): Uint32Array { + return this._suffixOrderedCallNodes; + } + + getSuffixOrderIndexes(): Uint32Array { + return this._suffixOrderIndexes; + } + + getSuffixOrderIndexRangeForCallNode( + callNodeIndex: IndexIntoCallNodeTable + ): [SuffixOrderIndex, SuffixOrderIndex] { + const callPath = this.getCallNodePathFromIndex(callNodeIndex); + return bisectEqualRange( + this._suffixOrderedCallNodes, + (callNodeIndex: IndexIntoCallNodeTable) => { + let currentCallNodeIndex = callNodeIndex; + for (let i = 0; i < callPath.length - 1; i++) { + const expectedFunc = callPath[i]; + const currentFunc = + this._nonInvertedCallNodeTable.func[currentCallNodeIndex]; + if (currentFunc < expectedFunc) { + return -1; + } + if (currentFunc > expectedFunc) { + return 1; + } + const prefix = + this._nonInvertedCallNodeTable.prefix[currentCallNodeIndex]; + if (prefix === -1) { + return -1; + } + currentCallNodeIndex = prefix; + } + const expectedFunc = callPath[callPath.length - 1]; + const currentFunc = + this._nonInvertedCallNodeTable.func[currentCallNodeIndex]; + return currentFunc - expectedFunc; + } + ); + } } diff --git a/src/profile-logic/profile-data.js b/src/profile-logic/profile-data.js index 3254a85081..80bad176e7 100644 --- a/src/profile-logic/profile-data.js +++ b/src/profile-logic/profile-data.js @@ -474,14 +474,74 @@ export function getInvertedCallNodeInfo( invertedStackIndexToCallNodeIndex[invertedStackIndex]; } } + + // TEMPORARY: Compute a suffix order for the entire non-inverted call node table. + // See the CallNodeInfoInverted interface for more details about the suffix order. + // By the end of this commit stack, the suffix order will be computed incrementally + // as inverted nodes are created; we won't compute the entire order upfront. + const nonInvertedCallNodeCount = nonInvertedCallNodeTable.length; + const suffixOrderedCallNodes = new Uint32Array(nonInvertedCallNodeCount); + const suffixOrderIndexes = new Uint32Array(nonInvertedCallNodeCount); + for (let i = 0; i < nonInvertedCallNodeCount; i++) { + suffixOrderedCallNodes[i] = i; + } + suffixOrderedCallNodes.sort((a, b) => + _compareNonInvertedCallNodesInSuffixOrder(a, b, nonInvertedCallNodeTable) + ); + for (let i = 0; i < suffixOrderedCallNodes.length; i++) { + suffixOrderIndexes[suffixOrderedCallNodes[i]] = i; + } + return new CallNodeInfoInvertedImpl( callNodeTable, nonInvertedCallNodeTable, nonInvertedStackIndexToCallNodeIndex, - stackIndexToNonInvertedCallNodeIndex + stackIndexToNonInvertedCallNodeIndex, + suffixOrderedCallNodes, + suffixOrderIndexes ); } +// Compare two non-inverted call nodes in "suffix order". +// The suffix order is defined as the lexicographical order of the inverted call +// path, or, in other words, the "backwards" lexicographical order of the +// non-inverted call paths. +// +// Example of some suffix ordered non-inverted call paths: +// [0] +// [0, 0] +// [2, 0] +// [4, 5, 1] +// [4, 5] +function _compareNonInvertedCallNodesInSuffixOrder( + callNodeA: IndexIntoCallNodeTable, + callNodeB: IndexIntoCallNodeTable, + nonInvertedCallNodeTable: CallNodeTable +): number { + // Walk up both and stop at the first non-matching function. + // Walking up the non-inverted tree is equivalent to walking down the + // inverted tree. + while (true) { + const funcA = nonInvertedCallNodeTable.func[callNodeA]; + const funcB = nonInvertedCallNodeTable.func[callNodeB]; + if (funcA !== funcB) { + return funcA - funcB; + } + callNodeA = nonInvertedCallNodeTable.prefix[callNodeA]; + callNodeB = nonInvertedCallNodeTable.prefix[callNodeB]; + if (callNodeA === callNodeB) { + break; + } + if (callNodeA === -1) { + return -1; + } + if (callNodeB === -1) { + return 1; + } + } + return 0; +} + // Given a stack index `needleStack` and a call node in the inverted tree // `invertedCallTreeNode`, find an ancestor stack of `needleStack` which // corresponds to the given call node in the inverted call tree. Returns null if diff --git a/src/test/unit/profile-data.test.js b/src/test/unit/profile-data.test.js index 0f16c6368f..e3259181ed 100644 --- a/src/test/unit/profile-data.test.js +++ b/src/test/unit/profile-data.test.js @@ -575,6 +575,111 @@ describe('profile-data', function () { }); }); +describe('getInvertedCallNodeInfo', function () { + function setup(plaintextSamples) { + const { profile, funcNamesDictPerThread } = + getProfileFromTextSamples(plaintextSamples); + + const thread = profile.threads[0]; + const funcNamesDict = funcNamesDictPerThread[0]; + const categories = ensureExists( + profile.meta.categories, + 'Expected to find categories' + ); + const defaultCategory = categories.findIndex((c) => c.name === 'Other'); + const nonInvertedCallNodeInfo = getCallNodeInfo( + thread.stackTable, + thread.frameTable, + thread.funcTable, + defaultCategory + ); + + const invertedCallNodeInfo = getInvertedCallNodeInfo( + thread, + nonInvertedCallNodeInfo.getNonInvertedCallNodeTable(), + nonInvertedCallNodeInfo.getStackIndexToNonInvertedCallNodeIndex(), + defaultCategory + ); + + // This function is used to test `getSuffixOrderIndexRangeForCallNode` and + // `getSuffixOrderedCallNodes`. To find the non-inverted call nodes with + // a call path suffix, `nodesWithSuffix` gets the inverted node X for the + // given call path suffix, and lists non-inverted nodes in X's "suffix + // order index range". + // These are the nodes whose call paths, if inverted, would correspond to + // inverted call nodes that are descendants of X. + function nodesWithSuffix(callPathSuffix) { + const invertedNodeForSuffix = ensureExists( + invertedCallNodeInfo.getCallNodeIndexFromPath( + [...callPathSuffix].reverse() + ) + ); + const [rangeStart, rangeEnd] = + invertedCallNodeInfo.getSuffixOrderIndexRangeForCallNode( + invertedNodeForSuffix + ); + const suffixOrderedCallNodes = + invertedCallNodeInfo.getSuffixOrderedCallNodes(); + const nonInvertedCallNodes = new Set(); + for (let i = rangeStart; i < rangeEnd; i++) { + nonInvertedCallNodes.add(suffixOrderedCallNodes[i]); + } + return nonInvertedCallNodes; + } + + return { + funcNamesDict, + nonInvertedCallNodeInfo, + invertedCallNodeInfo, + nodesWithSuffix, + }; + } + + it('creates a correct suffix order for this example profile', function () { + const { + funcNamesDict: { A, B, C }, + nonInvertedCallNodeInfo, + nodesWithSuffix, + } = setup(` + A A A A A A A + B B B A A C + A C B + `); + + const cnA = nonInvertedCallNodeInfo.getCallNodeIndexFromPath([A]); + const cnAB = nonInvertedCallNodeInfo.getCallNodeIndexFromPath([A, B]); + const cnABA = nonInvertedCallNodeInfo.getCallNodeIndexFromPath([A, B, A]); + const cnABC = nonInvertedCallNodeInfo.getCallNodeIndexFromPath([A, B, C]); + const cnAA = nonInvertedCallNodeInfo.getCallNodeIndexFromPath([A, A]); + const cnAAB = nonInvertedCallNodeInfo.getCallNodeIndexFromPath([A, A, B]); + const cnAC = nonInvertedCallNodeInfo.getCallNodeIndexFromPath([A, C]); + + expect(nodesWithSuffix([A])).toEqual(new Set([cnA, cnABA, cnAA])); + expect(nodesWithSuffix([B])).toEqual(new Set([cnAB, cnAAB])); + expect(nodesWithSuffix([A, B])).toEqual(new Set([cnAB, cnAAB])); + expect(nodesWithSuffix([A, A, B])).toEqual(new Set([cnAAB])); + expect(nodesWithSuffix([C])).toEqual(new Set([cnABC, cnAC])); + }); + + it('creates a correct suffix order for a different example profile', function () { + const { + funcNamesDict: { A, B, C }, + nonInvertedCallNodeInfo, + nodesWithSuffix, + } = setup(` + A A A C + B B + C + `); + + const cnABC = nonInvertedCallNodeInfo.getCallNodeIndexFromPath([A, B, C]); + const cnC = nonInvertedCallNodeInfo.getCallNodeIndexFromPath([C]); + + expect(nodesWithSuffix([B, C])).toEqual(new Set([cnABC])); + expect(nodesWithSuffix([C])).toEqual(new Set([cnABC, cnC])); + }); +}); + describe('symbolication', function () { describe('AddressLocator', function () { const libs = [ diff --git a/src/types/profile-derived.js b/src/types/profile-derived.js index af6090351d..9233ebec6d 100644 --- a/src/types/profile-derived.js +++ b/src/types/profile-derived.js @@ -187,8 +187,141 @@ export type SuffixOrderIndex = number; /** * A sub-interface of CallNodeInfo with additional functionality for the inverted * call tree. + * + * # The Suffix Order + * + * We define an alternative ordering of the *non-inverted* call nodes, called the + * "suffix order", which is useful when interacting with the *inverted* tree. + * The suffix order is stored by two Uint32Array side tables, returned by + * getSuffixOrderedCallNodes() and getSuffixOrderIndexes(). + * getSuffixOrderedCallNodes() maps a suffix order index to a non-inverted call + * node, and getSuffixOrderIndexes() is the reverse, mapping a non-inverted call + * node to its suffix order index. + * + * ## Background + * + * Many operations we do in the profiler require the ability to do an efficient + * "ancestor" check: + * + * - For a call node X in the call tree, what's its "total"? + * - When call node X in the call tree is selected, which samples should be + * highlighted in the activity graph, and which samples should contribute to + * the category breakdown in the sidebar? + * - For how many samples has the clicked call node X been observed in a certain + * line of code / in a certain instruction? + * + * We answer these questions by iterating over samples, getting the sample's + * call node Y, and checking whether the selected / clicked node X is an ancestor + * of Y. + * + * In the non-inverted call tree, the ordering in the call node table gives us a + * quick way to do these checks: For a call node X, all its descendant call nodes + * are in a contiguous range between X and callNodeTable.subtreeRangeEnd[X]. + * + * We want to have a similar ability for the *inverted* call tree, but without + * computing a full inverted call node table. The suffix order gives us this + * ability. It's based on the following insights: + * + * 1. Non-inverted call nodes are "enough" for many purposes even in inverted mode: + * + * When doing the per-sample checks listed above, we don't need an *inverted* + * call node for each sample. We just need an inverted call node for the + * clicked / selected node, and then we can check if the sample's + * *non-inverted* call node contributes to the selected / clicked *inverted* + * call node. + * A non-inverted call node is just a representation of a call path. You can + * read that call path from front to back, or you can read it from back to + * front. If you read it from back to front that's the inverted call path. + * + * 2. We can store multiple different orderings of the non-inverted call node + * table. + * + * The non-inverted call node table remains ordered in depth-first traversal + * order of the non-inverted tree, as described in the "Call node ordering" + * section on the CallNodeTable type. The suffix order is an additional, + * alternative ordering that we store on the side. + * + * ## Definition + * + * We define the suffix order as the lexicographic order of the inverted call path. + * Or as the lexicographic order of the non-inverted call paths "when reading back to front". + * + * D -> B comes before A -> C, because B comes before C. + * D -> B comes after A -> B, because B == B and D comes after A. + * D -> B comes before A -> D -> B, because B == B, D == D, and "end of path" comes before A. + * + * ## Example + * + * ### Non-inverted call tree: + * + * ``` + * Tree Left aligned Right aligned Reordered by suffix + * - [cn0] A = A = A [so0] [so0] [cn0] A + * - [cn1] B = A -> B = A -> B [so3] [so1] [cn4] A <- A + * - [cn2] A = A -> B -> A = A -> B -> A [so2] ↘↗ [so2] [cn2] A <- B <- A + * - [cn3] C = A -> B -> C = A -> B -> C [so6] ↗↘ [so3] [cn1] B <- A + * - [cn4] A = A -> A = A -> A [so1] [so4] [cn5] B <- A <- A + * - [cn5] B = A -> A -> B = A -> A -> B [so4] [so5] [cn6] C <- A + * - [cn6] C = A -> C = A -> C [so5] [so6] [cn3] C <- B <- A + * ``` + * + * ### Inverted call tree: + * + * ``` + * Represents call paths ending in + * - [in0] A (so:0..3) = A = ... A (cn0, cn4, cn2) + * - [in1] A (so:1..2) = A <- A = ... A -> A (cn4) + * - [in2] B (so:2..3) = A <- B = ... B -> A (cn2) + * - [in3] A (so:2..3) = A <- B <- A = ... A -> B -> A (cn2) + * - [in4] B (so:3..5) = B = ... B (cn1, cn5) + * - [in5] A (so:3..5) = B <- A = ... A -> B (cn1, cn5) + * - [in6] A (so:4..5) = B <- A <- A = ... A -> A -> B (cn5) + * - [in7] C (so:5..7) = C = ... C (cn6, cn3) + * - [in8] A (so:5..6) = C <- A = ... A -> C (cn6) + * - [in9] B (so:6..7) = C <- B = ... B -> C (cn3) + * - [in10] A (so:6..7) = C <- B <- A = ... A -> B -> C (cn3) + * ``` + * + * In the suffix order, call paths become grouped in such a way that call paths + * which belong to the same *inverted* tree node (i.e. which share a suffix) end + * up ordered next to each other. This makes it so that a node in the inverted + * tree can refer to all its represented call paths with a single contiguous range. + * + * In this example, inverted tree node `in5` represents all call paths which end + * in A -> B. Both `cn1` and `cn5` do so; `cn1` is A -> B and `cn5` is A -> A -> B. + * In the suffix order, `cn1` and `cn5` end up next to each other, at positions + * `so3` and `so4`. This means that the two paths can be referred to via the suffix + * order index range 3..5. + * + * Suffix ordered call nodes: [0, 4, 2, 1, 5, 6, 3] (soX -> cnY) + * Suffix order indexes: [0, 3, 2, 6, 1, 4, 5] (cnX -> soY) + * + * cnX: Non-inverted call node index X + * soX: Suffix order index X + * inX: Inverted call node index X + * so:X..Y: Suffix order index range soX..soY (soY excluded) */ export interface CallNodeInfoInverted extends CallNodeInfo { + // Get a mapping SuffixOrderIndex -> IndexIntoNonInvertedCallNodeTable. + // This array contains all non-inverted call node indexes, ordered by + // call path suffix. See "suffix order" in the documentation above. + getSuffixOrderedCallNodes(): Uint32Array; + + // Returns the inverse of getSuffixOrderedCallNodes(), i.e. a mapping + // IndexIntoNonInvertedCallNodeTable -> SuffixOrderIndex. + getSuffixOrderIndexes(): Uint32Array; + + // Get the [start, exclusiveEnd] range of suffix order indexes for this + // inverted tree node. This lets you list the non-inverted call nodes which + // "contribute to" the given inverted call node. Or put differently, it lets + // you iterate over the non-inverted call nodes whose call paths "end with" + // the call path suffix represented by the inverted node. + // By the definition of the suffix order, all non-inverted call nodes whose + // call path ends with the suffix defined by the inverted call node `callNodeIndex` + // will be in a contiguous range in the suffix order. + getSuffixOrderIndexRangeForCallNode( + callNodeIndex: IndexIntoCallNodeTable + ): [SuffixOrderIndex, SuffixOrderIndex]; } export type LineNumber = number; diff --git a/src/utils/bisect.js b/src/utils/bisect.js index 16dcc4e724..05fe86adbd 100644 --- a/src/utils/bisect.js +++ b/src/utils/bisect.js @@ -208,3 +208,132 @@ export function bisectionLeft( return low; } + +/* + * TEMPORARY: The functions below implement bisectEqualRange(). The implementation + * is copied from https://searchfox.org/mozilla-central/rev/8b0666aff1197e1dd8017de366343de9c21ee437/mfbt/BinarySearch.h#132-243 + * The only code calling bisectEqualRange will be removed by the end of this + * commit stack, so all the code added here will be removed again, too. + * + * bisectLowerBound(), bisectUpperBound(), and bisectEqualRange() are equivalent to + * std::lower_bound(), std::upper_bound(), and std::equal_range() respectively. + * + * bisectLowerBound() returns an index pointing to the first element in the range + * in which each element is considered *not less than* the given value passed + * via |aCompare|, or the length of |aContainer| if no such element is found. + * + * bisectUpperBound() returns an index pointing to the first element in the range + * in which each element is considered *greater than* the given value passed + * via |aCompare|, or the length of |aContainer| if no such element is found. + * + * bisectEqualRange() returns a range [first, second) containing all elements are + * considered equivalent to the given value via |aCompare|. If you need + * either the first or last index of the range, bisectLowerBound() or bisectUpperBound(), + * which is slightly faster than bisectEqualRange(), should suffice. + * + * Example (another example is given in TestBinarySearch.cpp): + * + * Vector sortedStrings = ... + * + * struct Comparator { + * const nsACString& mStr; + * explicit Comparator(const nsACString& aStr) : mStr(aStr) {} + * int32_t operator()(const char* aVal) const { + * return Compare(mStr, nsDependentCString(aVal)); + * } + * }; + * + * auto bounds = bisectEqualRange(sortedStrings, 0, sortedStrings.length(), + * Comparator("needle I'm looking for"_ns)); + * printf("Found the range [%zd %zd)\n", bounds.first(), bounds.second()); + * + */ +export function bisectLowerBound( + array: number[] | $TypedArray, + f: (number) => number, // < 0 if arg is before needle, > 0 if after, === 0 if same + low?: number, + high?: number +): number { + low = low || 0; + high = high || array.length; + + if (low < 0 || low > array.length || high < 0 || high > array.length) { + throw new TypeError("low and high must lie within the array's range"); + } + + while (high !== low) { + const middle = (low + high) >> 1; + const result = f(array[middle]); + + // The range returning from bisectLowerBound does include elements + // equivalent to the given value i.e. f(element) == 0 + if (result >= 0) { + high = middle; + } else { + low = middle + 1; + } + } + + return low; +} + +export function bisectUpperBound( + array: number[] | $TypedArray, + f: (number) => number, // < 0 if arg is before needle, > 0 if after, === 0 if same + low?: number, + high?: number +): number { + low = low || 0; + high = high || array.length; + + if (low < 0 || low > array.length || high < 0 || high > array.length) { + throw new TypeError("low and high must lie within the array's range"); + } + + while (high !== low) { + const middle = (low + high) >> 1; + const result = f(array[middle]); + + // The range returning from bisectUpperBound does NOT include elements + // equivalent to the given value i.e. f(element) == 0 + if (result > 0) { + high = middle; + } else { + low = middle + 1; + } + } + + return high; +} + +export function bisectEqualRange( + array: number[] | $TypedArray, + f: (number) => number, // < 0 if arg is before needle, > 0 if after, === 0 if same + low?: number, + high?: number +): [number, number] { + low = low || 0; + high = high || array.length; + + if (low < 0 || low > array.length || high < 0 || high > array.length) { + throw new TypeError("low and high must lie within the array's range"); + } + + while (high !== low) { + const middle = (low + high) >> 1; + const result = f(array[middle]); + + if (result > 0) { + high = middle; + } else if (result < 0) { + low = middle + 1; + } else { + return [ + bisectLowerBound(array, f, low, middle), + bisectUpperBound(array, f, middle + 1, high), + ]; + } + } + + return [low, high]; +} From e88ea5edfd148a8654859886881c1a1cae86184a Mon Sep 17 00:00:00 2001 From: Markus Stange Date: Fri, 19 Jan 2024 17:44:00 -0500 Subject: [PATCH 04/18] Use the suffix order in getMatchingAncestorStackForInvertedCallNode. This function is used by getNativeSymbolsForCallNodeInverted, getStackAddressInfoForCallNodeInverted, and getStackLineInfoForCallNodeInverted. This replaces a call to getStackIndexToCallNodeIndex() with a call to getStackIndexToNonInvertedCallNodeIndex(). It also mostly removes the use of the inverted call node table for this code. (There's still a place that accesses callNodeInfo.getCallNodeTable().depth, but this will be fixed in a later commit.) We want to eliminate all callers to getStackIndexToCallNodeIndex() because we don't want to compute a mapping from non-inverted stack index to inverted call node index upfront. --- src/profile-logic/address-timings.js | 26 ++++---- src/profile-logic/call-node-info.js | 4 ++ src/profile-logic/line-timings.js | 26 ++++---- src/profile-logic/profile-data.js | 94 ++++++++++++++++------------ src/types/profile-derived.js | 3 + 5 files changed, 91 insertions(+), 62 deletions(-) diff --git a/src/profile-logic/address-timings.js b/src/profile-logic/address-timings.js index 00080a3f3d..2e94663a2a 100644 --- a/src/profile-logic/address-timings.js +++ b/src/profile-logic/address-timings.js @@ -75,6 +75,7 @@ import type { StackTable, SamplesLikeTable, CallNodeInfo, + CallNodeInfoInverted, IndexIntoCallNodeTable, IndexIntoNativeSymbolTable, StackAddressInfo, @@ -202,12 +203,13 @@ export function getStackAddressInfoForCallNode( callNodeInfo: CallNodeInfo, nativeSymbol: IndexIntoNativeSymbolTable ): StackAddressInfo { - return callNodeInfo.isInverted() + const callNodeInfoInverted = callNodeInfo.asInverted(); + return callNodeInfoInverted !== null ? getStackAddressInfoForCallNodeInverted( stackTable, frameTable, callNodeIndex, - callNodeInfo, + callNodeInfoInverted, nativeSymbol ) : getStackAddressInfoForCallNodeNonInverted( @@ -426,16 +428,17 @@ export function getStackAddressInfoForCallNodeInverted( stackTable: StackTable, frameTable: FrameTable, callNodeIndex: IndexIntoCallNodeTable, - callNodeInfo: CallNodeInfo, + callNodeInfo: CallNodeInfoInverted, nativeSymbol: IndexIntoNativeSymbolTable ): StackAddressInfo { - const invertedCallNodeTable = callNodeInfo.getCallNodeTable(); - const depth = invertedCallNodeTable.depth[callNodeIndex]; - const endIndex = invertedCallNodeTable.subtreeRangeEnd[callNodeIndex]; - const callNodeIsRootOfInvertedTree = - invertedCallNodeTable.prefix[callNodeIndex] === -1; - const stackIndexToCallNodeIndex = callNodeInfo.getStackIndexToCallNodeIndex(); + const depth = callNodeInfo.getCallNodeTable().depth[callNodeIndex]; + const [rangeStart, rangeEnd] = + callNodeInfo.getSuffixOrderIndexRangeForCallNode(callNodeIndex); + const callNodeIsRootOfInvertedTree = callNodeInfo.isRoot(callNodeIndex); + const stackIndexToCallNodeIndex = + callNodeInfo.getStackIndexToNonInvertedCallNodeIndex(); const stackTablePrefixCol = stackTable.prefix; + const suffixOrderIndexes = callNodeInfo.getSuffixOrderIndexes(); // "self address" == "the address which a stack's self time is contributed to" const callNodeSelfAddressForAllStacks = []; @@ -449,8 +452,9 @@ export function getStackAddressInfoForCallNodeInverted( const stackForCallNode = getMatchingAncestorStackForInvertedCallNode( stackIndex, - callNodeIndex, - endIndex, + rangeStart, + rangeEnd, + suffixOrderIndexes, depth, stackIndexToCallNodeIndex, stackTablePrefixCol diff --git a/src/profile-logic/call-node-info.js b/src/profile-logic/call-node-info.js index 54fa8ba432..78b0757e6c 100644 --- a/src/profile-logic/call-node-info.js +++ b/src/profile-logic/call-node-info.js @@ -209,6 +209,10 @@ export class CallNodeInfoImpl implements CallNodeInfo { return null; } + + isRoot(callNodeIndex: IndexIntoCallNodeTable): boolean { + return this._callNodeTable.prefix[callNodeIndex] === -1; + } } /** diff --git a/src/profile-logic/line-timings.js b/src/profile-logic/line-timings.js index 0d0065f25c..3d0fd3443b 100644 --- a/src/profile-logic/line-timings.js +++ b/src/profile-logic/line-timings.js @@ -10,6 +10,7 @@ import type { StackTable, SamplesLikeTable, CallNodeInfo, + CallNodeInfoInverted, IndexIntoCallNodeTable, IndexIntoStringTable, StackLineInfo, @@ -125,12 +126,13 @@ export function getStackLineInfoForCallNode( callNodeIndex: IndexIntoCallNodeTable, callNodeInfo: CallNodeInfo ): StackLineInfo { - return callNodeInfo.isInverted() + const callNodeInfoInverted = callNodeInfo.asInverted(); + return callNodeInfoInverted !== null ? getStackLineInfoForCallNodeInverted( stackTable, frameTable, callNodeIndex, - callNodeInfo + callNodeInfoInverted ) : getStackLineInfoForCallNodeNonInverted( stackTable, @@ -282,15 +284,16 @@ export function getStackLineInfoForCallNodeInverted( stackTable: StackTable, frameTable: FrameTable, callNodeIndex: IndexIntoCallNodeTable, - callNodeInfo: CallNodeInfo + callNodeInfo: CallNodeInfoInverted ): StackLineInfo { - const invertedCallNodeTable = callNodeInfo.getCallNodeTable(); - const depth = invertedCallNodeTable.depth[callNodeIndex]; - const endIndex = invertedCallNodeTable.subtreeRangeEnd[callNodeIndex]; - const callNodeIsRootOfInvertedTree = - invertedCallNodeTable.prefix[callNodeIndex] === -1; - const stackIndexToCallNodeIndex = callNodeInfo.getStackIndexToCallNodeIndex(); + const depth = callNodeInfo.getCallNodeTable().depth[callNodeIndex]; + const [rangeStart, rangeEnd] = + callNodeInfo.getSuffixOrderIndexRangeForCallNode(callNodeIndex); + const callNodeIsRootOfInvertedTree = callNodeInfo.isRoot(callNodeIndex); + const stackIndexToCallNodeIndex = + callNodeInfo.getStackIndexToNonInvertedCallNodeIndex(); const stackTablePrefixCol = stackTable.prefix; + const suffixOrderIndexes = callNodeInfo.getSuffixOrderIndexes(); // "self line" == "the line which a stack's self time is contributed to" const callNodeSelfLineForAllStacks = []; @@ -304,8 +307,9 @@ export function getStackLineInfoForCallNodeInverted( const stackForCallNode = getMatchingAncestorStackForInvertedCallNode( stackIndex, - callNodeIndex, - endIndex, + rangeStart, + rangeEnd, + suffixOrderIndexes, depth, stackIndexToCallNodeIndex, stackTablePrefixCol diff --git a/src/profile-logic/profile-data.js b/src/profile-logic/profile-data.js index 80bad176e7..7154180c3f 100644 --- a/src/profile-logic/profile-data.js +++ b/src/profile-logic/profile-data.js @@ -83,6 +83,7 @@ import type { BottomBoxInfo, Bytes, ThreadWithReservedFunctions, + SuffixOrderIndex, } from 'firefox-profiler/types'; import type { UniqueStringArray } from 'firefox-profiler/utils/unique-string-array'; @@ -549,6 +550,17 @@ function _compareNonInvertedCallNodesInSuffixOrder( // // Also returns null for any stacks which aren't used as self stacks. // +// Note: This function doesn't actually have a parameter named `invertedCallTreeNode`. +// Instead, it has two parameters for the node's suffix order index range. This +// range is obtained by the caller and is enough to check whether a stack's call +// path ends with the path suffix represented by the inverted call node. The caller +// gets the suffix order index range as follows: +// +// ``` +// const [rangeStart, rangeEnd] = +// callNodeInfo.getSuffixOrderIndexRangeForCallNode(callNodeIndex); +// ``` +// // Example: // // Stack table (`:`): Inverted call tree: @@ -576,33 +588,32 @@ function _compareNonInvertedCallNodesInSuffixOrder( // that frame, for example the frame's address or line. export function getMatchingAncestorStackForInvertedCallNode( needleStack: IndexIntoStackTable, - invertedTreeCallNode: IndexIntoCallNodeTable, - invertedTreeCallNodeSubtreeEnd: IndexIntoCallNodeTable, + suffixOrderIndexRangeStart: SuffixOrderIndex, + suffixOrderIndexRangeEnd: SuffixOrderIndex, + suffixOrderIndexes: Uint32Array, invertedTreeCallNodeDepth: number, - stackIndexToInvertedCallNodeIndex: Int32Array, + stackIndexToCallNodeIndex: Int32Array, stackTablePrefixCol: Array ): IndexIntoStackTable | null { - // Get the inverted call tree node for the (non-inverted) stack. + // Get the non-inverted call tree node for the (non-inverted) stack. // For example, if the stack has the call path A -> B -> C, - // this will give us the node C <- B <- A in the inverted tree. - const needleCallNode = stackIndexToInvertedCallNodeIndex[needleStack]; + // this will give us the node A -> B -> C in the non-inverted tree. + const needleCallNode = stackIndexToCallNodeIndex[needleStack]; + const needleSuffixOrderIndex = suffixOrderIndexes[needleCallNode]; - // Check if needleCallNode is a descendant of invertedTreeCallNode in the - // inverted tree. + // Check if needleCallNode's call path ends with the call path suffix represented + // by the inverted call node. if ( - needleCallNode >= invertedTreeCallNode && - needleCallNode < invertedTreeCallNodeSubtreeEnd + needleSuffixOrderIndex >= suffixOrderIndexRangeStart && + needleSuffixOrderIndex < suffixOrderIndexRangeEnd ) { - // needleCallNode is a descendant of invertedTreeCallNode in the inverted tree. - // That means that needleStack's self time contributes to the total time of - // invertedTreeCallNode. It also means that the non-inverted call path of - // needleStack "ends with" the suffix described by invertedTreeCallNode. - // For example, if invertedTreeCallNode is C <- B, and needleStack has the + // Yes, needleCallNode's call path ends with the call path suffix represented + // by the inverted call node. + // For example, if our node is C <- B in the inverted tree, and needleStack has the // non-inverted call path A -> B -> C, then we now know that A -> B -> C ends // with B -> C. - // Now we strip off this suffix. In the example, we strip off "-> C" at the - // end so that we end up with a stack for A -> B. - // Stripping off the suffix is equivalent to "walking down" in the inverted tree. + // Now we strip off this suffix. In the example, invertedTreeCallNodeDepth is 1 + // so we strip off "-> C" at the end and return a stack for A -> B. return getNthPrefixStack( needleStack, invertedTreeCallNodeDepth, @@ -610,7 +621,7 @@ export function getMatchingAncestorStackForInvertedCallNode( ); } - // Not a descendant; return null. + // The stack's call path doesn't end with the suffix we were looking for; return null. return null; } @@ -3560,20 +3571,20 @@ export function getNativeSymbolsForCallNode( stackTable: StackTable, frameTable: FrameTable ): IndexIntoNativeSymbolTable[] { - if (callNodeInfo.isInverted()) { - return getNativeSymbolsForCallNodeInverted( - callNodeIndex, - callNodeInfo, - stackTable, - frameTable - ); - } - return getNativeSymbolsForCallNodeNonInverted( - callNodeIndex, - callNodeInfo, - stackTable, - frameTable - ); + const callNodeInfoInverted = callNodeInfo.asInverted(); + return callNodeInfoInverted !== null + ? getNativeSymbolsForCallNodeInverted( + callNodeIndex, + callNodeInfoInverted, + stackTable, + frameTable + ) + : getNativeSymbolsForCallNodeNonInverted( + callNodeIndex, + callNodeInfo, + stackTable, + frameTable + ); } export function getNativeSymbolsForCallNodeNonInverted( @@ -3599,21 +3610,24 @@ export function getNativeSymbolsForCallNodeNonInverted( export function getNativeSymbolsForCallNodeInverted( callNodeIndex: IndexIntoCallNodeTable, - callNodeInfo: CallNodeInfo, + callNodeInfo: CallNodeInfoInverted, stackTable: StackTable, frameTable: FrameTable ): IndexIntoNativeSymbolTable[] { - const invertedCallNodeTable = callNodeInfo.getCallNodeTable(); - const depth = invertedCallNodeTable.depth[callNodeIndex]; - const endIndex = invertedCallNodeTable.subtreeRangeEnd[callNodeIndex]; + const depth = callNodeInfo.getCallNodeTable().depth[callNodeIndex]; + const [rangeStart, rangeEnd] = + callNodeInfo.getSuffixOrderIndexRangeForCallNode(callNodeIndex); const stackTablePrefixCol = stackTable.prefix; - const stackIndexToCallNodeIndex = callNodeInfo.getStackIndexToCallNodeIndex(); + const stackIndexToCallNodeIndex = + callNodeInfo.getStackIndexToNonInvertedCallNodeIndex(); + const suffixOrderIndexes = callNodeInfo.getSuffixOrderIndexes(); const set = new Set(); for (let stackIndex = 0; stackIndex < stackTable.length; stackIndex++) { const stackForNode = getMatchingAncestorStackForInvertedCallNode( stackIndex, - callNodeIndex, - endIndex, + rangeStart, + rangeEnd, + suffixOrderIndexes, depth, stackIndexToCallNodeIndex, stackTablePrefixCol diff --git a/src/types/profile-derived.js b/src/types/profile-derived.js index 9233ebec6d..cc188b3f37 100644 --- a/src/types/profile-derived.js +++ b/src/types/profile-derived.js @@ -179,6 +179,9 @@ export interface CallNodeInfo { parent: IndexIntoCallNodeTable | -1, func: IndexIntoFuncTable ): IndexIntoCallNodeTable | null; + + // Returns whether the given node is a root node. + isRoot(callNodeIndex: IndexIntoCallNodeTable): boolean; } // An index into SuffixOrderedCallNodes. From 1dd684e49e28f9841fbec544835f2bc88acd58c3 Mon Sep 17 00:00:00 2001 From: Markus Stange Date: Fri, 19 Jan 2024 18:42:29 -0500 Subject: [PATCH 05/18] Implement getSamplesSelectedStates for inverted trees with the suffix order. This replaces a call to getStackIndexToCallNodeIndex() with a call to getStackIndexToNonInvertedCallNodeIndex(). It also removes a call to getCallNodeTable(). And it replaces a SampleIndexToCallNodeIndex mapping with a SampleIndexToNonInvertedCallNodeIndex mapping. --- src/profile-logic/profile-data.js | 84 +++++++++++++++++++----- src/selectors/per-thread/stack-sample.js | 21 +++--- src/test/unit/profile-data.test.js | 13 ++-- 3 files changed, 85 insertions(+), 33 deletions(-) diff --git a/src/profile-logic/profile-data.js b/src/profile-logic/profile-data.js index 7154180c3f..cf5ca1a901 100644 --- a/src/profile-logic/profile-data.js +++ b/src/profile-logic/profile-data.js @@ -661,7 +661,7 @@ export function getSampleIndexToCallNodeIndex( * This is an implementation of getSamplesSelectedStates for just the case where * no call node is selected. */ -function getSamplesSelectedStatesForNoSelection( +function _getSamplesSelectedStatesForNoSelection( sampleCallNodes: Array, activeTabFilteredCallNodes: Array ): SelectedState[] { @@ -694,7 +694,7 @@ function getSamplesSelectedStatesForNoSelection( } /** - * Given the call node for each sample and the call node selected states, + * Given the call node for each sample and the selected call node, * compute each sample's selected state. * * For samples that are not filtered out, the sample's selected state is based @@ -740,12 +740,15 @@ function getSamplesSelectedStatesForNoSelection( * In this example, the selected node has index 13 and the "selected index range" * is the range from 13 to 21 (not including 21). */ -function mapCallNodeSelectedStatesToSamples( +function _getSamplesSelectedStatesNonInverted( sampleCallNodes: Array, activeTabFilteredCallNodes: Array, selectedCallNodeIndex: IndexIntoCallNodeTable, - selectedCallNodeDescendantsEndIndex: IndexIntoCallNodeTable + callNodeInfo: CallNodeInfo ): SelectedState[] { + const callNodeTable = callNodeInfo.getNonInvertedCallNodeTable(); + const selectedCallNodeDescendantsEndIndex = + callNodeTable.subtreeRangeEnd[selectedCallNodeIndex]; const sampleCount = sampleCallNodes.length; const samplesSelectedStates = new Array(sampleCount); for (let sampleIndex = 0; sampleIndex < sampleCount; sampleIndex++) { @@ -773,6 +776,48 @@ function mapCallNodeSelectedStatesToSamples( return samplesSelectedStates; } +/** + * The implementation of getSamplesSelectedStates for the inverted tree. + * + * This uses the suffix order, see the documentation of CallNodeInfoInverted. + */ +function _getSamplesSelectedStatesInverted( + sampleNonInvertedCallNodes: Array, + activeTabFilteredNonInvertedCallNodes: Array, + selectedInvertedCallNodeIndex: IndexIntoCallNodeTable, + callNodeInfo: CallNodeInfoInverted +): SelectedState[] { + const suffixOrderIndexes = callNodeInfo.getSuffixOrderIndexes(); + const [selectedSubtreeRangeStart, selectedSubtreeRangeEnd] = + callNodeInfo.getSuffixOrderIndexRangeForCallNode( + selectedInvertedCallNodeIndex + ); + const sampleCount = sampleNonInvertedCallNodes.length; + const samplesSelectedStates = new Array(sampleCount); + for (let sampleIndex = 0; sampleIndex < sampleCount; sampleIndex++) { + let sampleSelectedState: SelectedState = 'SELECTED'; + const callNodeIndex = sampleNonInvertedCallNodes[sampleIndex]; + if (callNodeIndex !== null) { + const suffixOrderIndex = suffixOrderIndexes[callNodeIndex]; + if (suffixOrderIndex < selectedSubtreeRangeStart) { + sampleSelectedState = 'UNSELECTED_ORDERED_BEFORE_SELECTED'; + } else if (suffixOrderIndex >= selectedSubtreeRangeEnd) { + sampleSelectedState = 'UNSELECTED_ORDERED_AFTER_SELECTED'; + } + } else { + // This sample was filtered out. + sampleSelectedState = + activeTabFilteredNonInvertedCallNodes[sampleIndex] === null + ? // This sample was not part of the active tab. + 'FILTERED_OUT_BY_ACTIVE_TAB' + : // This sample was filtered out in the transform pipeline. + 'FILTERED_OUT_BY_TRANSFORM'; + } + samplesSelectedStates[sampleIndex] = sampleSelectedState; + } + return samplesSelectedStates; +} + /** * Go through the samples, and determine their current state with respect to * the selection. @@ -782,24 +827,31 @@ function mapCallNodeSelectedStatesToSamples( */ export function getSamplesSelectedStates( callNodeInfo: CallNodeInfo, - sampleCallNodes: Array, - activeTabFilteredCallNodes: Array, + sampleNonInvertedCallNodes: Array, + activeTabFilteredNonInvertedCallNodes: Array, selectedCallNodeIndex: IndexIntoCallNodeTable | null ): SelectedState[] { if (selectedCallNodeIndex === null || selectedCallNodeIndex === -1) { - return getSamplesSelectedStatesForNoSelection( - sampleCallNodes, - activeTabFilteredCallNodes + return _getSamplesSelectedStatesForNoSelection( + sampleNonInvertedCallNodes, + activeTabFilteredNonInvertedCallNodes ); } - const callNodeTable = callNodeInfo.getCallNodeTable(); - return mapCallNodeSelectedStatesToSamples( - sampleCallNodes, - activeTabFilteredCallNodes, - selectedCallNodeIndex, - callNodeTable.subtreeRangeEnd[selectedCallNodeIndex] - ); + const callNodeInfoInverted = callNodeInfo.asInverted(); + return callNodeInfoInverted !== null + ? _getSamplesSelectedStatesInverted( + sampleNonInvertedCallNodes, + activeTabFilteredNonInvertedCallNodes, + selectedCallNodeIndex, + callNodeInfoInverted + ) + : _getSamplesSelectedStatesNonInverted( + sampleNonInvertedCallNodes, + activeTabFilteredNonInvertedCallNodes, + selectedCallNodeIndex, + callNodeInfo + ); } /** diff --git a/src/selectors/per-thread/stack-sample.js b/src/selectors/per-thread/stack-sample.js index bd897ea1ad..86e3a6ce1b 100644 --- a/src/selectors/per-thread/stack-sample.js +++ b/src/selectors/per-thread/stack-sample.js @@ -263,35 +263,35 @@ export function getStackAndSampleSelectorsPerThread( ) ); - const getSampleIndexToCallNodeIndexForTabFilteredThread: Selector< + const _getSampleIndexToNonInvertedCallNodeIndexForTabFilteredThread: Selector< Array, > = createSelector( (state) => threadSelectors.getTabFilteredThread(state).samples.stack, - (state) => getCallNodeInfo(state).getStackIndexToCallNodeIndex(), - (tabFilteredThreadSampleStacks, stackIndexToCallNodeIndex) => + (state) => getCallNodeInfo(state).getStackIndexToNonInvertedCallNodeIndex(), + (tabFilteredThreadSampleStacks, stackIndexToNonInvertedCallNodeIndex) => ProfileData.getSampleIndexToCallNodeIndex( tabFilteredThreadSampleStacks, - stackIndexToCallNodeIndex + stackIndexToNonInvertedCallNodeIndex ) ); const getSamplesSelectedStatesInFilteredThread: Selector< null | SelectedState[], > = createSelector( - getSampleIndexToCallNodeIndexForFilteredThread, - getSampleIndexToCallNodeIndexForTabFilteredThread, + getSampleIndexToNonInvertedCallNodeIndexForFilteredThread, + _getSampleIndexToNonInvertedCallNodeIndexForTabFilteredThread, getCallNodeInfo, getSelectedCallNodeIndex, ( - sampleIndexToCallNodeIndex, - activeTabFilteredCallNodeIndex, + sampleIndexToNonInvertedCallNodeIndex, + activeTabFilteredNonInvertedCallNodes, callNodeInfo, selectedCallNode ) => { return ProfileData.getSamplesSelectedStates( callNodeInfo, - sampleIndexToCallNodeIndex, - activeTabFilteredCallNodeIndex, + sampleIndexToNonInvertedCallNodeIndex, + activeTabFilteredNonInvertedCallNodes, selectedCallNode ); } @@ -462,7 +462,6 @@ export function getStackAndSampleSelectorsPerThread( getExpandedCallNodeIndexes, getSampleIndexToCallNodeIndexForFilteredThread, getSampleIndexToNonInvertedCallNodeIndexForFilteredThread, - getSampleIndexToCallNodeIndexForTabFilteredThread, getSamplesSelectedStatesInFilteredThread, getTreeOrderComparatorInFilteredThread, getCallTree, diff --git a/src/test/unit/profile-data.test.js b/src/test/unit/profile-data.test.js index e3259181ed..cd3e90e8ab 100644 --- a/src/test/unit/profile-data.test.js +++ b/src/test/unit/profile-data.test.js @@ -1123,6 +1123,7 @@ describe('getSamplesSelectedStates', function () { * */ const { callNodeInfoInverted, + sampleCallNodes, sampleInvertedCallNodes, funcNamesDict: { A, B, C }, } = setup(` @@ -1146,8 +1147,8 @@ describe('getSamplesSelectedStates', function () { expect( getSamplesSelectedStates( callNodeInfoInverted, - sampleInvertedCallNodes, - sampleInvertedCallNodes, + sampleCallNodes, + sampleCallNodes, inBA ) ).toEqual([ @@ -1164,8 +1165,8 @@ describe('getSamplesSelectedStates', function () { expect( getSamplesSelectedStates( callNodeInfoInverted, - sampleInvertedCallNodes, - sampleInvertedCallNodes, + sampleCallNodes, + sampleCallNodes, inCBA ) ).toEqual([ @@ -1182,8 +1183,8 @@ describe('getSamplesSelectedStates', function () { expect( getSamplesSelectedStates( callNodeInfoInverted, - sampleInvertedCallNodes, - sampleInvertedCallNodes, + sampleCallNodes, + sampleCallNodes, inB ) ).toEqual([ From 6fee1343d90042626d45f841290c8bb464faa295 Mon Sep 17 00:00:00 2001 From: Markus Stange Date: Fri, 19 Jan 2024 19:11:41 -0500 Subject: [PATCH 06/18] Use suffix order in getTimingsForCallNodeIndex. This replaces a call to getStackIndexToCallNodeIndex() with a call to getStackIndexToNonInvertedCallNodeIndex(). It also removes a call to getCallNodeTable(). --- src/profile-logic/profile-data.js | 105 +++++++++++++++++++----------- 1 file changed, 66 insertions(+), 39 deletions(-) diff --git a/src/profile-logic/profile-data.js b/src/profile-logic/profile-data.js index cf5ca1a901..80369527b4 100644 --- a/src/profile-logic/profile-data.js +++ b/src/profile-logic/profile-data.js @@ -1132,51 +1132,78 @@ export function getTimingsForCallNodeIndex( return { forPath: pathTimings, rootTime }; } - const callNodeTable = callNodeInfo.getCallNodeTable(); - const stackIndexToCallNodeIndex = callNodeInfo.getStackIndexToCallNodeIndex(); - - const needleDescendantsEndIndex = - callNodeTable.subtreeRangeEnd[needleNodeIndex]; - - const isInvertedTree = callNodeInfo.isInverted(); - const needleNodeIsRootOfInvertedTree = - isInvertedTree && callNodeTable.prefix[needleNodeIndex] === -1; - - // Loop over each sample and accumulate the self time, running time, and - // the implementation breakdown. - for (let sampleIndex = 0; sampleIndex < samples.length; sampleIndex++) { - // Get the call node for this sample. - // TODO: Consider using sampleCallNodes for this, to save one indirection on - // a hot path. - const thisStackIndex = samples.stack[sampleIndex]; - if (thisStackIndex === null) { - continue; + const callNodeTable = callNodeInfo.getNonInvertedCallNodeTable(); + const stackIndexToCallNodeIndex = + callNodeInfo.getStackIndexToNonInvertedCallNodeIndex(); + const callNodeInfoInverted = callNodeInfo.asInverted(); + if (callNodeInfoInverted !== null) { + // Inverted case + const needleNodeIsRootOfInvertedTree = + callNodeInfoInverted.isRoot(needleNodeIndex); + const suffixOrderIndexes = callNodeInfoInverted.getSuffixOrderIndexes(); + const [rangeStart, rangeEnd] = + callNodeInfoInverted.getSuffixOrderIndexRangeForCallNode(needleNodeIndex); + + // Loop over each sample and accumulate the self time, running time, and + // the implementation breakdown. + for (let sampleIndex = 0; sampleIndex < samples.length; sampleIndex++) { + // Get the call node for this sample. + // TODO: Consider using sampleCallNodes for this, to save one indirection on + // a hot path. + const thisStackIndex = samples.stack[sampleIndex]; + if (thisStackIndex === null) { + continue; + } + const thisNodeIndex = stackIndexToCallNodeIndex[thisStackIndex]; + const thisNodeSuffixOrderIndex = suffixOrderIndexes[thisNodeIndex]; + const weight = samples.weight ? samples.weight[sampleIndex] : 1; + rootTime += Math.abs(weight); + + if ( + thisNodeSuffixOrderIndex >= rangeStart && + thisNodeSuffixOrderIndex < rangeEnd + ) { + // One of the parents is the exact passed path. + accumulateDataToTimings(pathTimings.totalTime, sampleIndex, weight); + + if (needleNodeIsRootOfInvertedTree) { + // This root node matches the passed call node path. + // Just increment the selfTime value. + // We don't call accumulateDataToTimings(pathTimings.selfTime, ...) + // here, mainly because this would be the same as for the total time. + pathTimings.selfTime.value += weight; + } + } } - const thisNodeIndex = stackIndexToCallNodeIndex[thisStackIndex]; - - const weight = samples.weight ? samples.weight[sampleIndex] : 1; - - rootTime += Math.abs(weight); + } else { + // Non-inverted case + const needleSubtreeRangeEnd = + callNodeTable.subtreeRangeEnd[needleNodeIndex]; + + // Loop over each sample and accumulate the self time, running time, and + // the implementation breakdown. + for (let sampleIndex = 0; sampleIndex < samples.length; sampleIndex++) { + // Get the call node for this sample. + // TODO: Consider using sampleCallNodes for this, to save one indirection on + // a hot path. + const thisStackIndex = samples.stack[sampleIndex]; + if (thisStackIndex === null) { + continue; + } + const thisNodeIndex = stackIndexToCallNodeIndex[thisStackIndex]; + const weight = samples.weight ? samples.weight[sampleIndex] : 1; + rootTime += Math.abs(weight); - if (!isInvertedTree) { // For non-inverted trees, we compute the self time from the stacks' leaf nodes. if (thisNodeIndex === needleNodeIndex) { accumulateDataToTimings(pathTimings.selfTime, sampleIndex, weight); } - } - - if ( - thisNodeIndex >= needleNodeIndex && - thisNodeIndex < needleDescendantsEndIndex - ) { - // One of the parents is the exact passed path. - accumulateDataToTimings(pathTimings.totalTime, sampleIndex, weight); - - if (needleNodeIsRootOfInvertedTree) { - // This root node matches the passed call node path. - // This is the only place where we don't accumulate timings, mainly - // because this would be the same as for the total time. - pathTimings.selfTime.value += weight; + if ( + thisNodeIndex >= needleNodeIndex && + thisNodeIndex < needleSubtreeRangeEnd + ) { + // One of the parents is the exact passed path. + accumulateDataToTimings(pathTimings.totalTime, sampleIndex, weight); } } } From bbf576076b55f0414cc3baeb949a468bf744f9ea Mon Sep 17 00:00:00 2001 From: Markus Stange Date: Sat, 20 Jan 2024 00:00:33 -0500 Subject: [PATCH 07/18] Implement getTreeOrderComparator for the inverted tree with non-inverted call nodes. This function is used when hovering or clicking the activity graph. This commit replaces a SampleIndexToCallNodeIndex mapping with a SampleIndexToNonInvertedCallNodeIndex mapping. --- src/profile-logic/profile-data.js | 45 +++++++++++++++++++ src/selectors/per-thread/stack-sample.js | 3 +- src/test/unit/profile-data.test.js | 57 +++++++++++------------- 3 files changed, 73 insertions(+), 32 deletions(-) diff --git a/src/profile-logic/profile-data.js b/src/profile-logic/profile-data.js index 80369527b4..ad7cffb479 100644 --- a/src/profile-logic/profile-data.js +++ b/src/profile-logic/profile-data.js @@ -2798,6 +2798,19 @@ export function getFuncNamesAndOriginsForPath( * highlighted area for a selected subtree is contiguous in the graph. */ export function getTreeOrderComparator( + sampleNonInvertedCallNodes: Array, + callNodeInfo: CallNodeInfo +): (IndexIntoSamplesTable, IndexIntoSamplesTable) => number { + const callNodeInfoInverted = callNodeInfo.asInverted(); + return callNodeInfoInverted !== null + ? _getTreeOrderComparatorInverted( + sampleNonInvertedCallNodes, + callNodeInfoInverted + ) + : _getTreeOrderComparatorNonInverted(sampleNonInvertedCallNodes); +} + +export function _getTreeOrderComparatorNonInverted( sampleCallNodes: Array ): (IndexIntoSamplesTable, IndexIntoSamplesTable) => number { /** @@ -2831,6 +2844,38 @@ export function getTreeOrderComparator( }; } +function _getTreeOrderComparatorInverted( + sampleNonInvertedCallNodes: Array, + callNodeInfo: CallNodeInfoInverted +): (IndexIntoSamplesTable, IndexIntoSamplesTable) => number { + const callNodeTable = callNodeInfo.getNonInvertedCallNodeTable(); + return function treeOrderComparator( + sampleA: IndexIntoSamplesTable, + sampleB: IndexIntoSamplesTable + ): number { + const callNodeA = sampleNonInvertedCallNodes[sampleA]; + const callNodeB = sampleNonInvertedCallNodes[sampleB]; + + if (callNodeA === callNodeB) { + // Both are filtered out or both are the same. + return 0; + } + if (callNodeA === null) { + // A filtered out, B not filtered out. A goes after B. + return 1; + } + if (callNodeB === null) { + // B filtered out, A not filtered out. B goes after A. + return -1; + } + return _compareNonInvertedCallNodesInSuffixOrder( + callNodeA, + callNodeB, + callNodeTable + ); + }; +} + export function getFriendlyStackTypeName( implementation: StackImplementation ): string { diff --git a/src/selectors/per-thread/stack-sample.js b/src/selectors/per-thread/stack-sample.js index 86e3a6ce1b..fe742edd76 100644 --- a/src/selectors/per-thread/stack-sample.js +++ b/src/selectors/per-thread/stack-sample.js @@ -300,7 +300,8 @@ export function getStackAndSampleSelectorsPerThread( const getTreeOrderComparatorInFilteredThread: Selector< (IndexIntoSamplesTable, IndexIntoSamplesTable) => number, > = createSelector( - getSampleIndexToCallNodeIndexForFilteredThread, + getSampleIndexToNonInvertedCallNodeIndexForFilteredThread, + getCallNodeInfo, ProfileData.getTreeOrderComparator ); diff --git a/src/test/unit/profile-data.test.js b/src/test/unit/profile-data.test.js index cd3e90e8ab..8e35e25b52 100644 --- a/src/test/unit/profile-data.test.js +++ b/src/test/unit/profile-data.test.js @@ -994,17 +994,10 @@ describe('getSamplesSelectedStates', function () { stackIndexToCallNodeIndex, defaultCategory ); - const stackIndexToInvertedCallNodeIndex = - callNodeInfoInverted.getStackIndexToCallNodeIndex(); - const sampleInvertedCallNodes = getSampleIndexToCallNodeIndex( - thread.samples.stack, - stackIndexToInvertedCallNodeIndex - ); return { callNodeInfo, callNodeInfoInverted, - sampleInvertedCallNodes, sampleCallNodes, funcNamesDict, }; @@ -1086,7 +1079,7 @@ describe('getSamplesSelectedStates', function () { }); it('can sort the samples based on their selection status', function () { - const comparator = getTreeOrderComparator(sampleCallNodes); + const comparator = getTreeOrderComparator(sampleCallNodes, callNodeInfo); const samples = [4, 1, 3, 0, 2]; // some random order samples.sort(comparator); expect(samples).toEqual([0, 2, 4, 1, 3]); @@ -1100,31 +1093,30 @@ describe('getSamplesSelectedStates', function () { describe('inverted', function () { /** - * - [cn0] A = A - * - [cn1] B = A -> B - * - [cn2] A = A -> B -> A - * - [cn3] C = A -> B -> C - * - [cn4] A = A -> A - * - [cn5] B = A -> A -> B - * - [cn6] C = A -> C + * - [cn0] A = A = A [so0] [so0] [cn0] A + * - [cn1] B = A -> B = A -> B [so3] [so1] [cn4] A <- A + * - [cn2] A = A -> B -> A = A -> B -> A [so2] ↘↗ [so2] [cn2] A <- B <- A + * - [cn3] C = A -> B -> C = A -> B -> C [so6] ↗↘ [so3] [cn1] B <- A + * - [cn4] A = A -> A = A -> A [so1] [so4] [cn5] B <- A <- A + * - [cn5] B = A -> A -> B = A -> A -> B [so4] [so5] [cn6] C <- A + * - [cn6] C = A -> C = A -> C [so5] [so6] [cn3] C <- B <- A * - * - * - [in0] A - * - [in1] A - * - [in2] B - * - [in3] A - * - [in4] B - * - [in5] A - * - [in6] A - * - [in7] C - * - [in8] A - * - [in9] B - * - [in10] A + * Represents call paths ending in + * - [in0] A (so:0..3) = A = ... A (cn0, cn4, cn2) + * - [in1] A (so:1..2) = A <- A = ... A -> A (cn4) + * - [in2] B (so:2..3) = A <- B = ... B -> A (cn2) + * - [in3] A (so:2..3) = A <- B <- A = ... A -> B -> A (cn2) + * - [in4] B (so:3..5) = B = ... B (cn1, cn5) + * - [in5] A (so:3..5) = B <- A = ... A -> B (cn1, cn5) + * - [in6] A (so:4..5) = B <- A <- A = ... A -> A -> B (cn5) + * - [in7] C (so:5..7) = C = ... C (cn6, cn3) + * - [in8] A (so:5..6) = C <- A = ... A -> C (cn6) + * - [in9] B (so:6..7) = C <- B = ... B -> C (cn3) + * - [in10] A (so:6..7) = C <- B <- A = ... A -> B -> C (cn3) * */ const { callNodeInfoInverted, sampleCallNodes, - sampleInvertedCallNodes, funcNamesDict: { A, B, C }, } = setup(` A A A A A A A @@ -1199,16 +1191,19 @@ describe('getSamplesSelectedStates', function () { }); it('can sort the samples based on their selection status', function () { - const comparator = getTreeOrderComparator(sampleInvertedCallNodes); + const comparator = getTreeOrderComparator( + sampleCallNodes, + callNodeInfoInverted + ); /** - * original order (non-inverted): + * in original order: * 0 1 2 3 4 5 6 * A A A A A A A * A B B C B A * A C B * - * sorted order ("inverted" if you read from bottom to top): + * in suffix order: * A A A * A B A A A B * A A A B B C C From 0f1d3a2ca355bc89ab0b7c313787401c8d61f83a Mon Sep 17 00:00:00 2001 From: Markus Stange Date: Sat, 20 Jan 2024 00:04:38 -0500 Subject: [PATCH 08/18] Use SampleIndexToNonInvertedCallNodeIndex for the stack chart. The stack chart is always non-inverted, so this commit is functionally neutral. This lets us remove the now-unused function getSampleIndexToCallNodeIndexForFilteredThread. --- src/selectors/per-thread/stack-sample.js | 15 +-------------- 1 file changed, 1 insertion(+), 14 deletions(-) diff --git a/src/selectors/per-thread/stack-sample.js b/src/selectors/per-thread/stack-sample.js index fe742edd76..545b8245bb 100644 --- a/src/selectors/per-thread/stack-sample.js +++ b/src/selectors/per-thread/stack-sample.js @@ -230,18 +230,6 @@ export function getStackAndSampleSelectorsPerThread( ) ); - const getSampleIndexToCallNodeIndexForFilteredThread: Selector< - Array, - > = createSelector( - (state) => threadSelectors.getFilteredThread(state).samples.stack, - (state) => getCallNodeInfo(state).getStackIndexToCallNodeIndex(), - (filteredThreadSampleStacks, stackIndexToCallNodeIndex) => - ProfileData.getSampleIndexToCallNodeIndex( - filteredThreadSampleStacks, - stackIndexToCallNodeIndex - ) - ); - const getSampleIndexToCallNodeIndexForPreviewFilteredThread: Selector< Array, > = createSelector( @@ -410,7 +398,7 @@ export function getStackAndSampleSelectorsPerThread( const getStackTimingByDepth: Selector = createSelector( threadSelectors.getFilteredSamplesForCallTree, - getSampleIndexToCallNodeIndexForFilteredThread, + getSampleIndexToNonInvertedCallNodeIndexForFilteredThread, getCallNodeInfo, getFilteredCallNodeMaxDepthPlusOne, ProfileSelectors.getProfileInterval, @@ -461,7 +449,6 @@ export function getStackAndSampleSelectorsPerThread( getSelectedCallNodeIndex, getExpandedCallNodePaths, getExpandedCallNodeIndexes, - getSampleIndexToCallNodeIndexForFilteredThread, getSampleIndexToNonInvertedCallNodeIndexForFilteredThread, getSamplesSelectedStatesInFilteredThread, getTreeOrderComparatorInFilteredThread, From 0b195fd10eea1f02381221b794977c4ffbf267cb Mon Sep 17 00:00:00 2001 From: Markus Stange Date: Sat, 20 Jan 2024 00:09:27 -0500 Subject: [PATCH 09/18] Remove now-unused stack-to-inverted-call-node mapping. --- src/profile-logic/call-node-info.js | 13 ------ src/profile-logic/profile-data.js | 40 +++---------------- .../__snapshots__/profile-view.test.js.snap | 33 --------------- src/types/profile-derived.js | 18 +-------- 4 files changed, 7 insertions(+), 97 deletions(-) diff --git a/src/profile-logic/call-node-info.js b/src/profile-logic/call-node-info.js index 78b0757e6c..0524646895 100644 --- a/src/profile-logic/call-node-info.js +++ b/src/profile-logic/call-node-info.js @@ -32,11 +32,6 @@ export class CallNodeInfoImpl implements CallNodeInfo { // The non-inverted call node table, regardless of isInverted(). _nonInvertedCallNodeTable: CallNodeTable; - // The mapping of stack index to corresponding call node index. This maps to - // either the inverted or the non-inverted call node table, depending on - // isInverted(). - _stackIndexToCallNodeIndex: Int32Array; - // The mapping of stack index to corresponding non-inverted call node index. // This always maps to the non-inverted call node table, regardless of // isInverted(). @@ -50,12 +45,10 @@ export class CallNodeInfoImpl implements CallNodeInfo { constructor( callNodeTable: CallNodeTable, nonInvertedCallNodeTable: CallNodeTable, - stackIndexToCallNodeIndex: Int32Array, stackIndexToNonInvertedCallNodeIndex: Int32Array ) { this._callNodeTable = callNodeTable; this._nonInvertedCallNodeTable = nonInvertedCallNodeTable; - this._stackIndexToCallNodeIndex = stackIndexToCallNodeIndex; this._stackIndexToNonInvertedCallNodeIndex = stackIndexToNonInvertedCallNodeIndex; } @@ -74,10 +67,6 @@ export class CallNodeInfoImpl implements CallNodeInfo { return this._callNodeTable; } - getStackIndexToCallNodeIndex(): Int32Array { - return this._stackIndexToCallNodeIndex; - } - getNonInvertedCallNodeTable(): CallNodeTable { return this._nonInvertedCallNodeTable; } @@ -239,7 +228,6 @@ export class CallNodeInfoInvertedImpl constructor( callNodeTable: CallNodeTable, nonInvertedCallNodeTable: CallNodeTable, - stackIndexToCallNodeIndex: Int32Array, stackIndexToNonInvertedCallNodeIndex: Int32Array, suffixOrderedCallNodes: Uint32Array, suffixOrderIndexes: Uint32Array @@ -247,7 +235,6 @@ export class CallNodeInfoInvertedImpl super( callNodeTable, nonInvertedCallNodeTable, - stackIndexToCallNodeIndex, stackIndexToNonInvertedCallNodeIndex ); this._suffixOrderedCallNodes = suffixOrderedCallNodes; diff --git a/src/profile-logic/profile-data.js b/src/profile-logic/profile-data.js index ad7cffb479..34734dc389 100644 --- a/src/profile-logic/profile-data.js +++ b/src/profile-logic/profile-data.js @@ -110,7 +110,6 @@ export function getCallNodeInfo( return new CallNodeInfoImpl( callNodeTable, callNodeTable, - stackIndexToCallNodeIndex, stackIndexToCallNodeIndex ); } @@ -435,47 +434,19 @@ export function getInvertedCallNodeInfo( defaultCategory: IndexIntoCategoryList ): CallNodeInfoInverted { // We compute an inverted stack table, but we don't let it escape this function. - const { - invertedThread, - oldStackToNewStack: nonInvertedStackToInvertedStack, - } = _computeThreadWithInvertedStackTable(thread, defaultCategory); + const { invertedThread } = _computeThreadWithInvertedStackTable( + thread, + defaultCategory + ); // Create an inverted call node table based on the inverted stack table. - const { - callNodeTable, - stackIndexToCallNodeIndex: invertedStackIndexToCallNodeIndex, - } = computeCallNodeTable( + const { callNodeTable } = computeCallNodeTable( invertedThread.stackTable, invertedThread.frameTable, invertedThread.funcTable, defaultCategory ); - // Create a mapping that maps a stack index from the non-inverted thread to - // its corresponding call node in the inverted tree. - const nonInvertedStackIndexToCallNodeIndex = new Int32Array( - thread.stackTable.length - ); - for ( - let nonInvertedStackIndex = 0; - nonInvertedStackIndex < nonInvertedStackIndexToCallNodeIndex.length; - nonInvertedStackIndex++ - ) { - const invertedStackIndex = nonInvertedStackToInvertedStack.get( - nonInvertedStackIndex - ); - if (invertedStackIndex === undefined) { - // This stack is not used as a self stack, only as a prefix stack. - // There may or may not be an inverted call node that corresponds to it, - // but we haven't checked that and we don't need to know it. - // nonInvertedStackIndexToCallNodeIndex only needs useful values for self stacks. - nonInvertedStackIndexToCallNodeIndex[nonInvertedStackIndex] = -1; - } else { - nonInvertedStackIndexToCallNodeIndex[nonInvertedStackIndex] = - invertedStackIndexToCallNodeIndex[invertedStackIndex]; - } - } - // TEMPORARY: Compute a suffix order for the entire non-inverted call node table. // See the CallNodeInfoInverted interface for more details about the suffix order. // By the end of this commit stack, the suffix order will be computed incrementally @@ -496,7 +467,6 @@ export function getInvertedCallNodeInfo( return new CallNodeInfoInvertedImpl( callNodeTable, nonInvertedCallNodeTable, - nonInvertedStackIndexToCallNodeIndex, stackIndexToNonInvertedCallNodeIndex, suffixOrderedCallNodes, suffixOrderIndexes diff --git a/src/test/store/__snapshots__/profile-view.test.js.snap b/src/test/store/__snapshots__/profile-view.test.js.snap index 41a98d367e..491c27db39 100644 --- a/src/test/store/__snapshots__/profile-view.test.js.snap +++ b/src/test/store/__snapshots__/profile-view.test.js.snap @@ -2424,17 +2424,6 @@ CallNodeInfoImpl { 9, ], }, - "_stackIndexToCallNodeIndex": Int32Array [ - 0, - 1, - 2, - 3, - 4, - 5, - 6, - 7, - 8, - ], "_stackIndexToNonInvertedCallNodeIndex": Int32Array [ 0, 1, @@ -2659,17 +2648,6 @@ CallTree { 9, ], }, - "_stackIndexToCallNodeIndex": Int32Array [ - 0, - 1, - 2, - 3, - 4, - 5, - 6, - 7, - 8, - ], "_stackIndexToNonInvertedCallNodeIndex": Int32Array [ 0, 1, @@ -3065,17 +3043,6 @@ CallTree { 9, ], }, - "_stackIndexToCallNodeIndex": Int32Array [ - 0, - 1, - 2, - 3, - 4, - 5, - 6, - 7, - 8, - ], "_stackIndexToNonInvertedCallNodeIndex": Int32Array [ 0, 1, diff --git a/src/types/profile-derived.js b/src/types/profile-derived.js index cc188b3f37..402be9a66e 100644 --- a/src/types/profile-derived.js +++ b/src/types/profile-derived.js @@ -139,25 +139,11 @@ export interface CallNodeInfo { // This is always the non-inverted call node table, regardless of isInverted(). getNonInvertedCallNodeTable(): CallNodeTable; - // Returns a mapping from the stack table to the call node table. + // Returns a mapping from the stack table to the non-inverted call node table. // The Int32Array should be used as if it were a // Map. // - // If this CallNodeInfo is for the non-inverted tree, this maps the stack index - // to its corresponding call node index, and all entries are >= 0. - // If this CallNodeInfo is for the inverted tree, this maps the non-inverted - // stack index to the inverted call node index. For example, the stack - // A -> B -> C -> D is mapped to the inverted call node describing the - // call path D <- C <- B <- A, i.e. the node with function A under the D root - // of the inverted tree. Stacks which are only used as prefixes are not mapped - // to an inverted call node; for those, the entry will be -1. In the example - // above, if the stack node A -> B -> C only exists so that it can be the prefix - // of the A -> B -> C -> D stack and no sample / marker / allocation has - // A -> B -> C as its stack, then there is no need to have a call node - // C <- B <- A in the inverted call node table. - getStackIndexToCallNodeIndex(): Int32Array; - - // Returns a mapping from the stack table to the non-inverted call node table. + // All entries are >= 0. // This always maps to the non-inverted call node table, regardless of isInverted(). getStackIndexToNonInvertedCallNodeIndex(): Int32Array; From c21a2ab19989b94d5e25222ff4a0fffe97f42bef Mon Sep 17 00:00:00 2001 From: Markus Stange Date: Sat, 20 Jan 2024 00:30:17 -0500 Subject: [PATCH 10/18] Call getNonInvertedCallNodeTable() in more stack-chart-related places. This removes a few more uses of getCallNodeTable(). --- src/components/stack-chart/Canvas.js | 2 +- src/profile-logic/stack-timing.js | 2 +- src/test/unit/profile-data.test.js | 4 ++-- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/components/stack-chart/Canvas.js b/src/components/stack-chart/Canvas.js index ce865cc563..df25e28f73 100644 --- a/src/components/stack-chart/Canvas.js +++ b/src/components/stack-chart/Canvas.js @@ -262,7 +262,7 @@ class StackChartCanvasImpl extends React.PureComponent { categoryForUserTiming = 0; } - const callNodeTable = callNodeInfo.getCallNodeTable(); + const callNodeTable = callNodeInfo.getNonInvertedCallNodeTable(); // Only draw the stack frames that are vertically within view. for (let depth = startDepth; depth < endDepth; depth++) { diff --git a/src/profile-logic/stack-timing.js b/src/profile-logic/stack-timing.js index fc8c728a69..c1aff9f5ce 100644 --- a/src/profile-logic/stack-timing.js +++ b/src/profile-logic/stack-timing.js @@ -66,7 +66,7 @@ export function getStackTimingByDepth( maxDepthPlusOne: number, interval: Milliseconds ): StackTimingByDepth { - const callNodeTable = callNodeInfo.getCallNodeTable(); + const callNodeTable = callNodeInfo.getNonInvertedCallNodeTable(); const { prefix: callNodeTablePrefixColumn, subtreeRangeEnd: callNodeTableSubtreeRangeEndColumn, diff --git a/src/test/unit/profile-data.test.js b/src/test/unit/profile-data.test.js index 8e35e25b52..4fa5033511 100644 --- a/src/test/unit/profile-data.test.js +++ b/src/test/unit/profile-data.test.js @@ -895,14 +895,14 @@ describe('funcHasDirectRecursiveCall and funcHasRecursiveCall', function () { thread.frameTable, thread.funcTable, defaultCategory - ).getCallNodeTable(); + ).getNonInvertedCallNodeTable(); const jsOnlyThread = filterThreadByImplementation(thread, 'js'); const jsOnlyCallNodeTable = getCallNodeInfo( jsOnlyThread.stackTable, jsOnlyThread.frameTable, jsOnlyThread.funcTable, defaultCategory - ).getCallNodeTable(); + ).getNonInvertedCallNodeTable(); return { callNodeTable, jsOnlyCallNodeTable, funcNames }; } From 17ca128251d6508041ac8fb4d97647d3442604f4 Mon Sep 17 00:00:00 2001 From: Markus Stange Date: Tue, 6 Aug 2024 15:07:28 -0400 Subject: [PATCH 11/18] Compute inverted call tree timings differently. This replaces lots of uses of getCallNodeTable() with uses of getNonInvertedCallNodeTable(). It also replaces lots of uses of getStackIndexToCallNodeIndex() with uses of getStackIndexToNonInvertedCallNodeIndex(). We now compute the call tree timings quite differently for inverted mode compared to non-inverted mode. There's one part of the work that's shared: The getCallNodeLeafAndSummary computes the self time for each non-inverted node, and the result is used for both the inverted and the non-inverted call tree timings. The CallTreeTimings Flow type is turned into an enum, with a different type for CallTreeTimingsNonInverted and for CallTreeTimingsInverted. A new implementation for the CallTreeInternal interface is added. --- src/components/flame-graph/Canvas.js | 4 +- src/components/flame-graph/FlameGraph.js | 7 +- src/profile-logic/call-node-info.js | 34 ++ src/profile-logic/call-tree.js | 389 +++++++++++++++--- src/profile-logic/flame-graph.js | 4 +- src/selectors/per-thread/stack-sample.js | 58 ++- src/test/fixtures/utils.js | 4 +- .../__snapshots__/profile-view.test.js.snap | 2 +- src/test/store/profile-view.test.js | 9 +- src/test/unit/profile-tree.test.js | 31 +- src/types/profile-derived.js | 6 + 11 files changed, 440 insertions(+), 108 deletions(-) diff --git a/src/components/flame-graph/Canvas.js b/src/components/flame-graph/Canvas.js index 1f6f11ce2c..8c691bfd26 100644 --- a/src/components/flame-graph/Canvas.js +++ b/src/components/flame-graph/Canvas.js @@ -50,7 +50,7 @@ import type { import type { CallTree, - CallTreeTimings, + CallTreeTimingsNonInverted, } from 'firefox-profiler/profile-logic/call-tree'; export type OwnProps = {| @@ -77,7 +77,7 @@ export type OwnProps = {| +callTreeSummaryStrategy: CallTreeSummaryStrategy, +samples: SamplesLikeTable, +unfilteredSamples: SamplesLikeTable, - +tracedTiming: CallTreeTimings | null, + +tracedTiming: CallTreeTimingsNonInverted | null, +displayImplementation: boolean, +displayStackType: boolean, |}; diff --git a/src/components/flame-graph/FlameGraph.js b/src/components/flame-graph/FlameGraph.js index 2d221359a9..86123df087 100644 --- a/src/components/flame-graph/FlameGraph.js +++ b/src/components/flame-graph/FlameGraph.js @@ -344,6 +344,11 @@ class FlameGraphImpl extends React.PureComponent { displayStackType, } = this.props; + const tracedTimingNonInverted = + tracedTiming !== null && tracedTiming.type === 'NON_INVERTED' + ? tracedTiming.timings + : null; + const maxViewportHeight = maxStackDepthPlusOne * STACK_FRAME_HEIGHT; return ( @@ -394,7 +399,7 @@ class FlameGraphImpl extends React.PureComponent { isInverted, samples, unfilteredSamples, - tracedTiming, + tracedTiming: tracedTimingNonInverted, displayImplementation, displayStackType, }} diff --git a/src/profile-logic/call-node-info.js b/src/profile-logic/call-node-info.js index 0524646895..61f764c4f3 100644 --- a/src/profile-logic/call-node-info.js +++ b/src/profile-logic/call-node-info.js @@ -199,9 +199,43 @@ export class CallNodeInfoImpl implements CallNodeInfo { return null; } + getRoots(): IndexIntoCallNodeTable[] { + const roots = []; + if (this._callNodeTable.length !== 0) { + for ( + let invertedRoot = 0; + invertedRoot !== -1; + invertedRoot = this._callNodeTable.nextSibling[invertedRoot] + ) { + roots.push(invertedRoot); + } + } + return roots; + } + isRoot(callNodeIndex: IndexIntoCallNodeTable): boolean { return this._callNodeTable.prefix[callNodeIndex] === -1; } + + getChildren(callNodeIndex: IndexIntoCallNodeTable): IndexIntoCallNodeTable[] { + if ( + this._callNodeTable.subtreeRangeEnd[callNodeIndex] === + callNodeIndex + 1 + ) { + return []; + } + + const children = []; + const firstChild = callNodeIndex + 1; + for ( + let childCallNodeIndex = firstChild; + childCallNodeIndex !== -1; + childCallNodeIndex = this._callNodeTable.nextSibling[childCallNodeIndex] + ) { + children.push(childCallNodeIndex); + } + return children; + } } /** diff --git a/src/profile-logic/call-tree.js b/src/profile-logic/call-tree.js index b8e01e0525..5db559e467 100644 --- a/src/profile-logic/call-tree.js +++ b/src/profile-logic/call-tree.js @@ -22,6 +22,7 @@ import type { CallNodePath, IndexIntoCallNodeTable, CallNodeInfo, + CallNodeInfoInverted, CallNodeData, CallNodeDisplayData, Milliseconds, @@ -39,13 +40,32 @@ import type { CallTreeSummaryStrategy } from '../types/actions'; type CallNodeChildren = IndexIntoCallNodeTable[]; -export type CallTreeTimings = { +export type CallTreeTimingsNonInverted = {| callNodeHasChildren: Uint8Array, self: Float32Array, leaf: Float32Array, total: Float32Array, + rootTotalSummary: number, // sum of absolute values, this is used for computing percentages +|}; + +type TotalAndHasChildren = {| total: number, hasChildren: boolean |}; + +export type InvertedCallTreeRoot = {| + totalAndHasChildren: TotalAndHasChildren, + func: IndexIntoFuncTable, +|}; + +export type CallTreeTimingsInverted = {| + callNodeLeaf: Float32Array, rootTotalSummary: number, -}; + sortedRoots: IndexIntoFuncTable[], + totalPerRootNode: Map, + rootNodesWithChildren: Set, +|}; + +export type CallTreeTimings = + | {| type: 'NON_INVERTED', timings: CallTreeTimingsNonInverted |} + | {| type: 'INVERTED', timings: CallTreeTimingsInverted |}; function extractFaviconFromLibname(libname: string): string | null { try { @@ -74,15 +94,18 @@ interface CallTreeInternal { ): CallNodePath; } -export class CallTreeInternalImpl implements CallTreeInternal { +export class CallTreeInternalNonInverted implements CallTreeInternal { _callNodeInfo: CallNodeInfo; _callNodeTable: CallNodeTable; - _callTreeTimings: CallTreeTimings; + _callTreeTimings: CallTreeTimingsNonInverted; _callNodeHasChildren: Uint8Array; // A table column matching the callNodeTable - constructor(callNodeInfo: CallNodeInfo, callTreeTimings: CallTreeTimings) { + constructor( + callNodeInfo: CallNodeInfo, + callTreeTimings: CallTreeTimingsNonInverted + ) { this._callNodeInfo = callNodeInfo; - this._callNodeTable = callNodeInfo.getCallNodeTable(); + this._callNodeTable = callNodeInfo.getNonInvertedCallNodeTable(); this._callTreeTimings = callTreeTimings; this._callNodeHasChildren = callTreeTimings.callNodeHasChildren; } @@ -157,6 +180,127 @@ export class CallTreeInternalImpl implements CallTreeInternal { } } +class CallTreeInternalInverted implements CallTreeInternal { + _callNodeInfo: CallNodeInfoInverted; + _nonInvertedCallNodeTable: CallNodeTable; + _callNodeLeaf: Float32Array; + _rootNodes: IndexIntoCallNodeTable[]; + _funcCount: number; + _totalPerRootNode: Map; + _rootNodesWithChildren: Set; + _totalAndHasChildrenPerNonRootNode: Map< + IndexIntoCallNodeTable, + TotalAndHasChildren, + > = new Map(); + + constructor( + callNodeInfo: CallNodeInfoInverted, + callTreeTimingsInverted: CallTreeTimingsInverted + ) { + this._callNodeInfo = callNodeInfo; + this._nonInvertedCallNodeTable = callNodeInfo.getNonInvertedCallNodeTable(); + this._callNodeLeaf = callTreeTimingsInverted.callNodeLeaf; + const { sortedRoots, totalPerRootNode, rootNodesWithChildren } = + callTreeTimingsInverted; + this._totalPerRootNode = totalPerRootNode; + this._rootNodesWithChildren = rootNodesWithChildren; + this._rootNodes = sortedRoots; + } + + createRoots(): IndexIntoCallNodeTable[] { + return this._rootNodes; + } + + hasChildren(callNodeIndex: IndexIntoCallNodeTable): boolean { + if (this._callNodeInfo.isRoot(callNodeIndex)) { + return this._rootNodesWithChildren.has(callNodeIndex); + } + return this._getTotalAndHasChildren(callNodeIndex).hasChildren; + } + + createChildren(nodeIndex: IndexIntoCallNodeTable): CallNodeChildren { + if (!this.hasChildren(nodeIndex)) { + return []; + } + + const children = this._callNodeInfo + .getChildren(nodeIndex) + .filter((child) => { + const { total, hasChildren } = this._getTotalAndHasChildren(child); + return total !== 0 || hasChildren; + }); + children.sort( + (a, b) => + Math.abs(this._getTotalAndHasChildren(b).total) - + Math.abs(this._getTotalAndHasChildren(a).total) + ); + return children; + } + + getSelfAndTotal(callNodeIndex: IndexIntoCallNodeTable): SelfAndTotal { + if (this._callNodeInfo.isRoot(callNodeIndex)) { + const total = ensureExists(this._totalPerRootNode.get(callNodeIndex)); + return { self: total, total }; + } + const { total } = this._getTotalAndHasChildren(callNodeIndex); + return { self: 0, total }; + } + + _getTotalAndHasChildren( + callNodeIndex: IndexIntoCallNodeTable + ): TotalAndHasChildren { + if (this._callNodeInfo.isRoot(callNodeIndex)) { + throw new Error('This function should not be called for roots'); + } + + const cached = this._totalAndHasChildrenPerNonRootNode.get(callNodeIndex); + if (cached !== undefined) { + return cached; + } + + const totalAndHasChildren = _getInvertedTreeNodeTotalAndHasChildren( + callNodeIndex, + this._callNodeInfo, + this._callNodeLeaf + ); + this._totalAndHasChildrenPerNonRootNode.set( + callNodeIndex, + totalAndHasChildren + ); + return totalAndHasChildren; + } + + findHeaviestPathInSubtree( + callNodeIndex: IndexIntoCallNodeTable + ): CallNodePath { + const [rangeStart, rangeEnd] = + this._callNodeInfo.getSuffixOrderIndexRangeForCallNode(callNodeIndex); + const orderedCallNodes = this._callNodeInfo.getSuffixOrderedCallNodes(); + + // Find the non-inverted node with the highest self time. + let maxNode = -1; + let maxAbs = 0; + for (let i = rangeStart; i < rangeEnd; i++) { + const nodeIndex = orderedCallNodes[i]; + const nodeSelf = Math.abs(this._callNodeLeaf[nodeIndex]); + if (maxNode === -1 || nodeSelf > maxAbs) { + maxNode = nodeIndex; + maxAbs = nodeSelf; + } + } + + const callPath = []; + for ( + let currentNode = maxNode; + currentNode !== -1; + currentNode = this._nonInvertedCallNodeTable.prefix[currentNode] + ) { + callPath.push(this._nonInvertedCallNodeTable.func[currentNode]); + } + return callPath; + } +} + export class CallTree { _categories: CategoryList; _internal: CallTreeInternal; @@ -424,7 +568,7 @@ export class CallTree { } /** - * Take a CallNodeIndex, and compute an inverted path for it. + * Take a IndexIntoCallNodeTable, and compute an inverted path for it. * * e.g: * (invertedPath, invertedCallTree) => path @@ -453,43 +597,6 @@ export class CallTree { } } -// 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; - callNodeIndex++ - ) { - const prefixCallNode = callNodeTable.prefix[callNodeIndex]; - if (prefixCallNode === -1) { - // callNodeIndex is a root node - callNodeToRoot[callNodeIndex] = callNodeIndex; - } else { - // The callNodeTable guarantees that a callNode's prefix always comes - // before the callNode; prefix references are always to lower callNode - // indexes and never to higher indexes. - // We are iterating the callNodeTable in forwards direction (starting at - // index 0) so we know that we have already visited the current call - // node's prefix call node and can reuse its stored root node, which - // recursively is the value we're looking for. - callNodeToRoot[callNodeIndex] = callNodeToRoot[prefixCallNode]; - } - callNodeSelf[callNodeToRoot[callNodeIndex]] += callNodeLeaf[callNodeIndex]; - } - - return callNodeSelf; -} - /** * Compute the leaf time for each call node, and the sum of the absolute leaf * values. @@ -523,22 +630,155 @@ export function computeCallNodeLeafAndSummary( return { callNodeLeaf, rootTotalSummary }; } +export function getSelfAndTotalForCallNode( + callNodeIndex: IndexIntoCallNodeTable, + callNodeInfo: CallNodeInfo, + callTreeTimings: CallTreeTimings +): SelfAndTotal { + switch (callTreeTimings.type) { + case 'NON_INVERTED': { + const { timings } = callTreeTimings; + const self = timings.self[callNodeIndex]; + const total = timings.total[callNodeIndex]; + return { self, total }; + } + case 'INVERTED': { + const callNodeInfoInverted = ensureExists(callNodeInfo.asInverted()); + const { timings } = callTreeTimings; + const { callNodeLeaf, totalPerRootNode } = timings; + if (callNodeInfoInverted.isRoot(callNodeIndex)) { + const total = totalPerRootNode.get(callNodeIndex) ?? 0; + return { self: total, total }; + } + const { total } = _getInvertedTreeNodeTotalAndHasChildren( + callNodeIndex, + callNodeInfoInverted, + callNodeLeaf + ); + return { self: 0, total }; + } + default: + throw assertExhaustiveCheck(callTreeTimings.type); + } +} + +function _getInvertedTreeNodeTotalAndHasChildren( + callNodeIndex: IndexIntoCallNodeTable, + callNodeInfo: CallNodeInfoInverted, + callNodeLeaf: Float32Array +): TotalAndHasChildren { + const nodeDepth = callNodeInfo.getCallNodeTable().depth[callNodeIndex]; + const [rangeStart, rangeEnd] = + callNodeInfo.getSuffixOrderIndexRangeForCallNode(callNodeIndex); + const suffixOrderedCallNodes = callNodeInfo.getSuffixOrderedCallNodes(); + const callNodeTableDepthCol = + callNodeInfo.getNonInvertedCallNodeTable().depth; + let total = 0; + let hasChildren = false; + for (let i = rangeStart; i < rangeEnd; i++) { + const leafNode = suffixOrderedCallNodes[i]; + const leaf = callNodeLeaf[leafNode]; + total += leaf; + + // The inverted call node has children if any deeper call paths with non-zero + // self time contribute to it. + hasChildren = + hasChildren || + (leaf !== 0 && callNodeTableDepthCol[leafNode] > nodeDepth); + } + return { total, hasChildren }; +} + +export function computeCallTreeTimingsInverted( + callNodeInfo: CallNodeInfoInverted, + { callNodeLeaf, rootTotalSummary }: CallNodeLeafAndSummary +): CallTreeTimingsInverted { + const roots = callNodeInfo.getRoots(); + const invertedCallNodeTable = callNodeInfo.getCallNodeTable(); + const callNodeTable = callNodeInfo.getNonInvertedCallNodeTable(); + const callNodeTableFuncCol = callNodeTable.func; + const callNodeTableDepthCol = callNodeTable.depth; + const totalPerRootNode = new Map(); + const rootNodesWithChildren = new Set(); + const seenRoots = new Set(); + for (let i = 0; i < callNodeLeaf.length; i++) { + const leaf = callNodeLeaf[i]; + if (leaf === 0) { + continue; + } + + // Map the non-inverted call node to its corresponding root in the inverted + // call tree. This is done by finding the inverted root which corresponds to + // the leaf function of the non-inverted call node. + const func = callNodeTableFuncCol[i]; + const rootNode = roots.find( + (invertedCallNode) => + invertedCallNodeTable.func[invertedCallNode] === func + ); + if (rootNode === undefined) { + throw new Error( + "Couldn't find the inverted root for a function with non-zero self time." + ); + } + + totalPerRootNode.set( + rootNode, + (totalPerRootNode.get(rootNode) ?? 0) + leaf + ); + seenRoots.add(rootNode); + if (callNodeTableDepthCol[i] !== 0) { + rootNodesWithChildren.add(rootNode); + } + } + const sortedRoots = [...seenRoots]; + sortedRoots.sort( + (a, b) => + Math.abs(totalPerRootNode.get(b) ?? 0) - + Math.abs(totalPerRootNode.get(a) ?? 0) + ); + return { + callNodeLeaf, + rootTotalSummary, + sortedRoots, + totalPerRootNode, + rootNodesWithChildren, + }; +} + +export function computeCallTreeTimings( + callNodeInfo: CallNodeInfo, + callNodeLeafAndSummary: CallNodeLeafAndSummary +): CallTreeTimings { + const callNodeInfoInverted = callNodeInfo.asInverted(); + if (callNodeInfoInverted !== null) { + return { + type: 'INVERTED', + timings: computeCallTreeTimingsInverted( + callNodeInfoInverted, + callNodeLeafAndSummary + ), + }; + } + return { + type: 'NON_INVERTED', + timings: computeCallTreeTimingsNonInverted( + callNodeInfo, + callNodeLeafAndSummary + ), + }; +} + /** * 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. */ -export function computeCallTreeTimings( +export function computeCallTreeTimingsNonInverted( callNodeInfo: CallNodeInfo, callNodeLeafAndSummary: CallNodeLeafAndSummary -): CallTreeTimings { - const callNodeTable = callNodeInfo.getCallNodeTable(); +): CallTreeTimingsNonInverted { + const callNodeTable = callNodeInfo.getNonInvertedCallNodeTable(); 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. - const callNodeSelf = callNodeInfo.isInverted() - ? _getInvertedCallNodeSelf(callNodeLeaf, callNodeTable) - : callNodeLeaf; + const callNodeSelf = callNodeLeaf; // Compute the following variables: const callNodeTotalSummary = new Float32Array(callNodeTable.length); @@ -588,15 +828,40 @@ export function getCallTree( weightType: WeightType ): CallTree { return timeCode('getCallTree', () => { - return new CallTree( - thread, - categories, - callNodeInfo, - new CallTreeInternalImpl(callNodeInfo, callTreeTimings), - callTreeTimings.rootTotalSummary, - Boolean(thread.isJsTracer), - weightType - ); + switch (callTreeTimings.type) { + case 'NON_INVERTED': { + const { timings } = callTreeTimings; + return new CallTree( + thread, + categories, + callNodeInfo, + new CallTreeInternalNonInverted(callNodeInfo, timings), + timings.rootTotalSummary, + Boolean(thread.isJsTracer), + weightType + ); + } + case 'INVERTED': { + const { timings } = callTreeTimings; + return new CallTree( + thread, + categories, + callNodeInfo, + new CallTreeInternalInverted( + ensureExists(callNodeInfo.asInverted()), + timings + ), + timings.rootTotalSummary, + Boolean(thread.isJsTracer), + weightType + ); + } + default: + throw assertExhaustiveCheck( + callTreeTimings.type, + 'Unhandled CallTreeTimings type.' + ); + } }); } diff --git a/src/profile-logic/flame-graph.js b/src/profile-logic/flame-graph.js index fe74e4e25a..c4ae89b253 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 { CallTreeTimings } from './call-tree'; +import type { CallTreeTimingsNonInverted } from './call-tree'; import { bisectionRightByStrKey } from 'firefox-profiler/utils/bisect'; @@ -229,7 +229,7 @@ export function computeFlameGraphRows( export function getFlameGraphTiming( flameGraphRows: FlameGraphRows, callNodeTable: CallNodeTable, - callTreeTimings: CallTreeTimings + callTreeTimings: CallTreeTimingsNonInverted ): FlameGraphTiming { const { total, self, rootTotalSummary } = callTreeTimings; const { prefix } = callNodeTable; diff --git a/src/selectors/per-thread/stack-sample.js b/src/selectors/per-thread/stack-sample.js index 545b8245bb..5c66006e70 100644 --- a/src/selectors/per-thread/stack-sample.js +++ b/src/selectors/per-thread/stack-sample.js @@ -44,6 +44,7 @@ import type { $ReturnType, ThreadsKey, SelfAndTotal, + CallNodeLeafAndSummary, } from 'firefox-profiler/types'; import type { ThreadSelectorsPerThread } from './thread'; @@ -230,12 +231,12 @@ export function getStackAndSampleSelectorsPerThread( ) ); - const getSampleIndexToCallNodeIndexForPreviewFilteredThread: Selector< + const getSampleIndexToNonInvertedCallNodeIndexForPreviewFilteredThread: Selector< Array, > = createSelector( (state) => threadSelectors.getPreviewFilteredSamplesForCallTree(state).stack, - (state) => getCallNodeInfo(state).getStackIndexToCallNodeIndex(), + (state) => getCallNodeInfo(state).getStackIndexToNonInvertedCallNodeIndex(), ProfileData.getSampleIndexToCallNodeIndex ); @@ -318,23 +319,33 @@ export function getStackAndSampleSelectorsPerThread( (samples) => samples.weightType || 'samples' ); + const getCallNodeLeafAndSummary: Selector = + createSelector( + threadSelectors.getPreviewFilteredSamplesForCallTree, + getSampleIndexToNonInvertedCallNodeIndexForPreviewFilteredThread, + getCallNodeInfo, + (samples, sampleIndexToCallNodeIndex, callNodeInfo) => { + return CallTree.computeCallNodeLeafAndSummary( + samples, + sampleIndexToCallNodeIndex, + callNodeInfo.getNonInvertedCallNodeTable().length + ); + } + ); + const getCallTreeTimings: Selector = createSelector( - threadSelectors.getPreviewFilteredSamplesForCallTree, - getSampleIndexToCallNodeIndexForPreviewFilteredThread, getCallNodeInfo, - (samples, sampleIndexToCallNodeIndex, callNodeInfo) => { - const callNodeLeafAndSummary = CallTree.computeCallNodeLeafAndSummary( - samples, - sampleIndexToCallNodeIndex, - callNodeInfo.getCallNodeTable().length - ); - return CallTree.computeCallTreeTimings( - callNodeInfo, - callNodeLeafAndSummary - ); - } + getCallNodeLeafAndSummary, + CallTree.computeCallTreeTimings ); + const getCallTreeTimingsNonInverted: Selector = + createSelector( + getCallNodeInfo, + getCallNodeLeafAndSummary, + CallTree.computeCallTreeTimingsNonInverted + ); + const getCallTree: Selector = createSelector( threadSelectors.getPreviewFilteredThread, getCallNodeInfo, @@ -360,7 +371,7 @@ export function getStackAndSampleSelectorsPerThread( const getTracedTiming: Selector = createSelector( threadSelectors.getPreviewFilteredSamplesForCallTree, - getSampleIndexToCallNodeIndexForPreviewFilteredThread, + getSampleIndexToNonInvertedCallNodeIndexForPreviewFilteredThread, getCallNodeInfo, ProfileSelectors.getProfileInterval, (samples, sampleIndexToCallNodeIndex, callNodeInfo, interval) => { @@ -368,7 +379,7 @@ export function getStackAndSampleSelectorsPerThread( CallTree.computeCallNodeTracedLeafAndSummary( samples, sampleIndexToCallNodeIndex, - callNodeInfo.getCallNodeTable().length, + callNodeInfo.getNonInvertedCallNodeTable().length, interval ); if (callNodeLeafAndSummary === null) { @@ -384,14 +395,17 @@ export function getStackAndSampleSelectorsPerThread( const getTracedSelfAndTotalForSelectedCallNode: Selector = createSelector( getSelectedCallNodeIndex, + getCallNodeInfo, getTracedTiming, - (selectedCallNodeIndex, tracedTiming) => { + (selectedCallNodeIndex, callNodeInfo, tracedTiming) => { if (selectedCallNodeIndex === null || tracedTiming === null) { return null; } - const total = tracedTiming.total[selectedCallNodeIndex]; - const self = tracedTiming.self[selectedCallNodeIndex]; - return { total, self }; + return CallTree.getSelfAndTotalForCallNode( + selectedCallNodeIndex, + callNodeInfo, + tracedTiming + ); } ); @@ -416,7 +430,7 @@ export function getStackAndSampleSelectorsPerThread( createSelector( getFlameGraphRows, (state) => getCallNodeInfo(state).getNonInvertedCallNodeTable(), - getCallTreeTimings, + getCallTreeTimingsNonInverted, FlameGraph.getFlameGraphTiming ); diff --git a/src/test/fixtures/utils.js b/src/test/fixtures/utils.js index b007e1fe4b..d9804c5f85 100644 --- a/src/test/fixtures/utils.js +++ b/src/test/fixtures/utils.js @@ -137,9 +137,9 @@ export function callTreeFromProfile( thread.samples, getSampleIndexToCallNodeIndex( thread.samples.stack, - callNodeInfo.getStackIndexToCallNodeIndex() + callNodeInfo.getStackIndexToNonInvertedCallNodeIndex() ), - callNodeInfo.getCallNodeTable().length + callNodeInfo.getNonInvertedCallNodeTable().length ) ); return getCallTree( diff --git a/src/test/store/__snapshots__/profile-view.test.js.snap b/src/test/store/__snapshots__/profile-view.test.js.snap index 491c27db39..b969ea3b9a 100644 --- a/src/test/store/__snapshots__/profile-view.test.js.snap +++ b/src/test/store/__snapshots__/profile-view.test.js.snap @@ -2823,7 +2823,7 @@ CallTree { ], "_children": Array [], "_displayDataByIndex": Map {}, - "_internal": CallTreeInternalImpl { + "_internal": CallTreeInternalNonInverted { "_callNodeHasChildren": Uint8Array [ 1, 1, diff --git a/src/test/store/profile-view.test.js b/src/test/store/profile-view.test.js index 92fec9d6a1..52618e8a75 100644 --- a/src/test/store/profile-view.test.js +++ b/src/test/store/profile-view.test.js @@ -51,6 +51,7 @@ import { processCounter, type BreakdownByCategory, } from '../../profile-logic/profile-data'; +import { getSelfAndTotalForCallNode } from '../../profile-logic/call-tree'; import type { TrackReference, @@ -3526,7 +3527,7 @@ describe('traced timing', function () { const callNodeInfo = selectedThreadSelectors.getCallNodeInfo(getState()); - const { total, self } = ensureExists( + const tracedTiming = ensureExists( selectedThreadSelectors.getTracedTiming(getState()), 'Expected to get a traced timing.' ); @@ -3537,7 +3538,11 @@ describe('traced timing', function () { const callNodeIndex = ensureExists( callNodeInfo.getCallNodeIndexFromPath(callNodePath) ); - return { self: self[callNodeIndex], total: total[callNodeIndex] }; + return getSelfAndTotalForCallNode( + callNodeIndex, + callNodeInfo, + tracedTiming + ); }, profile, }; diff --git a/src/test/unit/profile-tree.test.js b/src/test/unit/profile-tree.test.js index 4ba030b5d8..cc2a92e413 100644 --- a/src/test/unit/profile-tree.test.js +++ b/src/test/unit/profile-tree.test.js @@ -76,17 +76,20 @@ describe('unfiltered call tree', function () { thread.samples, getSampleIndexToCallNodeIndex( thread.samples.stack, - callNodeInfo.getStackIndexToCallNodeIndex() + callNodeInfo.getStackIndexToNonInvertedCallNodeIndex() ), - callNodeInfo.getCallNodeTable().length + callNodeInfo.getNonInvertedCallNodeTable().length ) ); 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]), - leaf: new Float32Array([0, 0, 0, 0, 1, 0, 1, 0, 1]), - total: new Float32Array([3, 3, 2, 1, 1, 1, 1, 1, 1]), + type: 'NON_INVERTED', + timings: { + 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]), + leaf: new Float32Array([0, 0, 0, 0, 1, 0, 1, 0, 1]), + total: new Float32Array([3, 3, 2, 1, 1, 1, 1, 1, 1]), + }, }); }); }); @@ -438,9 +441,9 @@ describe('inverted call tree', function () { thread.samples, getSampleIndexToCallNodeIndex( thread.samples.stack, - callNodeInfo.getStackIndexToCallNodeIndex() + callNodeInfo.getStackIndexToNonInvertedCallNodeIndex() ), - callNodeInfo.getCallNodeTable().length + callNodeInfo.getNonInvertedCallNodeTable().length ) ); const callTree = getCallTree( @@ -480,9 +483,9 @@ describe('inverted call tree', function () { thread.samples, getSampleIndexToCallNodeIndex( thread.samples.stack, - invertedCallNodeInfo.getStackIndexToCallNodeIndex() + invertedCallNodeInfo.getStackIndexToNonInvertedCallNodeIndex() ), - invertedCallNodeInfo.getCallNodeTable().length + invertedCallNodeInfo.getNonInvertedCallNodeTable().length ) ); const invertedCallTree = getCallTree( @@ -632,12 +635,12 @@ describe('diffing trees', function () { thread.samples, getSampleIndexToCallNodeIndex( thread.samples.stack, - callNodeInfo.getStackIndexToCallNodeIndex() + callNodeInfo.getStackIndexToNonInvertedCallNodeIndex() ), - callNodeInfo.getCallNodeTable().length + callNodeInfo.getNonInvertedCallNodeTable().length ) ); - expect(callTreeTimings.rootTotalSummary).toBe(12); + expect(callTreeTimings.timings.rootTotalSummary).toBe(12); }); }); diff --git a/src/types/profile-derived.js b/src/types/profile-derived.js index 402be9a66e..090e2203c6 100644 --- a/src/types/profile-derived.js +++ b/src/types/profile-derived.js @@ -166,8 +166,14 @@ export interface CallNodeInfo { func: IndexIntoFuncTable ): IndexIntoCallNodeTable | null; + // Returns the list of root nodes. + getRoots(): IndexIntoCallNodeTable[]; + // Returns whether the given node is a root node. isRoot(callNodeIndex: IndexIntoCallNodeTable): boolean; + + // Returns the list of children of a node. + getChildren(callNodeIndex: IndexIntoCallNodeTable): IndexIntoCallNodeTable[]; } // An index into SuffixOrderedCallNodes. From 9bf0e5ced51777d90364e898df0841bc77e66cf7 Mon Sep 17 00:00:00 2001 From: Markus Stange Date: Fri, 19 Jan 2024 15:21:06 -0500 Subject: [PATCH 12/18] Rename leaf to self in many places. All these places now deal with non-inverted call nodes, and for those, what we meant by "leaf" and by "self" was always the same thing. And I prefer the word "self" because "leaf" usually means "has no children" and that's not the case here. We still use the word "leaf" in many parts of the documentation. --- src/profile-logic/call-tree.js | 93 +++++++++---------- src/selectors/per-thread/stack-sample.js | 18 ++-- src/test/fixtures/utils.js | 4 +- .../__snapshots__/profile-view.test.js.snap | 11 --- src/test/unit/profile-tree.test.js | 11 +-- src/types/profile-derived.js | 8 +- 6 files changed, 65 insertions(+), 80 deletions(-) diff --git a/src/profile-logic/call-tree.js b/src/profile-logic/call-tree.js index 5db559e467..97fc54723f 100644 --- a/src/profile-logic/call-tree.js +++ b/src/profile-logic/call-tree.js @@ -28,7 +28,7 @@ import type { Milliseconds, ExtraBadgeInfo, BottomBoxInfo, - CallNodeLeafAndSummary, + CallNodeSelfAndSummary, SelfAndTotal, } from 'firefox-profiler/types'; @@ -43,7 +43,6 @@ type CallNodeChildren = IndexIntoCallNodeTable[]; export type CallTreeTimingsNonInverted = {| callNodeHasChildren: Uint8Array, self: Float32Array, - leaf: Float32Array, total: Float32Array, rootTotalSummary: number, // sum of absolute values, this is used for computing percentages |}; @@ -56,7 +55,7 @@ export type InvertedCallTreeRoot = {| |}; export type CallTreeTimingsInverted = {| - callNodeLeaf: Float32Array, + callNodeSelf: Float32Array, rootTotalSummary: number, sortedRoots: IndexIntoFuncTable[], totalPerRootNode: Map, @@ -165,14 +164,14 @@ export class CallTreeInternalNonInverted implements CallTreeInternal { ): CallNodePath { const rangeEnd = this._callNodeTable.subtreeRangeEnd[callNodeIndex]; - // Find the call node with the highest leaf time. + // Find the call node with the highest self 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) { + const nodeSelf = Math.abs(this._callTreeTimings.self[nodeIndex]); + if (maxNode === -1 || nodeSelf > maxAbs) { maxNode = nodeIndex; - maxAbs = nodeLeaf; + maxAbs = nodeSelf; } } @@ -183,7 +182,7 @@ export class CallTreeInternalNonInverted implements CallTreeInternal { class CallTreeInternalInverted implements CallTreeInternal { _callNodeInfo: CallNodeInfoInverted; _nonInvertedCallNodeTable: CallNodeTable; - _callNodeLeaf: Float32Array; + _callNodeSelf: Float32Array; _rootNodes: IndexIntoCallNodeTable[]; _funcCount: number; _totalPerRootNode: Map; @@ -199,7 +198,7 @@ class CallTreeInternalInverted implements CallTreeInternal { ) { this._callNodeInfo = callNodeInfo; this._nonInvertedCallNodeTable = callNodeInfo.getNonInvertedCallNodeTable(); - this._callNodeLeaf = callTreeTimingsInverted.callNodeLeaf; + this._callNodeSelf = callTreeTimingsInverted.callNodeSelf; const { sortedRoots, totalPerRootNode, rootNodesWithChildren } = callTreeTimingsInverted; this._totalPerRootNode = totalPerRootNode; @@ -261,7 +260,7 @@ class CallTreeInternalInverted implements CallTreeInternal { const totalAndHasChildren = _getInvertedTreeNodeTotalAndHasChildren( callNodeIndex, this._callNodeInfo, - this._callNodeLeaf + this._callNodeSelf ); this._totalAndHasChildrenPerNonRootNode.set( callNodeIndex, @@ -282,7 +281,7 @@ class CallTreeInternalInverted implements CallTreeInternal { let maxAbs = 0; for (let i = rangeStart; i < rangeEnd; i++) { const nodeIndex = orderedCallNodes[i]; - const nodeSelf = Math.abs(this._callNodeLeaf[nodeIndex]); + const nodeSelf = Math.abs(this._callNodeSelf[nodeIndex]); if (maxNode === -1 || nodeSelf > maxAbs) { maxNode = nodeIndex; maxAbs = nodeSelf; @@ -575,7 +574,7 @@ export class CallTree { * (path, callTree) => invertedPath * * Call trees are sorted with the CallNodes with the heaviest total time as the first - * entry. This function walks to the tip of the heaviest branches to find the leaf node, + * entry. This function walks to the tip of the heaviest branches to find the self node, * then construct an inverted CallNodePath with the result. This gives a pretty decent * result, but it doesn't guarantee that it will select the heaviest CallNodePath for the * INVERTED call tree. This would require doing a round trip through the reducers or @@ -598,15 +597,15 @@ export class CallTree { } /** - * Compute the leaf time for each call node, and the sum of the absolute leaf + * Compute the self time for each call node, and the sum of the absolute self * values. */ -export function computeCallNodeLeafAndSummary( +export function computeCallNodeSelfAndSummary( samples: SamplesLikeTable, sampleIndexToCallNodeIndex: Array, callNodeCount: number -): CallNodeLeafAndSummary { - const callNodeLeaf = new Float32Array(callNodeCount); +): CallNodeSelfAndSummary { + const callNodeSelf = new Float32Array(callNodeCount); for ( let sampleIndex = 0; sampleIndex < sampleIndexToCallNodeIndex.length; @@ -615,7 +614,7 @@ export function computeCallNodeLeafAndSummary( const callNodeIndex = sampleIndexToCallNodeIndex[sampleIndex]; if (callNodeIndex !== null) { const weight = samples.weight ? samples.weight[sampleIndex] : 1; - callNodeLeaf[callNodeIndex] += weight; + callNodeSelf[callNodeIndex] += weight; } } @@ -624,10 +623,10 @@ export function computeCallNodeLeafAndSummary( let rootTotalSummary = 0; for (let callNodeIndex = 0; callNodeIndex < callNodeCount; callNodeIndex++) { - rootTotalSummary += abs(callNodeLeaf[callNodeIndex]); + rootTotalSummary += abs(callNodeSelf[callNodeIndex]); } - return { callNodeLeaf, rootTotalSummary }; + return { callNodeSelf, rootTotalSummary }; } export function getSelfAndTotalForCallNode( @@ -645,7 +644,7 @@ export function getSelfAndTotalForCallNode( case 'INVERTED': { const callNodeInfoInverted = ensureExists(callNodeInfo.asInverted()); const { timings } = callTreeTimings; - const { callNodeLeaf, totalPerRootNode } = timings; + const { callNodeSelf, totalPerRootNode } = timings; if (callNodeInfoInverted.isRoot(callNodeIndex)) { const total = totalPerRootNode.get(callNodeIndex) ?? 0; return { self: total, total }; @@ -653,7 +652,7 @@ export function getSelfAndTotalForCallNode( const { total } = _getInvertedTreeNodeTotalAndHasChildren( callNodeIndex, callNodeInfoInverted, - callNodeLeaf + callNodeSelf ); return { self: 0, total }; } @@ -665,7 +664,7 @@ export function getSelfAndTotalForCallNode( function _getInvertedTreeNodeTotalAndHasChildren( callNodeIndex: IndexIntoCallNodeTable, callNodeInfo: CallNodeInfoInverted, - callNodeLeaf: Float32Array + callNodeSelf: Float32Array ): TotalAndHasChildren { const nodeDepth = callNodeInfo.getCallNodeTable().depth[callNodeIndex]; const [rangeStart, rangeEnd] = @@ -676,22 +675,22 @@ function _getInvertedTreeNodeTotalAndHasChildren( let total = 0; let hasChildren = false; for (let i = rangeStart; i < rangeEnd; i++) { - const leafNode = suffixOrderedCallNodes[i]; - const leaf = callNodeLeaf[leafNode]; - total += leaf; + const selfNode = suffixOrderedCallNodes[i]; + const self = callNodeSelf[selfNode]; + total += self; // The inverted call node has children if any deeper call paths with non-zero // self time contribute to it. hasChildren = hasChildren || - (leaf !== 0 && callNodeTableDepthCol[leafNode] > nodeDepth); + (self !== 0 && callNodeTableDepthCol[selfNode] > nodeDepth); } return { total, hasChildren }; } export function computeCallTreeTimingsInverted( callNodeInfo: CallNodeInfoInverted, - { callNodeLeaf, rootTotalSummary }: CallNodeLeafAndSummary + { callNodeSelf, rootTotalSummary }: CallNodeSelfAndSummary ): CallTreeTimingsInverted { const roots = callNodeInfo.getRoots(); const invertedCallNodeTable = callNodeInfo.getCallNodeTable(); @@ -701,15 +700,15 @@ export function computeCallTreeTimingsInverted( const totalPerRootNode = new Map(); const rootNodesWithChildren = new Set(); const seenRoots = new Set(); - for (let i = 0; i < callNodeLeaf.length; i++) { - const leaf = callNodeLeaf[i]; - if (leaf === 0) { + for (let i = 0; i < callNodeSelf.length; i++) { + const self = callNodeSelf[i]; + if (self === 0) { continue; } // Map the non-inverted call node to its corresponding root in the inverted // call tree. This is done by finding the inverted root which corresponds to - // the leaf function of the non-inverted call node. + // the self function of the non-inverted call node. const func = callNodeTableFuncCol[i]; const rootNode = roots.find( (invertedCallNode) => @@ -723,7 +722,7 @@ export function computeCallTreeTimingsInverted( totalPerRootNode.set( rootNode, - (totalPerRootNode.get(rootNode) ?? 0) + leaf + (totalPerRootNode.get(rootNode) ?? 0) + self ); seenRoots.add(rootNode); if (callNodeTableDepthCol[i] !== 0) { @@ -737,7 +736,7 @@ export function computeCallTreeTimingsInverted( Math.abs(totalPerRootNode.get(a) ?? 0) ); return { - callNodeLeaf, + callNodeSelf, rootTotalSummary, sortedRoots, totalPerRootNode, @@ -747,7 +746,7 @@ export function computeCallTreeTimingsInverted( export function computeCallTreeTimings( callNodeInfo: CallNodeInfo, - callNodeLeafAndSummary: CallNodeLeafAndSummary + CallNodeSelfAndSummary: CallNodeSelfAndSummary ): CallTreeTimings { const callNodeInfoInverted = callNodeInfo.asInverted(); if (callNodeInfoInverted !== null) { @@ -755,7 +754,7 @@ export function computeCallTreeTimings( type: 'INVERTED', timings: computeCallTreeTimingsInverted( callNodeInfoInverted, - callNodeLeafAndSummary + CallNodeSelfAndSummary ), }; } @@ -763,7 +762,7 @@ export function computeCallTreeTimings( type: 'NON_INVERTED', timings: computeCallTreeTimingsNonInverted( callNodeInfo, - callNodeLeafAndSummary + CallNodeSelfAndSummary ), }; } @@ -774,11 +773,10 @@ export function computeCallTreeTimings( */ export function computeCallTreeTimingsNonInverted( callNodeInfo: CallNodeInfo, - callNodeLeafAndSummary: CallNodeLeafAndSummary + CallNodeSelfAndSummary: CallNodeSelfAndSummary ): CallTreeTimingsNonInverted { const callNodeTable = callNodeInfo.getNonInvertedCallNodeTable(); - const { callNodeLeaf, rootTotalSummary } = callNodeLeafAndSummary; - const callNodeSelf = callNodeLeaf; + const { callNodeSelf, rootTotalSummary } = CallNodeSelfAndSummary; // Compute the following variables: const callNodeTotalSummary = new Float32Array(callNodeTable.length); @@ -792,7 +790,7 @@ export function computeCallTreeTimingsNonInverted( callNodeIndex >= 0; callNodeIndex-- ) { - callNodeTotalSummary[callNodeIndex] += callNodeLeaf[callNodeIndex]; + callNodeTotalSummary[callNodeIndex] += callNodeSelf[callNodeIndex]; const hasChildren = callNodeHasChildren[callNodeIndex] !== 0; const hasTotalValue = callNodeTotalSummary[callNodeIndex] !== 0; @@ -810,7 +808,6 @@ export function computeCallTreeTimingsNonInverted( return { self: callNodeSelf, - leaf: callNodeLeaf, total: callNodeTotalSummary, callNodeHasChildren, rootTotalSummary, @@ -936,7 +933,7 @@ export function extractSamplesLikeTable( } /** - * This function is extremely similar to computeCallNodeLeafAndSummary, + * This function is extremely similar to computeCallNodeSelfAndSummary, * 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 @@ -946,12 +943,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 computeCallNodeTracedLeafAndSummary( +export function computeCallNodeTracedSelfAndSummary( samples: SamplesLikeTable, sampleIndexToCallNodeIndex: Array, callNodeCount: number, interval: Milliseconds -): CallNodeLeafAndSummary | null { +): CallNodeSelfAndSummary | 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 @@ -963,7 +960,7 @@ export function computeCallNodeTracedLeafAndSummary( return null; } - const callNodeLeaf = new Float32Array(callNodeCount); + const callNodeSelf = new Float32Array(callNodeCount); let rootTotalSummary = 0; for (let sampleIndex = 0; sampleIndex < samples.length - 1; sampleIndex++) { @@ -971,7 +968,7 @@ export function computeCallNodeTracedLeafAndSummary( if (callNodeIndex !== null) { const sampleTracedTime = samples.time[sampleIndex + 1] - samples.time[sampleIndex]; - callNodeLeaf[callNodeIndex] += sampleTracedTime; + callNodeSelf[callNodeIndex] += sampleTracedTime; rootTotalSummary += sampleTracedTime; } } @@ -981,10 +978,10 @@ export function computeCallNodeTracedLeafAndSummary( if (callNodeIndex !== null) { // Use the sampling interval for the last sample. const sampleTracedTime = interval; - callNodeLeaf[callNodeIndex] += sampleTracedTime; + callNodeSelf[callNodeIndex] += sampleTracedTime; rootTotalSummary += sampleTracedTime; } } - return { callNodeLeaf, rootTotalSummary }; + return { callNodeSelf, rootTotalSummary }; } diff --git a/src/selectors/per-thread/stack-sample.js b/src/selectors/per-thread/stack-sample.js index 5c66006e70..44ffdbd893 100644 --- a/src/selectors/per-thread/stack-sample.js +++ b/src/selectors/per-thread/stack-sample.js @@ -44,7 +44,7 @@ import type { $ReturnType, ThreadsKey, SelfAndTotal, - CallNodeLeafAndSummary, + CallNodeSelfAndSummary, } from 'firefox-profiler/types'; import type { ThreadSelectorsPerThread } from './thread'; @@ -319,13 +319,13 @@ export function getStackAndSampleSelectorsPerThread( (samples) => samples.weightType || 'samples' ); - const getCallNodeLeafAndSummary: Selector = + const getCallNodeSelfAndSummary: Selector = createSelector( threadSelectors.getPreviewFilteredSamplesForCallTree, getSampleIndexToNonInvertedCallNodeIndexForPreviewFilteredThread, getCallNodeInfo, (samples, sampleIndexToCallNodeIndex, callNodeInfo) => { - return CallTree.computeCallNodeLeafAndSummary( + return CallTree.computeCallNodeSelfAndSummary( samples, sampleIndexToCallNodeIndex, callNodeInfo.getNonInvertedCallNodeTable().length @@ -335,14 +335,14 @@ export function getStackAndSampleSelectorsPerThread( const getCallTreeTimings: Selector = createSelector( getCallNodeInfo, - getCallNodeLeafAndSummary, + getCallNodeSelfAndSummary, CallTree.computeCallTreeTimings ); const getCallTreeTimingsNonInverted: Selector = createSelector( getCallNodeInfo, - getCallNodeLeafAndSummary, + getCallNodeSelfAndSummary, CallTree.computeCallTreeTimingsNonInverted ); @@ -375,19 +375,19 @@ export function getStackAndSampleSelectorsPerThread( getCallNodeInfo, ProfileSelectors.getProfileInterval, (samples, sampleIndexToCallNodeIndex, callNodeInfo, interval) => { - const callNodeLeafAndSummary = - CallTree.computeCallNodeTracedLeafAndSummary( + const CallNodeSelfAndSummary = + CallTree.computeCallNodeTracedSelfAndSummary( samples, sampleIndexToCallNodeIndex, callNodeInfo.getNonInvertedCallNodeTable().length, interval ); - if (callNodeLeafAndSummary === null) { + if (CallNodeSelfAndSummary === null) { return null; } return CallTree.computeCallTreeTimings( callNodeInfo, - callNodeLeafAndSummary + CallNodeSelfAndSummary ); } ); diff --git a/src/test/fixtures/utils.js b/src/test/fixtures/utils.js index d9804c5f85..3a620e39d7 100644 --- a/src/test/fixtures/utils.js +++ b/src/test/fixtures/utils.js @@ -4,7 +4,7 @@ // @flow import { getCallTree, - computeCallNodeLeafAndSummary, + computeCallNodeSelfAndSummary, computeCallTreeTimings, type CallTree, } from 'firefox-profiler/profile-logic/call-tree'; @@ -133,7 +133,7 @@ export function callTreeFromProfile( ); const callTreeTimings = computeCallTreeTimings( callNodeInfo, - computeCallNodeLeafAndSummary( + computeCallNodeSelfAndSummary( thread.samples, getSampleIndexToCallNodeIndex( thread.samples.stack, diff --git a/src/test/store/__snapshots__/profile-view.test.js.snap b/src/test/store/__snapshots__/profile-view.test.js.snap index b969ea3b9a..b49444b4a5 100644 --- a/src/test/store/__snapshots__/profile-view.test.js.snap +++ b/src/test/store/__snapshots__/profile-view.test.js.snap @@ -3170,17 +3170,6 @@ CallTree { 0, 0, ], - "leaf": Float32Array [ - 0, - 0, - 0, - 0, - 0, - 0, - 2, - 0, - 0, - ], "rootTotalSummary": 2, "self": Float32Array [ 0, diff --git a/src/test/unit/profile-tree.test.js b/src/test/unit/profile-tree.test.js index cc2a92e413..ed4a4dc341 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, - computeCallNodeLeafAndSummary, + computeCallNodeSelfAndSummary, computeCallTreeTimings, } from '../../profile-logic/call-tree'; import { computeFlameGraphRows } from '../../profile-logic/flame-graph'; @@ -72,7 +72,7 @@ describe('unfiltered call tree', function () { it('yields expected results', function () { const callTreeTimings = computeCallTreeTimings( callNodeInfo, - computeCallNodeLeafAndSummary( + computeCallNodeSelfAndSummary( thread.samples, getSampleIndexToCallNodeIndex( thread.samples.stack, @@ -87,7 +87,6 @@ describe('unfiltered call tree', function () { 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]), - leaf: new Float32Array([0, 0, 0, 0, 1, 0, 1, 0, 1]), total: new Float32Array([3, 3, 2, 1, 1, 1, 1, 1, 1]), }, }); @@ -437,7 +436,7 @@ describe('inverted call tree', function () { ); const callTreeTimings = computeCallTreeTimings( callNodeInfo, - computeCallNodeLeafAndSummary( + computeCallNodeSelfAndSummary( thread.samples, getSampleIndexToCallNodeIndex( thread.samples.stack, @@ -479,7 +478,7 @@ describe('inverted call tree', function () { ); const invertedCallTreeTimings = computeCallTreeTimings( invertedCallNodeInfo, - computeCallNodeLeafAndSummary( + computeCallNodeSelfAndSummary( thread.samples, getSampleIndexToCallNodeIndex( thread.samples.stack, @@ -631,7 +630,7 @@ describe('diffing trees', function () { ); const callTreeTimings = computeCallTreeTimings( callNodeInfo, - computeCallNodeLeafAndSummary( + computeCallNodeSelfAndSummary( thread.samples, getSampleIndexToCallNodeIndex( thread.samples.stack, diff --git a/src/types/profile-derived.js b/src/types/profile-derived.js index 090e2203c6..16332341bc 100644 --- a/src/types/profile-derived.js +++ b/src/types/profile-derived.js @@ -805,11 +805,11 @@ export type ProfileFilterPageData = {| favicon: string | null, |}; -export type CallNodeLeafAndSummary = {| +export type CallNodeSelfAndSummary = {| // 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. + // stacks' self nodes. + callNodeSelf: Float32Array, + // The sum of absolute values in callNodeSelf. // This is used for computing the percentages displayed in the call tree. rootTotalSummary: number, |}; From 0eb70a194c18ee4ecaac12b6c021e967cb860544 Mon Sep 17 00:00:00 2001 From: Markus Stange Date: Wed, 7 Aug 2024 18:14:12 -0400 Subject: [PATCH 13/18] Use the non-inverted call node table to check for recursion. Whether a function recurses (directly or indirectly) is the same in the inverted call node table and in the non-inverted call node table. --- src/actions/profile-view.js | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/actions/profile-view.js b/src/actions/profile-view.js index 6fe354ece7..3c1eb3268c 100644 --- a/src/actions/profile-view.js +++ b/src/actions/profile-view.js @@ -2041,6 +2041,7 @@ export function handleCallNodeTransformShortcut( const callNodePath = callNodeInfo.getCallNodePathFromIndex(callNodeIndex); const funcIndex = callNodeTable.func[callNodeIndex]; const category = callNodeTable.category[callNodeIndex]; + const nonInvertedCallNodeTable = callNodeInfo.getNonInvertedCallNodeTable(); switch (event.key) { case 'F': @@ -2099,7 +2100,7 @@ export function handleCallNodeTransformShortcut( break; } case 'r': { - if (funcHasRecursiveCall(callNodeTable, funcIndex)) { + if (funcHasRecursiveCall(nonInvertedCallNodeTable, funcIndex)) { dispatch( addTransformToStack(threadsKey, { type: 'collapse-recursion', @@ -2110,7 +2111,7 @@ export function handleCallNodeTransformShortcut( break; } case 'R': { - if (funcHasDirectRecursiveCall(callNodeTable, funcIndex)) { + if (funcHasDirectRecursiveCall(nonInvertedCallNodeTable, funcIndex)) { dispatch( addTransformToStack(threadsKey, { type: 'collapse-direct-recursion', From 66bb0d54f44a459d1b9e140b1a1dfab2e8833344 Mon Sep 17 00:00:00 2001 From: Markus Stange Date: Mon, 13 Nov 2023 14:20:47 -0500 Subject: [PATCH 14/18] Replace all remaining callers of getCallNodeTable() with xyzForNode() calls. --- src/actions/profile-view.js | 6 +- src/components/calltree/CallTree.js | 6 +- src/components/shared/CallNodeContextMenu.js | 17 ++- src/components/stack-chart/Canvas.js | 3 +- src/components/stack-chart/index.js | 3 +- src/components/tooltip/CallNode.js | 9 +- src/profile-logic/address-timings.js | 2 +- src/profile-logic/call-node-info.js | 40 +++++++ src/profile-logic/call-tree.js | 25 ++--- src/profile-logic/line-timings.js | 2 +- src/profile-logic/profile-data.js | 5 +- src/profile-logic/transforms.js | 4 +- src/selectors/per-thread/index.js | 3 +- .../__snapshots__/profile-view.test.js.snap | 103 ------------------ src/types/profile-derived.js | 23 ++++ 15 files changed, 100 insertions(+), 151 deletions(-) diff --git a/src/actions/profile-view.js b/src/actions/profile-view.js index 3c1eb3268c..223cadbd71 100644 --- a/src/actions/profile-view.js +++ b/src/actions/profile-view.js @@ -2035,12 +2035,12 @@ export function handleCallNodeTransformShortcut( const threadSelectors = getThreadSelectorsFromThreadsKey(threadsKey); const unfilteredThread = threadSelectors.getThread(getState()); const callNodeInfo = threadSelectors.getCallNodeInfo(getState()); - const callNodeTable = callNodeInfo.getCallNodeTable(); const implementation = getImplementationFilter(getState()); const inverted = getInvertCallstack(getState()); const callNodePath = callNodeInfo.getCallNodePathFromIndex(callNodeIndex); - const funcIndex = callNodeTable.func[callNodeIndex]; - const category = callNodeTable.category[callNodeIndex]; + const funcIndex = callNodeInfo.funcForNode(callNodeIndex); + const category = callNodeInfo.categoryForNode(callNodeIndex); + const nonInvertedCallNodeTable = callNodeInfo.getNonInvertedCallNodeTable(); switch (event.key) { diff --git a/src/components/calltree/CallTree.js b/src/components/calltree/CallTree.js index 4ad5c7110d..206f2fe70d 100644 --- a/src/components/calltree/CallTree.js +++ b/src/components/calltree/CallTree.js @@ -320,7 +320,6 @@ class CallTreeImpl extends PureComponent { // This tree is empty. return; } - const callNodeTable = callNodeInfo.getCallNodeTable(); newExpandedCallNodeIndexes.push(currentCallNodeIndex); for (let i = 0; i < maxInterestingDepth; i++) { const children = tree.getChildren(currentCallNodeIndex); @@ -330,7 +329,8 @@ class CallTreeImpl extends PureComponent { // Let's find if there's a non idle children. const firstNonIdleNode = children.find( - (nodeIndex) => callNodeTable.category[nodeIndex] !== idleCategoryIndex + (nodeIndex) => + callNodeInfo.categoryForNode(nodeIndex) !== idleCategoryIndex ); // If there's a non idle children, use it; otherwise use the first @@ -341,7 +341,7 @@ class CallTreeImpl extends PureComponent { } this._onExpandedCallNodesChange(newExpandedCallNodeIndexes); - const categoryIndex = callNodeTable.category[currentCallNodeIndex]; + const categoryIndex = callNodeInfo.categoryForNode(currentCallNodeIndex); if (categoryIndex !== idleCategoryIndex) { // If we selected the call node with a "idle" category, we'd have a // completely dimmed activity graph because idle stacks are not drawn in diff --git a/src/components/shared/CallNodeContextMenu.js b/src/components/shared/CallNodeContextMenu.js index 2a9dbbc1bb..0f9a662c40 100644 --- a/src/components/shared/CallNodeContextMenu.js +++ b/src/components/shared/CallNodeContextMenu.js @@ -138,8 +138,7 @@ class CallNodeContextMenuImpl extends React.PureComponent { callNodeInfo, } = rightClickedCallNodeInfo; - const callNodeTable = callNodeInfo.getCallNodeTable(); - const funcIndex = callNodeTable.func[callNodeIndex]; + const funcIndex = callNodeInfo.funcForNode(callNodeIndex); const isJS = funcTable.isJS[funcIndex]; const stringIndex = funcTable.name[funcIndex]; const functionCall = stringTable.getString(stringIndex); @@ -176,8 +175,7 @@ class CallNodeContextMenuImpl extends React.PureComponent { callNodeInfo, } = rightClickedCallNodeInfo; - const callNodeTable = callNodeInfo.getCallNodeTable(); - const funcIndex = callNodeTable.func[callNodeIndex]; + const funcIndex = callNodeInfo.funcForNode(callNodeIndex); const stringIndex = funcTable.fileName[funcIndex]; if (stringIndex === null) { return null; @@ -303,8 +301,7 @@ class CallNodeContextMenuImpl extends React.PureComponent { const { threadsKey, callNodePath, thread, callNodeIndex, callNodeInfo } = rightClickedCallNodeInfo; const selectedFunc = callNodePath[callNodePath.length - 1]; - const callNodeTable = callNodeInfo.getCallNodeTable(); - const category = callNodeTable.category[callNodeIndex]; + const category = callNodeInfo.categoryForNode(callNodeIndex); switch (type) { case 'focus-subtree': addTransformToStack(threadsKey, { @@ -488,9 +485,8 @@ class CallNodeContextMenuImpl extends React.PureComponent { callNodeInfo, } = rightClickedCallNodeInfo; - const callNodeTable = callNodeInfo.getCallNodeTable(); - const categoryIndex = callNodeTable.category[callNodeIndex]; - const funcIndex = callNodeTable.func[callNodeIndex]; + const categoryIndex = callNodeInfo.categoryForNode(callNodeIndex); + const funcIndex = callNodeInfo.funcForNode(callNodeIndex); const isJS = funcTable.isJS[funcIndex]; const hasCategory = categoryIndex !== -1; // This could be the C++ library, or the JS filename. @@ -504,6 +500,9 @@ class CallNodeContextMenuImpl extends React.PureComponent { const fileName = filePath && parseFileNameFromSymbolication(filePath).path.match(/[^\\/]+$/)?.[0]; + + const callNodeTable = callNodeInfo.getNonInvertedCallNodeTable(); + return ( <> {fileName ? ( diff --git a/src/components/stack-chart/Canvas.js b/src/components/stack-chart/Canvas.js index df25e28f73..90ab920f3f 100644 --- a/src/components/stack-chart/Canvas.js +++ b/src/components/stack-chart/Canvas.js @@ -128,8 +128,7 @@ class StackChartCanvasImpl extends React.PureComponent { return; } - const callNodeTable = callNodeInfo.getCallNodeTable(); - const depth = callNodeTable.depth[selectedCallNodeIndex]; + const depth = callNodeInfo.depthForNode(selectedCallNodeIndex); const y = depth * ROW_CSS_PIXELS_HEIGHT; if (y < this.props.viewport.viewportTop) { diff --git a/src/components/stack-chart/index.js b/src/components/stack-chart/index.js index 90120e458e..23744a792e 100644 --- a/src/components/stack-chart/index.js +++ b/src/components/stack-chart/index.js @@ -181,8 +181,7 @@ class StackChartImpl extends React.PureComponent { event.preventDefault(); const { callNodeInfo, selectedCallNodeIndex, thread } = this.props; if (selectedCallNodeIndex !== null) { - const callNodeTable = callNodeInfo.getCallNodeTable(); - const funcIndex = callNodeTable.func[selectedCallNodeIndex]; + const funcIndex = callNodeInfo.funcForNode(selectedCallNodeIndex); const funcName = thread.stringTable.getString( thread.funcTable.name[funcIndex] ); diff --git a/src/components/tooltip/CallNode.js b/src/components/tooltip/CallNode.js index 9bc4568b68..6eed705c81 100644 --- a/src/components/tooltip/CallNode.js +++ b/src/components/tooltip/CallNode.js @@ -365,12 +365,11 @@ export class TooltipCallNode extends React.PureComponent { callNodeInfo, displayStackType, } = this.props; - const callNodeTable = callNodeInfo.getCallNodeTable(); - const categoryIndex = callNodeTable.category[callNodeIndex]; + const categoryIndex = callNodeInfo.categoryForNode(callNodeIndex); const categoryColor = categories[categoryIndex].color; - const subcategoryIndex = callNodeTable.subcategory[callNodeIndex]; - const funcIndex = callNodeTable.func[callNodeIndex]; - const innerWindowID = callNodeTable.innerWindowID[callNodeIndex]; + const subcategoryIndex = callNodeInfo.subcategoryForNode(callNodeIndex); + const funcIndex = callNodeInfo.funcForNode(callNodeIndex); + const innerWindowID = callNodeInfo.innerWindowIDForNode(callNodeIndex); const funcStringIndex = thread.funcTable.name[funcIndex]; const funcName = thread.stringTable.getString(funcStringIndex); diff --git a/src/profile-logic/address-timings.js b/src/profile-logic/address-timings.js index 2e94663a2a..e026ef5775 100644 --- a/src/profile-logic/address-timings.js +++ b/src/profile-logic/address-timings.js @@ -431,7 +431,7 @@ export function getStackAddressInfoForCallNodeInverted( callNodeInfo: CallNodeInfoInverted, nativeSymbol: IndexIntoNativeSymbolTable ): StackAddressInfo { - const depth = callNodeInfo.getCallNodeTable().depth[callNodeIndex]; + const depth = callNodeInfo.depthForNode(callNodeIndex); const [rangeStart, rangeEnd] = callNodeInfo.getSuffixOrderIndexRangeForCallNode(callNodeIndex); const callNodeIsRootOfInvertedTree = callNodeInfo.isRoot(callNodeIndex); diff --git a/src/profile-logic/call-node-info.js b/src/profile-logic/call-node-info.js index 61f764c4f3..818eff1c8f 100644 --- a/src/profile-logic/call-node-info.js +++ b/src/profile-logic/call-node-info.js @@ -15,6 +15,8 @@ import type { CallNodePath, IndexIntoCallNodeTable, SuffixOrderIndex, + IndexIntoCategoryList, + IndexIntoNativeSymbolTable, } from 'firefox-profiler/types'; /** @@ -236,6 +238,44 @@ export class CallNodeInfoImpl implements CallNodeInfo { } return children; } + + prefixForNode( + callNodeIndex: IndexIntoCallNodeTable + ): IndexIntoCallNodeTable | -1 { + return this._callNodeTable.prefix[callNodeIndex]; + } + + funcForNode(callNodeIndex: IndexIntoCallNodeTable): IndexIntoFuncTable { + return this._callNodeTable.func[callNodeIndex]; + } + + categoryForNode( + callNodeIndex: IndexIntoCallNodeTable + ): IndexIntoCategoryList { + return this._callNodeTable.category[callNodeIndex]; + } + + subcategoryForNode( + callNodeIndex: IndexIntoCallNodeTable + ): IndexIntoCategoryList { + return this._callNodeTable.subcategory[callNodeIndex]; + } + + innerWindowIDForNode( + callNodeIndex: IndexIntoCallNodeTable + ): IndexIntoCategoryList { + return this._callNodeTable.innerWindowID[callNodeIndex]; + } + + depthForNode(callNodeIndex: IndexIntoCallNodeTable): number { + return this._callNodeTable.depth[callNodeIndex]; + } + + sourceFramesInlinedIntoSymbolForNode( + callNodeIndex: IndexIntoCallNodeTable + ): IndexIntoNativeSymbolTable | -1 | null { + return this._callNodeTable.sourceFramesInlinedIntoSymbol[callNodeIndex]; + } } /** diff --git a/src/profile-logic/call-tree.js b/src/profile-logic/call-tree.js index 97fc54723f..3cfbd11bce 100644 --- a/src/profile-logic/call-tree.js +++ b/src/profile-logic/call-tree.js @@ -304,7 +304,6 @@ export class CallTree { _categories: CategoryList; _internal: CallTreeInternal; _callNodeInfo: CallNodeInfo; - _callNodeTable: CallNodeTable; _thread: Thread; _rootTotalSummary: number; _displayDataByIndex: Map; @@ -327,7 +326,6 @@ export class CallTree { this._categories = categories; this._internal = internal; this._callNodeInfo = callNodeInfo; - this._callNodeTable = callNodeInfo.getCallNodeTable(); this._thread = thread; this._rootTotalSummary = rootTotalSummary; this._displayDataByIndex = new Map(); @@ -375,15 +373,15 @@ export class CallTree { getParent( callNodeIndex: IndexIntoCallNodeTable ): IndexIntoCallNodeTable | -1 { - return this._callNodeTable.prefix[callNodeIndex]; + return this._callNodeInfo.prefixForNode(callNodeIndex); } getDepth(callNodeIndex: IndexIntoCallNodeTable): number { - return this._callNodeTable.depth[callNodeIndex]; + return this._callNodeInfo.depthForNode(callNodeIndex); } getNodeData(callNodeIndex: IndexIntoCallNodeTable): CallNodeData { - const funcIndex = this._callNodeTable.func[callNodeIndex]; + const funcIndex = this._callNodeInfo.funcForNode(callNodeIndex); const funcName = this._thread.stringTable.getString( this._thread.funcTable.name[funcIndex] ); @@ -407,7 +405,7 @@ export class CallTree { ): ExtraBadgeInfo | void { const calledFunction = getFunctionName(funcName); const inlinedIntoNativeSymbol = - this._callNodeTable.sourceFramesInlinedIntoSymbol[callNodeIndex]; + this._callNodeInfo.sourceFramesInlinedIntoSymbolForNode(callNodeIndex); if (inlinedIntoNativeSymbol === null) { return undefined; } @@ -442,9 +440,10 @@ export class CallTree { if (displayData === undefined) { const { funcName, total, totalRelative, self } = this.getNodeData(callNodeIndex); - const funcIndex = this._callNodeTable.func[callNodeIndex]; - const categoryIndex = this._callNodeTable.category[callNodeIndex]; - const subcategoryIndex = this._callNodeTable.subcategory[callNodeIndex]; + const funcIndex = this._callNodeInfo.funcForNode(callNodeIndex); + const categoryIndex = this._callNodeInfo.categoryForNode(callNodeIndex); + const subcategoryIndex = + this._callNodeInfo.subcategoryForNode(callNodeIndex); const badge = this._getInliningBadge(callNodeIndex, funcName); const resourceIndex = this._thread.funcTable.resource[funcIndex]; const resourceType = this._thread.resourceTable.type[resourceIndex]; @@ -590,7 +589,7 @@ export class CallTree { } const heaviestPath = this._internal.findHeaviestPathInSubtree(callNodeIndex); - const startingDepth = this._callNodeTable.depth[callNodeIndex]; + const startingDepth = this._callNodeInfo.depthForNode(callNodeIndex); const partialPath = heaviestPath.slice(startingDepth); return partialPath.reverse(); } @@ -666,7 +665,7 @@ function _getInvertedTreeNodeTotalAndHasChildren( callNodeInfo: CallNodeInfoInverted, callNodeSelf: Float32Array ): TotalAndHasChildren { - const nodeDepth = callNodeInfo.getCallNodeTable().depth[callNodeIndex]; + const nodeDepth = callNodeInfo.depthForNode(callNodeIndex); const [rangeStart, rangeEnd] = callNodeInfo.getSuffixOrderIndexRangeForCallNode(callNodeIndex); const suffixOrderedCallNodes = callNodeInfo.getSuffixOrderedCallNodes(); @@ -693,7 +692,6 @@ export function computeCallTreeTimingsInverted( { callNodeSelf, rootTotalSummary }: CallNodeSelfAndSummary ): CallTreeTimingsInverted { const roots = callNodeInfo.getRoots(); - const invertedCallNodeTable = callNodeInfo.getCallNodeTable(); const callNodeTable = callNodeInfo.getNonInvertedCallNodeTable(); const callNodeTableFuncCol = callNodeTable.func; const callNodeTableDepthCol = callNodeTable.depth; @@ -711,8 +709,7 @@ export function computeCallTreeTimingsInverted( // the self function of the non-inverted call node. const func = callNodeTableFuncCol[i]; const rootNode = roots.find( - (invertedCallNode) => - invertedCallNodeTable.func[invertedCallNode] === func + (invertedCallNode) => callNodeInfo.funcForNode(invertedCallNode) === func ); if (rootNode === undefined) { throw new Error( diff --git a/src/profile-logic/line-timings.js b/src/profile-logic/line-timings.js index 3d0fd3443b..9907eb7359 100644 --- a/src/profile-logic/line-timings.js +++ b/src/profile-logic/line-timings.js @@ -286,7 +286,7 @@ export function getStackLineInfoForCallNodeInverted( callNodeIndex: IndexIntoCallNodeTable, callNodeInfo: CallNodeInfoInverted ): StackLineInfo { - const depth = callNodeInfo.getCallNodeTable().depth[callNodeIndex]; + const depth = callNodeInfo.depthForNode(callNodeIndex); const [rangeStart, rangeEnd] = callNodeInfo.getSuffixOrderIndexRangeForCallNode(callNodeIndex); const callNodeIsRootOfInvertedTree = callNodeInfo.isRoot(callNodeIndex); diff --git a/src/profile-logic/profile-data.js b/src/profile-logic/profile-data.js index 34734dc389..f8336efa4c 100644 --- a/src/profile-logic/profile-data.js +++ b/src/profile-logic/profile-data.js @@ -3708,7 +3708,7 @@ export function getNativeSymbolsForCallNodeInverted( stackTable: StackTable, frameTable: FrameTable ): IndexIntoNativeSymbolTable[] { - const depth = callNodeInfo.getCallNodeTable().depth[callNodeIndex]; + const depth = callNodeInfo.depthForNode(callNodeIndex); const [rangeStart, rangeEnd] = callNodeInfo.getSuffixOrderIndexRangeForCallNode(callNodeIndex); const stackTablePrefixCol = stackTable.prefix; @@ -3786,8 +3786,7 @@ export function getBottomBoxInfoForCallNode( nativeSymbols, } = thread; - const callNodeTable = callNodeInfo.getCallNodeTable(); - const funcIndex = callNodeTable.func[callNodeIndex]; + const funcIndex = callNodeInfo.funcForNode(callNodeIndex); const fileName = funcTable.fileName[funcIndex]; const sourceFile = fileName !== null ? stringTable.getString(fileName) : null; const resource = funcTable.resource[funcIndex]; diff --git a/src/profile-logic/transforms.js b/src/profile-logic/transforms.js index 699bdb0b4f..3aa23bb8e2 100644 --- a/src/profile-logic/transforms.js +++ b/src/profile-logic/transforms.js @@ -602,8 +602,6 @@ function _removeOtherCategoryFunctionsInNodePathWithFunction( callNodePath: CallNodePath, callNodeInfo: CallNodeInfo ): CallNodePath { - const callNodeTable = callNodeInfo.getCallNodeTable(); - const newCallNodePath = []; let prefix = -1; @@ -618,7 +616,7 @@ function _removeOtherCategoryFunctionsInNodePathWithFunction( ); } - if (callNodeTable.category[callNodeIndex] === category) { + if (callNodeInfo.categoryForNode(callNodeIndex) === category) { newCallNodePath.push(funcIndex); } diff --git a/src/selectors/per-thread/index.js b/src/selectors/per-thread/index.js index 91ce053c85..395a8b24da 100644 --- a/src/selectors/per-thread/index.js +++ b/src/selectors/per-thread/index.js @@ -288,8 +288,7 @@ export const selectedNodeSelectors: NodeSelectors = (() => { if (sourceViewFile === null || selectedCallNodeIndex === null) { return null; } - const callNodeTable = callNodeInfo.getCallNodeTable(); - const selectedFunc = callNodeTable.func[selectedCallNodeIndex]; + const selectedFunc = callNodeInfo.funcForNode(selectedCallNodeIndex); const selectedFuncFile = funcTable.fileName[selectedFunc]; if ( selectedFuncFile === null || diff --git a/src/test/store/__snapshots__/profile-view.test.js.snap b/src/test/store/__snapshots__/profile-view.test.js.snap index b49444b4a5..82963dba34 100644 --- a/src/test/store/__snapshots__/profile-view.test.js.snap +++ b/src/test/store/__snapshots__/profile-view.test.js.snap @@ -2660,109 +2660,6 @@ CallTree { 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, - ], - }, "_categories": Array [ Object { "color": "grey", diff --git a/src/types/profile-derived.js b/src/types/profile-derived.js index 16332341bc..9754b794c1 100644 --- a/src/types/profile-derived.js +++ b/src/types/profile-derived.js @@ -174,6 +174,29 @@ export interface CallNodeInfo { // Returns the list of children of a node. getChildren(callNodeIndex: IndexIntoCallNodeTable): IndexIntoCallNodeTable[]; + + // These functions return various properties about each node. You could also + // get these properties from the call node table, but that only works if the + // call node is a non-inverted call node (because we only have a non-inverted + // call node table). If your code is generic over inverted / non-inverted mode, + // and you just have a IndexIntoCallNodeTable and a CallNodeInfo instance, + // call the functions below. + + prefixForNode( + callNodeIndex: IndexIntoCallNodeTable + ): IndexIntoCallNodeTable | -1; + funcForNode(callNodeIndex: IndexIntoCallNodeTable): IndexIntoFuncTable; + categoryForNode(callNodeIndex: IndexIntoCallNodeTable): IndexIntoCategoryList; + subcategoryForNode( + callNodeIndex: IndexIntoCallNodeTable + ): IndexIntoCategoryList; + innerWindowIDForNode( + callNodeIndex: IndexIntoCallNodeTable + ): IndexIntoCategoryList; + depthForNode(callNodeIndex: IndexIntoCallNodeTable): number; + sourceFramesInlinedIntoSymbolForNode( + callNodeIndex: IndexIntoCallNodeTable + ): IndexIntoNativeSymbolTable | -1 | null; } // An index into SuffixOrderedCallNodes. From 15d5cf9900a98238c4261ab5aa0f77b653421751 Mon Sep 17 00:00:00 2001 From: Markus Stange Date: Sat, 20 Jan 2024 00:32:48 -0500 Subject: [PATCH 15/18] Remove now-unused getCallNodeTable(). This just stops exposing it from the interface. The way we compute it will change in the next commit. --- src/profile-logic/call-node-info.js | 4 ---- src/types/profile-derived.js | 4 ---- 2 files changed, 8 deletions(-) diff --git a/src/profile-logic/call-node-info.js b/src/profile-logic/call-node-info.js index 818eff1c8f..b5c37536f1 100644 --- a/src/profile-logic/call-node-info.js +++ b/src/profile-logic/call-node-info.js @@ -65,10 +65,6 @@ export class CallNodeInfoImpl implements CallNodeInfo { return null; } - getCallNodeTable(): CallNodeTable { - return this._callNodeTable; - } - getNonInvertedCallNodeTable(): CallNodeTable { return this._nonInvertedCallNodeTable; } diff --git a/src/types/profile-derived.js b/src/types/profile-derived.js index 9754b794c1..40d92715a1 100644 --- a/src/types/profile-derived.js +++ b/src/types/profile-derived.js @@ -131,10 +131,6 @@ export interface CallNodeInfo { // Returns this object as CallNodeInfoInverted if isInverted(), otherwise null. asInverted(): CallNodeInfoInverted | null; - // Returns the call node table. If isInverted() is true, this is an inverted - // call node table, otherwise this is the non-inverted call node table. - getCallNodeTable(): CallNodeTable; - // Returns the non-inverted call node table. // This is always the non-inverted call node table, regardless of isInverted(). getNonInvertedCallNodeTable(): CallNodeTable; From f35b4aa3bcbfc0a062e6f012750ba406414260b3 Mon Sep 17 00:00:00 2001 From: Markus Stange Date: Sat, 20 Jan 2024 11:47:47 -0500 Subject: [PATCH 16/18] Create inverted call nodes lazily. This is the main commit of this PR. Now that nothing is relying on having an inverted call node for each sample, or on having a fully-computed inverted call node table, we can make it so that we only add entries to the inverted call node table when we actually need a node, for example because it was revealed in the call tree. This makes it a lot faster to click the "Invert call stack" checkbox - before this commit, we were computing a lot of inverted call nodes that were never shown to the user. After this commit, CallNodeInfoInvertedImpl no longer inherits from CallNodeInfoImpl - it is now a fully separate implementation. --- src/profile-logic/call-node-info.js | 850 ++++++++++++++++-- src/profile-logic/profile-data.js | 176 +--- src/selectors/per-thread/stack-sample.js | 8 +- .../ProfileCallTreeView.test.js.snap | 16 +- .../__snapshots__/profile-view.test.js.snap | 315 +------ src/test/unit/address-timings.test.js | 4 +- src/test/unit/line-timings.test.js | 4 +- src/test/unit/profile-data.test.js | 12 +- src/test/unit/profile-tree.test.js | 4 +- src/types/profile-derived.js | 44 +- src/utils/bisect.js | 129 --- src/utils/path.js | 13 +- 12 files changed, 910 insertions(+), 665 deletions(-) diff --git a/src/profile-logic/call-node-info.js b/src/profile-logic/call-node-info.js index b5c37536f1..f2018d55e4 100644 --- a/src/profile-logic/call-node-info.js +++ b/src/profile-logic/call-node-info.js @@ -4,8 +4,12 @@ // @flow -import { hashPath } from 'firefox-profiler/utils/path'; -import { bisectEqualRange } from 'firefox-profiler/utils/bisect'; +import { + hashPath, + concatHash, + hashPathSingleFunc, +} from 'firefox-profiler/utils/path'; +import { ensureExists } from '../utils/flow'; import type { IndexIntoFuncTable, @@ -17,6 +21,8 @@ import type { SuffixOrderIndex, IndexIntoCategoryList, IndexIntoNativeSymbolTable, + IndexIntoSubcategoryListForCategory, + InnerWindowID, } from 'firefox-profiler/types'; /** @@ -26,17 +32,11 @@ import type { * By the end of this commit stack, it will no longer inherit from this class and * will have its own implementation. */ -export class CallNodeInfoImpl implements CallNodeInfo { - // The call node table. This is either the inverted or the non-inverted call - // node table, depending on isInverted(). +export class CallNodeInfoNonInvertedImpl implements CallNodeInfo { + // The call node table. (always non-inverted) _callNodeTable: CallNodeTable; - // The non-inverted call node table, regardless of isInverted(). - _nonInvertedCallNodeTable: CallNodeTable; - // The mapping of stack index to corresponding non-inverted call node index. - // This always maps to the non-inverted call node table, regardless of - // isInverted(). _stackIndexToNonInvertedCallNodeIndex: Int32Array; // This is a Map. This map speeds up @@ -46,27 +46,23 @@ export class CallNodeInfoImpl implements CallNodeInfo { constructor( callNodeTable: CallNodeTable, - nonInvertedCallNodeTable: CallNodeTable, stackIndexToNonInvertedCallNodeIndex: Int32Array ) { this._callNodeTable = callNodeTable; - this._nonInvertedCallNodeTable = nonInvertedCallNodeTable; this._stackIndexToNonInvertedCallNodeIndex = stackIndexToNonInvertedCallNodeIndex; } isInverted(): boolean { - // Overridden in subclass return false; } asInverted(): CallNodeInfoInverted | null { - // Overridden in subclass return null; } getNonInvertedCallNodeTable(): CallNodeTable { - return this._nonInvertedCallNodeTable; + return this._callNodeTable; } getStackIndexToNonInvertedCallNodeIndex(): Int32Array { @@ -274,51 +270,298 @@ export class CallNodeInfoImpl implements CallNodeInfo { } } +// A "subtype" of IndexIntoCallNodeTable, used in places where it is known that +// we are referring to an inverted call node. We just use it as a convention, +// Flow doesn't actually treat this any different from any other index and won't +// catch incorrect uses. +type InvertedCallNodeHandle = number; + +// An index into InvertedNonRootCallNodeTable. This is usually created by +// taking an InvertedCallNodeHandle and subtracting rootCount. +type IndexIntoInvertedNonRootCallNodeTable = number; + +// Information about the roots of the inverted call tree. We compute this +// information upfront for all roots. The root count is fixed, so most of the +// arrays in this struct are fixed-size typed arrays. +// The number of roots is the same as the number of functions in the funcTable. +type InvertedRootCallNodeTable = {| + category: Int32Array, // IndexIntoFuncTable -> IndexIntoCategoryList + subcategory: Int32Array, // IndexIntoFuncTable -> IndexIntoSubcategoryListForCategory + innerWindowID: Float64Array, // IndexIntoFuncTable -> InnerWindowID + // IndexIntoNativeSymbolTable: all frames that collapsed into this call node inlined into the same native symbol + // -1: divergent: some, but not all, frames that collapsed into this call node were inlined, or they are from different symbols + // null: no inlining + sourceFramesInlinedIntoSymbol: Array, + // The (exclusive) end of the suffix order index range for each root node. + // The beginning of the range is given by suffixOrderIndexRangeEnd[i - 1], or by + // zero. This is possible because both the inverted root order and the suffix order + // are determined by the func order. + suffixOrderIndexRangeEnd: Uint32Array, // IndexIntoFuncTable -> SuffixOrderIndex, + length: number, +|}; + +// Information about the non-root nodes of the inverted call tree. This table +// grows on-demand, as new inverted call nodes are materialized. +type InvertedNonRootCallNodeTable = {| + prefix: InvertedCallNodeHandle[], + func: IndexIntoFuncTable[], // IndexIntoInvertedNonRootCallNodeTable -> IndexIntoFuncTable + pathHash: string[], // IndexIntoInvertedNonRootCallNodeTable -> string + category: IndexIntoCategoryList[], // IndexIntoInvertedNonRootCallNodeTable -> IndexIntoCategoryList + subcategory: IndexIntoSubcategoryListForCategory[], // IndexIntoInvertedNonRootCallNodeTable -> IndexIntoSubcategoryListForCategory + innerWindowID: InnerWindowID[], // IndexIntoInvertedNonRootCallNodeTable -> InnerWindowID + // IndexIntoNativeSymbolTable: all frames that collapsed into this call node inlined into the same native symbol + // -1: divergent: some, but not all, frames that collapsed into this call node were inlined, or they are from different symbols + // null: no inlining + sourceFramesInlinedIntoSymbol: Array, + suffixOrderIndexRangeStart: SuffixOrderIndex[], // IndexIntoInvertedNonRootCallNodeTable -> SuffixOrderIndex + suffixOrderIndexRangeEnd: SuffixOrderIndex[], // IndexIntoInvertedNonRootCallNodeTable -> SuffixOrderIndex + + // Non-null for non-root nodes whose children haven't been created yet. + // For a non-root node x of the inverted tree, let k = depth[x] its depth in the inverted tree, + // and deepNodes = deepNodesForSuffixOrderIndexRange[x] be its non-null deep nodes. + // Then, for every index i in suffixOrderIndexRangeStart[x]..suffixOrderIndexRangeEnd[x], + // the k'th prefix node of suffixOrderedCallNodes[i] is stored at deepNodes[i - suffixOrderIndexRangeStart[x]]. + deepNodesForSuffixOrderIndexRange: Array, // IndexIntoInvertedNonRootCallNodeTable -> (Uint32Array | null) + + depth: number[], // IndexIntoInvertedNonRootCallNodeTable -> number + length: number, +|}; + +// Compute the InvertedRootCallNodeTable. +// We compute this information upfront for all roots. The root count is fixed - +// the number of roots is the same as the number of functions in the funcTable. +function _createInvertedRootCallNodeTable( + callNodeTable: CallNodeTable, + rootSuffixOrderIndexRangeEndCol: Uint32Array, + suffixOrderedCallNodes: Uint32Array, + defaultCategory: IndexIntoCategoryList +): InvertedRootCallNodeTable { + const funcCount = rootSuffixOrderIndexRangeEndCol.length; + const category = new Int32Array(funcCount); + const subcategory = new Int32Array(funcCount); + const innerWindowID = new Float64Array(funcCount); + const sourceFramesInlinedIntoSymbol = new Array(funcCount); + let previousRootSuffixOrderIndexRangeEnd = 0; + for (let funcIndex = 0; funcIndex < funcCount; funcIndex++) { + const callNodesuffixOrderIndexRangeStart = + previousRootSuffixOrderIndexRangeEnd; + const callNodesuffixOrderIndexRangeEnd = + rootSuffixOrderIndexRangeEndCol[funcIndex]; + previousRootSuffixOrderIndexRangeEnd = callNodesuffixOrderIndexRangeEnd; + if ( + callNodesuffixOrderIndexRangeStart === callNodesuffixOrderIndexRangeEnd + ) { + sourceFramesInlinedIntoSymbol[funcIndex] = null; + // Leave the remaining columns at zero for this root. + continue; + } + + // Fill the remaining fields with the conflict-resolved versions of the values + // in the non-inverted call node table. + const firstNonInvertedCallNodeIndex = + suffixOrderedCallNodes[callNodesuffixOrderIndexRangeStart]; + let resolvedCategory = + callNodeTable.category[firstNonInvertedCallNodeIndex]; + let resolvedSubcategory = + callNodeTable.subcategory[firstNonInvertedCallNodeIndex]; + const resolvedInnerWindowID = + callNodeTable.innerWindowID[firstNonInvertedCallNodeIndex]; + let resolvedSourceFramesInlinedIntoSymbol = + callNodeTable.sourceFramesInlinedIntoSymbol[ + firstNonInvertedCallNodeIndex + ]; + + // Resolve conflicts in the same way as for the non-inverted call node table. + for ( + let orderingIndex = callNodesuffixOrderIndexRangeStart + 1; + orderingIndex < callNodesuffixOrderIndexRangeEnd; + orderingIndex++ + ) { + const currentNonInvertedCallNodeIndex = + suffixOrderedCallNodes[orderingIndex]; + // Resolve category conflicts, by resetting a conflicting subcategory or + // category to the default category. + if ( + resolvedCategory !== + callNodeTable.category[currentNonInvertedCallNodeIndex] + ) { + // Conflicting origin stack categories -> default category + subcategory. + resolvedCategory = defaultCategory; + resolvedSubcategory = 0; + } else if ( + resolvedSubcategory !== + callNodeTable.subcategory[currentNonInvertedCallNodeIndex] + ) { + // Conflicting origin stack subcategories -> "Other" subcategory. + resolvedSubcategory = 0; + } + + // Resolve "inlined into" conflicts. This can happen if you have two + // function calls A -> B where only one of the B calls is inlined, or + // if you use call tree transforms in such a way that a function B which + // was inlined into two different callers (A -> B, C -> B) gets collapsed + // into one call node. + if ( + resolvedSourceFramesInlinedIntoSymbol !== + callNodeTable.sourceFramesInlinedIntoSymbol[ + currentNonInvertedCallNodeIndex + ] + ) { + // Conflicting inlining: -1. + resolvedSourceFramesInlinedIntoSymbol = -1; + } + + // FIXME: Resolve conflicts of InnerWindowID + } + + category[funcIndex] = resolvedCategory; + subcategory[funcIndex] = resolvedSubcategory; + innerWindowID[funcIndex] = resolvedInnerWindowID; + sourceFramesInlinedIntoSymbol[funcIndex] = + resolvedSourceFramesInlinedIntoSymbol; + } + + return { + category, + subcategory, + innerWindowID, + sourceFramesInlinedIntoSymbol, + suffixOrderIndexRangeEnd: rootSuffixOrderIndexRangeEndCol, + length: funcCount, + }; +} + +function _createEmptyInvertedNonRootCallNodeTable(): InvertedNonRootCallNodeTable { + return { + prefix: [], + func: [], + pathHash: [], + category: [], + subcategory: [], + innerWindowID: [], + sourceFramesInlinedIntoSymbol: [], + suffixOrderIndexRangeStart: [], + suffixOrderIndexRangeEnd: [], + deepNodesForSuffixOrderIndexRange: [], + depth: [], + length: 0, + }; +} + +type PreparedChildrenAndSpecialChild = {| + // The node indexes in the inverted tree of this node's children. + childCallNodes: InvertedCallNodeHandle[], + // The node index of the child corresponding to specialChildFunc, if found. + specialChild: InvertedCallNodeHandle | null, +|}; + /** - * A subclass of CallNodeInfoImpl for "invert call stack" mode. - * - * This currently shares its implementation with CallNodeInfoImpl; - * this._callNodeTable is the inverted call node table. - * - * By the end of this commit stack, we will no longer have an inverted call node - * table and this class will stop inheriting from CallNodeInfoImpl. + * This is the implementation of the CallNodeInfoInverted interface. */ -export class CallNodeInfoInvertedImpl - extends CallNodeInfoImpl - implements CallNodeInfoInverted -{ +export class CallNodeInfoInvertedImpl implements CallNodeInfoInverted { + // The non-inverted call node table. + _callNodeTable: CallNodeTable; + + // The part of the inverted call node table for the roots of the inverted tree. + _invertedRootCallNodeTable: InvertedRootCallNodeTable; + + // The dynamically growing part of the inverted call node table for just the + // non-root nodes. Entries are added to this table as needed, whenever a caller + // asks us for children of a node for which we haven't needed children before, + // or when a caller asks us to translate an inverted call path that we haven't + // seend before to an inverted call node index. + _invertedNonRootCallNodeTable: InvertedNonRootCallNodeTable; + + // The mapping of non-inverted stack index to non-inverted call node index. + _stackIndexToNonInvertedCallNodeIndex: Int32Array; + + // The number of roots, i.e. this._roots.length. + _rootCount: number; + + // All inverted call tree roots. The roots of the inverted call tree are the + // "self" functions of the non-inverted call paths. + _roots: InvertedCallNodeHandle[]; + // This is a Map. // It lists the non-inverted call nodes in "suffix order", i.e. ordered by // comparing their call paths from back to front. _suffixOrderedCallNodes: Uint32Array; + // This is the inverse of _suffixOrderedCallNodes; i.e. it is a // Map. _suffixOrderIndexes: Uint32Array; + // The default category (usually "Other"), used when creating new inverted + // call nodes based on divergently-categorized functions. + _defaultCategory: IndexIntoCategoryList; + + // A scratch array of length funcTable.length, which can be used to count the + // number of occurrences of a function. Used for incremental sorting of call nodes. + _funcCountBuf: Uint32Array; + + // This is a Map. This map speeds up + // the look-up process by caching every CallNodePath we handle which avoids + // looking up parents again and again. + _cache: Map = new Map(); + + // For every inverted call node, the list of its child nodes, if we've computed + // it already. Computed on-demand by _getOrCreateChildren(). + _children: Map = new Map(); + constructor( callNodeTable: CallNodeTable, - nonInvertedCallNodeTable: CallNodeTable, stackIndexToNonInvertedCallNodeIndex: Int32Array, - suffixOrderedCallNodes: Uint32Array, - suffixOrderIndexes: Uint32Array + suffixOrderedCallNodes: Uint32Array, // IndexIntoCallNodeTable[], + suffixOrderIndexes: Uint32Array, // Map, + rootSuffixOrderIndexRangeEndCol: Uint32Array, + defaultCategory: IndexIntoCategoryList, + funcCountBuf: Uint32Array ) { - super( - callNodeTable, - nonInvertedCallNodeTable, - stackIndexToNonInvertedCallNodeIndex - ); + this._callNodeTable = callNodeTable; + this._stackIndexToNonInvertedCallNodeIndex = + stackIndexToNonInvertedCallNodeIndex; this._suffixOrderedCallNodes = suffixOrderedCallNodes; this._suffixOrderIndexes = suffixOrderIndexes; + this._defaultCategory = defaultCategory; + + const rootCount = rootSuffixOrderIndexRangeEndCol.length; + this._rootCount = rootCount; + + const roots = new Array(rootCount); + for (let i = 0; i < rootCount; i++) { + roots[i] = i; + } + this._roots = roots; + + this._funcCountBuf = funcCountBuf; + this._funcCountBuf.fill(0); + const invertedRootCallNodeTable = _createInvertedRootCallNodeTable( + callNodeTable, + rootSuffixOrderIndexRangeEndCol, + suffixOrderedCallNodes, + defaultCategory + ); + this._invertedRootCallNodeTable = invertedRootCallNodeTable; + this._invertedNonRootCallNodeTable = + _createEmptyInvertedNonRootCallNodeTable(); } isInverted(): boolean { return true; } - asInverted(): CallNodeInfoInverted | null { + asInverted(): CallNodeInfoInvertedImpl | null { return this; } + getNonInvertedCallNodeTable(): CallNodeTable { + return this._callNodeTable; + } + + getStackIndexToNonInvertedCallNodeIndex(): Int32Array { + return this._stackIndexToNonInvertedCallNodeIndex; + } + getSuffixOrderedCallNodes(): Uint32Array { return this._suffixOrderedCallNodes; } @@ -327,36 +570,521 @@ export class CallNodeInfoInvertedImpl return this._suffixOrderIndexes; } + getRoots(): Array { + return this._roots; + } + + isRoot(nodeHandle: InvertedCallNodeHandle): boolean { + return nodeHandle < this._rootCount; + } + getSuffixOrderIndexRangeForCallNode( - callNodeIndex: IndexIntoCallNodeTable + nodeHandle: InvertedCallNodeHandle ): [SuffixOrderIndex, SuffixOrderIndex] { - const callPath = this.getCallNodePathFromIndex(callNodeIndex); - return bisectEqualRange( - this._suffixOrderedCallNodes, - (callNodeIndex: IndexIntoCallNodeTable) => { - let currentCallNodeIndex = callNodeIndex; - for (let i = 0; i < callPath.length - 1; i++) { - const expectedFunc = callPath[i]; - const currentFunc = - this._nonInvertedCallNodeTable.func[currentCallNodeIndex]; - if (currentFunc < expectedFunc) { - return -1; - } - if (currentFunc > expectedFunc) { - return 1; - } - const prefix = - this._nonInvertedCallNodeTable.prefix[currentCallNodeIndex]; - if (prefix === -1) { - return -1; - } - currentCallNodeIndex = prefix; + if (nodeHandle < this._rootCount) { + const funcIndex = nodeHandle; + const rangeStart = + funcIndex === 0 + ? 0 + : this._invertedRootCallNodeTable.suffixOrderIndexRangeEnd[ + funcIndex - 1 + ]; + const rangeEnd = + this._invertedRootCallNodeTable.suffixOrderIndexRangeEnd[funcIndex]; + return [rangeStart, rangeEnd]; + } + const nonRootIndex = nodeHandle - this._rootCount; + const rangeStart = + this._invertedNonRootCallNodeTable.suffixOrderIndexRangeStart[ + nonRootIndex + ]; + const rangeEnd = + this._invertedNonRootCallNodeTable.suffixOrderIndexRangeEnd[nonRootIndex]; + return [rangeStart, rangeEnd]; + } + + /** + * Materialize inverted call nodes for parentNodeHandle's children in the + * inverted tree. + * + * Some callers of this function are interested in the index of the new child + * for a specific function. If so, they set specialChildFunc to that function + * index, and the corresponding child node will be returned in rv.specialChild. + * + * This function refines the suffix order so that it's correct for the newly- + * created children. It also adds entries to this._invertedNonRootCallNodeTable + * for each child. + * + * As we go deeper into the inverted tree, we go higher up in the non-inverted + * tree: To create the children of an inverted node, we need to look at the + * parents / "prefixes" of the corresponding non-inverted nodes. + * For each non-inverted node, we store both its "self" node and its + * "deep node". For an inverted node at depth k, the deep node is the kth parent + * of the self node. The deep node's function determines which one of the + * children in the *inverted* tree the non-inverted node is assigned to. + */ + _createChildren( + parentNodeHandle: InvertedCallNodeHandle, + specialChildFunc: IndexIntoFuncTable | null + ): PreparedChildrenAndSpecialChild { + const invertedNonRootCallNodeTable = this._invertedNonRootCallNodeTable; + const callNodeTable = this._callNodeTable; + const suffixOrderedCallNodes = this._suffixOrderedCallNodes; + const suffixOrderIndexes = this._suffixOrderIndexes; + + const parentDeepNodes = + this._takeDeepNodesForInvertedNode(parentNodeHandle); + const [parentIndexRangeStart, parentIndexRangeEnd] = + this.getSuffixOrderIndexRangeForCallNode(parentNodeHandle); + if ( + parentDeepNodes.length !== + parentIndexRangeEnd - parentIndexRangeStart + ) { + throw new Error('indexes out of sync'); + } + + // We need to sort the next level in [parentIndexRangeStart, parentIndexRangeEnd). + // To do the sorting, we use a trick from radix sort: First, we traverse the + // child call nodes and count how many occurrences there are of each func + // (i.e. build a histogram). Then we iterate over the funcs and compute the + // start indexes in the sorted array for each func, by accumulating the counts. + // Then we do another pass over the child call nodes and use the start indexes + // to put each call node at its new sorted position. + const countPerFunc = this._funcCountBuf; + const nodesWhichEndHere = []; + + // These three columns write down { selfNode, deepNode, func } per call node. + const unsortedCallNodesSelfNodeCol = []; + const unsortedCallNodesDeepNodeCol = []; + const unsortedCallNodesFuncCol = []; + + const uniqueFuncs = []; + + // Pass 1: Build a histogram (`countPerFunc`) by traversing the child call nodes. + // We also build a list of which funcs have non-zero counts, in `uniqueFuncs`. + // And we write down { selfNode, deepNode, func } per call node. + for (let i = 0; i < parentDeepNodes.length; i++) { + const selfNode = suffixOrderedCallNodes[parentIndexRangeStart + i]; + const parentDeepNode = parentDeepNodes[i]; + const deepNode = callNodeTable.prefix[parentDeepNode]; + if (deepNode !== -1) { + const func = callNodeTable.func[deepNode]; + const previousCountForThisFunc = countPerFunc[func]; + countPerFunc[func] = previousCountForThisFunc + 1; + if (previousCountForThisFunc === 0) { + uniqueFuncs.push(func); } - const expectedFunc = callPath[callPath.length - 1]; - const currentFunc = - this._nonInvertedCallNodeTable.func[currentCallNodeIndex]; - return currentFunc - expectedFunc; + unsortedCallNodesSelfNodeCol.push(selfNode); + unsortedCallNodesDeepNodeCol.push(deepNode); + unsortedCallNodesFuncCol.push(func); + } else { + nodesWhichEndHere.push(selfNode); } + } + + const childrenCount = uniqueFuncs.length; + const childrenFuncs = new Uint32Array(uniqueFuncs); + childrenFuncs.sort(); // Fast typed-array sort + + const childNodesIndexRangeStartCol = new Uint32Array(childrenCount); + const childNodesIndexRangeEndCol = new Uint32Array(childrenCount); + + const childRangeStart = parentIndexRangeStart + nodesWhichEndHere.length; + const childRangeLength = unsortedCallNodesFuncCol.length; + if (childRangeStart + childRangeLength !== parentIndexRangeEnd) { + throw new Error('indexes out of sync'); + } + + // Pass 2: Compute the accumulated sort index ranges (with the start index + // going into `nextIndexPerFunc`) for each child node func, based on + // the counts in the histogram. `previousEndIndex` is the accumulator. + const nextIndexPerFunc = countPerFunc; // WARNING: We are using the same array for both! + let previousEndIndex = 0; + for (let i = 0; i < childrenFuncs.length; i++) { + const func = childrenFuncs[i]; + const count = countPerFunc[func]; + if (count === 0) { + throw new Error( + 'childrenFuncs should only contain funcs with non-zero counts' + ); + } + + const startIndex = previousEndIndex; + const endIndex = previousEndIndex + count; + nextIndexPerFunc[func] = startIndex; + + childNodesIndexRangeStartCol[i] = startIndex; + childNodesIndexRangeEndCol[i] = endIndex; + + previousEndIndex = endIndex; + } + + // Pass 3: (two loops) Put the call nodes into their new spots in the sorted + // ordering, with the help of the computed index ranges. + + // BEGIN Apply new ordering. Warning: Between here and "END Apply new ordering", + // suffixOrderIndexes and suffixOrderedCallNodes will be in an inconsistent state. + + // First, apply the new ordering to nodesWhichEndHere. + for (let i = 0; i < nodesWhichEndHere.length; i++) { + const selfNode = nodesWhichEndHere[i]; + const orderingIndex = parentIndexRangeStart + i; + suffixOrderIndexes[selfNode] = orderingIndex; + suffixOrderedCallNodes[orderingIndex] = selfNode; + } + + // Then, apply the new ordering to unsortedCallNodes. + const childrenDeepNodes = new Uint32Array(childRangeLength); + for (let i = 0; i < unsortedCallNodesFuncCol.length; i++) { + const func = unsortedCallNodesFuncCol[i]; + const index = nextIndexPerFunc[func]++; + const selfNode = unsortedCallNodesSelfNodeCol[i]; + childrenDeepNodes[index] = unsortedCallNodesDeepNodeCol[i]; + const orderingIndex = childRangeStart + index; + suffixOrderIndexes[selfNode] = orderingIndex; + suffixOrderedCallNodes[orderingIndex] = selfNode; + } + + // END apply new ordering. + // The new ordering has been applied, and suffixOrderIndexes and + // suffixOrderedCallNodes are well-defined bijections again. + + // Clear nextIndexPerFunc so that we don't need to clear it next time when we reuse it. + for (let i = 0; i < childrenFuncs.length; i++) { + const func = childrenFuncs[i]; + nextIndexPerFunc[func] = 0; + } + + const parentNodeCallPathHash = this._pathHashForNode(parentNodeHandle); + const childrenDepth = this.depthForNode(parentNodeHandle) + 1; + + const childCallNodes = []; + let specialChild = null; // will be set to the index corresponding to specialChildFunc + for (let i = 0; i < childrenFuncs.length; i++) { + const func = childrenFuncs[i]; + const indexRangeStart = childNodesIndexRangeStartCol[i]; + const indexRangeEnd = childNodesIndexRangeEndCol[i]; + const suffixOrderIndexRangeStart = childRangeStart + indexRangeStart; + const suffixOrderIndexRangeEnd = childRangeStart + indexRangeEnd; + + const firstNodeNode = childrenDeepNodes[indexRangeStart]; + let currentCategory = callNodeTable.category[firstNodeNode]; + let currentSubcategory = callNodeTable.subcategory[firstNodeNode]; + const currentInnerWindowID = callNodeTable.innerWindowID[firstNodeNode]; + let currentSourceFramesInlinedIntoSymbol = + callNodeTable.sourceFramesInlinedIntoSymbol[firstNodeNode]; + + for (let index = indexRangeStart + 1; index < indexRangeEnd; index++) { + const nodeNode = childrenDeepNodes[index]; + + // Resolve category conflicts, by resetting a conflicting subcategory or + // category to the default category. + if (currentCategory !== callNodeTable.category[nodeNode]) { + // Conflicting origin stack categories -> default category + subcategory. + currentCategory = this._defaultCategory; + currentSubcategory = 0; + } else if (currentSubcategory !== callNodeTable.subcategory[nodeNode]) { + // Conflicting origin stack subcategories -> "Other" subcategory. + currentSubcategory = 0; + } + + // Resolve "inlined into" conflicts. This can happen if you have two + // function calls A -> B where only one of the B calls is inlined, or + // if you use call tree transforms in such a way that a function B which + // was inlined into two different callers (A -> B, C -> B) gets collapsed + // into one call node. + if ( + currentSourceFramesInlinedIntoSymbol !== + callNodeTable.sourceFramesInlinedIntoSymbol[nodeNode] + ) { + // Conflicting inlining: -1. + currentSourceFramesInlinedIntoSymbol = -1; + } + + // FIXME: Resolve conflicts of InnerWindowID + } + + const deepNodesForSuffixOrderIndexRange = childrenDeepNodes.subarray( + indexRangeStart, + indexRangeEnd + ); + + const newIndex = invertedNonRootCallNodeTable.length++; + const newHandle = this._rootCount + newIndex; + + const pathHash = concatHash(parentNodeCallPathHash, func); + invertedNonRootCallNodeTable.prefix[newIndex] = parentNodeHandle; + invertedNonRootCallNodeTable.func[newIndex] = func; + invertedNonRootCallNodeTable.pathHash[newIndex] = pathHash; + invertedNonRootCallNodeTable.category[newIndex] = currentCategory; + invertedNonRootCallNodeTable.subcategory[newIndex] = currentSubcategory; + invertedNonRootCallNodeTable.innerWindowID[newIndex] = + currentInnerWindowID; + invertedNonRootCallNodeTable.sourceFramesInlinedIntoSymbol[newIndex] = + currentSourceFramesInlinedIntoSymbol; + invertedNonRootCallNodeTable.deepNodesForSuffixOrderIndexRange[newIndex] = + deepNodesForSuffixOrderIndexRange; + invertedNonRootCallNodeTable.suffixOrderIndexRangeStart[newIndex] = + suffixOrderIndexRangeStart; + invertedNonRootCallNodeTable.suffixOrderIndexRangeEnd[newIndex] = + suffixOrderIndexRangeEnd; + invertedNonRootCallNodeTable.depth[newIndex] = childrenDepth; + childCallNodes.push(newHandle); + + this._cache.set(pathHash, newHandle); + + if (func === specialChildFunc) { + specialChild = newHandle; + } + } + this._children.set(parentNodeHandle, childCallNodes); + return { childCallNodes, specialChild }; + } + + _getChildWithFunc( + childrenSortedByFunc: InvertedCallNodeHandle[], + func: IndexIntoFuncTable + ): InvertedCallNodeHandle | null { + // TODO: Use bisection + for (let i = 0; i < childrenSortedByFunc.length; i++) { + const childNodeHandle = childrenSortedByFunc[i]; + if (this.funcForNode(childNodeHandle) === func) { + return childNodeHandle; + } + } + return null; + } + + _getOrCreateChildren( + parent: InvertedCallNodeHandle, + specialChildFunc: IndexIntoFuncTable | null + ): PreparedChildrenAndSpecialChild { + const childCallNodes = this._children.get(parent); + if (childCallNodes === undefined) { + return this._createChildren(parent, specialChildFunc); + } + + const specialChild = + specialChildFunc !== null + ? this._getChildWithFunc(childCallNodes, specialChildFunc) + : null; + return { childCallNodes, specialChild }; + } + + _findDeepestKnownAncestor(callPath: CallNodePath): InvertedCallNodeHandle { + const completePathNode = this._cache.get(hashPath(callPath)); + if (completePathNode !== undefined) { + return completePathNode; + } + + let bestNode = callPath[0]; + let remainingDepthRangeStart = 1; + let remainingDepthRangeEnd = callPath.length - 1; + while (remainingDepthRangeStart < remainingDepthRangeEnd) { + const currentDepth = + (remainingDepthRangeStart + remainingDepthRangeEnd) >> 1; + // assert(currentDepth < remainingDepthRangeEnd); + const currentPartialPath = callPath.slice(0, currentDepth + 1); + const currentNode = this._cache.get(hashPath(currentPartialPath)); + if (currentNode !== undefined) { + bestNode = currentNode; + remainingDepthRangeStart = currentDepth + 1; + } else { + remainingDepthRangeEnd = currentDepth; + } + } + return bestNode; + } + + getChildren(nodeIndex: InvertedCallNodeHandle): InvertedCallNodeHandle[] { + const { childCallNodes } = this._getOrCreateChildren(nodeIndex, null); + return childCallNodes; + } + + /** + * For an inverted call node whose children haven't been created yet, this + * returns the "deep nodes" corresponding to its suffix ordered call nodes. + * A deep node is the k'th parent node of a non-inverted call node, where k + * is the depth of the *inverted* call node. + */ + _takeDeepNodesForInvertedNode( + callNodeHandle: InvertedCallNodeHandle + ): Uint32Array { + if (callNodeHandle < this._rootCount) { + // This is a root. + const [rangeStart, rangeEnd] = + this.getSuffixOrderIndexRangeForCallNode(callNodeHandle); + return this._suffixOrderedCallNodes.subarray(rangeStart, rangeEnd); + } + + // callNodeHandle is a non-root node. + const nonRootIndex: IndexIntoInvertedNonRootCallNodeTable = + callNodeHandle - this._rootCount; + const deepNodesForSuffixOrderIndexRange = ensureExists( + this._invertedNonRootCallNodeTable.deepNodesForSuffixOrderIndexRange[ + nonRootIndex + ], + '_takeDeepNodesForInvertedNode should only be called once for each node, and only after its parent prepared its children.' ); + // Null it out, because we won't need it any more and because the order will + // be stale. + this._invertedNonRootCallNodeTable.deepNodesForSuffixOrderIndexRange[ + nonRootIndex + ] = null; + return deepNodesForSuffixOrderIndexRange; + } + + // This function returns a CallNodePath from a InvertedCallNodeHandle. + getCallNodePathFromIndex( + callNodeHandle: InvertedCallNodeHandle | null + ): CallNodePath { + if (callNodeHandle === null || callNodeHandle === -1) { + return []; + } + + const rootCount = this._rootCount; + const callNodePath = []; + let currentHandle = callNodeHandle; + while (currentHandle >= rootCount) { + const nonRootIndex = currentHandle - rootCount; + callNodePath.push(this._invertedNonRootCallNodeTable.func[nonRootIndex]); + currentHandle = this._invertedNonRootCallNodeTable.prefix[nonRootIndex]; + } + const rootFunc = currentHandle; + callNodePath.push(rootFunc); + callNodePath.reverse(); + return callNodePath; + } + + // Returns a CallNodeIndex from a CallNodePath, using and contributing to the + // cache parameter. + getCallNodeIndexFromPath( + callNodePath: CallNodePath + ): InvertedCallNodeHandle | null { + if (callNodePath.length === 0) { + return null; + } + + if (callNodePath.length === 1) { + return callNodePath[0]; // For roots, IndexIntoFuncTable === InvertedCallNodeHandle + } + + const pathDepth = callNodePath.length - 1; + let deepestKnownAncestor = this._findDeepestKnownAncestor(callNodePath); + let deepestKnownAncestorDepth = this.depthForNode(deepestKnownAncestor); + + while (deepestKnownAncestorDepth < pathDepth) { + const currentChildFunc = callNodePath[deepestKnownAncestorDepth + 1]; + const { specialChild } = this._getOrCreateChildren( + deepestKnownAncestor, + currentChildFunc + ); + if (specialChild === null) { + // No child matches the func we were looking for. + // This can happen when the provided call path doesn't exist. In that case + // we return null. + return null; + } + deepestKnownAncestor = specialChild; + deepestKnownAncestorDepth++; + } + return deepestKnownAncestor; + } + + // Returns the CallNodeIndex that matches the function `func` and whose parent's + // CallNodeIndex is `parent`. + getCallNodeIndexFromParentAndFunc( + parent: InvertedCallNodeHandle | -1, + func: IndexIntoFuncTable + ): InvertedCallNodeHandle | null { + if (parent === -1) { + return func; // For roots, IndexIntoFuncTable === InvertedCallNodeHandle + } + return this._getOrCreateChildren(parent, func).specialChild; + } + + _pathHashForNode(callNodeHandle: InvertedCallNodeHandle): string { + if (callNodeHandle < this._rootCount) { + return hashPathSingleFunc(callNodeHandle); + } + const nonRootIndex = callNodeHandle - this._rootCount; + return this._invertedNonRootCallNodeTable.pathHash[nonRootIndex]; + } + + prefixForNode( + callNodeHandle: InvertedCallNodeHandle + ): InvertedCallNodeHandle | -1 { + if (callNodeHandle < this._rootCount) { + // This is a root. + return -1; + } + const nonRootIndex = callNodeHandle - this._rootCount; + return this._invertedNonRootCallNodeTable.prefix[nonRootIndex]; + } + + funcForNode(callNodeHandle: InvertedCallNodeHandle): IndexIntoFuncTable { + if (callNodeHandle < this._rootCount) { + // This is a root. For roots, InvertedCallNodeHandle === IndexIntoFuncTable. + return callNodeHandle; + } + const nonRootIndex = callNodeHandle - this._rootCount; + return this._invertedNonRootCallNodeTable.func[nonRootIndex]; + } + + categoryForNode( + callNodeHandle: InvertedCallNodeHandle + ): IndexIntoCategoryList { + if (callNodeHandle < this._rootCount) { + const rootFunc = callNodeHandle; + return this._invertedRootCallNodeTable.category[rootFunc]; + } + const nonRootIndex = callNodeHandle - this._rootCount; + return this._invertedNonRootCallNodeTable.category[nonRootIndex]; + } + + subcategoryForNode( + callNodeHandle: InvertedCallNodeHandle + ): IndexIntoCategoryList { + if (callNodeHandle < this._rootCount) { + const rootFunc = callNodeHandle; + return this._invertedRootCallNodeTable.subcategory[rootFunc]; + } + const nonRootIndex = callNodeHandle - this._rootCount; + return this._invertedNonRootCallNodeTable.subcategory[nonRootIndex]; + } + + innerWindowIDForNode( + callNodeHandle: InvertedCallNodeHandle + ): IndexIntoCategoryList { + if (callNodeHandle < this._rootCount) { + const rootFunc = callNodeHandle; + return this._invertedRootCallNodeTable.innerWindowID[rootFunc]; + } + const nonRootIndex = callNodeHandle - this._rootCount; + return this._invertedNonRootCallNodeTable.innerWindowID[nonRootIndex]; + } + + depthForNode(callNodeHandle: InvertedCallNodeHandle): number { + if (callNodeHandle < this._rootCount) { + // Roots have depth 0. + return 0; + } + const nonRootIndex = callNodeHandle - this._rootCount; + return this._invertedNonRootCallNodeTable.depth[nonRootIndex]; + } + + sourceFramesInlinedIntoSymbolForNode( + callNodeHandle: InvertedCallNodeHandle + ): IndexIntoNativeSymbolTable | -1 | null { + if (callNodeHandle < this._rootCount) { + const rootFunc = callNodeHandle; + return this._invertedRootCallNodeTable.sourceFramesInlinedIntoSymbol[ + rootFunc + ]; + } + const nonRootIndex = callNodeHandle - this._rootCount; + return this._invertedNonRootCallNodeTable.sourceFramesInlinedIntoSymbol[ + nonRootIndex + ]; } } diff --git a/src/profile-logic/profile-data.js b/src/profile-logic/profile-data.js index f8336efa4c..f0ab4d125f 100644 --- a/src/profile-logic/profile-data.js +++ b/src/profile-logic/profile-data.js @@ -16,7 +16,10 @@ import { shallowCloneFrameTable, shallowCloneFuncTable, } from './data-structures'; -import { CallNodeInfoImpl, CallNodeInfoInvertedImpl } from './call-node-info'; +import { + CallNodeInfoNonInvertedImpl, + CallNodeInfoInvertedImpl, +} from './call-node-info'; import { INSTANT, INTERVAL, @@ -107,8 +110,7 @@ export function getCallNodeInfo( funcTable, defaultCategory ); - return new CallNodeInfoImpl( - callNodeTable, + return new CallNodeInfoNonInvertedImpl( callNodeTable, stackIndexToCallNodeIndex ); @@ -428,48 +430,56 @@ function _createCallNodeTableFromUnorderedComponents( * Generate the inverted CallNodeInfo for a thread. */ export function getInvertedCallNodeInfo( - thread: Thread, nonInvertedCallNodeTable: CallNodeTable, stackIndexToNonInvertedCallNodeIndex: Int32Array, - defaultCategory: IndexIntoCategoryList + defaultCategory: IndexIntoCategoryList, + funcCount: number ): CallNodeInfoInverted { - // We compute an inverted stack table, but we don't let it escape this function. - const { invertedThread } = _computeThreadWithInvertedStackTable( - thread, - defaultCategory - ); - - // Create an inverted call node table based on the inverted stack table. - const { callNodeTable } = computeCallNodeTable( - invertedThread.stackTable, - invertedThread.frameTable, - invertedThread.funcTable, - defaultCategory - ); - - // TEMPORARY: Compute a suffix order for the entire non-inverted call node table. - // See the CallNodeInfoInverted interface for more details about the suffix order. - // By the end of this commit stack, the suffix order will be computed incrementally - // as inverted nodes are created; we won't compute the entire order upfront. - const nonInvertedCallNodeCount = nonInvertedCallNodeTable.length; - const suffixOrderedCallNodes = new Uint32Array(nonInvertedCallNodeCount); - const suffixOrderIndexes = new Uint32Array(nonInvertedCallNodeCount); - for (let i = 0; i < nonInvertedCallNodeCount; i++) { - suffixOrderedCallNodes[i] = i; - } - suffixOrderedCallNodes.sort((a, b) => - _compareNonInvertedCallNodesInSuffixOrder(a, b, nonInvertedCallNodeTable) - ); - for (let i = 0; i < suffixOrderedCallNodes.length; i++) { - suffixOrderIndexes[suffixOrderedCallNodes[i]] = i; - } + const callNodeCount = nonInvertedCallNodeTable.length; + const suffixOrderedCallNodes = new Uint32Array(callNodeCount); + const suffixOrderingIndexes = new Uint32Array(callNodeCount); + const callNodeTableFuncCol = nonInvertedCallNodeTable.func; + + // Compute, per func, how many non-inverted call nodes end in this func + const nextIndexPerFunc = new Uint32Array(funcCount); + for (let i = 0; i < callNodeTableFuncCol.length; i++) { + const func = callNodeTableFuncCol[i]; + nextIndexPerFunc[func]++; + } + // Compute cumulative start index + let previousEndIndex = 0; + for (let func = 0; func < nextIndexPerFunc.length; func++) { + const count = nextIndexPerFunc[func]; + nextIndexPerFunc[func] = previousEndIndex; + previousEndIndex += count; + } + // Compute a first pass of the suffix ordering. This is just good enough for + // the roots of the inverted tree. This sort will be refined by + // CallNodeInfoInvertedImpl as more inverted nodes are created on-demand. + for (let i = 0; i < callNodeTableFuncCol.length; i++) { + const func = callNodeTableFuncCol[i]; + const nextIndex = nextIndexPerFunc[func]++; + suffixOrderedCallNodes[nextIndex] = i; + suffixOrderingIndexes[i] = nextIndex; + } + // Compute the suffix order index ranges for each root. We only compute the + // end of each range because the start is implied by the end of the previous + // inverted node (or 0 for the first inverted node). + const rootOrderingIndexRangeEndCol = new Uint32Array(funcCount); + for (let func = 0; func < funcCount - 1; func++) { + const endIndex = nextIndexPerFunc[func]; + rootOrderingIndexRangeEndCol[func] = endIndex; + } + rootOrderingIndexRangeEndCol[funcCount - 1] = suffixOrderedCallNodes.length; return new CallNodeInfoInvertedImpl( - callNodeTable, nonInvertedCallNodeTable, stackIndexToNonInvertedCallNodeIndex, suffixOrderedCallNodes, - suffixOrderIndexes + suffixOrderingIndexes, + rootOrderingIndexRangeEndCol, + defaultCategory, + nextIndexPerFunc ); } @@ -2157,98 +2167,6 @@ export function computeCallNodeMaxDepthPlusOne( return maxDepth + 1; } -function _computeThreadWithInvertedStackTable( - thread: Thread, - defaultCategory: IndexIntoCategoryList -): { - invertedThread: Thread, - oldStackToNewStack: Map, -} { - return timeCode('_computeThreadWithInvertedStackTable', () => { - const { stackTable, frameTable } = thread; - - const newStackTable = { - length: 0, - frame: [], - category: [], - subcategory: [], - prefix: [], - }; - // Create a Map that keys off of two values, both the prefix and frame combination - // by using a bit of math: prefix * frameCount + frame => stackIndex - const prefixAndFrameToStack = new Map(); - const frameCount = frameTable.length; - - // Returns the stackIndex for a specific frame (that is, a function and its - // context), and a specific prefix. If it doesn't exist yet it will create - // a new stack entry and return its index. - function stackFor(prefix, frame, category, subcategory) { - const prefixAndFrameIndex = - (prefix === null ? -1 : prefix) * frameCount + frame; - let stackIndex = prefixAndFrameToStack.get(prefixAndFrameIndex); - if (stackIndex === undefined) { - stackIndex = newStackTable.length++; - newStackTable.prefix[stackIndex] = prefix; - newStackTable.frame[stackIndex] = frame; - newStackTable.category[stackIndex] = category; - newStackTable.subcategory[stackIndex] = subcategory; - prefixAndFrameToStack.set(prefixAndFrameIndex, stackIndex); - } else if (newStackTable.category[stackIndex] !== category) { - // If two stack nodes from the non-inverted stack tree with different - // categories happen to collapse into the same stack node in the - // inverted tree, discard their category and set the category to the - // default category. - newStackTable.category[stackIndex] = defaultCategory; - newStackTable.subcategory[stackIndex] = 0; - } else if (newStackTable.subcategory[stackIndex] !== subcategory) { - // If two stack nodes from the non-inverted stack tree with the same - // category but different subcategories happen to collapse into the same - // stack node in the inverted tree, discard their subcategory and set it - // to the "Other" subcategory. - newStackTable.subcategory[stackIndex] = 0; - } - return stackIndex; - } - - const oldStackToNewStack = new Map(); - - // For one specific stack, this will ensure that stacks are created for all - // of its ancestors, by walking its prefix chain up to the root. - function convertStack(stackIndex) { - if (stackIndex === null) { - return null; - } - let newStack = oldStackToNewStack.get(stackIndex); - if (newStack === undefined) { - newStack = null; - for ( - let currentStack = stackIndex; - currentStack !== null; - currentStack = stackTable.prefix[currentStack] - ) { - // Notice how we reuse the previous stack as the prefix. This is what - // effectively inverts the call tree. - newStack = stackFor( - newStack, - stackTable.frame[currentStack], - stackTable.category[currentStack], - stackTable.subcategory[currentStack] - ); - } - oldStackToNewStack.set(stackIndex, ensureExists(newStack)); - } - return newStack; - } - - const invertedThread = updateThreadStacks( - thread, - newStackTable, - convertStack - ); - return { invertedThread, oldStackToNewStack }; - }); -} - /** * Sometimes we want to update the stacks for a thread, for instance while searching * for a text string, or doing a call tree transformation. This function abstracts diff --git a/src/selectors/per-thread/stack-sample.js b/src/selectors/per-thread/stack-sample.js index 44ffdbd893..a76c3efc5c 100644 --- a/src/selectors/per-thread/stack-sample.js +++ b/src/selectors/per-thread/stack-sample.js @@ -116,15 +116,15 @@ export function getStackAndSampleSelectorsPerThread( const _getInvertedCallNodeInfo: Selector = createSelectorWithTwoCacheSlots( - threadSelectors.getFilteredThread, _getNonInvertedCallNodeInfo, ProfileSelectors.getDefaultCategory, - (thread, nonInvertedCallNodeInfo, defaultCategory) => { + (state) => threadSelectors.getFilteredThread(state).funcTable.length, + (nonInvertedCallNodeInfo, defaultCategory, funcCount) => { return ProfileData.getInvertedCallNodeInfo( - thread, nonInvertedCallNodeInfo.getNonInvertedCallNodeTable(), nonInvertedCallNodeInfo.getStackIndexToNonInvertedCallNodeIndex(), - defaultCategory + defaultCategory, + funcCount ); } ); diff --git a/src/test/components/__snapshots__/ProfileCallTreeView.test.js.snap b/src/test/components/__snapshots__/ProfileCallTreeView.test.js.snap index 18baa59885..f347850326 100644 --- a/src/test/components/__snapshots__/ProfileCallTreeView.test.js.snap +++ b/src/test/components/__snapshots__/ProfileCallTreeView.test.js.snap @@ -4374,7 +4374,7 @@ for understanding where time was actually spent in a program." class="react-contextmenu-wrapper treeViewContextMenu" >
@@ -4751,7 +4751,7 @@ for understanding where time was actually spent in a program." aria-level="2" aria-selected="false" class="treeViewRow treeViewRowScrolledColumns odd" - id="treeViewRow-6" + id="treeViewRow-9" role="treeitem" style="height: 16px; line-height: 16px;" > @@ -4781,7 +4781,7 @@ for understanding where time was actually spent in a program." aria-level="3" aria-selected="false" class="treeViewRow treeViewRowScrolledColumns even" - id="treeViewRow-7" + id="treeViewRow-10" role="treeitem" style="height: 16px; line-height: 16px;" > @@ -4813,7 +4813,7 @@ for understanding where time was actually spent in a program." aria-level="4" aria-selected="false" class="treeViewRow treeViewRowScrolledColumns odd" - id="treeViewRow-8" + id="treeViewRow-11" role="treeitem" style="height: 16px; line-height: 16px;" > @@ -4842,7 +4842,7 @@ for understanding where time was actually spent in a program." aria-level="5" aria-selected="true" class="treeViewRow treeViewRowScrolledColumns even isSelected" - id="treeViewRow-9" + id="treeViewRow-13" role="treeitem" style="height: 16px; line-height: 16px;" > @@ -4872,7 +4872,7 @@ for understanding where time was actually spent in a program." aria-level="4" aria-selected="false" class="treeViewRow treeViewRowScrolledColumns odd" - id="treeViewRow-10" + id="treeViewRow-12" role="treeitem" style="height: 16px; line-height: 16px;" > @@ -4902,7 +4902,7 @@ for understanding where time was actually spent in a program." aria-level="1" aria-selected="false" class="treeViewRow treeViewRowScrolledColumns even" - id="treeViewRow-0" + id="treeViewRow-4" role="treeitem" style="height: 16px; line-height: 16px;" > diff --git a/src/test/store/__snapshots__/profile-view.test.js.snap b/src/test/store/__snapshots__/profile-view.test.js.snap index 82963dba34..a693358fd2 100644 --- a/src/test/store/__snapshots__/profile-view.test.js.snap +++ b/src/test/store/__snapshots__/profile-view.test.js.snap @@ -2216,7 +2216,7 @@ Object { `; exports[`snapshots of selectors/profile matches the last stored run of selectedThreadSelector.getCallNodeInfo 1`] = ` -CallNodeInfoImpl { +CallNodeInfoNonInvertedImpl { "_cache": Map {}, "_callNodeTable": Object { "category": Int32Array [ @@ -2321,109 +2321,6 @@ CallNodeInfoImpl { 9, ], }, - "_nonInvertedCallNodeTable": 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, - ], - }, "_stackIndexToNonInvertedCallNodeIndex": Int32Array [ 0, 1, @@ -2440,7 +2337,7 @@ CallNodeInfoImpl { exports[`snapshots of selectors/profile matches the last stored run of selectedThreadSelector.getCallTree 1`] = ` CallTree { - "_callNodeInfo": CallNodeInfoImpl { + "_callNodeInfo": CallNodeInfoNonInvertedImpl { "_cache": Map {}, "_callNodeTable": Object { "category": Int32Array [ @@ -2545,109 +2442,6 @@ CallTree { 9, ], }, - "_nonInvertedCallNodeTable": 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, - ], - }, "_stackIndexToNonInvertedCallNodeIndex": Int32Array [ 0, 1, @@ -2732,7 +2526,7 @@ CallTree { 0, 0, ], - "_callNodeInfo": CallNodeInfoImpl { + "_callNodeInfo": CallNodeInfoNonInvertedImpl { "_cache": Map {}, "_callNodeTable": Object { "category": Int32Array [ @@ -2837,109 +2631,6 @@ CallTree { 9, ], }, - "_nonInvertedCallNodeTable": 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, - ], - }, "_stackIndexToNonInvertedCallNodeIndex": Int32Array [ 0, 1, diff --git a/src/test/unit/address-timings.test.js b/src/test/unit/address-timings.test.js index ecf594a7c6..91309ed040 100644 --- a/src/test/unit/address-timings.test.js +++ b/src/test/unit/address-timings.test.js @@ -166,10 +166,10 @@ describe('getAddressTimings for getStackAddressInfoForCallNode', function () { ); const callNodeInfo = isInverted ? getInvertedCallNodeInfo( - thread, nonInvertedCallNodeInfo.getNonInvertedCallNodeTable(), nonInvertedCallNodeInfo.getStackIndexToNonInvertedCallNodeIndex(), - defaultCat + defaultCat, + funcTable.length ) : nonInvertedCallNodeInfo; const callNodeIndex = ensureExists( diff --git a/src/test/unit/line-timings.test.js b/src/test/unit/line-timings.test.js index aec1d1ade2..2cac8355e8 100644 --- a/src/test/unit/line-timings.test.js +++ b/src/test/unit/line-timings.test.js @@ -129,10 +129,10 @@ describe('getLineTimings for getStackLineInfoForCallNode', function () { ); const callNodeInfo = isInverted ? getInvertedCallNodeInfo( - thread, nonInvertedCallNodeInfo.getNonInvertedCallNodeTable(), nonInvertedCallNodeInfo.getStackIndexToNonInvertedCallNodeIndex(), - defaultCat + defaultCat, + funcTable.length ) : nonInvertedCallNodeInfo; const callNodeIndex = ensureExists( diff --git a/src/test/unit/profile-data.test.js b/src/test/unit/profile-data.test.js index 4fa5033511..5107668566 100644 --- a/src/test/unit/profile-data.test.js +++ b/src/test/unit/profile-data.test.js @@ -595,10 +595,10 @@ describe('getInvertedCallNodeInfo', function () { ); const invertedCallNodeInfo = getInvertedCallNodeInfo( - thread, nonInvertedCallNodeInfo.getNonInvertedCallNodeTable(), nonInvertedCallNodeInfo.getStackIndexToNonInvertedCallNodeIndex(), - defaultCategory + defaultCategory, + thread.funcTable.length ); // This function is used to test `getSuffixOrderIndexRangeForCallNode` and @@ -989,10 +989,10 @@ describe('getSamplesSelectedStates', function () { ); const defaultCategory = categories.findIndex((c) => c.name === 'Other'); const callNodeInfoInverted = getInvertedCallNodeInfo( - thread, callNodeInfo.getNonInvertedCallNodeTable(), stackIndexToCallNodeIndex, - defaultCategory + defaultCategory, + thread.funcTable.length ); return { @@ -1476,10 +1476,10 @@ describe('getNativeSymbolsForCallNode', function () { defaultCategory ); const callNodeInfo = getInvertedCallNodeInfo( - thread, nonInvertedCallNodeInfo.getNonInvertedCallNodeTable(), nonInvertedCallNodeInfo.getStackIndexToNonInvertedCallNodeIndex(), - defaultCategory + defaultCategory, + thread.funcTable.length ); const c = callNodeInfo.getCallNodeIndexFromPath([funC]); expect(c).not.toBeNull(); diff --git a/src/test/unit/profile-tree.test.js b/src/test/unit/profile-tree.test.js index ed4a4dc341..23c0164c4c 100644 --- a/src/test/unit/profile-tree.test.js +++ b/src/test/unit/profile-tree.test.js @@ -471,10 +471,10 @@ describe('inverted call tree', function () { // Now compute the inverted tree and check it. const invertedCallNodeInfo = getInvertedCallNodeInfo( - thread, callNodeInfo.getNonInvertedCallNodeTable(), callNodeInfo.getStackIndexToNonInvertedCallNodeIndex(), - defaultCategory + defaultCategory, + thread.funcTable.length ); const invertedCallTreeTimings = computeCallTreeTimings( invertedCallNodeInfo, diff --git a/src/types/profile-derived.js b/src/types/profile-derived.js index 40d92715a1..f01be3ab54 100644 --- a/src/types/profile-derived.js +++ b/src/types/profile-derived.js @@ -284,16 +284,16 @@ export type SuffixOrderIndex = number; * ``` * Represents call paths ending in * - [in0] A (so:0..3) = A = ... A (cn0, cn4, cn2) - * - [in1] A (so:1..2) = A <- A = ... A -> A (cn4) - * - [in2] B (so:2..3) = A <- B = ... B -> A (cn2) - * - [in3] A (so:2..3) = A <- B <- A = ... A -> B -> A (cn2) - * - [in4] B (so:3..5) = B = ... B (cn1, cn5) + * - [in3] A (so:1..2) = A <- A = ... A -> A (cn4) + * - [in4] B (so:2..3) = A <- B = ... B -> A (cn2) + * - [in6] A (so:2..3) = A <- B <- A = ... A -> B -> A (cn2) + * - [in1] B (so:3..5) = B = ... B (cn1, cn5) * - [in5] A (so:3..5) = B <- A = ... A -> B (cn1, cn5) - * - [in6] A (so:4..5) = B <- A <- A = ... A -> A -> B (cn5) - * - [in7] C (so:5..7) = C = ... C (cn6, cn3) - * - [in8] A (so:5..6) = C <- A = ... A -> C (cn6) - * - [in9] B (so:6..7) = C <- B = ... B -> C (cn3) - * - [in10] A (so:6..7) = C <- B <- A = ... A -> B -> C (cn3) + * - [in10] A (so:4..5) = B <- A <- A = ... A -> A -> B (cn5) + * - [in2] C (so:5..7) = C = ... C (cn6, cn3) + * - [in7] A (so:5..6) = C <- A = ... A -> C (cn6) + * - [in8] B (so:6..7) = C <- B = ... B -> C (cn3) + * - [in9] A (so:6..7) = C <- B <- A = ... A -> B -> C (cn3) * ``` * * In the suffix order, call paths become grouped in such a way that call paths @@ -314,15 +314,41 @@ export type SuffixOrderIndex = number; * soX: Suffix order index X * inX: Inverted call node index X * so:X..Y: Suffix order index range soX..soY (soY excluded) + * + * ## Incremental order refinement + * + * Sorting all non-inverted nodes upfront would take a long time on large profiles. + * So we don't do that. Instead, we refine the order as new inverted tree nodes + * are materialized on demand. + * + * The ground rules are: + * - For any inverted call node X, getSuffixOrderIndexRangeForCallNode(X) must + * always return the same range. + * - For any inverted call node X, the *set* of suffix ordered call nodes in the + * range returned by getSuffixOrderIndexRangeForCallNode(X) must always be the + * same. Notably, the order in the range does *not* necessarily need to remain + * the same. + * + * This means that, whenever you have a handle X of an inverted call node, you + * can be confident that your checks of the form "is non-inverted call node Y + * part of X's range" will work correctly. */ export interface CallNodeInfoInverted extends CallNodeInfo { // Get a mapping SuffixOrderIndex -> IndexIntoNonInvertedCallNodeTable. // This array contains all non-inverted call node indexes, ordered by // call path suffix. See "suffix order" in the documentation above. + // Note that the contents of this array will be mutated by CallNodeInfoInverted + // when new inverted nodes are created on demand (e.g. during a call to + // getChildren or to getCallNodeIndexFromPath). So callers should not hold on + // to this array across calls which can create new inverted call nodes. getSuffixOrderedCallNodes(): Uint32Array; // Returns the inverse of getSuffixOrderedCallNodes(), i.e. a mapping // IndexIntoNonInvertedCallNodeTable -> SuffixOrderIndex. + // Note that the contents of this array will be mutated by CallNodeInfoInverted + // when new inverted nodes are created on demand (e.g. during a call to + // getChildren or to getCallNodeIndexFromPath). So callers should not hold on + // to this array across calls which can create new inverted call nodes. getSuffixOrderIndexes(): Uint32Array; // Get the [start, exclusiveEnd] range of suffix order indexes for this diff --git a/src/utils/bisect.js b/src/utils/bisect.js index 05fe86adbd..16dcc4e724 100644 --- a/src/utils/bisect.js +++ b/src/utils/bisect.js @@ -208,132 +208,3 @@ export function bisectionLeft( return low; } - -/* - * TEMPORARY: The functions below implement bisectEqualRange(). The implementation - * is copied from https://searchfox.org/mozilla-central/rev/8b0666aff1197e1dd8017de366343de9c21ee437/mfbt/BinarySearch.h#132-243 - * The only code calling bisectEqualRange will be removed by the end of this - * commit stack, so all the code added here will be removed again, too. - * - * bisectLowerBound(), bisectUpperBound(), and bisectEqualRange() are equivalent to - * std::lower_bound(), std::upper_bound(), and std::equal_range() respectively. - * - * bisectLowerBound() returns an index pointing to the first element in the range - * in which each element is considered *not less than* the given value passed - * via |aCompare|, or the length of |aContainer| if no such element is found. - * - * bisectUpperBound() returns an index pointing to the first element in the range - * in which each element is considered *greater than* the given value passed - * via |aCompare|, or the length of |aContainer| if no such element is found. - * - * bisectEqualRange() returns a range [first, second) containing all elements are - * considered equivalent to the given value via |aCompare|. If you need - * either the first or last index of the range, bisectLowerBound() or bisectUpperBound(), - * which is slightly faster than bisectEqualRange(), should suffice. - * - * Example (another example is given in TestBinarySearch.cpp): - * - * Vector sortedStrings = ... - * - * struct Comparator { - * const nsACString& mStr; - * explicit Comparator(const nsACString& aStr) : mStr(aStr) {} - * int32_t operator()(const char* aVal) const { - * return Compare(mStr, nsDependentCString(aVal)); - * } - * }; - * - * auto bounds = bisectEqualRange(sortedStrings, 0, sortedStrings.length(), - * Comparator("needle I'm looking for"_ns)); - * printf("Found the range [%zd %zd)\n", bounds.first(), bounds.second()); - * - */ -export function bisectLowerBound( - array: number[] | $TypedArray, - f: (number) => number, // < 0 if arg is before needle, > 0 if after, === 0 if same - low?: number, - high?: number -): number { - low = low || 0; - high = high || array.length; - - if (low < 0 || low > array.length || high < 0 || high > array.length) { - throw new TypeError("low and high must lie within the array's range"); - } - - while (high !== low) { - const middle = (low + high) >> 1; - const result = f(array[middle]); - - // The range returning from bisectLowerBound does include elements - // equivalent to the given value i.e. f(element) == 0 - if (result >= 0) { - high = middle; - } else { - low = middle + 1; - } - } - - return low; -} - -export function bisectUpperBound( - array: number[] | $TypedArray, - f: (number) => number, // < 0 if arg is before needle, > 0 if after, === 0 if same - low?: number, - high?: number -): number { - low = low || 0; - high = high || array.length; - - if (low < 0 || low > array.length || high < 0 || high > array.length) { - throw new TypeError("low and high must lie within the array's range"); - } - - while (high !== low) { - const middle = (low + high) >> 1; - const result = f(array[middle]); - - // The range returning from bisectUpperBound does NOT include elements - // equivalent to the given value i.e. f(element) == 0 - if (result > 0) { - high = middle; - } else { - low = middle + 1; - } - } - - return high; -} - -export function bisectEqualRange( - array: number[] | $TypedArray, - f: (number) => number, // < 0 if arg is before needle, > 0 if after, === 0 if same - low?: number, - high?: number -): [number, number] { - low = low || 0; - high = high || array.length; - - if (low < 0 || low > array.length || high < 0 || high > array.length) { - throw new TypeError("low and high must lie within the array's range"); - } - - while (high !== low) { - const middle = (low + high) >> 1; - const result = f(array[middle]); - - if (result > 0) { - high = middle; - } else if (result < 0) { - low = middle + 1; - } else { - return [ - bisectLowerBound(array, f, low, middle), - bisectUpperBound(array, f, middle + 1, high), - ]; - } - } - - return [low, high]; -} diff --git a/src/utils/path.js b/src/utils/path.js index 7bc9bc8d09..814d741858 100644 --- a/src/utils/path.js +++ b/src/utils/path.js @@ -4,7 +4,7 @@ // @flow -import type { CallNodePath } from 'firefox-profiler/types'; +import type { CallNodePath, IndexIntoFuncTable } from 'firefox-profiler/types'; export function arePathsEqual(a: CallNodePath, b: CallNodePath): boolean { if (a === b) { @@ -34,6 +34,17 @@ export function hashPath(a: CallNodePath): string { return a.join('-'); } +export function concatHash( + hash: string, + extraFunc: IndexIntoFuncTable +): string { + return hash + '-' + extraFunc; +} + +export function hashPathSingleFunc(func: IndexIntoFuncTable): string { + return '' + func; +} + // This class implements all of the methods of the native Set, but provides a // unique list of CallNodePaths. These paths can be different objects, but as // long as they contain the same data, they are considered to be the same. From ed5cd5b6bdb2592806d8da364459d7b0488b2437 Mon Sep 17 00:00:00 2001 From: Markus Stange Date: Sat, 20 Jan 2024 12:17:25 -0500 Subject: [PATCH 17/18] Optimize handling of roots. The new structure gives us a nice guarantee about roots of the inverted tree: There is an inverted root for every func, and their indexes are identical. This makes it really cheap to translate between the call node index and the func index (no conversion or lookup is necessary) and also makes it cheap to check if a node is a root. This commit also replaces a few maps and sets with typed arrays for performance. This is easier now that the root indexes are all contiguous. --- src/profile-logic/call-node-info.js | 28 ++++++-------- src/profile-logic/call-tree.js | 60 ++++++++++++----------------- src/types/profile-derived.js | 8 ++-- 3 files changed, 41 insertions(+), 55 deletions(-) diff --git a/src/profile-logic/call-node-info.js b/src/profile-logic/call-node-info.js index f2018d55e4..330fb05741 100644 --- a/src/profile-logic/call-node-info.js +++ b/src/profile-logic/call-node-info.js @@ -475,13 +475,16 @@ export class CallNodeInfoInvertedImpl implements CallNodeInfoInverted { // The mapping of non-inverted stack index to non-inverted call node index. _stackIndexToNonInvertedCallNodeIndex: Int32Array; - // The number of roots, i.e. this._roots.length. + // The number of roots, which is also the number of functions. Each root of + // the inverted tree represents a "self" function, i.e. all call paths which + // end in a certain function. + // We have roots even for functions which aren't used as "self" functions in + // any sampled stacks, for simplicity. The actual displayed number of roots + // in the call tree will usually be lower because roots with a zero total sample + // count will be filtered out. But any data in this class is fully independent + // from sample counts. _rootCount: number; - // All inverted call tree roots. The roots of the inverted call tree are the - // "self" functions of the non-inverted call paths. - _roots: InvertedCallNodeHandle[]; - // This is a Map. // It lists the non-inverted call nodes in "suffix order", i.e. ordered by // comparing their call paths from back to front. @@ -523,16 +526,7 @@ export class CallNodeInfoInvertedImpl implements CallNodeInfoInverted { this._suffixOrderedCallNodes = suffixOrderedCallNodes; this._suffixOrderIndexes = suffixOrderIndexes; this._defaultCategory = defaultCategory; - - const rootCount = rootSuffixOrderIndexRangeEndCol.length; - this._rootCount = rootCount; - - const roots = new Array(rootCount); - for (let i = 0; i < rootCount; i++) { - roots[i] = i; - } - this._roots = roots; - + this._rootCount = rootSuffixOrderIndexRangeEndCol.length; this._funcCountBuf = funcCountBuf; this._funcCountBuf.fill(0); const invertedRootCallNodeTable = _createInvertedRootCallNodeTable( @@ -570,8 +564,8 @@ export class CallNodeInfoInvertedImpl implements CallNodeInfoInverted { return this._suffixOrderIndexes; } - getRoots(): Array { - return this._roots; + getFuncCount(): number { + return this._rootCount; } isRoot(nodeHandle: InvertedCallNodeHandle): boolean { diff --git a/src/profile-logic/call-tree.js b/src/profile-logic/call-tree.js index 3cfbd11bce..42b09be8ac 100644 --- a/src/profile-logic/call-tree.js +++ b/src/profile-logic/call-tree.js @@ -58,8 +58,8 @@ export type CallTreeTimingsInverted = {| callNodeSelf: Float32Array, rootTotalSummary: number, sortedRoots: IndexIntoFuncTable[], - totalPerRootNode: Map, - rootNodesWithChildren: Set, + totalPerRootFunc: Float32Array, + hasChildrenPerRootFunc: Uint8Array, |}; export type CallTreeTimings = @@ -185,8 +185,8 @@ class CallTreeInternalInverted implements CallTreeInternal { _callNodeSelf: Float32Array; _rootNodes: IndexIntoCallNodeTable[]; _funcCount: number; - _totalPerRootNode: Map; - _rootNodesWithChildren: Set; + _totalPerRootFunc: Float32Array; + _hasChildrenPerRootFunc: Uint8Array; _totalAndHasChildrenPerNonRootNode: Map< IndexIntoCallNodeTable, TotalAndHasChildren, @@ -199,10 +199,10 @@ class CallTreeInternalInverted implements CallTreeInternal { this._callNodeInfo = callNodeInfo; this._nonInvertedCallNodeTable = callNodeInfo.getNonInvertedCallNodeTable(); this._callNodeSelf = callTreeTimingsInverted.callNodeSelf; - const { sortedRoots, totalPerRootNode, rootNodesWithChildren } = + const { sortedRoots, totalPerRootFunc, hasChildrenPerRootFunc } = callTreeTimingsInverted; - this._totalPerRootNode = totalPerRootNode; - this._rootNodesWithChildren = rootNodesWithChildren; + this._totalPerRootFunc = totalPerRootFunc; + this._hasChildrenPerRootFunc = hasChildrenPerRootFunc; this._rootNodes = sortedRoots; } @@ -212,7 +212,7 @@ class CallTreeInternalInverted implements CallTreeInternal { hasChildren(callNodeIndex: IndexIntoCallNodeTable): boolean { if (this._callNodeInfo.isRoot(callNodeIndex)) { - return this._rootNodesWithChildren.has(callNodeIndex); + return this._hasChildrenPerRootFunc[callNodeIndex] !== 0; } return this._getTotalAndHasChildren(callNodeIndex).hasChildren; } @@ -238,7 +238,7 @@ class CallTreeInternalInverted implements CallTreeInternal { getSelfAndTotal(callNodeIndex: IndexIntoCallNodeTable): SelfAndTotal { if (this._callNodeInfo.isRoot(callNodeIndex)) { - const total = ensureExists(this._totalPerRootNode.get(callNodeIndex)); + const total = this._totalPerRootFunc[callNodeIndex]; return { self: total, total }; } const { total } = this._getTotalAndHasChildren(callNodeIndex); @@ -643,9 +643,9 @@ export function getSelfAndTotalForCallNode( case 'INVERTED': { const callNodeInfoInverted = ensureExists(callNodeInfo.asInverted()); const { timings } = callTreeTimings; - const { callNodeSelf, totalPerRootNode } = timings; + const { callNodeSelf, totalPerRootFunc } = timings; if (callNodeInfoInverted.isRoot(callNodeIndex)) { - const total = totalPerRootNode.get(callNodeIndex) ?? 0; + const total = totalPerRootFunc[callNodeIndex]; return { self: total, total }; } const { total } = _getInvertedTreeNodeTotalAndHasChildren( @@ -691,13 +691,14 @@ export function computeCallTreeTimingsInverted( callNodeInfo: CallNodeInfoInverted, { callNodeSelf, rootTotalSummary }: CallNodeSelfAndSummary ): CallTreeTimingsInverted { - const roots = callNodeInfo.getRoots(); + const funcCount = callNodeInfo.getFuncCount(); const callNodeTable = callNodeInfo.getNonInvertedCallNodeTable(); const callNodeTableFuncCol = callNodeTable.func; const callNodeTableDepthCol = callNodeTable.depth; - const totalPerRootNode = new Map(); - const rootNodesWithChildren = new Set(); - const seenRoots = new Set(); + const totalPerRootFunc = new Float32Array(funcCount); + const hasChildrenPerRootFunc = new Uint8Array(funcCount); + const seenPerRootFunc = new Uint8Array(funcCount); + const sortedRoots = []; for (let i = 0; i < callNodeSelf.length; i++) { const self = callNodeSelf[i]; if (self === 0) { @@ -708,36 +709,25 @@ export function computeCallTreeTimingsInverted( // call tree. This is done by finding the inverted root which corresponds to // the self function of the non-inverted call node. const func = callNodeTableFuncCol[i]; - const rootNode = roots.find( - (invertedCallNode) => callNodeInfo.funcForNode(invertedCallNode) === func - ); - if (rootNode === undefined) { - throw new Error( - "Couldn't find the inverted root for a function with non-zero self time." - ); - } - totalPerRootNode.set( - rootNode, - (totalPerRootNode.get(rootNode) ?? 0) + self - ); - seenRoots.add(rootNode); + totalPerRootFunc[func] += self; + if (seenPerRootFunc[func] === 0) { + seenPerRootFunc[func] = 1; + sortedRoots.push(func); + } if (callNodeTableDepthCol[i] !== 0) { - rootNodesWithChildren.add(rootNode); + hasChildrenPerRootFunc[func] = 1; } } - const sortedRoots = [...seenRoots]; sortedRoots.sort( - (a, b) => - Math.abs(totalPerRootNode.get(b) ?? 0) - - Math.abs(totalPerRootNode.get(a) ?? 0) + (a, b) => Math.abs(totalPerRootFunc[b]) - Math.abs(totalPerRootFunc[a]) ); return { callNodeSelf, rootTotalSummary, sortedRoots, - totalPerRootNode, - rootNodesWithChildren, + totalPerRootFunc, + hasChildrenPerRootFunc, }; } diff --git a/src/types/profile-derived.js b/src/types/profile-derived.js index f01be3ab54..b78883958a 100644 --- a/src/types/profile-derived.js +++ b/src/types/profile-derived.js @@ -162,9 +162,6 @@ export interface CallNodeInfo { func: IndexIntoFuncTable ): IndexIntoCallNodeTable | null; - // Returns the list of root nodes. - getRoots(): IndexIntoCallNodeTable[]; - // Returns whether the given node is a root node. isRoot(callNodeIndex: IndexIntoCallNodeTable): boolean; @@ -334,6 +331,11 @@ export type SuffixOrderIndex = number; * part of X's range" will work correctly. */ export interface CallNodeInfoInverted extends CallNodeInfo { + // Get the number of functions. There is one root per function. + // So this is also the number of roots at the same time. + // The inverted call node index for a root is the same as the function index. + getFuncCount(): number; + // Get a mapping SuffixOrderIndex -> IndexIntoNonInvertedCallNodeTable. // This array contains all non-inverted call node indexes, ordered by // call path suffix. See "suffix order" in the documentation above. From 086aec73f3c3ca7241c5b091fbb5bebc442df893 Mon Sep 17 00:00:00 2001 From: Markus Stange Date: Sat, 20 Jan 2024 12:23:08 -0500 Subject: [PATCH 18/18] Make sourceFramesInlinedIntoSymbol an Int32Array. This avoids a CompareIC when comparing to null in _createInvertedRootCallNodeTable, because we'll now only be comparing integers. This speeds up _createInvertedRootCallNodeTable by almost 2x. --- src/profile-logic/call-node-info.js | 16 ++-- src/profile-logic/call-tree.js | 2 +- src/profile-logic/data-structures.js | 2 +- src/profile-logic/profile-data.js | 12 +-- .../__snapshots__/profile-view.test.js.snap | 80 +++++++++---------- src/types/profile-derived.js | 6 +- 6 files changed, 59 insertions(+), 59 deletions(-) diff --git a/src/profile-logic/call-node-info.js b/src/profile-logic/call-node-info.js index 330fb05741..057a82e871 100644 --- a/src/profile-logic/call-node-info.js +++ b/src/profile-logic/call-node-info.js @@ -265,7 +265,7 @@ export class CallNodeInfoNonInvertedImpl implements CallNodeInfo { sourceFramesInlinedIntoSymbolForNode( callNodeIndex: IndexIntoCallNodeTable - ): IndexIntoNativeSymbolTable | -1 | null { + ): IndexIntoNativeSymbolTable | -1 | -2 { return this._callNodeTable.sourceFramesInlinedIntoSymbol[callNodeIndex]; } } @@ -290,8 +290,8 @@ type InvertedRootCallNodeTable = {| innerWindowID: Float64Array, // IndexIntoFuncTable -> InnerWindowID // IndexIntoNativeSymbolTable: all frames that collapsed into this call node inlined into the same native symbol // -1: divergent: some, but not all, frames that collapsed into this call node were inlined, or they are from different symbols - // null: no inlining - sourceFramesInlinedIntoSymbol: Array, + // -2: no inlining + sourceFramesInlinedIntoSymbol: Int32Array, // IndexIntoFuncTable -> IndexIntoNativeSymbolTable | -1 | -2 // The (exclusive) end of the suffix order index range for each root node. // The beginning of the range is given by suffixOrderIndexRangeEnd[i - 1], or by // zero. This is possible because both the inverted root order and the suffix order @@ -311,8 +311,8 @@ type InvertedNonRootCallNodeTable = {| innerWindowID: InnerWindowID[], // IndexIntoInvertedNonRootCallNodeTable -> InnerWindowID // IndexIntoNativeSymbolTable: all frames that collapsed into this call node inlined into the same native symbol // -1: divergent: some, but not all, frames that collapsed into this call node were inlined, or they are from different symbols - // null: no inlining - sourceFramesInlinedIntoSymbol: Array, + // -2: no inlining + sourceFramesInlinedIntoSymbol: Array, suffixOrderIndexRangeStart: SuffixOrderIndex[], // IndexIntoInvertedNonRootCallNodeTable -> SuffixOrderIndex suffixOrderIndexRangeEnd: SuffixOrderIndex[], // IndexIntoInvertedNonRootCallNodeTable -> SuffixOrderIndex @@ -340,7 +340,7 @@ function _createInvertedRootCallNodeTable( const category = new Int32Array(funcCount); const subcategory = new Int32Array(funcCount); const innerWindowID = new Float64Array(funcCount); - const sourceFramesInlinedIntoSymbol = new Array(funcCount); + const sourceFramesInlinedIntoSymbol = new Int32Array(funcCount); let previousRootSuffixOrderIndexRangeEnd = 0; for (let funcIndex = 0; funcIndex < funcCount; funcIndex++) { const callNodesuffixOrderIndexRangeStart = @@ -351,7 +351,7 @@ function _createInvertedRootCallNodeTable( if ( callNodesuffixOrderIndexRangeStart === callNodesuffixOrderIndexRangeEnd ) { - sourceFramesInlinedIntoSymbol[funcIndex] = null; + sourceFramesInlinedIntoSymbol[funcIndex] = -2; // Leave the remaining columns at zero for this root. continue; } @@ -1069,7 +1069,7 @@ export class CallNodeInfoInvertedImpl implements CallNodeInfoInverted { sourceFramesInlinedIntoSymbolForNode( callNodeHandle: InvertedCallNodeHandle - ): IndexIntoNativeSymbolTable | -1 | null { + ): IndexIntoNativeSymbolTable | -1 | -2 { if (callNodeHandle < this._rootCount) { const rootFunc = callNodeHandle; return this._invertedRootCallNodeTable.sourceFramesInlinedIntoSymbol[ diff --git a/src/profile-logic/call-tree.js b/src/profile-logic/call-tree.js index 42b09be8ac..0cbf2510dc 100644 --- a/src/profile-logic/call-tree.js +++ b/src/profile-logic/call-tree.js @@ -406,7 +406,7 @@ export class CallTree { const calledFunction = getFunctionName(funcName); const inlinedIntoNativeSymbol = this._callNodeInfo.sourceFramesInlinedIntoSymbolForNode(callNodeIndex); - if (inlinedIntoNativeSymbol === null) { + if (inlinedIntoNativeSymbol === -2) { return undefined; } diff --git a/src/profile-logic/data-structures.js b/src/profile-logic/data-structures.js index 0a50aea02d..2c32fbf695 100644 --- a/src/profile-logic/data-structures.js +++ b/src/profile-logic/data-structures.js @@ -428,7 +428,7 @@ export function getEmptyCallNodeTable(): CallNodeTable { category: new Int32Array(0), subcategory: new Int32Array(0), innerWindowID: new Float64Array(0), - sourceFramesInlinedIntoSymbol: [], + sourceFramesInlinedIntoSymbol: new Int32Array(0), depth: [], maxDepth: -1, length: 0, diff --git a/src/profile-logic/profile-data.js b/src/profile-logic/profile-data.js index f0ab4d125f..7edb85546f 100644 --- a/src/profile-logic/profile-data.js +++ b/src/profile-logic/profile-data.js @@ -148,7 +148,7 @@ export function computeCallNodeTable( const subcategory: Array = []; const innerWindowID: Array = []; const sourceFramesInlinedIntoSymbol: Array< - IndexIntoNativeSymbolTable | -1 | null, + IndexIntoNativeSymbolTable | -1 | -2, > = []; let length = 0; @@ -168,7 +168,7 @@ export function computeCallNodeTable( categoryIndex: IndexIntoCategoryList, subcategoryIndex: IndexIntoSubcategoryListForCategory, windowID: InnerWindowID, - inlinedIntoSymbol: IndexIntoNativeSymbolTable | null + inlinedIntoSymbol: IndexIntoNativeSymbolTable | -1 | -2 ) { const index = length++; prefix[index] = prefixIndex; @@ -221,8 +221,8 @@ export function computeCallNodeTable( const subcategoryIndex = stackTable.subcategory[stackIndex]; const inlinedIntoSymbol = frameTable.inlineDepth[frameIndex] > 0 - ? frameTable.nativeSymbol[frameIndex] - : null; + ? (frameTable.nativeSymbol[frameIndex] ?? -2) + : -2; const funcIndex = frameTable.func[frameIndex]; // Check if the call node for this stack already exists. @@ -318,7 +318,7 @@ function _createCallNodeTableFromUnorderedComponents( category: Array, subcategory: Array, innerWindowID: Array, - sourceFramesInlinedIntoSymbol: Array, + sourceFramesInlinedIntoSymbol: Array, length: number, stackIndexToCallNodeIndex: Int32Array ): CallNodeTableAndStackMap { @@ -337,7 +337,7 @@ function _createCallNodeTableFromUnorderedComponents( const categorySorted = new Int32Array(length); const subcategorySorted = new Int32Array(length); const innerWindowIDSorted = new Float64Array(length); - const sourceFramesInlinedIntoSymbolSorted = new Array(length); + const sourceFramesInlinedIntoSymbolSorted = new Int32Array(length); const depthSorted = new Array(length); let maxDepth = 0; diff --git a/src/test/store/__snapshots__/profile-view.test.js.snap b/src/test/store/__snapshots__/profile-view.test.js.snap index a693358fd2..173db0ff28 100644 --- a/src/test/store/__snapshots__/profile-view.test.js.snap +++ b/src/test/store/__snapshots__/profile-view.test.js.snap @@ -2287,16 +2287,16 @@ CallNodeInfoNonInvertedImpl { 1, 7, ], - "sourceFramesInlinedIntoSymbol": Array [ - null, - null, - null, - null, - null, - null, - null, - null, - null, + "sourceFramesInlinedIntoSymbol": Int32Array [ + -2, + -2, + -2, + -2, + -2, + -2, + -2, + -2, + -2, ], "subcategory": Int32Array [ 0, @@ -2408,16 +2408,16 @@ CallTree { 1, 7, ], - "sourceFramesInlinedIntoSymbol": Array [ - null, - null, - null, - null, - null, - null, - null, - null, - null, + "sourceFramesInlinedIntoSymbol": Int32Array [ + -2, + -2, + -2, + -2, + -2, + -2, + -2, + -2, + -2, ], "subcategory": Int32Array [ 0, @@ -2597,16 +2597,16 @@ CallTree { 1, 7, ], - "sourceFramesInlinedIntoSymbol": Array [ - null, - null, - null, - null, - null, - null, - null, - null, - null, + "sourceFramesInlinedIntoSymbol": Int32Array [ + -2, + -2, + -2, + -2, + -2, + -2, + -2, + -2, + -2, ], "subcategory": Int32Array [ 0, @@ -2712,16 +2712,16 @@ CallTree { 1, 7, ], - "sourceFramesInlinedIntoSymbol": Array [ - null, - null, - null, - null, - null, - null, - null, - null, - null, + "sourceFramesInlinedIntoSymbol": Int32Array [ + -2, + -2, + -2, + -2, + -2, + -2, + -2, + -2, + -2, ], "subcategory": Int32Array [ 0, diff --git a/src/types/profile-derived.js b/src/types/profile-derived.js index b78883958a..3b9b2ae3a0 100644 --- a/src/types/profile-derived.js +++ b/src/types/profile-derived.js @@ -109,10 +109,10 @@ export type CallNodeTable = { category: Int32Array, // IndexIntoCallNodeTable -> IndexIntoCategoryList subcategory: Int32Array, // IndexIntoCallNodeTable -> IndexIntoSubcategoryListForCategory innerWindowID: Float64Array, // IndexIntoCallNodeTable -> InnerWindowID - // null: no inlining // IndexIntoNativeSymbolTable: all frames that collapsed into this call node inlined into the same native symbol // -1: divergent: not all frames that collapsed into this call node were inlined, or they are from different symbols - sourceFramesInlinedIntoSymbol: Array, + // -2: no inlining + sourceFramesInlinedIntoSymbol: Int32Array, // The depth of the call node. Roots have depth 0. depth: number[], // The maximum value in the depth column, or -1 if this table is empty. @@ -189,7 +189,7 @@ export interface CallNodeInfo { depthForNode(callNodeIndex: IndexIntoCallNodeTable): number; sourceFramesInlinedIntoSymbolForNode( callNodeIndex: IndexIntoCallNodeTable - ): IndexIntoNativeSymbolTable | -1 | null; + ): IndexIntoNativeSymbolTable | -1 | -2; } // An index into SuffixOrderedCallNodes.