From 0b537cb60a5c49337fbdaaab3c8014a9a46a588b Mon Sep 17 00:00:00 2001 From: Markus Stange Date: Tue, 28 Nov 2023 13:47:11 -0500 Subject: [PATCH 1/2] Work around Math.abs being slow in Firefox in React development environments. --- src/profile-logic/call-tree.js | 5 ++++- src/profile-logic/flame-graph.js | 7 +++++-- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/src/profile-logic/call-tree.js b/src/profile-logic/call-tree.js index 46eca35e26..9f2f7876dc 100644 --- a/src/profile-logic/call-tree.js +++ b/src/profile-logic/call-tree.js @@ -529,6 +529,9 @@ export function computeCallTreeCountsAndSummary( let rootTotalSummary = 0; let rootCount = 0; + // Workaround for https://bugzilla.mozilla.org/show_bug.cgi?id=1858310 + const abs = Math.abs; + // We loop the call node table in reverse, so that we find the children // before their parents, and the total is known at the time we reach a // node. @@ -538,7 +541,7 @@ export function computeCallTreeCountsAndSummary( callNodeIndex-- ) { callNodeTotalSummary[callNodeIndex] += callNodeLeaf[callNodeIndex]; - rootTotalSummary += Math.abs(callNodeLeaf[callNodeIndex]); + rootTotalSummary += abs(callNodeLeaf[callNodeIndex]); const hasChildren = callNodeChildCount[callNodeIndex] !== 0; const hasTotalValue = callNodeTotalSummary[callNodeIndex] !== 0; diff --git a/src/profile-logic/flame-graph.js b/src/profile-logic/flame-graph.js index f894a3b652..d983071bd0 100644 --- a/src/profile-logic/flame-graph.js +++ b/src/profile-logic/flame-graph.js @@ -211,6 +211,9 @@ export function getFlameGraphTiming( // Keep track of time offset by depth level. const timeOffset = [0.0]; + // Workaround for https://bugzilla.mozilla.org/show_bug.cgi?id=1858310 + const abs = Math.abs; + while (stack.length) { const { depth, nodeIndex } = stack.pop(); @@ -228,10 +231,10 @@ export function getFlameGraphTiming( } // Take the absolute value, as native deallocations can be negative. - const totalRelative = Math.abs( + const totalRelative = abs( callNodeSummary.total[nodeIndex] / rootTotalSummary ); - const selfRelative = Math.abs( + const selfRelative = abs( callNodeSummary.self[nodeIndex] / rootTotalSummary ); From 30ae592ac8e52d4d72dd11af45df603bdefef01d Mon Sep 17 00:00:00 2001 From: Markus Stange Date: Tue, 10 Oct 2023 16:45:55 -0400 Subject: [PATCH 2/2] Improve flame graph performance by caching rows. Before: https://share.firefox.dev/47ydcmp After: https://share.firefox.dev/40Y48og Fixes #844. The old code was saving time by sorting siblings only for the nodes that were displayed based on the current (preview) range selection. This made it cheaper to compute the flame graph for a small range, but it meant that we had to re-do the sort every time the selection changed. Now we do the sorting once, based on the entire call node table. This is expensive but it is a one-time cost (and it's still cheaper than if you were looking at the entire call tree with the old implementation). Then we cache the "ordered rows" and don't have to sort again on each range selection change. --- src/profile-logic/flame-graph.js | 406 ++++++++++++----------- src/selectors/per-thread/stack-sample.js | 11 +- src/test/unit/bisect.test.js | 142 +++++++- src/test/unit/profile-tree.test.js | 65 ++-- src/utils/bisect.js | 135 +++++++- 5 files changed, 526 insertions(+), 233 deletions(-) diff --git a/src/profile-logic/flame-graph.js b/src/profile-logic/flame-graph.js index d983071bd0..ff7ebe1b6b 100644 --- a/src/profile-logic/flame-graph.js +++ b/src/profile-logic/flame-graph.js @@ -5,13 +5,15 @@ // @flow import type { UnitIntervalOfProfileRange, - CallNodeInfo, CallNodeTable, + FuncTable, IndexIntoCallNodeTable, - Thread, } from 'firefox-profiler/types'; +import type { UniqueStringArray } from 'firefox-profiler/utils/unique-string-array'; import type { CallTreeCountsAndSummary } from './call-tree'; +import { bisectionRightByStrKey } from 'firefox-profiler/utils/bisect'; + export type FlameGraphDepth = number; export type IndexIntoFlameGraphTiming = number; @@ -38,229 +40,257 @@ export type FlameGraphTiming = Array<{ length: number, }>; -type RootsAndChildren = { - /** - * Conceptually, `children` is a collection of arrays, one for each - * callnode in the tree. Each array contains all immediate callnode - * children (with a non-zero total time) of a given callnode. - * - * To avoid heavy allocations for large call trees, the elements of - * this collection are not real array instances. Instead, one has to - * work with slices of one large array. - * - * Given a callnode `p` with callnode index `pi`, and `start` and - * `end` defined as: - * let start = children.offsets[pi]; - * let end = children.offsets[pi + 1]; - * - * then children.array.slice(start, end) is a sorted list of all - * callnode indices whose callnodes have `p` as their direct parent. - * The list is sorted in descending order with respect to the - * function names of the callnodes. - */ - children: { - // Array of IndexIntoCallNodeTable. This is a concatenation of all - // children sub-arrays. - array: Uint32Array, - // This array maps a given IndexIntoCallNodeTable to a slice - // within `array` by providing start and end indices. - offsets: Uint32Array, - }, - - // A list of every root CallNodeIndex in the call tree. - roots: IndexIntoCallNodeTable[], -}; - /** - * Obtain collections of callnode indices needed for building the - * flame graph. + * FlameGraphRows is an array of rows, where each row is an array of call node + * indexes. This is a timing-invariant representation of the flame graph and can + * be cached independently of the sample / timing information. + * + * When combined with the timing information, it is used to produce FlameGraphTiming. + * + * In FlameGraphRows, rows[depth] contains all the call nodes of that depth. + * 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 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. + * + * # Example ([call node index] [function name]) + * + * ``` + * - 0 A + * - 1 D + * - 2 I + * - 3 B + * - 4 C + * - 5 F + * - 6 E + * - 7 G + * - 8 H + * ``` + * + * The call node table above produces the following FlameGraphRows: + * Depth 0: [0, 8] // ["A", "H"] + * Depth 1: [3, 1, 7] // ["B", "D", "G"] + * Depth 2: [4, 6, 5, 2] // ["C", "E", "F", "I"] * - * The returned object contains two arrays, one for the roots and one - * for all children of the call tree. Along with the children array is - * an offset array used to index into it. + * Note that [4, 6, 5] are the children of 3 ("B"), sorted by name, and these + * children have been moved before 2 ("I") to match the order of their parents. */ -export function getRootsAndChildren( - thread: Thread, +export type FlameGraphRows = IndexIntoCallNodeTable[][]; + +/** + * Compute the FlameGraphRows. The result is independent of timing information. + */ +export function computeFlameGraphRows( callNodeTable: CallNodeTable, - callNodeChildCount: Uint32Array, - totalTime: Float32Array -): RootsAndChildren { - const roots = []; - const array = new Uint32Array(callNodeTable.length); - const offsets = new Uint32Array(callNodeTable.length + 1); + funcTable: FuncTable, + stringTable: UniqueStringArray +): FlameGraphRows { + const callNodeCount = callNodeTable.length; + if (callNodeCount === 0) { + return [[]]; + } - /* For performance reasons the array is of type Uint32Array. This - * means we cannot use values such as `undefined` or `null` to - * indicate uninitialized values, as we build up the array. But - * since `callNodeTable` is ordered is such a way that a given - * callnode index always comes _after_ its parent callnode index, we - * know that callnode index zero never can be a child. It is always - * a root. (Not counting the special -1 root, but we don't need it - * here). Hence, we are free to use the value 0 in the children - * array to mark elements as not initialized, since 0 is never a - * valid child. Since the default values of Uint32Array is 0, we - * conveniently get an array where all its values are uninitialized - * from start. */ + const { func, nextSibling, subtreeRangeEnd } = callNodeTable; + const funcTableNameColumn = funcTable.name; - let callNodeIndex = 0; - let ptr = 0; - for (; callNodeIndex < callNodeTable.length; callNodeIndex++) { - offsets[callNodeIndex] = ptr; - ptr += callNodeChildCount[callNodeIndex]; + // flameGraphRows is what we'll return from this function. We add a row to + // flameGraphRows any time we descend to a call node that's deeper than + // flameGraphRows.length - 1. + // + // Each row is conceptually partitioned into two parts: "Finished nodes" and + // "pending nodes". + // + // For row d, flameGraphRows[d] is partitioned as follows (a..b is the half-open + // range which includes a but excludes b): + // + // - flameGraphRows[d][0..pendingRangeStartAtDepth[d]] are "finished" + // - flameGraphRows[d][pendingRangeStartAtDepth[d]..] are "pending" + // + // A node starts out as "pending" when we initially add it to the row. + // A node becomes "finished" once we've decided to process its children. + // + // This is used to queue up a bunch of siblings before we process their + // children. + // We need to queue up nodes before we can process their children because + // we can only process children once their parents are in the right order. + const flameGraphRows = [[]]; + const pendingRangeStartAtDepth = [0]; - if (totalTime[callNodeIndex] === 0) { - continue; - } + // At the beginning of each turn of this loop, add currentCallNode and all its + // siblings as "pending" to row[currentDepth], ordered by name. Then find the + // first pending call node with children, and go to the next iteration. + let currentCallNode = 0; + let currentDepth = 0; // always set to depth[currentCallNode] + outer: while (true) { + // assert(depth[currentCallNode] === currentDepth); - const parent = callNodeTable.prefix[callNodeIndex]; - if (parent === -1) { - roots.push(callNodeIndex); - continue; + // Add currentCallNode and all its siblings to the current row. Ensure correct + // ordering when inserting each sibling. + const rowAtThisDepth = flameGraphRows[currentDepth]; + const siblingIndexRangeStart = rowAtThisDepth.length; // index into rowAtThisDepth + for ( + let currentSibling = currentCallNode; + currentSibling !== -1; + currentSibling = nextSibling[currentSibling] + ) { + const siblingIndexRangeEnd = rowAtThisDepth.length; + if (siblingIndexRangeStart === siblingIndexRangeEnd) { + // This is the first sibling that we see. We don't need to compute an + // insertion index because we don't have any other siblings to compare + // to yet. + rowAtThisDepth.push(currentSibling); + } else { + // There are other siblings already present in rowAtThisDepth[siblingIndexRangeStart..]. + // Do an ordered insert, to keep siblings ordered by function name. + // assert(siblingIndexRangeStart < siblingIndexRangeEnd) + const thisFunc = func[currentSibling]; + const funcName = stringTable.getString(funcTableNameColumn[thisFunc]); + const insertionIndex = bisectionRightByStrKey( + rowAtThisDepth, + funcName, + (cn) => stringTable.getString(funcTableNameColumn[func[cn]]), + siblingIndexRangeStart, + siblingIndexRangeEnd + ); + rowAtThisDepth.splice(insertionIndex, 0, currentSibling); + } } - const funcName = thread.stringTable.getString( - thread.funcTable.name[callNodeTable.func[callNodeIndex]] - ); + // Now currentCallNode and all its siblings have been added to the row, and + // they are ordered correctly. Find a queued up node in this row which has + // children, and descend into it. + let candidateDepth = currentDepth; + let candidateRow = rowAtThisDepth; + let indexInCandidateRow = pendingRangeStartAtDepth[candidateDepth]; + let candidateNode = candidateRow[indexInCandidateRow]; + // "while (!hasChildren(candidateNode)) {" + while (subtreeRangeEnd[candidateNode] === candidateNode + 1) { + // candidateNode does not have any children. + // "Finish" candidateNode by incrementing pendingRangeStartAtDepth[candidateDepth]. + indexInCandidateRow++; + pendingRangeStartAtDepth[candidateDepth] = indexInCandidateRow; - /* From the parent, we can now know the slice allotted for all - * its children. */ - const start = offsets[parent]; - const end = offsets[parent] + callNodeChildCount[parent] - 1; - - /* Find the place in `array` where this callnode should be - * inserted, swapping elements in the array as we go - * along. Continue as long as this callnode's function name is - * lexically smaller than the function names of the callnodes - * already placed in the array. This ensures that all slices have - * children in descending order. Any callnode indices equal to 0 - * means that they are uninitialized, so just breeze through - * them. When we stop, when have found the right position to - * insert our callnode. - * - * This effectively is an insertion sort, which is O(n^2), but - * since n is typically small (the number of children of a given - * callnode), it should be just fine. - */ - let i = start; - while (i < end) { - if ( - array[i + 1] !== 0 && - funcName > - thread.stringTable.getString( - thread.funcTable.name[callNodeTable.func[array[i + 1]]] - ) - ) { - // We've found our spot if the next slot in the array is - // occupied with a callnode whose function name is less than - // ours. - break; + // Check if the current row contains any other pending call nodes. + while (indexInCandidateRow === candidateRow.length) { + // There are no more pending nodes in the current row. + if (candidateDepth === 0) { + // We're completely done. + break outer; + } + // Go up a level and continue the search there. + candidateDepth--; + candidateRow = flameGraphRows[candidateDepth]; + indexInCandidateRow = pendingRangeStartAtDepth[candidateDepth]; } - array[i] = array[i + 1]; - i++; + // We have found a pending call node. + candidateNode = candidateRow[indexInCandidateRow]; } - array[i] = callNodeIndex; - } - offsets[callNodeIndex] = ptr; - // The children are already sorted, but the roots aren't. - // Let's sort the roots in descending order, just like the children. - roots.sort((rootA, rootB) => { - const [nameA, nameB] = [rootA, rootB].map((callNodeIndex) => - thread.stringTable.getString( - thread.funcTable.name[callNodeTable.func[callNodeIndex]] - ) - ); - if (nameA < nameB) { - return 1; - } - if (nameA === nameB) { - return 0; + // Now candidateNode is a pending node which has at least one child. + // "Finish" candidateNode by incrementing pendingRangeStartAtDepth[candidateDepth]. + pendingRangeStartAtDepth[candidateDepth] = indexInCandidateRow + 1; + + // Advance to candidateNode's first child. Due to the way call nodes are ordered, + // the first child of x (if present) is always at x + 1. + currentCallNode = candidateNode + 1; // "currentCallNode = firstChild[candidateNode];" + currentDepth = candidateDepth + 1; + + // Make sure flameGraphRows and pendingRangeStartAtDepth are initialized for + // the new currentDepth. + if (currentDepth === flameGraphRows.length) { + flameGraphRows[currentDepth] = []; + pendingRangeStartAtDepth[currentDepth] = 0; } - return -1; - }); - return { roots, children: { array, offsets } }; + } + + return flameGraphRows; } /** * Build a FlameGraphTiming table from a call tree. */ export function getFlameGraphTiming( - thread: Thread, - callNodeInfo: CallNodeInfo, + flameGraphRows: FlameGraphRows, + callNodeTable: CallNodeTable, callTreeCountsAndSummary: CallTreeCountsAndSummary ): FlameGraphTiming { - const { callNodeChildCount, callNodeSummary, rootTotalSummary } = - callTreeCountsAndSummary; + const { callNodeSummary, rootTotalSummary } = callTreeCountsAndSummary; + const { total, self } = callNodeSummary; + const { prefix } = callNodeTable; - const { roots, children } = getRootsAndChildren( - thread, - callNodeInfo.callNodeTable, - callNodeChildCount, - callNodeSummary.total - ); + // This is where we build up the return value, one row at a time. const timing = []; - // Array of call nodes to recursively process in the loop below. - // Start with the roots of the call tree. - const stack: Array<{ - depth: number, - nodeIndex: IndexIntoCallNodeTable, - }> = roots.map((nodeIndex) => ({ nodeIndex, depth: 0 })); - - // Keep track of time offset by depth level. - const timeOffset = [0.0]; + // This is used to adjust the start position of a call node's box based on the + // start position of its prefix node's box. + const startPerCallNode = new Float32Array(callNodeTable.length); // Workaround for https://bugzilla.mozilla.org/show_bug.cgi?id=1858310 const abs = Math.abs; - while (stack.length) { - const { depth, nodeIndex } = stack.pop(); + // Go row by row. + for (let depth = 0; depth < flameGraphRows.length; depth++) { + const rowNodes = flameGraphRows[depth]; - // Select an existing row, or create a new one. - let row = timing[depth]; - if (row === undefined) { - row = { - start: [], - end: [], - selfRelative: [], - callNode: [], - length: 0, - }; - timing[depth] = row; - } + const start = []; + const end = []; + const selfRelative = []; + const timingCallNodes = []; - // Take the absolute value, as native deallocations can be negative. - const totalRelative = abs( - callNodeSummary.total[nodeIndex] / rootTotalSummary - ); - const selfRelative = abs( - callNodeSummary.self[nodeIndex] / rootTotalSummary - ); + // Process the call nodes in this row. Sibling boxes are adjacent to each other. + // Whenever the prefix changes, we need to add a gap so that the child boxes + // start at the same position as the parent box. + // + // Previous row: [B ][D ] [G ] + // Current row: [C][E][F] [I ] + // (Note that this is upside down from how the flame graph is usually displayed) + let currentStart = 0.0; + let previousPrefixCallNode = -1; + for (let indexInRow = 0; indexInRow < rowNodes.length; indexInRow++) { + const nodeIndex = rowNodes[indexInRow]; + const totalVal = total[nodeIndex]; + if (totalVal === 0) { + // Skip boxes with zero width. + continue; + } - // Compute the timing information. - row.start.push(timeOffset[depth]); - row.end.push(timeOffset[depth] + totalRelative); - row.selfRelative.push(selfRelative); - row.callNode.push(nodeIndex); - row.length++; + const nodePrefix = prefix[nodeIndex]; + if (nodePrefix !== previousPrefixCallNode) { + // We have advanced to a node with a different parent, so we need to + // jump ahead to the parent box's start position. + currentStart = startPerCallNode[nodePrefix]; + previousPrefixCallNode = nodePrefix; + } - // Before we add the total time of this node to the time offset, - // we'll make sure that the first child (if any) begins with the - // same time offset. - timeOffset[depth + 1] = timeOffset[depth]; - timeOffset[depth] += totalRelative; + // Write down the start position of this call node so that it can be + // checked later by this node's children. + startPerCallNode[nodeIndex] = currentStart; - // The items in the children array are sorted in descending order, - // but since they are popped from the stack at the top of the - // while loop they'll be processed in ascending order. - for ( - let offset = children.offsets[nodeIndex]; - offset < children.offsets[nodeIndex + 1]; - offset++ - ) { - stack.push({ nodeIndex: children.array[offset], depth: depth + 1 }); + // Take the absolute value, as native deallocations can be negative. + const totalRelativeVal = abs(totalVal / rootTotalSummary); + const selfRelativeVal = abs(self[nodeIndex] / rootTotalSummary); + + const currentEnd = currentStart + totalRelativeVal; + start.push(currentStart); + end.push(currentEnd); + selfRelative.push(selfRelativeVal); + timingCallNodes.push(nodeIndex); + + // The start position of the next box is the end position of the current box. + currentStart = currentEnd; } + timing[depth] = { + start, + end, + selfRelative, + callNode: timingCallNodes, + length: timingCallNodes.length, + }; } + return timing; } diff --git a/src/selectors/per-thread/stack-sample.js b/src/selectors/per-thread/stack-sample.js index 156d582bef..3dc2d88dac 100644 --- a/src/selectors/per-thread/stack-sample.js +++ b/src/selectors/per-thread/stack-sample.js @@ -355,10 +355,17 @@ export function getStackAndSampleSelectorsPerThread( StackTiming.getStackTimingByDepth ); + const getFlameGraphRows: Selector = createSelector( + (state) => getCallNodeInfo(state).callNodeTable, + (state) => threadSelectors.getFilteredThread(state).funcTable, + (state) => threadSelectors.getFilteredThread(state).stringTable, + FlameGraph.computeFlameGraphRows + ); + const getFlameGraphTiming: Selector = createSelector( - threadSelectors.getPreviewFilteredThread, - getCallNodeInfo, + getFlameGraphRows, + (state) => getCallNodeInfo(state).callNodeTable, getCallTreeCountsAndSummary, FlameGraph.getFlameGraphTiming ); diff --git a/src/test/unit/bisect.test.js b/src/test/unit/bisect.test.js index 813a994f42..5dbb2375a6 100644 --- a/src/test/unit/bisect.test.js +++ b/src/test/unit/bisect.test.js @@ -4,7 +4,12 @@ // @flow -import { bisectionLeft, bisectionRight } from '../../utils/bisect'; +import { + bisectionLeft, + bisectionRight, + bisectionRightByKey, + bisectionRightByStrKey, +} from '../../utils/bisect'; describe('bisectionRight', function () { const array = [0, 1, 2, 3, 3, 3, 3, 3, 5, 6, 7, 8, 9]; @@ -43,6 +48,141 @@ describe('bisectionRight', function () { }); }); +describe('bisectionRightByKey', function () { + // T is string + // We're going to use a key function which maps a string to its length. + // This array is sorted by string length. + const array = [ + '', + 'a', + 'in', + 'cat', + 'dog', + 'moo', + 'bus', + 'fob', + 'camel', + 'stairs', + 'kitchen', + 'profiler', + 'profiling', + ]; + + function strLen(s: string): number { + return s.length; + } + + // 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(bisectionRightByKey(array, 4, strLen)).toBe(8); + expect(bisectionRightByKey(array, 3, strLen)).toBe(8); + }); + + it('returns index of the first element whose key is greater than x, occuring after low', function () { + expect(bisectionRightByKey(array, 4, strLen, 8)).toBe(8); + expect(bisectionRightByKey(array, 3, strLen, 9)).toBe(9); + }); + + it('returns index of the first element whose key is greater than x, between low and high', function () { + expect(bisectionRightByKey(array, 4, strLen, 1, 7)).toBe(7); + expect(bisectionRightByKey(array, 3, strLen, 4, 6)).toBe(6); + }); + + it('returns 0 if the keys of all elements are greater than x', function () { + expect(bisectionRightByKey(array, -5, strLen)).toBe(0); + }); + + it('returns array length if x is greater than the key of all elements', function () { + expect(bisectionRightByKey(array, 15, strLen)).toBe(13); + }); + + it('throws TypeError if either low or high are outside the range of the array', function () { + expect(() => bisectionRightByKey(array, 15, strLen, -2)).toThrow(TypeError); + expect(() => bisectionRightByKey(array, 15, strLen, 100)).toThrow( + TypeError + ); + expect(() => bisectionRightByKey(array, 15, strLen, 2, -10)).toThrow( + TypeError + ); + expect(() => bisectionRightByKey(array, 15, strLen, 2, 100)).toThrow( + TypeError + ); + expect(() => bisectionRightByKey(array, 15, strLen, -20, -10)).toThrow( + TypeError + ); + expect(() => bisectionRightByKey(array, 15, strLen, 100, 200)).toThrow( + TypeError + ); + }); +}); + +describe('bisectionRightByStrKey', function () { + const array = [ + { name: 'a0' }, + { name: 'a1' }, + { name: 'a2' }, + { name: 'a3' }, + { name: 'a3' }, + { name: 'a3' }, + { name: 'a3' }, + { name: 'a3' }, + { name: 'a5' }, + { name: 'a6' }, + { name: 'a7' }, + { name: 'a8' }, + { name: 'a9' }, + ]; + + function getName(x): string { + return x.name; + } + + // 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); + }); + + 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); + }); + + 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); + }); + + it('returns 0 if all the keys of elements are greater than x', function () { + expect(bisectionRightByStrKey(array, '_', getName)).toBe(0); + }); + + it('returns array length if x is greater than keys of all elements', function () { + expect(bisectionRightByStrKey(array, 'b', getName)).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( + TypeError + ); + expect(() => bisectionRightByStrKey(array, 'b', getName, 100)).toThrow( + TypeError + ); + expect(() => bisectionRightByStrKey(array, 'b', getName, 2, -10)).toThrow( + TypeError + ); + expect(() => bisectionRightByStrKey(array, 'b', getName, 2, 100)).toThrow( + TypeError + ); + expect(() => bisectionRightByStrKey(array, 'b', getName, -20, -10)).toThrow( + TypeError + ); + expect(() => bisectionRightByStrKey(array, 'b', getName, 100, 200)).toThrow( + TypeError + ); + }); +}); + describe('bisectionLeft', function () { const array = [0, 1, 2, 3, 3, 3, 3, 3, 5, 6, 7, 8, 9]; diff --git a/src/test/unit/profile-tree.test.js b/src/test/unit/profile-tree.test.js index 7a6d7f493f..825fae3dd9 100644 --- a/src/test/unit/profile-tree.test.js +++ b/src/test/unit/profile-tree.test.js @@ -10,7 +10,7 @@ import { getCallTree, computeCallTreeCountsAndSummary, } from '../../profile-logic/call-tree'; -import { getRootsAndChildren } from '../../profile-logic/flame-graph'; +import { computeFlameGraphRows } from '../../profile-logic/flame-graph'; import { getCallNodeInfo, invertCallstack, @@ -93,8 +93,8 @@ describe('unfiltered call tree', function () { }); }); - describe('roots and children for flame graph', function () { - it('returns roots and children', function () { + describe('ordered rows of call node indexes for flame graph', function () { + it('returns ordered rows', function () { // On purpose, we build a profile where the call node indexes won't be in // the same order than the function names. const { @@ -110,46 +110,35 @@ describe('unfiltered call tree', function () { profile.meta.categories, 'Expected to find categories' ).findIndex((c) => c.name === 'Other'); - const callNodeInfo = getCallNodeInfo( + const { callNodeTable } = getCallNodeInfo( thread.stackTable, thread.frameTable, thread.funcTable, defaultCategory ); - - const { callNodeChildCount, callNodeSummary } = - computeCallTreeCountsAndSummary( - thread.samples, - getSampleIndexToCallNodeIndex( - thread.samples.stack, - callNodeInfo.stackIndexToCallNodeIndex - ), - callNodeInfo, - false /* inverted */ - ); - - expect( - getRootsAndChildren( - thread, - callNodeInfo.callNodeTable, - callNodeChildCount, - callNodeSummary.total - ) - ).toEqual({ - // Roots are ordered in lexically descending order. - roots: [Z, K, G], - children: { - // Children are ordered in lexically descending order - array: new Uint32Array([X, Y, W, H, J, I, N, M, 0, 0, 0]), - offsets: new Uint32Array([0, 1, 3, 3, 3, 4, 6, 6, 6, 8, 8, 8]), - /* Z, X, Y, W, G, H, I, J, K, M, N, */ - /* offsets represent where children of a parent are located in "array". - * For example: - * children of G are located between indexes 3 and 4 (excluded). That is H. - * children of H are located between indexes 4 and 6 (excluded). That is J and I. - */ - }, - }); + const cnZ = getCallNodeIndexFromPath([Z], callNodeTable); + const cnZX = getCallNodeIndexFromPath([Z, X], callNodeTable); + const cnZXY = getCallNodeIndexFromPath([Z, X, Y], callNodeTable); + const cnZXW = getCallNodeIndexFromPath([Z, X, W], callNodeTable); + const cnG = getCallNodeIndexFromPath([G], callNodeTable); + const cnGH = getCallNodeIndexFromPath([G, H], callNodeTable); + const cnGHI = getCallNodeIndexFromPath([G, H, I], callNodeTable); + const cnGHJ = getCallNodeIndexFromPath([G, H, J], callNodeTable); + const cnK = getCallNodeIndexFromPath([K], callNodeTable); + const cnKM = getCallNodeIndexFromPath([K, M], callNodeTable); + const cnKN = getCallNodeIndexFromPath([K, N], callNodeTable); + + const rows = computeFlameGraphRows( + callNodeTable, + thread.funcTable, + thread.stringTable + ); + expect(rows).toEqual([ + // Siblings are ordered in lexically ascending order. + [cnG, cnK, cnZ], + [cnGH, cnKM, cnKN, cnZX], + [cnGHI, cnGHJ, cnZXW, cnZXY], + ]); }); }); diff --git a/src/utils/bisect.js b/src/utils/bisect.js index 27614251ed..193168b717 100644 --- a/src/utils/bisect.js +++ b/src/utils/bisect.js @@ -20,10 +20,76 @@ THE SOFTWARE. */ -/* - The two functions here, bisectionLeft and bisectionRight, return the index where a new element - would be inserted, respectively at the left or at the right of elements with the same value. -*/ +/** + * These functions are used on sorted arrays. + * + * You commonly use them in one of two cases: + * + * 1. When you want to insert a new element into a sorted array, at a position + * such that the array remains in sorted order. Or + * 2. When you have a sorted array of interval start positions, and you want + * to find out which interval includes a certain number. + * + * For case 1, you can use either bisectionRight or bisectionLeft. The difference + * only matters if you care about the positioning of two equal elements. For + * example: + * bisectionRight([1, 2, 3], 2) === 2 + * bisectionLeft([1, 2, 3], 2) === 1 + * i.e. the returned index is either to the right or to the left of the equal + * element. If no exactly matching element is present in the array, both functions + * return the same value, i.e. the index of the first element that's larger than + * the passed in element (which is the index at which that element would need to + * be inserted). + * + * ```js + * const insertionIndexR = bisectionRight(array, x); + * assert(array[insertionIndexR] > x); + * assert(array[insertionIndexR - 1] <= x); + * + * const insertionIndexL = bisectionLeft(array, x); + * assert(array[insertionIndexL] >= x); + * assert(array[insertionIndexL - 1] < x); + * ``` + * + * For case 2, you'll have to use bisectionRight, and subtract 1 from the return + * value. For example, if you have the half-open intervals 2..4, 4..7, 7..Infinity, + * then your start position array will be [2, 4, 7] and bisectionRight() - 1 will + * be the index of the last interval whose start position is <= the checked element. + * bisectionRight([2, 4, 7], 1) - 1 === -1 + * bisectionRight([2, 4, 7], 2) - 1 === 0 + * bisectionRight([2, 4, 7], 3) - 1 === 0 + * bisectionRight([2, 4, 7], 4) - 1 === 1 + * bisectionRight([2, 4, 7], 5) - 1 === 1 + * bisectionRight([2, 4, 7], 6) - 1 === 1 + * bisectionRight([2, 4, 7], 7) - 1 === 2 + * bisectionRight([2, 4, 7], 20) - 1 === 2 + * + * Example code: + * + * ```js + * const intervalStarts = [2, 4, 7]; + * const insertionIndex = bisectionRight(intervalStarts, x); + * if (insertionIndex === 0) { + * // x is before the first interval. + * return null; + * } + * + * const intervalIndex = insertionIndex - 1; + * assert(x >= intervalStarts[intervalIndex]); + * + * // If there can be gaps between your intervals, you also need to check the + * // interval end: + * if (x >= intervalEnds[intervalIndex]) { + * // x isn't actually inside this interval! It's in the gap between + * // intervalIndex and intervalIndex+1. + * return null; + * } + * + * // Now we know that x is inside the interval. + * assert(x >= intervalStarts[intervalIndex] && x < intervalEnds[intervalIndex]); + * return intervalIndex; + * ``` + */ // @flow @@ -53,6 +119,67 @@ export function bisectionRight( return low; } +/** + * Like bisectionRight, but accepts a "key" function which maps an element of + * the array to a number "key". The array must be sorted by that key. + */ +export function bisectionRightByKey( + array: T[], + x: number, + toKey: (T) => number, + low?: number, + high?: number +): number { + low = low || 0; + high = high || array.length; + + if (low < 0 || low > array.length || high < 0 || high > array.length) { + throw new TypeError("low and high must lie within the array's range"); + } + + while (low < high) { + const mid = (low + high) >> 1; + + if (x < toKey(array[mid])) { + high = mid; + } else { + low = mid + 1; + } + } + + return low; +} + +// 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. +export function bisectionRightByStrKey( + array: T[], + x: string, + toKey: (T) => string, + low?: number, + high?: number +): number { + low = low || 0; + high = high || array.length; + + if (low < 0 || low > array.length || high < 0 || high > array.length) { + throw new TypeError("low and high must lie within the array's range"); + } + + while (low < high) { + const mid = (low + high) >> 1; + + if (x < toKey(array[mid])) { + high = mid; + } else { + low = mid + 1; + } + } + + return low; +} + export function bisectionLeft( array: number[] | $TypedArray, x: number,