From 51e429e16ddcf3ea8bdcae55f4e9e580ee8822bb Mon Sep 17 00:00:00 2001 From: Markus Stange Date: Tue, 20 Jan 2026 15:33:44 -0500 Subject: [PATCH 1/3] Stop computing self line/address hits for the double-clicked call node. We only use the total hits anyway, to compute which line to scroll to. --- src/profile-logic/address-timings.ts | 31 +++++++++++ src/profile-logic/bottom-box.ts | 18 ++++--- src/profile-logic/line-timings.ts | 31 +++++++++++ src/test/unit/address-timings.test.ts | 70 ++++++++++-------------- src/test/unit/line-timings.test.ts | 77 +++++++++++---------------- 5 files changed, 133 insertions(+), 94 deletions(-) diff --git a/src/profile-logic/address-timings.ts b/src/profile-logic/address-timings.ts index 144dadfeb8..2c11bd79a5 100644 --- a/src/profile-logic/address-timings.ts +++ b/src/profile-logic/address-timings.ts @@ -530,3 +530,34 @@ export function getAddressTimings( } return { totalAddressHits, selfAddressHits }; } + +// Like getAddressTimings, but computes only the totalAddressHits. +export function getTotalAddressTimings( + stackAddressInfo: StackAddressInfo | null, + samples: SamplesLikeTable +): Map { + if (stackAddressInfo === null) { + return new Map(); + } + const { stackAddresses } = stackAddressInfo; + const totalAddressHits: Map = new Map(); + + // Iterate over all the samples, and aggregate the sample's weight into the + // addresses which are hit by the sample's stack. + // TODO: Maybe aggregate sample count per stack first, and then visit each stack only once? + for (let sampleIndex = 0; sampleIndex < samples.length; sampleIndex++) { + const stackIndex = samples.stack[sampleIndex]; + if (stackIndex === null) { + continue; + } + const weight = samples.weight ? samples.weight[sampleIndex] : 1; + const setOfHitAddresses = stackAddresses[stackIndex]; + if (setOfHitAddresses !== null) { + for (const address of setOfHitAddresses) { + const oldHitCount = totalAddressHits.get(address) ?? 0; + totalAddressHits.set(address, oldHitCount + weight); + } + } + } + return totalAddressHits; +} diff --git a/src/profile-logic/bottom-box.ts b/src/profile-logic/bottom-box.ts index a470105912..98135818f1 100644 --- a/src/profile-logic/bottom-box.ts +++ b/src/profile-logic/bottom-box.ts @@ -16,11 +16,14 @@ import { getNativeSymbolInfo, getNativeSymbolsForCallNode, } from './profile-data'; -import { getLineTimings, getStackLineInfoForCallNode } from './line-timings'; +import { + getStackLineInfoForCallNode, + getTotalLineTimings, +} from './line-timings'; import { mapGetKeyWithMaxValue } from 'firefox-profiler/utils'; import { - getAddressTimings, getStackAddressInfoForCallNode, + getTotalAddressTimings, } from './address-timings'; /** @@ -84,8 +87,8 @@ export function getBottomBoxInfoForCallNode( callNodeIndex, callNodeInfo ); - const callNodeLineTimings = getLineTimings(stackLineInfo, samples); - const hottestLine = mapGetKeyWithMaxValue(callNodeLineTimings.totalLineHits); + const callNodeLineTimings = getTotalLineTimings(stackLineInfo, samples); + const hottestLine = mapGetKeyWithMaxValue(callNodeLineTimings); // Compute the hottest instruction, so we can ask the assembly view to scroll to it. let hottestInstructionAddress; @@ -97,10 +100,11 @@ export function getBottomBoxInfoForCallNode( callNodeInfo, nativeSymbolsForCallNode[initialNativeSymbol] ); - const callNodeAddressTimings = getAddressTimings(stackAddressInfo, samples); - hottestInstructionAddress = mapGetKeyWithMaxValue( - callNodeAddressTimings.totalAddressHits + const callNodeAddressTimings = getTotalAddressTimings( + stackAddressInfo, + samples ); + hottestInstructionAddress = mapGetKeyWithMaxValue(callNodeAddressTimings); } return { diff --git a/src/profile-logic/line-timings.ts b/src/profile-logic/line-timings.ts index 68c0a679b5..2d264ce720 100644 --- a/src/profile-logic/line-timings.ts +++ b/src/profile-logic/line-timings.ts @@ -404,3 +404,34 @@ export function getLineTimings( } return { totalLineHits, selfLineHits }; } + +// Like getLineTimings, but computes only the totalLineHits. +export function getTotalLineTimings( + stackLineInfo: StackLineInfo | null, + samples: SamplesLikeTable +): Map { + if (stackLineInfo === null) { + return new Map(); + } + const { stackLines } = stackLineInfo; + const totalLineHits: Map = new Map(); + + // Iterate over all the samples, and aggregate the sample's weight into the + // lines which are hit by the sample's stack. + // TODO: Maybe aggregate sample count per stack first, and then visit each stack only once? + for (let sampleIndex = 0; sampleIndex < samples.length; sampleIndex++) { + const stackIndex = samples.stack[sampleIndex]; + if (stackIndex === null) { + continue; + } + const weight = samples.weight ? samples.weight[sampleIndex] : 1; + const setOfHitLines = stackLines[stackIndex]; + if (setOfHitLines !== null) { + for (const line of setOfHitLines) { + const oldHitCount = totalLineHits.get(line) ?? 0; + totalLineHits.set(line, oldHitCount + weight); + } + } + } + return totalLineHits; +} diff --git a/src/test/unit/address-timings.test.ts b/src/test/unit/address-timings.test.ts index 3484ce5401..de3e45c60d 100644 --- a/src/test/unit/address-timings.test.ts +++ b/src/test/unit/address-timings.test.ts @@ -7,6 +7,7 @@ import { getStackAddressInfo, getStackAddressInfoForCallNode, getAddressTimings, + getTotalAddressTimings, } from 'firefox-profiler/profile-logic/address-timings'; import { getCallNodeInfo, @@ -18,6 +19,7 @@ import type { Thread, IndexIntoCategoryList, IndexIntoNativeSymbolTable, + Address, } from 'firefox-profiler/types'; describe('getStackAddressInfo', function () { @@ -158,7 +160,7 @@ describe('getAddressTimings for getStackAddressInfoForCallNode', function () { defaultCategory: IndexIntoCategoryList, nativeSymbol: IndexIntoNativeSymbolTable, isInverted: boolean - ) { + ): Map { const { stackTable, frameTable, funcTable, samples } = thread; const nonInvertedCallNodeInfo = getCallNodeInfo( stackTable, @@ -183,7 +185,7 @@ describe('getAddressTimings for getStackAddressInfoForCallNode', function () { callNodeInfo, nativeSymbol ); - return getAddressTimings(stackLineInfo, samples); + return getTotalAddressTimings(stackLineInfo, samples); } it('passes a basic test', function () { @@ -201,7 +203,7 @@ describe('getAddressTimings for getStackAddressInfoForCallNode', function () { const [thread] = derivedThreads; // Compute the address timings for the root call node. - // No self address hit, one total address hit at address 0x20. + // One total address hit at address 0x20. const addressTimingsRoot = getTimings( thread, [A], @@ -209,12 +211,11 @@ describe('getAddressTimings for getStackAddressInfoForCallNode', function () { Asym, false ); - expect(addressTimingsRoot.totalAddressHits.get(0x20)).toBe(1); - expect(addressTimingsRoot.totalAddressHits.size).toBe(1); // no other hits - expect(addressTimingsRoot.selfAddressHits.size).toBe(0); // no self hits + expect(addressTimingsRoot.get(0x20)).toBe(1); + expect(addressTimingsRoot.size).toBe(1); // no other hits // Compute the address timings for the child call node. - // One self address hit at address 0x30, which is also the only total address hit. + // One total address hit at address 0x30. const addressTimingsChild = getTimings( thread, [A, B], @@ -222,10 +223,8 @@ describe('getAddressTimings for getStackAddressInfoForCallNode', function () { Bsym, false ); - expect(addressTimingsChild.totalAddressHits.get(0x30)).toBe(1); - expect(addressTimingsChild.totalAddressHits.size).toBe(1); // no other hits - expect(addressTimingsChild.selfAddressHits.get(0x30)).toBe(1); - expect(addressTimingsChild.selfAddressHits.size).toBe(1); // no other hits + expect(addressTimingsChild.get(0x30)).toBe(1); + expect(addressTimingsChild.size).toBe(1); // no other hits }); it('passes a basic test with recursion', function () { @@ -245,7 +244,7 @@ describe('getAddressTimings for getStackAddressInfoForCallNode', function () { const [thread] = derivedThreads; // Compute the address timings for the root call node. - // No self address hit, one total address hit at address 0x20. + // One total address hit at address 0x20. const addressTimingsRoot = getTimings( thread, [A], @@ -253,12 +252,11 @@ describe('getAddressTimings for getStackAddressInfoForCallNode', function () { Asym, false ); - expect(addressTimingsRoot.totalAddressHits.get(0x20)).toBe(1); - expect(addressTimingsRoot.totalAddressHits.size).toBe(1); // no other hits - expect(addressTimingsRoot.selfAddressHits.size).toBe(0); // no self hits + expect(addressTimingsRoot.get(0x20)).toBe(1); + expect(addressTimingsRoot.size).toBe(1); // no other hits // Compute the address timings for the leaf call node. - // One self address hit at address 0x21, which is also the only total address hit. + // One total address hit at address 0x21. // In particular, we shouldn't record a hit for line 20, even though // the hit at line 20 is also in A. But it's in the wrong call node. const addressTimingsChild = getTimings( @@ -268,10 +266,8 @@ describe('getAddressTimings for getStackAddressInfoForCallNode', function () { Asym, false ); - expect(addressTimingsChild.totalAddressHits.get(0x21)).toBe(1); - expect(addressTimingsChild.totalAddressHits.size).toBe(1); // no other hits - expect(addressTimingsChild.selfAddressHits.get(0x21)).toBe(1); - expect(addressTimingsChild.selfAddressHits.size).toBe(1); // no other hits + expect(addressTimingsChild.get(0x21)).toBe(1); + expect(addressTimingsChild.size).toBe(1); // no other hits }); it('passes a test where the same function is called via different call paths', function () { @@ -298,11 +294,9 @@ describe('getAddressTimings for getStackAddressInfoForCallNode', function () { Csym, false ); - expect(addressTimingsABC.totalAddressHits.get(0x10)).toBe(1); - expect(addressTimingsABC.totalAddressHits.get(0x12)).toBe(1); - expect(addressTimingsABC.totalAddressHits.size).toBe(2); // no other hits - expect(addressTimingsABC.selfAddressHits.get(0x10)).toBe(1); - expect(addressTimingsABC.selfAddressHits.size).toBe(1); // no other hits + expect(addressTimingsABC.get(0x10)).toBe(1); + expect(addressTimingsABC.get(0x12)).toBe(1); + expect(addressTimingsABC.size).toBe(2); // no other hits }); it('passes a test with an inverted thread', function () { @@ -321,7 +315,7 @@ describe('getAddressTimings for getStackAddressInfoForCallNode', function () { const [{ C, D }] = funcNamesDictPerThread; const [{ Csym, Dsym }] = nativeSymbolsDictPerThread; const [thread] = derivedThreads; - // For the root D of the inverted tree, we have 3 self address hits. + // For the root D of the inverted tree, we have 3 address hits. const addressTimingsD = getTimings( thread, [D], @@ -329,15 +323,12 @@ describe('getAddressTimings for getStackAddressInfoForCallNode', function () { Dsym, true ); - expect(addressTimingsD.totalAddressHits.get(0x51)).toBe(2); - expect(addressTimingsD.totalAddressHits.get(0x52)).toBe(1); - expect(addressTimingsD.totalAddressHits.size).toBe(2); // no other hits - expect(addressTimingsD.selfAddressHits.get(0x51)).toBe(2); - expect(addressTimingsD.selfAddressHits.get(0x52)).toBe(1); - expect(addressTimingsD.selfAddressHits.size).toBe(2); // no other hits + expect(addressTimingsD.get(0x51)).toBe(2); + expect(addressTimingsD.get(0x52)).toBe(1); + expect(addressTimingsD.size).toBe(2); // no other hits // For the C call node which is a child (direct caller) of D, we have - // no self address hit and one hit at address 0x12. + // one hit at address 0x12. const addressTimingsDC = getTimings( thread, [D, C], @@ -345,9 +336,8 @@ describe('getAddressTimings for getStackAddressInfoForCallNode', function () { Csym, true ); - expect(addressTimingsDC.totalAddressHits.get(0x12)).toBe(1); - expect(addressTimingsDC.totalAddressHits.size).toBe(1); // no other hits - expect(addressTimingsDC.selfAddressHits.size).toBe(0); // no self address hits + expect(addressTimingsDC.get(0x12)).toBe(1); + expect(addressTimingsDC.size).toBe(1); // no other hits }); it('passes a test where a function is present in two different native symbols', function () { @@ -386,10 +376,8 @@ describe('getAddressTimings for getStackAddressInfoForCallNode', function () { Bsym, false ); - expect(addressTimingsABCForBsym.totalAddressHits.get(0x40)).toBe(1); - expect(addressTimingsABCForBsym.totalAddressHits.get(0x45)).toBe(1); - expect(addressTimingsABCForBsym.totalAddressHits.size).toBe(2); // no other hits - expect(addressTimingsABCForBsym.selfAddressHits.get(0x40)).toBe(1); - expect(addressTimingsABCForBsym.selfAddressHits.size).toBe(1); // no other hits + expect(addressTimingsABCForBsym.get(0x40)).toBe(1); + expect(addressTimingsABCForBsym.get(0x45)).toBe(1); + expect(addressTimingsABCForBsym.size).toBe(2); // no other hits }); }); diff --git a/src/test/unit/line-timings.test.ts b/src/test/unit/line-timings.test.ts index 2c3f68ca30..5241f8d16c 100644 --- a/src/test/unit/line-timings.test.ts +++ b/src/test/unit/line-timings.test.ts @@ -7,6 +7,7 @@ import { getStackLineInfo, getStackLineInfoForCallNode, getLineTimings, + getTotalLineTimings, } from 'firefox-profiler/profile-logic/line-timings'; import { getCallNodeInfo, @@ -17,6 +18,7 @@ import type { CallNodePath, Thread, IndexIntoCategoryList, + LineNumber, } from 'firefox-profiler/types'; describe('getStackLineInfo', function () { @@ -154,7 +156,7 @@ describe('getLineTimings for getStackLineInfoForCallNode', function () { callNodePath: CallNodePath, defaultCategory: IndexIntoCategoryList, isInverted: boolean - ) { + ): Map { const { stackTable, frameTable, funcTable, samples } = thread; const nonInvertedCallNodeInfo = getCallNodeInfo( stackTable, @@ -179,7 +181,7 @@ describe('getLineTimings for getStackLineInfoForCallNode', function () { callNodeIndex, callNodeInfo ); - return getLineTimings(stackLineInfo, samples); + return getTotalLineTimings(stackLineInfo, samples); } it('passes a basic test', function () { @@ -193,19 +195,16 @@ describe('getLineTimings for getStackLineInfoForCallNode', function () { const [thread] = derivedThreads; // Compute the line timings for the root call node. - // No self line hit, one total line hit in line 20. + // One total line hit in line 20. const lineTimingsRoot = getTimings(thread, [A], defaultCategory, false); - expect(lineTimingsRoot.totalLineHits.get(20)).toBe(1); - expect(lineTimingsRoot.totalLineHits.size).toBe(1); // no other hits - expect(lineTimingsRoot.selfLineHits.size).toBe(0); // no self hits + expect(lineTimingsRoot.get(20)).toBe(1); + expect(lineTimingsRoot.size).toBe(1); // no other hits // Compute the line timings for the child call node. - // One self line hit in line 30, which is also the only total line hit. + // One total line hit in line 30. const lineTimingsChild = getTimings(thread, [A, B], defaultCategory, false); - expect(lineTimingsChild.totalLineHits.get(30)).toBe(1); - expect(lineTimingsChild.totalLineHits.size).toBe(1); // no other hits - expect(lineTimingsChild.selfLineHits.get(30)).toBe(1); - expect(lineTimingsChild.selfLineHits.size).toBe(1); // no other hits + expect(lineTimingsChild.get(30)).toBe(1); + expect(lineTimingsChild.size).toBe(1); // no other hits }); it('passes a basic test with recursion', function () { @@ -220,14 +219,13 @@ describe('getLineTimings for getStackLineInfoForCallNode', function () { const [thread] = derivedThreads; // Compute the line timings for the root call node. - // No self line hit, one total line hit in line 20. + // One total line hit in line 20. const lineTimingsRoot = getTimings(thread, [A], defaultCategory, false); - expect(lineTimingsRoot.totalLineHits.get(20)).toBe(1); - expect(lineTimingsRoot.totalLineHits.size).toBe(1); // no other hits - expect(lineTimingsRoot.selfLineHits.size).toBe(0); // no self hits + expect(lineTimingsRoot.get(20)).toBe(1); + expect(lineTimingsRoot.size).toBe(1); // no other hits // Compute the line timings for the leaf call node. - // One self line hit in line 21, which is also the only total line hit. + // One total line hit in line 21. // In particular, we shouldn't record a hit for line 20, even though // the hit at line 20 is also in A. But it's in the wrong call node. const lineTimingsChild = getTimings( @@ -236,10 +234,8 @@ describe('getLineTimings for getStackLineInfoForCallNode', function () { defaultCategory, false ); - expect(lineTimingsChild.totalLineHits.get(21)).toBe(1); - expect(lineTimingsChild.totalLineHits.size).toBe(1); // no other hits - expect(lineTimingsChild.selfLineHits.get(21)).toBe(1); - expect(lineTimingsChild.selfLineHits.size).toBe(1); // no other hits + expect(lineTimingsChild.get(21)).toBe(1); + expect(lineTimingsChild.size).toBe(1); // no other hits }); it('passes a test where the same function is called via different call paths', function () { @@ -260,11 +256,9 @@ describe('getLineTimings for getStackLineInfoForCallNode', function () { defaultCategory, false ); - expect(lineTimingsABC.totalLineHits.get(10)).toBe(1); - expect(lineTimingsABC.totalLineHits.get(12)).toBe(1); - expect(lineTimingsABC.totalLineHits.size).toBe(2); // no other hits - expect(lineTimingsABC.selfLineHits.get(10)).toBe(1); - expect(lineTimingsABC.selfLineHits.size).toBe(1); // no other hits + expect(lineTimingsABC.get(10)).toBe(1); + expect(lineTimingsABC.get(12)).toBe(1); + expect(lineTimingsABC.size).toBe(2); // no other hits }); it('passes a test with an inverted thread', function () { @@ -279,29 +273,22 @@ describe('getLineTimings for getStackLineInfoForCallNode', function () { const [{ A, B, C, D }] = funcNamesDictPerThread; const [thread] = derivedThreads; - // For the root D of the inverted tree, we have 3 self line hits. + // For the root D of the inverted tree, we have 3 total line hits. const lineTimingsD = getTimings(thread, [D], defaultCategory, true); - expect(lineTimingsD.totalLineHits.get(51)).toBe(2); - expect(lineTimingsD.totalLineHits.get(52)).toBe(1); - expect(lineTimingsD.totalLineHits.size).toBe(2); // no other hits - expect(lineTimingsD.selfLineHits.get(51)).toBe(2); - expect(lineTimingsD.selfLineHits.get(52)).toBe(1); - expect(lineTimingsD.selfLineHits.size).toBe(2); // no other hits + expect(lineTimingsD.get(51)).toBe(2); + expect(lineTimingsD.get(52)).toBe(1); + expect(lineTimingsD.size).toBe(2); // no other hits // For the C call node which is a child (direct caller) of D, we have - // no self line hit and one hit in line 12. + // one hit in line 12. const lineTimingsDC = getTimings(thread, [D, C], defaultCategory, true); - expect(lineTimingsDC.totalLineHits.get(12)).toBe(1); - expect(lineTimingsDC.totalLineHits.size).toBe(1); // no other hits - expect(lineTimingsDC.selfLineHits.size).toBe(0); // no self line hits + expect(lineTimingsDC.get(12)).toBe(1); + expect(lineTimingsDC.size).toBe(1); // no other hits - // For the D <- B <- A call node, we have no self line hit, and one total - // hit in line 20. (No self line hit because that sample's self time is - // spent in D, not in A.) + // For the D <- B <- A call node, we have one total hit in line 20. const lineTimingsDBA = getTimings(thread, [D, B, A], defaultCategory, true); - expect(lineTimingsDBA.totalLineHits.get(20)).toBe(1); - expect(lineTimingsDBA.totalLineHits.size).toBe(1); // no other hits - expect(lineTimingsDC.selfLineHits.size).toBe(0); // no self line hits + expect(lineTimingsDBA.get(20)).toBe(1); + expect(lineTimingsDBA.size).toBe(1); // no other hits }); it('falls back to funcTable.lineNumber when frameTable.line is null', function () { @@ -327,9 +314,7 @@ describe('getLineTimings for getStackLineInfoForCallNode', function () { // Compute the line timings for the child call node. // The fallback should use funcTable.lineNumber[func] = 35 const lineTimingsChild = getTimings(thread, [A, B], defaultCategory, false); - expect(lineTimingsChild.totalLineHits.get(35)).toBe(1); - expect(lineTimingsChild.totalLineHits.size).toBe(1); // no other hits - expect(lineTimingsChild.selfLineHits.get(35)).toBe(1); - expect(lineTimingsChild.selfLineHits.size).toBe(1); // no other hits + expect(lineTimingsChild.get(35)).toBe(1); + expect(lineTimingsChild.size).toBe(1); // no other hits }); }); From 9add0bfb00568d5d393b6e078e9e65f9d9003330 Mon Sep 17 00:00:00 2001 From: Markus Stange Date: Mon, 12 Jan 2026 14:00:02 -0500 Subject: [PATCH 2/3] Add getCallNodeFramePerStack and use it in getNativeSymbolsForCallNode. I didn't add any tests for the new function; I think it is sufficiently covered by the existing tests for getNativeSymbolsForCallNode. And the next commit will make the address / line timings code use it, which will contribute some more test coverage. --- src/profile-logic/bottom-box.ts | 8 +- src/profile-logic/profile-data.ts | 228 ++++++++++++++++++++--------- src/test/unit/profile-data.test.ts | 38 ++--- 3 files changed, 188 insertions(+), 86 deletions(-) diff --git a/src/profile-logic/bottom-box.ts b/src/profile-logic/bottom-box.ts index 98135818f1..5eb190b7a9 100644 --- a/src/profile-logic/bottom-box.ts +++ b/src/profile-logic/bottom-box.ts @@ -13,6 +13,7 @@ import type { } from 'firefox-profiler/types'; import type { CallNodeInfo } from './call-node-info'; import { + getCallNodeFramePerStack, getNativeSymbolInfo, getNativeSymbolsForCallNode, } from './profile-data'; @@ -56,10 +57,13 @@ export function getBottomBoxInfoForCallNode( resource !== -1 && resourceTable.type[resource] === resourceTypes.library ? resourceTable.lib[resource] : null; - const nativeSymbolsForCallNode = getNativeSymbolsForCallNode( + const callNodeFramePerStack = getCallNodeFramePerStack( callNodeIndex, callNodeInfo, - stackTable, + stackTable + ); + const nativeSymbolsForCallNode = getNativeSymbolsForCallNode( + callNodeFramePerStack, frameTable ); diff --git a/src/profile-logic/profile-data.ts b/src/profile-logic/profile-data.ts index 39efe49ae0..3be6925778 100644 --- a/src/profile-logic/profile-data.ts +++ b/src/profile-logic/profile-data.ts @@ -727,6 +727,159 @@ export function getNthPrefixStack( return s; } +/** + * Given a call node `callNodeIndex`, answer, for each stack S: + * - Does a sample with stack S contribute to `callNodeIndex`'s total time? + * - If so, which of `callNodeIndex`'s frames does such a sample contribute its + * total time to? + * + * If the answer to the first question is "no", we put frame index -1 into the + * returned array for that stack index. + */ +export function getCallNodeFramePerStack( + callNodeIndex: IndexIntoCallNodeTable, + callNodeInfo: CallNodeInfo, + stackTable: StackTable +): Int32Array { + const callNodeInfoInverted = callNodeInfo.asInverted(); + return callNodeInfoInverted !== null + ? getCallNodeFramePerStackInverted( + callNodeIndex, + callNodeInfoInverted, + stackTable + ) + : getCallNodeFramePerStackNonInverted( + callNodeIndex, + callNodeInfo, + stackTable + ); +} + +/** + * This function handles the non-inverted case of getCallNodeFramePerStack. + * + * Gathers the frames which are hit in a given call node by each stack, + * or -1 if the stack isn't in the call node's subtree. + * + * This is best explained with an example. + * Let the call node be the node for the call path [A, B, C]. + * Let this be the stack tree: + * + * - stack 0, func A, frame 100 + * - stack 1, func B, frame 110 + * - stack 2, func C, frame 120 + * - stack 3, func C, frame 130 + * - stack 4, func B, frame 140 + * - stack 5, func C, frame 150 + * - stack 6, func C, frame 160 + * - stack 7, func D, frame 170 + * - stack 8, func E, frame 180 + * - stack 9, func F, frame 190 + * + * This maps to the following call tree: + * + * - call node 0, func A + * - call node 1, func B + * - call node 2, func C + * - call node 3, func D + * - call node 4, func E + * - call node 5, func F + * + * The call path [A, B, C] uniquely identifies call node 2. + * The following stacks all "collapse into" ("map to") call node 2: + * stack 2, 3, 5 and 6. + * Stack 7 maps to call node 3, which is a child of call node 2. + * Stacks 0, 1, 4, 8 and 9 are outside the call path [A, B, C]. + * + * Stacks 2, 3, 4 and 5 all make a "total time" contribution to call + * node 2, to the frames 120, 130, 150, and 160, respectively. + * Stack 7 also contributes total time to call node 2, to frame 160. + * Stacks 0, 1, 4, 8 and 9 don't contribute to call node 2's total time. + * + * So this function returns the following array in the example: + * new Int32Array([-1, -1, 120, 130, -1, 150, 160, 160, -1, -1]) + * // for stacks 0, 1, 2, 3, 4, 5, 6, 7, 8, 9 + */ +export function getCallNodeFramePerStackNonInverted( + callNodeIndex: IndexIntoCallNodeTable, + callNodeInfo: CallNodeInfo, + stackTable: StackTable +): Int32Array { + const stackIndexToCallNodeIndex = + callNodeInfo.getStackIndexToNonInvertedCallNodeIndex(); + + const { frame: frameCol, prefix: prefixCol, length: stackCount } = stackTable; + + const callNodeFramePerStack = new Int32Array(stackCount); + + // This loop takes advantage of the stack table's ordering: + // Prefix stacks are always visited before their descendants. + for (let stackIndex = 0; stackIndex < stackCount; stackIndex++) { + let frame = -1; + const callNodeForThisStack = stackIndexToCallNodeIndex[stackIndex]; + if (callNodeForThisStack === callNodeIndex) { + frame = frameCol[stackIndex]; + } else { + // We're either already in the call node's subtree, or we are + // outside the subtree. Either way, we can just inherit the frame + // that our prefix stack hits in this call node. + const prefix = prefixCol[stackIndex]; + if (prefix !== null) { + frame = callNodeFramePerStack[prefix]; + } + } + + callNodeFramePerStack[stackIndex] = frame; + } + return callNodeFramePerStack; +} + +/** + * This handles the inverted case of getCallNodeFramePerStack. + */ +export function getCallNodeFramePerStackInverted( + callNodeIndex: IndexIntoCallNodeTable, + callNodeInfo: CallNodeInfoInverted, + stackTable: StackTable +): Int32Array { + const depth = callNodeInfo.depthForNode(callNodeIndex); + const [rangeStart, rangeEnd] = + callNodeInfo.getSuffixOrderIndexRangeForCallNode(callNodeIndex); + const stackIndexToCallNodeIndex = + callNodeInfo.getStackIndexToNonInvertedCallNodeIndex(); + const stackTablePrefixCol = stackTable.prefix; + const suffixOrderIndexes = callNodeInfo.getSuffixOrderIndexes(); + + const callNodeFramePerStack = new Int32Array(stackTable.length); + + for (let stackIndex = 0; stackIndex < stackTable.length; stackIndex++) { + let callNodeFrame = -1; + + // 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 A -> B -> C in the non-inverted tree. + const thisStackCallNode = stackIndexToCallNodeIndex[stackIndex]; + const thisStackSuffixOrderIndex = suffixOrderIndexes[thisStackCallNode]; + + if ( + thisStackSuffixOrderIndex >= rangeStart && + thisStackSuffixOrderIndex < rangeEnd + ) { + const stackForCallNode = getNthPrefixStack( + stackIndex, + depth, + stackTablePrefixCol + ); + if (stackForCallNode !== null) { + callNodeFrame = stackTable.frame[stackForCallNode]; + } + } + + callNodeFramePerStack[stackIndex] = callNodeFrame; + } + return callNodeFramePerStack; +} + /** * Take a samples table, and return an array that contain indexes that point to the * leaf most call node, or null. @@ -3854,75 +4007,18 @@ export function calculateFunctionSizeLowerBound( * functions. */ export function getNativeSymbolsForCallNode( - callNodeIndex: IndexIntoCallNodeTable, - callNodeInfo: CallNodeInfo, - stackTable: StackTable, - frameTable: FrameTable -): IndexIntoNativeSymbolTable[] { - const callNodeInfoInverted = callNodeInfo.asInverted(); - return callNodeInfoInverted !== null - ? getNativeSymbolsForCallNodeInverted( - callNodeIndex, - callNodeInfoInverted, - stackTable, - frameTable - ) - : getNativeSymbolsForCallNodeNonInverted( - callNodeIndex, - callNodeInfo, - stackTable, - frameTable - ); -} - -export function getNativeSymbolsForCallNodeNonInverted( - callNodeIndex: IndexIntoCallNodeTable, - callNodeInfo: CallNodeInfo, - stackTable: StackTable, + callNodeFramePerStack: Int32Array, frameTable: FrameTable ): IndexIntoNativeSymbolTable[] { - const stackIndexToCallNodeIndex = - callNodeInfo.getStackIndexToNonInvertedCallNodeIndex(); const set: Set = new Set(); - for (let stackIndex = 0; stackIndex < stackTable.length; stackIndex++) { - if (stackIndexToCallNodeIndex[stackIndex] === callNodeIndex) { - const frame = stackTable.frame[stackIndex]; - const nativeSymbol = frameTable.nativeSymbol[frame]; - if (nativeSymbol !== null) { - set.add(nativeSymbol); - } - } - } - return [...set]; -} - -export function getNativeSymbolsForCallNodeInverted( - callNodeIndex: IndexIntoCallNodeTable, - callNodeInfo: CallNodeInfoInverted, - stackTable: StackTable, - frameTable: FrameTable -): IndexIntoNativeSymbolTable[] { - const depth = callNodeInfo.depthForNode(callNodeIndex); - const [rangeStart, rangeEnd] = - callNodeInfo.getSuffixOrderIndexRangeForCallNode(callNodeIndex); - const stackTablePrefixCol = stackTable.prefix; - const stackIndexToCallNodeIndex = - callNodeInfo.getStackIndexToNonInvertedCallNodeIndex(); - const suffixOrderIndexes = callNodeInfo.getSuffixOrderIndexes(); - const set: Set = new Set(); - for (let stackIndex = 0; stackIndex < stackTable.length; stackIndex++) { - const stackForNode = getMatchingAncestorStackForInvertedCallNode( - stackIndex, - rangeStart, - rangeEnd, - suffixOrderIndexes, - depth, - stackIndexToCallNodeIndex, - stackTablePrefixCol - ); - if (stackForNode !== null) { - const frame = stackTable.frame[stackForNode]; - const nativeSymbol = frameTable.nativeSymbol[frame]; + for ( + let stackIndex = 0; + stackIndex < callNodeFramePerStack.length; + stackIndex++ + ) { + const callNodeFrame = callNodeFramePerStack[stackIndex]; + if (callNodeFrame !== -1) { + const nativeSymbol = frameTable.nativeSymbol[callNodeFrame]; if (nativeSymbol !== null) { set.add(nativeSymbol); } diff --git a/src/test/unit/profile-data.test.ts b/src/test/unit/profile-data.test.ts index 6b1f3ef34e..96b85e6d94 100644 --- a/src/test/unit/profile-data.test.ts +++ b/src/test/unit/profile-data.test.ts @@ -25,6 +25,7 @@ import { getNativeSymbolsForCallNode, getNativeSymbolInfo, computeTimeColumnForRawSamplesTable, + getCallNodeFramePerStack, } from '../../profile-logic/profile-data'; import { resourceTypes } from '../../profile-logic/data-structures'; import { @@ -1473,21 +1474,22 @@ describe('getNativeSymbolsForCallNode', function () { // Both the call path [funA, funB] and the call path [funA, funB, funC] end // up at a call node with native symbol symB. + const callNodeFramePerStackAB = getCallNodeFramePerStack( + ensureExists(ab), + callNodeInfo, + thread.stackTable + ); expect( - getNativeSymbolsForCallNode( - ensureExists(ab), - callNodeInfo, - thread.stackTable, - thread.frameTable - ) + getNativeSymbolsForCallNode(callNodeFramePerStackAB, thread.frameTable) ).toEqual([symB]); + + const callNodeFramePerStackABC = getCallNodeFramePerStack( + ensureExists(abc), + callNodeInfo, + thread.stackTable + ); expect( - getNativeSymbolsForCallNode( - ensureExists(abc), - callNodeInfo, - thread.stackTable, - thread.frameTable - ) + getNativeSymbolsForCallNode(callNodeFramePerStackABC, thread.frameTable) ).toEqual([symB]); }); @@ -1523,14 +1525,14 @@ describe('getNativeSymbolsForCallNode', function () { // is called by funB and one sample where it's called by funD. The call to // funC was inlined into each of those functions. So the call node has two // native symbols, B and D. + const callNodeFramePerStackC = getCallNodeFramePerStack( + ensureExists(c), + callNodeInfo, + thread.stackTable + ); expect( new Set( - getNativeSymbolsForCallNode( - ensureExists(c), - callNodeInfo, - thread.stackTable, - thread.frameTable - ) + getNativeSymbolsForCallNode(callNodeFramePerStackC, thread.frameTable) ) ).toEqual(new Set([symB, symD])); }); From 6ba8c8cdcbb6c00b8a30df02ac4f3b26ebe907dd Mon Sep 17 00:00:00 2001 From: Markus Stange Date: Mon, 12 Jan 2026 14:00:02 -0500 Subject: [PATCH 3/3] Simplify and speed up the computation of the call-node-specific line timings / address timings with the help of getCallNodeFramePerStack. --- src/profile-logic/address-timings.ts | 366 +++----------------------- src/profile-logic/bottom-box.ts | 52 ++-- src/profile-logic/line-timings.ts | 305 ++------------------- src/test/unit/address-timings.test.ts | 17 +- src/test/unit/line-timings.test.ts | 23 +- 5 files changed, 107 insertions(+), 656 deletions(-) diff --git a/src/profile-logic/address-timings.ts b/src/profile-logic/address-timings.ts index 2c11bd79a5..571758abcd 100644 --- a/src/profile-logic/address-timings.ts +++ b/src/profile-logic/address-timings.ts @@ -72,16 +72,12 @@ import type { FuncTable, StackTable, SamplesLikeTable, - IndexIntoCallNodeTable, IndexIntoNativeSymbolTable, StackAddressInfo, AddressTimings, Address, } from 'firefox-profiler/types'; -import { getMatchingAncestorStackForInvertedCallNode } from './profile-data'; -import type { CallNodeInfo, CallNodeInfoInverted } from './call-node-info'; - /** * For each stack in `stackTable`, and one specific native symbol, compute the * sets of addresses for frames belonging to that native symbol that are hit by @@ -180,312 +176,6 @@ export function getStackAddressInfo( }; } -/** - * Gathers the addresses which are hit by a given call node. - * This is different from `getStackAddressInfo`: `getStackAddressInfo` counts - * address hits anywhere in the stack, and this function only counts hits *in - * the given call node*. - * - * This is useful when opening the assembly view from a call node: We can - * directly jump to the place in the assembly where *this particular call node* - * spends its time. - * - * Returns a StackAddressInfo object for the given stackTable and for the library - * which contains the call node's func. - */ -export function getStackAddressInfoForCallNode( - stackTable: StackTable, - frameTable: FrameTable, - callNodeIndex: IndexIntoCallNodeTable, - callNodeInfo: CallNodeInfo, - nativeSymbol: IndexIntoNativeSymbolTable -): StackAddressInfo { - const callNodeInfoInverted = callNodeInfo.asInverted(); - return callNodeInfoInverted !== null - ? getStackAddressInfoForCallNodeInverted( - stackTable, - frameTable, - callNodeIndex, - callNodeInfoInverted, - nativeSymbol - ) - : getStackAddressInfoForCallNodeNonInverted( - stackTable, - frameTable, - callNodeIndex, - callNodeInfo, - nativeSymbol - ); -} - -/** - * This function handles the non-inverted case of getStackAddressInfoForCallNode. - * - * Gathers the addresses which are hit by a given call node in a given native - * symbol. - * - * This is best explained with an example. We first start with a case that does - * not have any inlining, because this is already complicated enough. - * - * Let the call node be the node for the call path [A, B, C]. - * Let the native symbol be C. - * Let every frame have inlineDepth:0. - * Let there be a native symbol for every func, with the same name as the func. - * Let this be the stack tree: - * - * - stack 1, func A - * - stack 2, func B - * - stack 3, func C, address 0x30 - * - stack 4, func C, address 0x40 - * - stack 5, func B - * - stack 6, func C, address 0x60 - * - stack 7, func C, address 0x70 - * - stack 8, func D - * - stack 9, func E - * - stack 10, func F - * - * This maps to the following call tree: - * - * - call node 1, func A - * - call node 2, func B - * - call node 3, func C - * - call node 4, func D - * - call node 5, func E - * - call node 6, func F - * - * The call path [A, B, C] uniquely identifies call node 3. - * The following stacks all "collapse into" ("map to") call node 3: - * stack 3, 4, 6 and 7. - * Stack 8 maps to call node 4, which is a child of call node 3. - * Stacks 1, 2, 5, 9 and 10 are outside the call path [A, B, C]. - * - * In this function, we only compute "address hits" that are contributed to - * the given call node. - * Stacks 3, 4, 6 and 7 all contribute their time both as "self time" - * and as "total time" to call node 3, at the addresses 0x30, 0x40, 0x60, - * and 0x70, respectively. - * Stack 8 also hits call node 3 at address 0x70, but does not contribute to - * call node 3's "self time", it only contributes to its "total time". - * Stacks 1, 2, 5, 9 and 10 don't contribute to call node 3's self or total time. - * - * Now here's an example *with* inlining. - * - * Let the call node be the node for the call path [A, B, C]. - * Let the native symbol be B. - * Let this be the stack tree: - * - * - stack 1, func A, nativeSymbol A - * - stack 2, func B, nativeSymbol B, address 0x40 - * - stack 3, func C, nativeSymbol B, address 0x40, inlineDepth 1 - * - stack 4, func B, nativeSymbol B, address 0x45 - * - stack 5, func C, nativeSymbol B, address 0x45, inlineDepth 1 - * - stack 6, func D, nativeSymbol D - * - stack 7, func E, nativeSymbol E - * - stack 8, func A, nativeSymbol A, address 0x30 - * - stack 9, func B, nativeSymbol A, address 0x30, inlineDepth 1 - * - stack 10, func C, nativeSymbol A, address 0x30, inlineDepth 2 - * - * This maps to the following call tree: - * - * - call node 1, func A - * - call node 2, func B - * - call node 3, func C - * - call node 4, func D - * - call node 5, func E - * - * The funky part here is that call node 3 has frames from two different native - * symbols: Two from native symbol B, and one from native symbol A. That's - * because B is present both as its own native symbol (separate outer function) - * and as an inlined call from A. In other words, C has been inlined both into - * a standalone B and also into another copy of B which was inlined into A. - * - * This means that, if the user double clicks call node 3, there are two - * different symbols for which we may want to display the assembly code. And - * depending on whether the assembly for A or for B is displayed, we want to - * call this function for a different native symbol. - * - * In this example, we call this function for native symbol B. - * - * The call path [A, B, C] uniquely identifies call node 3. - * The following stacks all "collapse into" ("map to") call node 3: - * stack 3, 5 and 10. However, only stacks 3 and 5 belong to native symbol B; - * stack 10 belongs to native symbol A. - * Stack 6 maps to call node 4, which is a child of call node 3. - * Stacks 1, 2, 4, 7, 8 and 9 are outside the call path [A, B, C]. - * - * Stacks 3 and 5 both contribute their time both as "self time" and as "total - * time" to call node 3 and native symbol B, at the addresses 0x40 and 0x45, - * respectively. Stack 10 has the right call node but the wrong native symbol, - * so it contributes to neither self nor total time. - * Stack 6 also hits call node 3 at address 0x45, but does not contribute to - * call node 3's "self time", it only contributes to its "total time". - * Stacks 1, 2, 4, 7, 8 and 9 don't contribute to call node 3's self or total time. - * - * --- - * - * All stacks can contribute no more than one address in the given call node. - * This is different from the getStackAddressInfo function above, where each - * stack can hit many addresses of the same native symbol, because all of the ancestor - * stacks are taken into account, rather than just one of them. Concretely, - * this means that in the returned StackAddressInfo, each stackAddresses[stack] - * set will only contain at most one element. - * - * The returned StackAddressInfo is computed as follows: - * selfAddress[stack]: - * For stacks that map to the given call node and whose nativeSymbol is the - * given native symbol, this is stack.frame.address. - * For all other stacks this is null. - * stackAddresses[stack]: - * For stacks that map to the given call node or one of its descendant - * call nodes, and whose nativeSymbol is the given native symbol, this is a - * set containing one element, which is ancestorStack.frame.address, where - * ancestorStack maps to the given call node. - * For all other stacks, this is null. - */ -export function getStackAddressInfoForCallNodeNonInverted( - stackTable: StackTable, - frameTable: FrameTable, - callNodeIndex: IndexIntoCallNodeTable, - callNodeInfo: CallNodeInfo, - nativeSymbol: IndexIntoNativeSymbolTable -): StackAddressInfo { - const stackIndexToCallNodeIndex = - callNodeInfo.getStackIndexToNonInvertedCallNodeIndex(); - - // "self address" == "the address which a stack's self time is contributed to" - const callNodeSelfAddressForAllStacks = []; - // "total addresses" == "the set of addresses whose total time this stack contributes to" - // Either null or a single-element set. - const callNodeTotalAddressesForAllStacks: Array | null> = []; - - // This loop takes advantage of the fact that the stack table is topologically ordered: - // Prefix stacks are always visited before their descendants. - for (let stackIndex = 0; stackIndex < stackTable.length; stackIndex++) { - let selfAddress: Address | null = null; - let totalAddresses: Set
| null = null; - const frame = stackTable.frame[stackIndex]; - - if ( - stackIndexToCallNodeIndex[stackIndex] === callNodeIndex && - frameTable.nativeSymbol[frame] === nativeSymbol - ) { - // This stack contributes to the call node's self time for the right - // native symbol. We needed to check both, because multiple stacks for the - // same call node can have different native symbols. - selfAddress = frameTable.address[frame]; - if (selfAddress !== -1) { - totalAddresses = new Set([selfAddress]); - } - } else { - // This stack does not map to the given call node or has the wrong native - // symbol. So this stack contributes no self time to the call node for the - // requested native symbol, and we leave selfAddress at null. - // As for totalTime, this stack contributes to the same address's totalTime - // as its parent stack: If it is a descendant of a stack X which maps to - // the given call node, then it contributes to stack X's address's totalTime, - // otherwise it contributes to no address's totalTime. - // In the example above, this is how stack 8 contributes to call node 3's - // totalTime. - const prefixStack = stackTable.prefix[stackIndex]; - totalAddresses = - prefixStack !== null - ? callNodeTotalAddressesForAllStacks[prefixStack] - : null; - } - - callNodeSelfAddressForAllStacks.push(selfAddress); - callNodeTotalAddressesForAllStacks.push(totalAddresses); - } - return { - selfAddress: callNodeSelfAddressForAllStacks, - stackAddresses: callNodeTotalAddressesForAllStacks, - }; -} - -/** - * This handles the inverted case of getStackAddressInfoForCallNode. - * - * The returned StackAddressInfo is computed as follows: - * selfAddress[stack]: - * For (inverted thread) root stack nodes that map to the given call node - * and whose stack.frame.nativeSymbol is the given symbol, this is stack.frame.address. - * For (inverted thread) root stack nodes whose frame with a different symbol, - * or which don't map to the given call node, this is null. - * For (inverted thread) *non-root* stack nodes, this is the same as the selfAddress - * of the stack's prefix. This way, the selfAddress is always inherited from the - * subtree root. - * stackAddresses[stack]: - * For stacks that map to the given call node or one of its (inverted tree) - * descendant call nodes, this is a set containing one element, which is - * ancestorStack.frame.address, where ancestorStack maps to the given call - * node. - * For all other stacks, this is null. - */ -export function getStackAddressInfoForCallNodeInverted( - stackTable: StackTable, - frameTable: FrameTable, - callNodeIndex: IndexIntoCallNodeTable, - callNodeInfo: CallNodeInfoInverted, - nativeSymbol: IndexIntoNativeSymbolTable -): StackAddressInfo { - const depth = callNodeInfo.depthForNode(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 = []; - // "total addresses" == "the set of addresses whose total time this stack contributes to" - // Either null or a single-element set. - const callNodeTotalAddressesForAllStacks = []; - - for (let stackIndex = 0; stackIndex < stackTable.length; stackIndex++) { - let selfAddress: Address | null = null; - let totalAddresses: Set
| null = null; - - const stackForCallNode = getMatchingAncestorStackForInvertedCallNode( - stackIndex, - rangeStart, - rangeEnd, - suffixOrderIndexes, - depth, - stackIndexToCallNodeIndex, - stackTablePrefixCol - ); - if (stackForCallNode !== null) { - const frameForCallNode = stackTable.frame[stackForCallNode]; - if (frameTable.nativeSymbol[frameForCallNode] === nativeSymbol) { - // This stack contributes to the call node's total time for the right - // native symbol. We needed to check both, because multiple stacks for the - // same call node can have different native symbols. - const address = frameTable.address[frameForCallNode]; - if (address !== -1) { - totalAddresses = new Set([address]); - if (callNodeIsRootOfInvertedTree) { - // This is a root of the inverted tree, and it is the given - // call node. That means that we have a self address. - selfAddress = address; - } else { - // This is not a root stack node, so no self time is spent - // in the given call node for this stack node. - } - } - } - } - - callNodeSelfAddressForAllStacks.push(selfAddress); - callNodeTotalAddressesForAllStacks.push(totalAddresses); - } - return { - selfAddress: callNodeSelfAddressForAllStacks, - stackAddresses: callNodeTotalAddressesForAllStacks, - }; -} - // An AddressTimings instance without any hits. export const emptyAddressTimings: AddressTimings = { totalAddressHits: new Map(), @@ -531,33 +221,49 @@ export function getAddressTimings( return { totalAddressHits, selfAddressHits }; } -// Like getAddressTimings, but computes only the totalAddressHits. -export function getTotalAddressTimings( - stackAddressInfo: StackAddressInfo | null, - samples: SamplesLikeTable +// Returns the addresses which are hit within the specified native +// symbol in a specific call node, along with the total of the +// sample weights per address. +// callNodeFramePerStack needs to be a mapping from stackIndex to the +// corresponding frame in the call node of interest. +export function getTotalAddressTimingsForCallNode( + samples: SamplesLikeTable, + callNodeFramePerStack: Int32Array, + frameTable: FrameTable, + nativeSymbol: IndexIntoNativeSymbolTable | null ): Map { - if (stackAddressInfo === null) { + if (nativeSymbol === null) { return new Map(); } - const { stackAddresses } = stackAddressInfo; - const totalAddressHits: Map = new Map(); - // Iterate over all the samples, and aggregate the sample's weight into the - // addresses which are hit by the sample's stack. - // TODO: Maybe aggregate sample count per stack first, and then visit each stack only once? + const totalPerAddress = new Map(); for (let sampleIndex = 0; sampleIndex < samples.length; sampleIndex++) { - const stackIndex = samples.stack[sampleIndex]; - if (stackIndex === null) { + const stack = samples.stack[sampleIndex]; + if (stack === null) { continue; } - const weight = samples.weight ? samples.weight[sampleIndex] : 1; - const setOfHitAddresses = stackAddresses[stackIndex]; - if (setOfHitAddresses !== null) { - for (const address of setOfHitAddresses) { - const oldHitCount = totalAddressHits.get(address) ?? 0; - totalAddressHits.set(address, oldHitCount + weight); - } + const callNodeFrame = callNodeFramePerStack[stack]; + if (callNodeFrame === -1) { + // This sample does not contribute to the call node's total. Ignore. + continue; + } + + if (frameTable.nativeSymbol[callNodeFrame] !== nativeSymbol) { + continue; + } + + const address = frameTable.address[callNodeFrame]; + if (address === -1) { + continue; } + + const sampleWeight = + samples.weight !== null ? samples.weight[sampleIndex] : 1; + totalPerAddress.set( + address, + (totalPerAddress.get(address) ?? 0) + sampleWeight + ); } - return totalAddressHits; + + return totalPerAddress; } diff --git a/src/profile-logic/bottom-box.ts b/src/profile-logic/bottom-box.ts index 5eb190b7a9..4519ee5cc1 100644 --- a/src/profile-logic/bottom-box.ts +++ b/src/profile-logic/bottom-box.ts @@ -17,15 +17,9 @@ import { getNativeSymbolInfo, getNativeSymbolsForCallNode, } from './profile-data'; -import { - getStackLineInfoForCallNode, - getTotalLineTimings, -} from './line-timings'; import { mapGetKeyWithMaxValue } from 'firefox-profiler/utils'; -import { - getStackAddressInfoForCallNode, - getTotalAddressTimings, -} from './address-timings'; +import { getTotalLineTimingsForCallNode } from './line-timings'; +import { getTotalAddressTimingsForCallNode } from './address-timings'; /** * Calculate the BottomBoxInfo for a call node, i.e. information about which @@ -83,33 +77,25 @@ export function getBottomBoxInfoForCallNode( ) ); - // Compute the hottest line, so we can ask the source view to scroll to it. - const stackLineInfo = getStackLineInfoForCallNode( - stackTable, + // Compute the hottest line and instruction address, so we can ask the + // source and assembly view to scroll those into view. + const funcLine = funcTable.lineNumber[funcIndex]; + const lineTimings = getTotalLineTimingsForCallNode( + samples, + callNodeFramePerStack, frameTable, - funcTable, - callNodeIndex, - callNodeInfo + funcLine ); - const callNodeLineTimings = getTotalLineTimings(stackLineInfo, samples); - const hottestLine = mapGetKeyWithMaxValue(callNodeLineTimings); - - // Compute the hottest instruction, so we can ask the assembly view to scroll to it. - let hottestInstructionAddress; - if (initialNativeSymbol !== null) { - const stackAddressInfo = getStackAddressInfoForCallNode( - stackTable, - frameTable, - callNodeIndex, - callNodeInfo, - nativeSymbolsForCallNode[initialNativeSymbol] - ); - const callNodeAddressTimings = getTotalAddressTimings( - stackAddressInfo, - samples - ); - hottestInstructionAddress = mapGetKeyWithMaxValue(callNodeAddressTimings); - } + const hottestLine = mapGetKeyWithMaxValue(lineTimings); + const addressTimings = getTotalAddressTimingsForCallNode( + samples, + callNodeFramePerStack, + frameTable, + initialNativeSymbol !== null + ? nativeSymbolsForCallNode[initialNativeSymbol] + : null + ); + const hottestInstructionAddress = mapGetKeyWithMaxValue(addressTimings); return { libIndex, diff --git a/src/profile-logic/line-timings.ts b/src/profile-logic/line-timings.ts index 2d264ce720..fc9e09b13c 100644 --- a/src/profile-logic/line-timings.ts +++ b/src/profile-logic/line-timings.ts @@ -7,16 +7,12 @@ import type { FuncTable, StackTable, SamplesLikeTable, - IndexIntoCallNodeTable, StackLineInfo, LineTimings, LineNumber, IndexIntoSourceTable, } from 'firefox-profiler/types'; -import { getMatchingAncestorStackForInvertedCallNode } from './profile-data'; -import type { CallNodeInfo, CallNodeInfoInverted } from './call-node-info'; - /** * For each stack in `stackTable`, and one specific source file, compute the * sets of line numbers in file that are hit by the stack. @@ -110,256 +106,6 @@ export function getStackLineInfo( }; } -/** - * Gathers the line numbers which are hit by a given call node. - * This is different from `getStackLineInfo`: `getStackLineInfo` counts line hits - * anywhere in the stack, and this function only counts hits *in the given call node*. - * - * This is useful when opening a file from a call node: We can directly jump to the - * place in the file where *this particular call node* spends its time. - * - * Returns a StackLineInfo object for the given stackTable and for the source file - * which contains the call node's func. - */ -export function getStackLineInfoForCallNode( - stackTable: StackTable, - frameTable: FrameTable, - funcTable: FuncTable, - callNodeIndex: IndexIntoCallNodeTable, - callNodeInfo: CallNodeInfo -): StackLineInfo { - const callNodeInfoInverted = callNodeInfo.asInverted(); - return callNodeInfoInverted !== null - ? getStackLineInfoForCallNodeInverted( - stackTable, - frameTable, - funcTable, - callNodeIndex, - callNodeInfoInverted - ) - : getStackLineInfoForCallNodeNonInverted( - stackTable, - frameTable, - funcTable, - callNodeIndex, - callNodeInfo - ); -} - -/** - * This function handles the non-inverted case of getStackLineInfoForCallNode. - * - * Gathers the line numbers which are hit by a given call node. - * These line numbers are in the source file that contains that call node's func. - * - * This is best explained with an example. - * Let the call node be the node for the call path [A, B, C]. - * Let this be the stack tree: - * - * - stack 1, func A - * - stack 2, func B - * - stack 3, func C, line 30 - * - stack 4, func C, line 40 - * - stack 5, func B - * - stack 6, func C, line 60 - * - stack 7, func C, line 70 - * - stack 8, func D - * - stack 9, func E - * - stack 10, func F - * - * This maps to the following call tree: - * - * - call node 1, func A - * - call node 2, func B - * - call node 3, func C - * - call node 4, func D - * - call node 5, func E - * - call node 6, func F - * - * The call path [A, B, C] uniquely identifies call node 3. - * The following stacks all "collapse into" ("map to") call node 3: - * stack 3, 4, 6 and 7. - * Stack 8 maps to call node 4, which is a child of call node 3. - * Stacks 1, 2, 5, 9 and 10 are outside the call path [A, B, C]. - * - * In this function, we only compute "line hits" that are contributed to - * the given call node. - * Stacks 3, 4, 6 and 7 all contribute their time both as "self time" - * and as "total time" to call node 3, at the line numbers 30, 40, 60, - * and 70, respectively. - * Stack 8 also hits call node 3 at line 70, but does not contribute to - * call node 3's "self time", it only contributes to its "total time". - * Stacks 1, 2, 5, 9 and 10 don't contribute to call node 3's self or total time. - * - * All stacks can contribute no more than one line in the given call node. - * This is different from the getStackLineInfo function above, where each - * stack can hit many lines in the same file, because all of the ancestor - * stacks are taken into account, rather than just one of them. Concretely, - * this means that in the returned StackLineInfo, each stackLines[stack] - * set will only contain at most one element. - * - * The returned StackLineInfo is computed as follows: - * selfLine[stack]: - * For stacks that map to the given call node, this is stack.frame.line. - * For all other stacks this is null. - * stackLines[stack]: - * For stacks that map to the given call node or one of its descendant - * call nodes, this is a set containing one element, which is - * ancestorStack.frame.line, where ancestorStack maps to the given call - * node. - * For all other stacks, this is null. - */ -export function getStackLineInfoForCallNodeNonInverted( - stackTable: StackTable, - frameTable: FrameTable, - funcTable: FuncTable, - callNodeIndex: IndexIntoCallNodeTable, - callNodeInfo: CallNodeInfo -): StackLineInfo { - const stackIndexToCallNodeIndex = - callNodeInfo.getStackIndexToNonInvertedCallNodeIndex(); - - // "self line" == "the line which a stack's self time is contributed to" - const callNodeSelfLineForAllStacks = []; - // "total lines" == "the set of lines whose total time this stack contributes to" - // Either null or a single-element set. - const callNodeTotalLinesForAllStacks: Array | null> = []; - - // This loop takes advantage of the fact that the stack table is topologically ordered: - // Prefix stacks are always visited before their descendants. - for (let stackIndex = 0; stackIndex < stackTable.length; stackIndex++) { - let selfLine: LineNumber | null = null; - let totalLines: Set | null = null; - - if (stackIndexToCallNodeIndex[stackIndex] === callNodeIndex) { - // This stack contributes to the call node's self time. - // We don't need to check the stack's func or file because it'll be - // the same as the given call node's func and file. - const frame = stackTable.frame[stackIndex]; - selfLine = frameTable.line[frame]; - // Fallback to func line info if frame line info is not available - if (selfLine === null) { - const func = frameTable.func[frame]; - selfLine = funcTable.lineNumber[func]; - } - if (selfLine !== null) { - totalLines = new Set([selfLine]); - } - } else { - // This stack does not map to the given call node. - // So this stack contributes no self time to the call node, and we - // leave selfLine at null. - // As for totalTime, this stack contributes to the same line's totalTime - // as its parent stack: If it is a descendant of a stack X which maps to - // the given call node, then it contributes to stack X's line's totalTime, - // otherwise it contributes to no line's totalTime. - // In the example above, this is how stack 8 contributes to call node 3's - // totalTime. - const prefixStack = stackTable.prefix[stackIndex]; - totalLines = - prefixStack !== null - ? callNodeTotalLinesForAllStacks[prefixStack] - : null; - } - - callNodeSelfLineForAllStacks.push(selfLine); - callNodeTotalLinesForAllStacks.push(totalLines); - } - return { - selfLine: callNodeSelfLineForAllStacks, - stackLines: callNodeTotalLinesForAllStacks, - }; -} - -/** - * This handles the inverted case of getStackLineInfoForCallNode. - * - * The returned StackLineInfo is computed as follows: - * selfLine[stack]: - * For (inverted thread) root stack nodes that map to the given call node - * and whose stack.frame.func.file is the given file, this is stack.frame.line. - * For (inverted thread) root stack nodes whose frame is in a different file, - * or which don't map to the given call node, this is null. - * For (inverted thread) *non-root* stack nodes, this is the same as the selfLine - * of the stack's prefix. This way, the selfLine is always inherited from the - * subtree root. - * stackLines[stack]: - * For stacks that map to the given call node or one of its (inverted tree) - * descendant call nodes, this is a set containing one element, which is - * ancestorStack.frame.line, where ancestorStack maps to the given call - * node. - * For all other stacks, this is null. - */ -export function getStackLineInfoForCallNodeInverted( - stackTable: StackTable, - frameTable: FrameTable, - funcTable: FuncTable, - callNodeIndex: IndexIntoCallNodeTable, - callNodeInfo: CallNodeInfoInverted -): StackLineInfo { - const depth = callNodeInfo.depthForNode(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 = []; - // "total lines" == "the set of lines whose total time this stack contributes to" - // Either null or a single-element set. - const callNodeTotalLinesForAllStacks = []; - - for (let stackIndex = 0; stackIndex < stackTable.length; stackIndex++) { - let selfLine: LineNumber | null = null; - let totalLines: Set | null = null; - - const stackForCallNode = getMatchingAncestorStackForInvertedCallNode( - stackIndex, - rangeStart, - rangeEnd, - suffixOrderIndexes, - depth, - stackIndexToCallNodeIndex, - stackTablePrefixCol - ); - if (stackForCallNode !== null) { - const frameForCallNode = stackTable.frame[stackForCallNode]; - // assert(frameTable.func[frameForCallNode] === suffixPath[0]); - - // This stack contributes to the call node's total time. - // We don't need to check the stack's func or file because it'll be - // the same as the given call node's func and file. - let line = frameTable.line[frameForCallNode]; - // Fallback to func line info if frame line info is not available - if (line === null) { - const func = frameTable.func[frameForCallNode]; - line = funcTable.lineNumber[func]; - } - if (line !== null) { - totalLines = new Set([line]); - if (callNodeIsRootOfInvertedTree) { - // This is a root of the inverted tree, and it is the given - // call node. That means that we have a self address. - selfLine = line; - } else { - // This is not a root stack node, so no self time is spent - // in the given call node for this stack node. - } - } - } - - callNodeSelfLineForAllStacks.push(selfLine); - callNodeTotalLinesForAllStacks.push(totalLines); - } - return { - selfLine: callNodeSelfLineForAllStacks, - stackLines: callNodeTotalLinesForAllStacks, - }; -} - // A LineTimings instance without any hits. export const emptyLineTimings: LineTimings = { totalLineHits: new Map(), @@ -405,33 +151,38 @@ export function getLineTimings( return { totalLineHits, selfLineHits }; } -// Like getLineTimings, but computes only the totalLineHits. -export function getTotalLineTimings( - stackLineInfo: StackLineInfo | null, - samples: SamplesLikeTable +// Returns the line numbers which are hit in a specific call node, +// along with the total of the sample weights per line. +// callNodeFramePerStack needs to be a mapping from stackIndex to the +// corresponding frame in the call node of interest. +export function getTotalLineTimingsForCallNode( + samples: SamplesLikeTable, + callNodeFramePerStack: Int32Array, + frameTable: FrameTable, + funcLine: LineNumber | null ): Map { - if (stackLineInfo === null) { - return new Map(); - } - const { stackLines } = stackLineInfo; - const totalLineHits: Map = new Map(); - - // Iterate over all the samples, and aggregate the sample's weight into the - // lines which are hit by the sample's stack. - // TODO: Maybe aggregate sample count per stack first, and then visit each stack only once? + const totalPerLine = new Map(); for (let sampleIndex = 0; sampleIndex < samples.length; sampleIndex++) { - const stackIndex = samples.stack[sampleIndex]; - if (stackIndex === null) { + const stack = samples.stack[sampleIndex]; + if (stack === null) { continue; } - const weight = samples.weight ? samples.weight[sampleIndex] : 1; - const setOfHitLines = stackLines[stackIndex]; - if (setOfHitLines !== null) { - for (const line of setOfHitLines) { - const oldHitCount = totalLineHits.get(line) ?? 0; - totalLineHits.set(line, oldHitCount + weight); - } + const callNodeFrame = callNodeFramePerStack[stack]; + if (callNodeFrame === -1) { + // This sample does not contribute to the call node's total. Ignore. + continue; + } + + const frameLine = frameTable.line[callNodeFrame]; + const line = frameLine !== null ? frameLine : funcLine; + if (line === null) { + continue; } + + const sampleWeight = + samples.weight !== null ? samples.weight[sampleIndex] : 1; + totalPerLine.set(line, (totalPerLine.get(line) ?? 0) + sampleWeight); } - return totalLineHits; + + return totalPerLine; } diff --git a/src/test/unit/address-timings.test.ts b/src/test/unit/address-timings.test.ts index de3e45c60d..822d8ebac6 100644 --- a/src/test/unit/address-timings.test.ts +++ b/src/test/unit/address-timings.test.ts @@ -5,11 +5,11 @@ import { getProfileFromTextSamples } from '../fixtures/profiles/processed-profile'; import { getStackAddressInfo, - getStackAddressInfoForCallNode, + getTotalAddressTimingsForCallNode, getAddressTimings, - getTotalAddressTimings, } from 'firefox-profiler/profile-logic/address-timings'; import { + getCallNodeFramePerStack, getCallNodeInfo, getInvertedCallNodeInfo, } from '../../profile-logic/profile-data'; @@ -153,7 +153,7 @@ describe('getAddressTimings for getStackAddressInfo', function () { }); }); -describe('getAddressTimings for getStackAddressInfoForCallNode', function () { +describe('getTotalAddressTimingsForCallNode', function () { function getTimings( thread: Thread, callNodePath: CallNodePath, @@ -178,14 +178,17 @@ describe('getAddressTimings for getStackAddressInfoForCallNode', function () { callNodeInfo.getCallNodeIndexFromPath(callNodePath), 'invalid call node path' ); - const stackLineInfo = getStackAddressInfoForCallNode( - stackTable, - frameTable, + const callNodeFramePerStack = getCallNodeFramePerStack( callNodeIndex, callNodeInfo, + stackTable + ); + return getTotalAddressTimingsForCallNode( + samples, + callNodeFramePerStack, + frameTable, nativeSymbol ); - return getTotalAddressTimings(stackLineInfo, samples); } it('passes a basic test', function () { diff --git a/src/test/unit/line-timings.test.ts b/src/test/unit/line-timings.test.ts index 5241f8d16c..188cae794b 100644 --- a/src/test/unit/line-timings.test.ts +++ b/src/test/unit/line-timings.test.ts @@ -5,11 +5,11 @@ import { getProfileFromTextSamples } from '../fixtures/profiles/processed-profile'; import { getStackLineInfo, - getStackLineInfoForCallNode, getLineTimings, - getTotalLineTimings, + getTotalLineTimingsForCallNode, } from 'firefox-profiler/profile-logic/line-timings'; import { + getCallNodeFramePerStack, getCallNodeInfo, getInvertedCallNodeInfo, } from '../../profile-logic/profile-data'; @@ -150,7 +150,7 @@ describe('getLineTimings for getStackLineInfo', function () { }); }); -describe('getLineTimings for getStackLineInfoForCallNode', function () { +describe('getTotalLineTimingsForCallNode', function () { function getTimings( thread: Thread, callNodePath: CallNodePath, @@ -174,14 +174,19 @@ describe('getLineTimings for getStackLineInfoForCallNode', function () { callNodeInfo.getCallNodeIndexFromPath(callNodePath), 'invalid call node path' ); - const stackLineInfo = getStackLineInfoForCallNode( - stackTable, - frameTable, - funcTable, + const callNodeFramePerStack = getCallNodeFramePerStack( callNodeIndex, - callNodeInfo + callNodeInfo, + stackTable + ); + const funcLine = + funcTable.lineNumber[callNodeInfo.funcForNode(callNodeIndex)]; + return getTotalLineTimingsForCallNode( + samples, + callNodeFramePerStack, + frameTable, + funcLine ); - return getTotalLineTimings(stackLineInfo, samples); } it('passes a basic test', function () {