From cef27b1583166359057b31de08e46486cc365f84 Mon Sep 17 00:00:00 2001 From: Markus Stange Date: Thu, 15 Jan 2026 16:03:27 -0500 Subject: [PATCH 1/3] Make getNativeSymbolsForCallNode return a Set. --- src/profile-logic/bottom-box.ts | 8 +++++--- src/profile-logic/profile-data.ts | 6 +++--- src/test/unit/profile-data.test.ts | 8 +++----- 3 files changed, 11 insertions(+), 11 deletions(-) diff --git a/src/profile-logic/bottom-box.ts b/src/profile-logic/bottom-box.ts index 4519ee5cc1..4ed96d56f4 100644 --- a/src/profile-logic/bottom-box.ts +++ b/src/profile-logic/bottom-box.ts @@ -60,14 +60,16 @@ export function getBottomBoxInfoForCallNode( callNodeFramePerStack, frameTable ); + const nativeSymbolsForCallNodeArr = [...nativeSymbolsForCallNode]; // If we have at least one native symbol to show assembly for, pick // the first one arbitrarily. - // TODO: If the we have more than one native symbol, pick the one + // TODO: If we have more than one native symbol, pick the one // with the highest total sample count. - const initialNativeSymbol = nativeSymbolsForCallNode.length !== 0 ? 0 : null; + const initialNativeSymbol = + nativeSymbolsForCallNodeArr.length !== 0 ? 0 : null; - const nativeSymbolInfosForCallNode = nativeSymbolsForCallNode.map( + const nativeSymbolInfosForCallNode = nativeSymbolsForCallNodeArr.map( (nativeSymbolIndex) => getNativeSymbolInfo( nativeSymbolIndex, diff --git a/src/profile-logic/profile-data.ts b/src/profile-logic/profile-data.ts index 6e247fad55..83b252c830 100644 --- a/src/profile-logic/profile-data.ts +++ b/src/profile-logic/profile-data.ts @@ -4032,8 +4032,8 @@ export function calculateFunctionSizeLowerBound( export function getNativeSymbolsForCallNode( callNodeFramePerStack: Int32Array, frameTable: FrameTable -): IndexIntoNativeSymbolTable[] { - const set: Set = new Set(); +): Set { + const set = new Set(); for ( let stackIndex = 0; stackIndex < callNodeFramePerStack.length; @@ -4047,7 +4047,7 @@ export function getNativeSymbolsForCallNode( } } } - return [...set]; + return set; } /** diff --git a/src/test/unit/profile-data.test.ts b/src/test/unit/profile-data.test.ts index 96b85e6d94..9587a75ad6 100644 --- a/src/test/unit/profile-data.test.ts +++ b/src/test/unit/profile-data.test.ts @@ -1481,7 +1481,7 @@ describe('getNativeSymbolsForCallNode', function () { ); expect( getNativeSymbolsForCallNode(callNodeFramePerStackAB, thread.frameTable) - ).toEqual([symB]); + ).toEqual(new Set([symB])); const callNodeFramePerStackABC = getCallNodeFramePerStack( ensureExists(abc), @@ -1490,7 +1490,7 @@ describe('getNativeSymbolsForCallNode', function () { ); expect( getNativeSymbolsForCallNode(callNodeFramePerStackABC, thread.frameTable) - ).toEqual([symB]); + ).toEqual(new Set([symB])); }); it('finds multiple symbols', function () { @@ -1531,9 +1531,7 @@ describe('getNativeSymbolsForCallNode', function () { thread.stackTable ); expect( - new Set( - getNativeSymbolsForCallNode(callNodeFramePerStackC, thread.frameTable) - ) + getNativeSymbolsForCallNode(callNodeFramePerStackC, thread.frameTable) ).toEqual(new Set([symB, symD])); }); }); From 7ba1db4179b9764545aef8c91572479dfc5bc67a Mon Sep 17 00:00:00 2001 From: Markus Stange Date: Thu, 15 Jan 2026 16:03:27 -0500 Subject: [PATCH 2/3] Make initialNativeSymbol an IndexIntoNativeSymbol and go back to the "entry index" only at the end of the function. --- src/profile-logic/bottom-box.ts | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/src/profile-logic/bottom-box.ts b/src/profile-logic/bottom-box.ts index 4ed96d56f4..714bf497ed 100644 --- a/src/profile-logic/bottom-box.ts +++ b/src/profile-logic/bottom-box.ts @@ -66,8 +66,10 @@ export function getBottomBoxInfoForCallNode( // the first one arbitrarily. // TODO: If we have more than one native symbol, pick the one // with the highest total sample count. - const initialNativeSymbol = - nativeSymbolsForCallNodeArr.length !== 0 ? 0 : null; + let initialNativeSymbol = null; + if (nativeSymbolsForCallNodeArr.length !== 0) { + initialNativeSymbol = nativeSymbolsForCallNodeArr[0]; + } const nativeSymbolInfosForCallNode = nativeSymbolsForCallNodeArr.map( (nativeSymbolIndex) => @@ -93,9 +95,7 @@ export function getBottomBoxInfoForCallNode( samples, callNodeFramePerStack, frameTable, - initialNativeSymbol !== null - ? nativeSymbolsForCallNode[initialNativeSymbol] - : null + initialNativeSymbol ); const hottestInstructionAddress = mapGetKeyWithMaxValue(addressTimings); @@ -103,7 +103,10 @@ export function getBottomBoxInfoForCallNode( libIndex, sourceIndex, nativeSymbols: nativeSymbolInfosForCallNode, - initialNativeSymbol, + initialNativeSymbol: + initialNativeSymbol !== null + ? nativeSymbolsForCallNodeArr.indexOf(initialNativeSymbol) + : null, scrollToLineNumber: hottestLine, scrollToInstructionAddress: hottestInstructionAddress, highlightedLineNumber: null, From 0693702dcf107251e03bb2c77175313bac46589a Mon Sep 17 00:00:00 2001 From: Markus Stange Date: Thu, 15 Jan 2026 16:03:27 -0500 Subject: [PATCH 3/3] When double-clicking a node, display the assembly code for the native symbol with the highest sample count. --- src/profile-logic/bottom-box.ts | 30 +++-- src/profile-logic/profile-data.ts | 40 ++++++ src/test/unit/profile-data.test.ts | 187 +++++++++++++++++++++++++++++ 3 files changed, 250 insertions(+), 7 deletions(-) diff --git a/src/profile-logic/bottom-box.ts b/src/profile-logic/bottom-box.ts index 714bf497ed..a803a24978 100644 --- a/src/profile-logic/bottom-box.ts +++ b/src/profile-logic/bottom-box.ts @@ -16,6 +16,7 @@ import { getCallNodeFramePerStack, getNativeSymbolInfo, getNativeSymbolsForCallNode, + getTotalNativeSymbolTimingsForCallNode, } from './profile-data'; import { mapGetKeyWithMaxValue } from 'firefox-profiler/utils'; import { getTotalLineTimingsForCallNode } from './line-timings'; @@ -56,18 +57,33 @@ export function getBottomBoxInfoForCallNode( callNodeInfo, stackTable ); + + // If we have at least one native symbol to show assembly for, pick + // the one with the highest total. But first, create the full list of + // native symbols for this call node, including even those symbols + // that aren't hit by any samples in the current view, so that the + // list is stable regardless of the current preview selection. const nativeSymbolsForCallNode = getNativeSymbolsForCallNode( callNodeFramePerStack, frameTable ); - const nativeSymbolsForCallNodeArr = [...nativeSymbolsForCallNode]; - - // If we have at least one native symbol to show assembly for, pick - // the first one arbitrarily. - // TODO: If we have more than one native symbol, pick the one - // with the highest total sample count. let initialNativeSymbol = null; - if (nativeSymbolsForCallNodeArr.length !== 0) { + const nativeSymbolTimings = getTotalNativeSymbolTimingsForCallNode( + samples, + callNodeFramePerStack, + frameTable + ); + const hottestNativeSymbol = mapGetKeyWithMaxValue(nativeSymbolTimings); + if (hottestNativeSymbol !== undefined) { + nativeSymbolsForCallNode.add(hottestNativeSymbol); + initialNativeSymbol = hottestNativeSymbol; + } + const nativeSymbolsForCallNodeArr = [...nativeSymbolsForCallNode]; + nativeSymbolsForCallNodeArr.sort((a, b) => a - b); + if ( + nativeSymbolsForCallNodeArr.length !== 0 && + initialNativeSymbol === null + ) { initialNativeSymbol = nativeSymbolsForCallNodeArr[0]; } diff --git a/src/profile-logic/profile-data.ts b/src/profile-logic/profile-data.ts index 83b252c830..ceb60a8e11 100644 --- a/src/profile-logic/profile-data.ts +++ b/src/profile-logic/profile-data.ts @@ -4050,6 +4050,46 @@ export function getNativeSymbolsForCallNode( return set; } +/** + * Return the total of the sample weights per native symbol, by + * accumulating the weight from samples which contribute to the + * call node of interest's total time. + * callNodeFramePerStack needs to be a mapping from stackIndex to the + * corresponding frame in the call node of interest. + */ +export function getTotalNativeSymbolTimingsForCallNode( + samples: SamplesLikeTable, + callNodeFramePerStack: Int32Array, + frameTable: FrameTable +): Map { + const totalPerNativeSymbol = new Map(); + for (let sampleIndex = 0; sampleIndex < samples.length; sampleIndex++) { + const stack = samples.stack[sampleIndex]; + if (stack === null) { + continue; + } + const callNodeFrame = callNodeFramePerStack[stack]; + if (callNodeFrame === -1) { + // This sample does not contribute to the call node's total. Ignore. + continue; + } + + const nativeSymbol = frameTable.nativeSymbol[callNodeFrame]; + if (nativeSymbol === null) { + continue; + } + + const sampleWeight = + samples.weight !== null ? samples.weight[sampleIndex] : 1; + totalPerNativeSymbol.set( + nativeSymbol, + (totalPerNativeSymbol.get(nativeSymbol) ?? 0) + sampleWeight + ); + } + + return totalPerNativeSymbol; +} + /** * Convert a native symbol index into a NativeSymbolInfo object, to create * something that's meaningful outside of its associated thread. diff --git a/src/test/unit/profile-data.test.ts b/src/test/unit/profile-data.test.ts index 9587a75ad6..e28ce2de6d 100644 --- a/src/test/unit/profile-data.test.ts +++ b/src/test/unit/profile-data.test.ts @@ -26,6 +26,7 @@ import { getNativeSymbolInfo, computeTimeColumnForRawSamplesTable, getCallNodeFramePerStack, + getTotalNativeSymbolTimingsForCallNode, } from '../../profile-logic/profile-data'; import { resourceTypes } from '../../profile-logic/data-structures'; import { @@ -60,6 +61,8 @@ import type { RawProfileSharedData, IndexIntoFrameTable, IndexIntoSourceTable, + IndexIntoCategoryList, + IndexIntoNativeSymbolTable, } from 'firefox-profiler/types'; describe('string-table', function () { @@ -1536,6 +1539,190 @@ describe('getNativeSymbolsForCallNode', function () { }); }); +describe('getTotalNativeSymbolTimingsForCallNode', function () { + function getTimings( + thread: Thread, + callNodePath: CallNodePath, + defaultCategory: IndexIntoCategoryList, + isInverted: boolean + ): Map { + const { stackTable, frameTable, funcTable, samples } = thread; + const nonInvertedCallNodeInfo = getCallNodeInfo( + stackTable, + frameTable, + defaultCategory + ); + const callNodeInfo = isInverted + ? getInvertedCallNodeInfo( + nonInvertedCallNodeInfo, + defaultCategory, + funcTable.length + ) + : nonInvertedCallNodeInfo; + const callNodeIndex = ensureExists( + callNodeInfo.getCallNodeIndexFromPath(callNodePath), + 'invalid call node path' + ); + const callNodeFramePerStack = getCallNodeFramePerStack( + callNodeIndex, + callNodeInfo, + stackTable + ); + return getTotalNativeSymbolTimingsForCallNode( + samples, + callNodeFramePerStack, + frameTable + ); + } + + it('passes a basic test', function () { + const { + derivedThreads, + funcNamesDictPerThread, + nativeSymbolsDictPerThread, + defaultCategory, + } = getProfileFromTextSamples(` + A[lib:file][sym:Asym:20:] + B[lib:file][sym:Bsym:30:] + `); + const [{ A, B }] = funcNamesDictPerThread; + const [{ Asym, Bsym }] = nativeSymbolsDictPerThread; + const [thread] = derivedThreads; + + // Compute the timings for the root call node. + // One total hit at symbol Asym. + const timingsRoot = getTimings(thread, [A], defaultCategory, false); + expect(timingsRoot.get(Asym)).toBe(1); + expect(timingsRoot.size).toBe(1); // no other hits + + // Compute the timings for the child call node. + // One total hit at symbol Bsym. + const timingsChild = getTimings(thread, [A, B], defaultCategory, false); + expect(timingsChild.get(Bsym)).toBe(1); + expect(timingsChild.size).toBe(1); // no other hits + }); + + it('passes a basic test with recursion', function () { + const { + derivedThreads, + funcNamesDictPerThread, + nativeSymbolsDictPerThread, + defaultCategory, + } = getProfileFromTextSamples(` + A[lib:file][sym:Asym:20:] + B[lib:file][sym:Bsym:30:] + A[lib:file][sym:A2sym:40:] + `); + + const [{ A, B }] = funcNamesDictPerThread; + const [{ Asym, A2sym }] = nativeSymbolsDictPerThread; + const [thread] = derivedThreads; + + // Compute the timings for the root call node. + // One total hit at symbol Asym. + const timingsRoot = getTimings(thread, [A], defaultCategory, false); + expect(timingsRoot.get(Asym)).toBe(1); + expect(timingsRoot.size).toBe(1); // no other hits + + // Compute the timings for the leaf call node. + // One total hit at symbol A2sym. + // In particular, we shouldn't record a hit for symbol Asym, even though + // the frame in Asym is also in A. But it's in the wrong call node. + const timingsChild = getTimings(thread, [A, B, A], defaultCategory, false); + expect(timingsChild.get(A2sym)).toBe(1); + expect(timingsChild.size).toBe(1); // no other hits + }); + + it('passes a test where the same function is called via different call paths', function () { + const { + derivedThreads, + funcNamesDictPerThread, + nativeSymbolsDictPerThread, + defaultCategory, + } = getProfileFromTextSamples(` + A[lib:one][sym:Asym:20:] A[lib:one][sym:Asym:20:] A[lib:one][sym:Asym:20:] + B[lib:one][sym:Bsym:30:] D[lib:one][sym:Dsym:40:] B[lib:one][sym:Bsym:30:] + C[lib:two][sym:Csym:10:] C[lib:two][sym:C2sym:50:] C[lib:two][sym:C3sym:60:] + D[lib:one][sym:Dsym:40:] + `); + + const [{ A, B, C }] = funcNamesDictPerThread; + const [{ Csym, C3sym }] = nativeSymbolsDictPerThread; + const [thread] = derivedThreads; + + const timingsABC = getTimings(thread, [A, B, C], defaultCategory, false); + expect(timingsABC.get(Csym)).toBe(1); + expect(timingsABC.get(C3sym)).toBe(1); + expect(timingsABC.size).toBe(2); // no other hits + }); + + it('passes a test with an inverted thread', function () { + const { + derivedThreads, + funcNamesDictPerThread, + nativeSymbolsDictPerThread, + defaultCategory, + } = getProfileFromTextSamples(` + A[lib:one][sym:Asym:20:] A[lib:one][sym:Asym:20:] A[lib:one][sym:Asym:20:] + B[lib:one][sym:Bsym:30:] D[lib:one][sym:Dsym:40:] B[lib:one][sym:Bsym:30:] + D[lib:one][sym:Dsym:40:] D[lib:one][sym:D2sym:50:] C[lib:two][sym:Csym:10:] + D[lib:one][sym:Dsym:40:] + `); + + const [{ C, D }] = funcNamesDictPerThread; + const [{ Csym, Dsym, D2sym }] = nativeSymbolsDictPerThread; + const [thread] = derivedThreads; + // For the root D of the inverted tree, we have 3 native symbol hits. + const timingsD = getTimings(thread, [D], defaultCategory, true); + expect(timingsD.get(Dsym)).toBe(2); + expect(timingsD.get(D2sym)).toBe(1); + expect(timingsD.size).toBe(2); // no other hits + + // For the C call node which is a child (direct caller) of D, we have + // one hit at symbol Csym. + const timingsDC = getTimings(thread, [D, C], defaultCategory, true); + expect(timingsDC.get(Csym)).toBe(1); + expect(timingsDC.size).toBe(1); // no other hits + }); + + it('passes a test where a function is present in two different native symbols', function () { + // The funky part here is that the targeted call node has frames from two different native + // symbols: Two from native symbol Bsym, and one from native symbol Asym. 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 [A, B, C], there are two + // different symbols for which we may want to display the assembly code, and we + // should compute how much time is spent in each. + const { + derivedThreads, + funcNamesDictPerThread, + nativeSymbolsDictPerThread, + defaultCategory, + } = getProfileFromTextSamples(` + A[lib:one][sym:Asym:20:] A[lib:one][sym:Asym:20:] A[lib:one][sym:Asym:20:] A[lib:one][sym:Asym:20:] + B[lib:one][sym:Bsym:30:] B[lib:one][sym:Asym:20:][inl:1] B[lib:one][sym:Bsym:30:] E[lib:one][sym:Esym:30:] + C[lib:one][sym:Bsym:30:][inl:1] C[lib:one][sym:Asym:20:][inl:2] C[lib:one][sym:Bsym:30:] + D[lib:one][sym:Dsym:40:] + `); + + const [{ A, B, C }] = funcNamesDictPerThread; + const [{ Asym, Bsym }] = nativeSymbolsDictPerThread; + const [thread] = derivedThreads; + + const timingsABCForBsym = getTimings( + thread, + [A, B, C], + defaultCategory, + false + ); + expect(timingsABCForBsym.get(Asym)).toBe(1); + expect(timingsABCForBsym.get(Bsym)).toBe(2); + expect(timingsABCForBsym.size).toBe(2); // no other hits + }); +}); + describe('getNativeSymbolInfo', function () { it('calculates the correct native symbol info', function () { const { profile, nativeSymbolsDictPerThread } = getProfileFromTextSamples(`