Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make inverting the call tree fast, by computing inverted call nodes lazily #4900

Open
wants to merge 18 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
69 changes: 69 additions & 0 deletions src/profile-logic/call-node-info.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
// @flow

import { hashPath } from 'firefox-profiler/utils/path';
import { bisectEqualRange } from 'firefox-profiler/utils/bisect';

import type {
IndexIntoFuncTable,
Expand All @@ -13,6 +14,7 @@ import type {
CallNodeTable,
CallNodePath,
IndexIntoCallNodeTable,
SuffixOrderIndex,
} from 'firefox-profiler/types';

/**
Expand Down Expand Up @@ -222,11 +224,78 @@ export class CallNodeInfoInvertedImpl
extends CallNodeInfoImpl
implements CallNodeInfoInverted
{
// This is a Map<SuffixOrderIndex, IndexIntoNonInvertedCallNodeTable>.
// It lists the non-inverted call nodes in "suffix order", i.e. ordered by
// comparing their call paths from back to front.
_suffixOrderedCallNodes: Uint32Array;
// This is the inverse of _suffixOrderedCallNodes; i.e. it is a
// Map<IndexIntoNonInvertedCallNodeTable, SuffixOrderIndex>.
_suffixOrderIndexes: Uint32Array;

constructor(
callNodeTable: CallNodeTable,
nonInvertedCallNodeTable: CallNodeTable,
stackIndexToCallNodeIndex: Int32Array,
stackIndexToNonInvertedCallNodeIndex: Int32Array,
suffixOrderedCallNodes: Uint32Array,
suffixOrderIndexes: Uint32Array
) {
super(
callNodeTable,
nonInvertedCallNodeTable,
stackIndexToCallNodeIndex,
stackIndexToNonInvertedCallNodeIndex
);
this._suffixOrderedCallNodes = suffixOrderedCallNodes;
this._suffixOrderIndexes = suffixOrderIndexes;
}

isInverted(): boolean {
return true;
}

asInverted(): CallNodeInfoInverted | null {
return this;
}

getSuffixOrderedCallNodes(): Uint32Array {
return this._suffixOrderedCallNodes;
}

getSuffixOrderIndexes(): Uint32Array {
return this._suffixOrderIndexes;
}

getSuffixOrderIndexRangeForCallNode(
callNodeIndex: IndexIntoCallNodeTable
): [SuffixOrderIndex, SuffixOrderIndex] {
const callPath = this.getCallNodePathFromIndex(callNodeIndex);
return bisectEqualRange(
this._suffixOrderedCallNodes,
(callNodeIndex: IndexIntoCallNodeTable) => {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

some more comment around the compare function would be useful.
Esp give a few different examples with callnodes of different lengths, and explain why this gives the intended result.

Also it would probably be a good idea to call the 2 callNodeIndex differently: there's one as the parameter of the main function (the needleCallNodeIndex), and another one as the parameter of the compare function (the iteratingCallNodeIndex maybe?), that are not the same.

I don't understand it fully. Is it similar to _compareNonInvertedCallNodesInSuffixOrder below, except that we computed the needle's callPath once at the start for a faster access in the iteration, while it would be more costly than necessary for all the iterating callNodeIndexes?

let currentCallNodeIndex = callNodeIndex;
for (let i = 0; i < callPath.length - 1; i++) {
const expectedFunc = callPath[i];
const currentFunc =
this._nonInvertedCallNodeTable.func[currentCallNodeIndex];
if (currentFunc < expectedFunc) {
return -1;
}
if (currentFunc > expectedFunc) {
return 1;
}
const prefix =
this._nonInvertedCallNodeTable.prefix[currentCallNodeIndex];
if (prefix === -1) {
return -1;
}
currentCallNodeIndex = prefix;
}
const expectedFunc = callPath[callPath.length - 1];
const currentFunc =
this._nonInvertedCallNodeTable.func[currentCallNodeIndex];
return currentFunc - expectedFunc;
}
);
}
}
62 changes: 61 additions & 1 deletion src/profile-logic/profile-data.js
Original file line number Diff line number Diff line change
Expand Up @@ -474,14 +474,74 @@ export function getInvertedCallNodeInfo(
invertedStackIndexToCallNodeIndex[invertedStackIndex];
}
}

// TEMPORARY: Compute a suffix order for the entire non-inverted call node table.
// See the CallNodeInfoInverted interface for more details about the suffix order.
// By the end of this commit stack, the suffix order will be computed incrementally
// as inverted nodes are created; we won't compute the entire order upfront.
const nonInvertedCallNodeCount = nonInvertedCallNodeTable.length;
const suffixOrderedCallNodes = new Uint32Array(nonInvertedCallNodeCount);
const suffixOrderIndexes = new Uint32Array(nonInvertedCallNodeCount);
for (let i = 0; i < nonInvertedCallNodeCount; i++) {
suffixOrderedCallNodes[i] = i;
}
suffixOrderedCallNodes.sort((a, b) =>
_compareNonInvertedCallNodesInSuffixOrder(a, b, nonInvertedCallNodeTable)
);
for (let i = 0; i < suffixOrderedCallNodes.length; i++) {
suffixOrderIndexes[suffixOrderedCallNodes[i]] = i;
}

return new CallNodeInfoInvertedImpl(
callNodeTable,
nonInvertedCallNodeTable,
nonInvertedStackIndexToCallNodeIndex,
stackIndexToNonInvertedCallNodeIndex
stackIndexToNonInvertedCallNodeIndex,
suffixOrderedCallNodes,
suffixOrderIndexes
);
}

// Compare two non-inverted call nodes in "suffix order".
// The suffix order is defined as the lexicographical order of the inverted call
// path, or, in other words, the "backwards" lexicographical order of the
// non-inverted call paths.
//
// Example of some suffix ordered non-inverted call paths:
// [0]
// [0, 0]
// [2, 0]
// [4, 5, 1]
// [4, 5]
function _compareNonInvertedCallNodesInSuffixOrder(
callNodeA: IndexIntoCallNodeTable,
callNodeB: IndexIntoCallNodeTable,
nonInvertedCallNodeTable: CallNodeTable
): number {
// Walk up both and stop at the first non-matching function.
// Walking up the non-inverted tree is equivalent to walking down the
// inverted tree.
while (true) {
const funcA = nonInvertedCallNodeTable.func[callNodeA];
const funcB = nonInvertedCallNodeTable.func[callNodeB];
if (funcA !== funcB) {
return funcA - funcB;
}
callNodeA = nonInvertedCallNodeTable.prefix[callNodeA];
callNodeB = nonInvertedCallNodeTable.prefix[callNodeB];
if (callNodeA === callNodeB) {
break;
}
if (callNodeA === -1) {
return -1;
}
if (callNodeB === -1) {
return 1;
}
}
return 0;
}

// Given a stack index `needleStack` and a call node in the inverted tree
// `invertedCallTreeNode`, find an ancestor stack of `needleStack` which
// corresponds to the given call node in the inverted call tree. Returns null if
Expand Down
105 changes: 105 additions & 0 deletions src/test/unit/profile-data.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -575,6 +575,111 @@ describe('profile-data', function () {
});
});

describe('getInvertedCallNodeInfo', function () {
function setup(plaintextSamples) {
const { profile, funcNamesDictPerThread } =
getProfileFromTextSamples(plaintextSamples);

const thread = profile.threads[0];
const funcNamesDict = funcNamesDictPerThread[0];
const categories = ensureExists(
profile.meta.categories,
'Expected to find categories'
);
const defaultCategory = categories.findIndex((c) => c.name === 'Other');
const nonInvertedCallNodeInfo = getCallNodeInfo(
thread.stackTable,
thread.frameTable,
thread.funcTable,
defaultCategory
);

const invertedCallNodeInfo = getInvertedCallNodeInfo(
thread,
nonInvertedCallNodeInfo.getNonInvertedCallNodeTable(),
nonInvertedCallNodeInfo.getStackIndexToNonInvertedCallNodeIndex(),
defaultCategory
);

// This function is used to test `getSuffixOrderIndexRangeForCallNode` and
// `getSuffixOrderedCallNodes`. To find the non-inverted call nodes with
// a call path suffix, `nodesWithSuffix` gets the inverted node X for the
// given call path suffix, and lists non-inverted nodes in X's "suffix
// order index range".
// These are the nodes whose call paths, if inverted, would correspond to
// inverted call nodes that are descendants of X.
function nodesWithSuffix(callPathSuffix) {
const invertedNodeForSuffix = ensureExists(
invertedCallNodeInfo.getCallNodeIndexFromPath(
[...callPathSuffix].reverse()
)
);
const [rangeStart, rangeEnd] =
invertedCallNodeInfo.getSuffixOrderIndexRangeForCallNode(
invertedNodeForSuffix
);
const suffixOrderedCallNodes =
invertedCallNodeInfo.getSuffixOrderedCallNodes();
const nonInvertedCallNodes = new Set();
for (let i = rangeStart; i < rangeEnd; i++) {
nonInvertedCallNodes.add(suffixOrderedCallNodes[i]);
}
return nonInvertedCallNodes;
}

return {
funcNamesDict,
nonInvertedCallNodeInfo,
invertedCallNodeInfo,
nodesWithSuffix,
};
}

it('creates a correct suffix order for this example profile', function () {
const {
funcNamesDict: { A, B, C },
nonInvertedCallNodeInfo,
nodesWithSuffix,
} = setup(`
A A A A A A A
B B B A A C
A C B
`);

const cnA = nonInvertedCallNodeInfo.getCallNodeIndexFromPath([A]);
const cnAB = nonInvertedCallNodeInfo.getCallNodeIndexFromPath([A, B]);
const cnABA = nonInvertedCallNodeInfo.getCallNodeIndexFromPath([A, B, A]);
const cnABC = nonInvertedCallNodeInfo.getCallNodeIndexFromPath([A, B, C]);
const cnAA = nonInvertedCallNodeInfo.getCallNodeIndexFromPath([A, A]);
const cnAAB = nonInvertedCallNodeInfo.getCallNodeIndexFromPath([A, A, B]);
const cnAC = nonInvertedCallNodeInfo.getCallNodeIndexFromPath([A, C]);

expect(nodesWithSuffix([A])).toEqual(new Set([cnA, cnABA, cnAA]));
expect(nodesWithSuffix([B])).toEqual(new Set([cnAB, cnAAB]));
expect(nodesWithSuffix([A, B])).toEqual(new Set([cnAB, cnAAB]));
expect(nodesWithSuffix([A, A, B])).toEqual(new Set([cnAAB]));
expect(nodesWithSuffix([C])).toEqual(new Set([cnABC, cnAC]));
});

it('creates a correct suffix order for a different example profile', function () {
const {
funcNamesDict: { A, B, C },
nonInvertedCallNodeInfo,
nodesWithSuffix,
} = setup(`
A A A C
B B
C
`);

const cnABC = nonInvertedCallNodeInfo.getCallNodeIndexFromPath([A, B, C]);
const cnC = nonInvertedCallNodeInfo.getCallNodeIndexFromPath([C]);

expect(nodesWithSuffix([B, C])).toEqual(new Set([cnABC]));
expect(nodesWithSuffix([C])).toEqual(new Set([cnABC, cnC]));
});
});

describe('symbolication', function () {
describe('AddressLocator', function () {
const libs = [
Expand Down
Loading