From 38013438f96f5025d58e37c388d1d18c65959ca4 Mon Sep 17 00:00:00 2001 From: Markus Stange Date: Fri, 23 Oct 2026 16:53:26 -0400 Subject: [PATCH] Give flame graph siblings with the same function name a defined order. The flame graph ordered siblings by function name only. Siblings with the same name kept the order in which they appear in the call node table, which depends on the stack table. Two similar profiles could therefore show the same functions in opposite orders. Example: in the JS-only flame graph of https://share.firefox.dev/4jy20z8 vs https://share.firefox.dev/4k1tYTW, the call node for callHook in the stack below has two children named "view": view [../../src/components/Controls.js:9:5] callHook [../../node_modules/mithril/render/render.js:34:11] updateComponent [../../node_modules/mithril/render/render.js:462:11] updateNode [../../node_modules/mithril/render/render.js:396:11] updateNodes [../../node_modules/mithril/render/render.js:270:11] updateElement [../../node_modules/mithril/render/render.js:451:11] Both view functions come from the same bundle, so they have the same name and the same file name. The two profiles showed them in opposite orders. Siblings with the same function name are now compared by: 1. The generated file name. 2. The original (source-mapped) file name. 3. The line, then the column, where the function starts. This uses the original location if there is one, and the generated location otherwise. 4. The resource name, for example the library of a native function. 5. The function index, so that two different functions never compare equal. The inverted flame graph uses the same order. It previously ordered siblings with the same name by function index only. The new makeFlameGraphFuncComparator function creates this comparison function. computeFlameGraphRows and getInvertedFlameGraphTiming now take the comparison function instead of the FuncTable and the StringTable. bisectionRightByStrKey is replaced with bisectionRightBy, which takes a comparison function. --- src/profile-logic/flame-graph.ts | 167 ++++++++++++++++++----- src/selectors/per-thread/stack-sample.ts | 17 ++- src/test/store/actions.test.ts | 19 +++ src/test/unit/bisect.test.ts | 44 +++--- src/test/unit/profile-tree.test.ts | 157 ++++++++++++++++++++- src/utils/bisect.ts | 17 ++- 6 files changed, 352 insertions(+), 69 deletions(-) diff --git a/src/profile-logic/flame-graph.ts b/src/profile-logic/flame-graph.ts index 7963d7c286..02623ea3e8 100644 --- a/src/profile-logic/flame-graph.ts +++ b/src/profile-logic/flame-graph.ts @@ -6,7 +6,12 @@ import type { CallNodeTable, FuncTable, IndexIntoCallNodeTable, + IndexIntoFuncTable, + SourceTable, + SourceLocationTable, + ResourceTable, } from 'firefox-profiler/types'; +import { FuncFlag } from 'firefox-profiler/types'; import type { StringTable } from 'firefox-profiler/utils/string-table'; import type { CallTreeTimingsInverted, @@ -14,7 +19,7 @@ import type { } from './call-tree'; import type { CallNodeInfoInverted } from './call-node-info'; -import { bisectionRightByStrKey } from 'firefox-profiler/utils/bisect'; +import { bisectionRightBy } from 'firefox-profiler/utils/bisect'; export type FlameGraphDepth = number; export type IndexIntoFlameGraphTiming = number; @@ -200,7 +205,8 @@ class FlameGraphTimingNonInverted implements FlameGraphTiming { * The call nodes are ordered in the same order that they'll be displayed in the * flame graph: * - * - Siblings are ordered by function name. + * - Siblings are ordered by function name. Siblings with the same function + * name are ordered by the other criteria of makeFlameGraphFuncComparator. * - Siblings are grouped, i.e. all nodes with the same prefix are next to each other. * - The order of these groups with respect to each other is determined by how * their prefix call nodes are ordered in the previous row. @@ -229,20 +235,125 @@ class FlameGraphTimingNonInverted implements FlameGraphTiming { */ export type FlameGraphRows = IndexIntoCallNodeTable[][]; +function compareStrings(a: string, b: string): number { + if (a < b) { + return -1; + } + if (a > b) { + return 1; + } + return 0; +} + +// Compares two functions. Returns a negative number if funcA comes first. +export type FlameGraphFuncComparator = ( + funcA: IndexIntoFuncTable, + funcB: IndexIntoFuncTable +) => number; + +/** + * Returns a comparator for ordering siblings in the flame graph. Functions + * are ordered by name. Functions with the same name are ordered by file name, + * then by original (source-mapped) file name, then by the line and column + * where the function starts, then by resource name. The function index is the + * final tie-breaker, so that two different functions never compare equal. + */ +export function makeFlameGraphFuncComparator( + funcTable: FuncTable, + resourceTable: ResourceTable, + sources: SourceTable, + sourceLocationTable: SourceLocationTable, + stringTable: StringTable +): FlameGraphFuncComparator { + function getFuncName(funcIndex: IndexIntoFuncTable): string { + return stringTable.getString(funcTable.name[funcIndex]); + } + + // Returns the file name of a function, or the empty string if the function + // has no source. + function getFuncFileName(funcIndex: IndexIntoFuncTable): string { + if ((funcTable.flags[funcIndex] & FuncFlag.HasSource) === 0) { + return ''; + } + return stringTable.getString(sources.filename[funcTable.source[funcIndex]]); + } + + function hasOriginalLocation(funcIndex: IndexIntoFuncTable): boolean { + return (funcTable.flags[funcIndex] & FuncFlag.HasOriginalLocation) !== 0; + } + + // Returns the original (source-mapped) file name of a function, or the empty + // string if the function has no original location. + function getFuncOriginalFileName(funcIndex: IndexIntoFuncTable): string { + if (!hasOriginalLocation(funcIndex)) { + return ''; + } + const location = funcTable.originalLocation[funcIndex]; + return stringTable.getString( + sources.filename[sourceLocationTable.source[location]] + ); + } + + // Returns the line where a function starts, preferring the original + // location. Returns 0 if the line is unknown. + function getFuncStartLine(funcIndex: IndexIntoFuncTable): number { + if (hasOriginalLocation(funcIndex)) { + return sourceLocationTable.line[funcTable.originalLocation[funcIndex]]; + } + if ((funcTable.flags[funcIndex] & FuncFlag.HasLine) !== 0) { + return funcTable.lineNumber[funcIndex]; + } + return 0; + } + + // Returns the column where a function starts, preferring the original + // location. Returns 0 if the column is unknown. + function getFuncStartColumn(funcIndex: IndexIntoFuncTable): number { + if (hasOriginalLocation(funcIndex)) { + return sourceLocationTable.column[funcTable.originalLocation[funcIndex]]; + } + if ((funcTable.flags[funcIndex] & FuncFlag.HasColumn) !== 0) { + return funcTable.columnNumber[funcIndex]; + } + return 0; + } + + // Returns the resource name of a function, for example the library name of a + // native function, or the empty string if the function has no resource. + function getFuncResourceName(funcIndex: IndexIntoFuncTable): string { + if ((funcTable.flags[funcIndex] & FuncFlag.HasResource) === 0) { + return ''; + } + return stringTable.getString( + resourceTable.name[funcTable.resource[funcIndex]] + ); + } + + return (funcA, funcB) => + compareStrings(getFuncName(funcA), getFuncName(funcB)) || + compareStrings(getFuncFileName(funcA), getFuncFileName(funcB)) || + compareStrings( + getFuncOriginalFileName(funcA), + getFuncOriginalFileName(funcB) + ) || + getFuncStartLine(funcA) - getFuncStartLine(funcB) || + getFuncStartColumn(funcA) - getFuncStartColumn(funcB) || + compareStrings(getFuncResourceName(funcA), getFuncResourceName(funcB)) || + funcA - funcB; +} + /** * Compute the FlameGraphRows. The result is independent of timing information. */ export function computeFlameGraphRows( callNodeTable: CallNodeTable, - funcTable: FuncTable, - stringTable: StringTable + compareFuncs: FlameGraphFuncComparator ): FlameGraphRows { if (callNodeTable.length === 0) { return [[]]; } const { func, nextSibling, subtreeRangeEnd, maxDepth } = callNodeTable; - const funcTableNameColumn = funcTable.name; // flameGraphRows is what we'll return from this function. // @@ -293,14 +404,12 @@ export function computeFlameGraphRows( rowAtThisDepth.push(currentSibling); } else { // There are other siblings already present in rowAtThisDepth[siblingIndexRangeStart..]. - // Do an ordered insert, to keep siblings ordered by function name. + // Do an ordered insert, to keep siblings ordered by compareFuncs. // assert(siblingIndexRangeStart < siblingIndexRangeEnd) const thisFunc = func[currentSibling]; - const funcName = stringTable.getString(funcTableNameColumn[thisFunc]); - const insertionIndex = bisectionRightByStrKey( + const insertionIndex = bisectionRightBy( rowAtThisDepth, - funcName, - (cn) => stringTable.getString(funcTableNameColumn[func[cn]]), + (cn) => compareFuncs(func[cn], thisFunc), siblingIndexRangeStart, siblingIndexRangeEnd ); @@ -410,8 +519,7 @@ const MIN_INVERTED_FLAME_GRAPH_BOX_WIDTH = 1 / 16384; class FlameGraphTimingInverted implements FlameGraphTiming { _callNodeInfo: CallNodeInfoInverted; _callTreeTimings: CallTreeTimingsInverted; - _funcTable: FuncTable; - _stringTable: StringTable; + _compareFuncs: FlameGraphFuncComparator; _nonRootTimingCache: Map = new Map(); _timingRows: FlameGraphTimingRow[] = []; @@ -421,13 +529,11 @@ class FlameGraphTimingInverted implements FlameGraphTiming { constructor( callNodeInfo: CallNodeInfoInverted, callTreeTimings: CallTreeTimingsInverted, - funcTable: FuncTable, - stringTable: StringTable + compareFuncs: FlameGraphFuncComparator ) { this._callNodeInfo = callNodeInfo; this._callTreeTimings = callTreeTimings; - this._funcTable = funcTable; - this._stringTable = stringTable; + this._compareFuncs = compareFuncs; const { rootTotalSummary } = callTreeTimings; this._rowCount = @@ -466,24 +572,15 @@ class FlameGraphTimingInverted implements FlameGraphTiming { return _getBoxWidth(this, depth, indexInRow); } - _compareCallNodesByFuncName = ( + _compareCallNodesByFunc = ( a: IndexIntoCallNodeTable, b: IndexIntoCallNodeTable ): number => { const callNodeInfo = this._callNodeInfo; - const { name } = this._funcTable; - const stringTable = this._stringTable; - const funcA = callNodeInfo.funcForNode(a); - const funcB = callNodeInfo.funcForNode(b); - const funcNameA = stringTable.getString(name[funcA]); - const funcNameB = stringTable.getString(name[funcB]); - if (funcNameA < funcNameB) { - return -1; - } - if (funcNameA > funcNameB) { - return 1; - } - return funcA - funcB; + return this._compareFuncs( + callNodeInfo.funcForNode(a), + callNodeInfo.funcForNode(b) + ); }; _getNodeTiming(nodeIndex: IndexIntoCallNodeTable): InvertedNodeTiming { @@ -538,7 +635,7 @@ class FlameGraphTimingInverted implements FlameGraphTiming { displayedChildren.push(child); } } - displayedChildren.sort(this._compareCallNodesByFuncName); + displayedChildren.sort(this._compareCallNodesByFunc); return displayedChildren; } @@ -555,7 +652,7 @@ class FlameGraphTimingInverted implements FlameGraphTiming { for (let i = 0; i < sortedRoots.length; i++) { roots.push(sortedRoots[i]); } - roots.sort(this._compareCallNodesByFuncName); + roots.sort(this._compareCallNodesByFunc); const rootRowItems: InvertedFlameGraphRowItem[] = []; let currentRootStart = 0; @@ -632,13 +729,11 @@ class FlameGraphTimingInverted implements FlameGraphTiming { export function getInvertedFlameGraphTiming( callNodeInfo: CallNodeInfoInverted, callTreeTimings: CallTreeTimingsInverted, - funcTable: FuncTable, - stringTable: StringTable + compareFuncs: FlameGraphFuncComparator ): FlameGraphTiming { return new FlameGraphTimingInverted( callNodeInfo, callTreeTimings, - funcTable, - stringTable + compareFuncs ); } diff --git a/src/selectors/per-thread/stack-sample.ts b/src/selectors/per-thread/stack-sample.ts index 63347c19c1..6ae25be4ae 100644 --- a/src/selectors/per-thread/stack-sample.ts +++ b/src/selectors/per-thread/stack-sample.ts @@ -594,10 +594,20 @@ export function getStackAndSampleSelectorsPerThread( > = (state) => _getStackTimingByDepthWithMap(state).sameWidthsIndexToTimestampMap; + const _getFlameGraphFuncComparator: Selector = + createSelector( + (state: State) => threadSelectors.getFilteredThread(state).funcTable, + (state: State) => threadSelectors.getFilteredThread(state).resourceTable, + (state: State) => threadSelectors.getFilteredThread(state).sources, + (state: State) => + threadSelectors.getFilteredThread(state).sourceLocationTable, + (state: State) => threadSelectors.getFilteredThread(state).stringTable, + FlameGraph.makeFlameGraphFuncComparator + ); + const getFlameGraphRows: Selector = createSelector( (state: State) => _getNonInvertedCallNodeInfo(state).getCallNodeTable(), - (state: State) => threadSelectors.getFilteredThread(state).funcTable, - (state: State) => threadSelectors.getFilteredThread(state).stringTable, + _getFlameGraphFuncComparator, FlameGraph.computeFlameGraphRows ); @@ -620,8 +630,7 @@ export function getStackAndSampleSelectorsPerThread( createSelector( _getInvertedCallNodeInfo, getCallTreeTimingsInverted, - (state: State) => threadSelectors.getFilteredThread(state).funcTable, - (state: State) => threadSelectors.getFilteredThread(state).stringTable, + _getFlameGraphFuncComparator, FlameGraph.getInvertedFlameGraphTiming ); diff --git a/src/test/store/actions.test.ts b/src/test/store/actions.test.ts index 95f968d15a..a8653682e5 100644 --- a/src/test/store/actions.test.ts +++ b/src/test/store/actions.test.ts @@ -338,6 +338,25 @@ describe('selectors/getFlameGraphTiming', function () { ]); }); + it('orders inverted siblings with the same function name by file name', function () { + const { + profile, + funcNamesPerThread: [funcNames], + } = getProfileFromTextSamples(` + X Y + B[file:b.js] B[file:a.js] + `); + + const store = storeWithProfile(profile); + store.dispatch(changeInvertCallstack(true)); + + // The root B from a.js comes first, so its caller Y comes first too. + expect(getHumanReadableFlameGraphRanges(store, funcNames)).toEqual([ + 'B (0:0.5) | B (0.5:1)', + 'Y (0:0.5) | X (0.5:1)', + ]); + }); + it('contains totalTime, selfTime and selfRelative', function () { const { profile, diff --git a/src/test/unit/bisect.test.ts b/src/test/unit/bisect.test.ts index 8bdc34cbbb..3e9b38f5b2 100644 --- a/src/test/unit/bisect.test.ts +++ b/src/test/unit/bisect.test.ts @@ -6,7 +6,7 @@ import { bisectionLeft, bisectionRight, bisectionRightByKey, - bisectionRightByStrKey, + bisectionRightBy, } from '../../utils/bisect'; describe('bisectionRight', function () { @@ -114,7 +114,7 @@ describe('bisectionRightByKey', function () { }); }); -describe('bisectionRightByStrKey', function () { +describe('bisectionRightBy', function () { const array = [ { name: 'a0' }, { name: 'a1' }, @@ -131,51 +131,59 @@ describe('bisectionRightByStrKey', function () { { name: 'a9' }, ]; - function getName(x: { name: string }): string { - return x.name; + function compareNameTo(x: string): (elem: { name: string }) => number { + return (elem) => { + if (elem.name < x) { + return -1; + } + if (elem.name > x) { + return 1; + } + return 0; + }; } // The new element to be inserted into this array is referred to as x it('returns index of the first element whose key is greater than x', function () { - expect(bisectionRightByStrKey(array, 'a4', getName)).toBe(8); - expect(bisectionRightByStrKey(array, 'a3', getName)).toBe(8); + expect(bisectionRightBy(array, compareNameTo('a4'))).toBe(8); + expect(bisectionRightBy(array, compareNameTo('a3'))).toBe(8); }); it('returns index of the first element whose key is greater than x, occuring after low', function () { - expect(bisectionRightByStrKey(array, 'a4', getName, 8)).toBe(8); - expect(bisectionRightByStrKey(array, 'a3', getName, 9)).toBe(9); + expect(bisectionRightBy(array, compareNameTo('a4'), 8)).toBe(8); + expect(bisectionRightBy(array, compareNameTo('a3'), 9)).toBe(9); }); it('returns index of the first element whose key is greater than x, between low and high', function () { - expect(bisectionRightByStrKey(array, 'a4', getName, 1, 7)).toBe(7); - expect(bisectionRightByStrKey(array, 'a3', getName, 4, 6)).toBe(6); + expect(bisectionRightBy(array, compareNameTo('a4'), 1, 7)).toBe(7); + expect(bisectionRightBy(array, compareNameTo('a3'), 4, 6)).toBe(6); }); it('returns 0 if all the keys of elements are greater than x', function () { - expect(bisectionRightByStrKey(array, '_', getName)).toBe(0); + expect(bisectionRightBy(array, compareNameTo('_'))).toBe(0); }); it('returns array length if x is greater than keys of all elements', function () { - expect(bisectionRightByStrKey(array, 'b', getName)).toBe(13); + expect(bisectionRightBy(array, compareNameTo('b'))).toBe(13); }); it('throws TypeError if either low or high are outside the range of the array', function () { - expect(() => bisectionRightByStrKey(array, 'b', getName, -2)).toThrow( + expect(() => bisectionRightBy(array, compareNameTo('b'), -2)).toThrow( TypeError ); - expect(() => bisectionRightByStrKey(array, 'b', getName, 100)).toThrow( + expect(() => bisectionRightBy(array, compareNameTo('b'), 100)).toThrow( TypeError ); - expect(() => bisectionRightByStrKey(array, 'b', getName, 2, -10)).toThrow( + expect(() => bisectionRightBy(array, compareNameTo('b'), 2, -10)).toThrow( TypeError ); - expect(() => bisectionRightByStrKey(array, 'b', getName, 2, 100)).toThrow( + expect(() => bisectionRightBy(array, compareNameTo('b'), 2, 100)).toThrow( TypeError ); - expect(() => bisectionRightByStrKey(array, 'b', getName, -20, -10)).toThrow( + expect(() => bisectionRightBy(array, compareNameTo('b'), -20, -10)).toThrow( TypeError ); - expect(() => bisectionRightByStrKey(array, 'b', getName, 100, 200)).toThrow( + expect(() => bisectionRightBy(array, compareNameTo('b'), 100, 200)).toThrow( TypeError ); }); diff --git a/src/test/unit/profile-tree.test.ts b/src/test/unit/profile-tree.test.ts index d535aadd59..a1ebad4d39 100644 --- a/src/test/unit/profile-tree.test.ts +++ b/src/test/unit/profile-tree.test.ts @@ -11,7 +11,10 @@ import { computeCallNodeSelfAndSummary, computeCallTreeTimings, } from '../../profile-logic/call-tree'; -import { computeFlameGraphRows } from '../../profile-logic/flame-graph'; +import { + computeFlameGraphRows, + makeFlameGraphFuncComparator, +} from '../../profile-logic/flame-graph'; import { computeFrameTableFromRawFrameTable, computeFuncTableFromRawFuncTable, @@ -31,7 +34,17 @@ import { addSourceToTable, } from '../fixtures/utils'; import { ensureExists } from 'firefox-profiler/utils/types'; -import type { CallNodePath } from 'firefox-profiler/types'; +import type { CallNodePath, Thread } from 'firefox-profiler/types'; + +function makeFuncComparator(thread: Thread) { + return makeFlameGraphFuncComparator( + thread.funcTable, + thread.resourceTable, + thread.sources, + thread.sourceLocationTable, + thread.stringTable + ); +} describe('unfiltered call tree', function () { // These values are hoisted at the top for the ease of access. In the profile fixture @@ -128,8 +141,7 @@ describe('unfiltered call tree', function () { const rows = computeFlameGraphRows( callNodeInfo.getCallNodeTable(), - thread.funcTable, - thread.stringTable + makeFuncComparator(thread) ); expect(rows).toEqual([ // Siblings are ordered in lexically ascending order. @@ -138,6 +150,143 @@ describe('unfiltered call tree', function () { [cnGHI, cnGHJ, cnZXW, cnZXY], ]); }); + + it('orders siblings with the same function name by file name', function () { + const { derivedThreads, defaultCategory } = getProfileFromTextSamples(` + A[file:b.js] A[file:a.js] A + B[file:b.js] B B[file:a.js] + `); + const [thread] = derivedThreads; + const { funcTable, sources, stringTable } = thread; + const callNodeInfo = getCallNodeInfo( + thread.stackTable, + thread.frameTable, + defaultCategory + ); + const callNodeTable = callNodeInfo.getCallNodeTable(); + const rows = computeFlameGraphRows( + callNodeTable, + makeFuncComparator(thread) + ); + + // Describe each call node as "funcName fileName". + const describe = (callNodeIndex: number) => { + const funcIndex = callNodeTable.func[callNodeIndex]; + const name = stringTable.getString(funcTable.name[funcIndex]); + const fileName = + (funcTable.flags[funcIndex] & FuncFlag.HasSource) !== 0 + ? stringTable.getString( + sources.filename[funcTable.source[funcIndex]] + ) + : '(no file)'; + return `${name} ${fileName}`; + }; + expect(rows.map((row) => row.map(describe))).toEqual([ + // Functions without a file come first. + ['A (no file)', 'A a.js', 'A b.js'], + // Children are grouped by their parent, in the parent's order. + ['B a.js', 'B (no file)', 'B b.js'], + ]); + }); + + it('orders siblings with the same function name by resource name', function () { + const { derivedThreads, defaultCategory } = getProfileFromTextSamples(` + A[lib:libb.so] A[lib:liba.so] A + `); + const [thread] = derivedThreads; + const callNodeInfo = getCallNodeInfo( + thread.stackTable, + thread.frameTable, + defaultCategory + ); + const rows = computeFlameGraphRows( + callNodeInfo.getCallNodeTable(), + makeFuncComparator(thread) + ); + const { resourceTable, funcTable, stringTable } = thread; + const describe = (callNodeIndex: number) => { + const funcIndex = callNodeInfo.funcForNode(callNodeIndex); + return (funcTable.flags[funcIndex] & FuncFlag.HasResource) !== 0 + ? stringTable.getString( + resourceTable.name[funcTable.resource[funcIndex]] + ) + : '(no resource)'; + }; + expect(rows.map((row) => row.map(describe))).toEqual([ + ['(no resource)', 'liba.so', 'libb.so'], + ]); + }); + + it('orders siblings from the same file by their original location', function () { + // The fixture merges functions with the same name and file, so create + // them with different names and rename them afterwards. + const { + derivedThreads, + funcNamesDictPerThread: [{ A, view1, view2, view3, view4 }], + defaultCategory, + } = getProfileFromTextSamples(` + A A A A + view1[file:bundle.js] view2[file:bundle.js] view3[file:bundle.js] view4[file:bundle.js] + `); + const [thread] = derivedThreads; + const { funcTable, sources, sourceLocationTable, stringTable } = thread; + const viewNameIndex = stringTable.indexForString('view'); + for (const funcIndex of [view1, view2, view3, view4]) { + funcTable.name[funcIndex] = viewNameIndex; + } + + function addSource(fileName: string): number { + sources.id.push(null); + sources.filename.push(stringTable.indexForString(fileName)); + sources.startLine.push(1); + sources.startColumn.push(1); + sources.sourceMapURL.push(null); + sources.content.push(null); + return sources.length++; + } + function setOriginalLocation( + funcIndex: number, + source: number, + line: number, + column: number + ) { + sourceLocationTable.source.push(source); + sourceLocationTable.line.push(line); + sourceLocationTable.column.push(column); + funcTable.originalLocation[funcIndex] = sourceLocationTable.length++; + funcTable.flags[funcIndex] |= FuncFlag.HasOriginalLocation; + } + const controls = addSource('Controls.js'); + const todoItem = addSource('TodoItem.js'); + setOriginalLocation(view1, todoItem, 3, 1); + setOriginalLocation(view2, controls, 20, 5); + setOriginalLocation(view3, controls, 9, 7); + setOriginalLocation(view4, controls, 9, 5); + + const callNodeInfo = getCallNodeInfo( + thread.stackTable, + thread.frameTable, + defaultCategory + ); + const callNodeTable = callNodeInfo.getCallNodeTable(); + const rows = computeFlameGraphRows( + callNodeTable, + makeFuncComparator(thread) + ); + const cnA = callNodeInfo.getCallNodeIndexFromPath([A]); + const cnForView = (funcIndex: number) => + callNodeInfo.getCallNodeIndexFromPath([A, funcIndex]); + expect(rows).toEqual([ + [cnA], + // Ordered by original file name, then line, then column. + [ + cnForView(view4), + cnForView(view3), + cnForView(view2), + cnForView(view1), + ], + ]); + }); }); /** diff --git a/src/utils/bisect.ts b/src/utils/bisect.ts index a2a1168af8..218d4b46ca 100644 --- a/src/utils/bisect.ts +++ b/src/utils/bisect.ts @@ -157,14 +157,17 @@ export function bisectionRightByKey( } /** - * This is the same as bisectionRightByKey but uses string as the key type. - * If you can find a way to make Flow accept a single function that handles - * both string and number keys, please remove this duplication. + * This is the same as bisectionRightByKey but uses a comparison function. + * cmp(elem) compares an element of the array to the looked-up value, which is + * usually captured by the closure: + * - rv > 0 if elem > value, + * - rv === 0 if elem == value, + * - rv < 0 if elem < value, + * The returned index is the index of the first element for which rv > 0. */ -export function bisectionRightByStrKey( +export function bisectionRightBy( array: T[], - x: string, - toKey: (arg: T) => string, + cmp: (elem: T) => number, low: number = 0, high: number = array.length ): number { @@ -175,7 +178,7 @@ export function bisectionRightByStrKey( while (low < high) { const mid = (low + high) >> 1; - if (x < toKey(array[mid])) { + if (cmp(array[mid]) > 0) { high = mid; } else { low = mid + 1;