diff --git a/src/import/firefox.ts b/src/import/firefox.ts index 3918818..082bf10 100644 --- a/src/import/firefox.ts +++ b/src/import/firefox.ts @@ -158,7 +158,7 @@ export function importFromFirefox(firefoxProfile: FirefoxProfile): Profile { ? cpuProfile.threads[0] : cpuProfile.threads.filter(t => t.name === 'GeckoMain')[0] - const frameIdToFrameInfo = new Map() + const frameKeyToFrameInfo = new Map() function extractStack(sample: Sample): FrameInfo[] { let stackFrameId: number | null = sample[0] @@ -185,7 +185,7 @@ export function importFromFirefox(firefoxProfile: FirefoxProfile): Profile { return null } - return getOrInsert(frameIdToFrameInfo, f, () => ({ + return getOrInsert(frameKeyToFrameInfo, location, () => ({ key: location, name: match[1]!, file: match[2]!, @@ -205,24 +205,22 @@ export function importFromFirefox(firefoxProfile: FirefoxProfile): Profile { // Find lowest common ancestor of the current stack and the previous one let lca: FrameInfo | null = null - // This is O(n^2), but n should be relatively small here (stack height), - // so hopefully this isn't much of a problem - for (let i = stack.length - 1; i >= 0 && prevStack.indexOf(stack[i]) === -1; i--) {} + for (let i = 0; i < Math.min(stack.length, prevStack.length); i++) { + if (prevStack[i] !== stack[i]) { + break + } + lca = stack[i] + } // Close frames that are no longer open - while (prevStack.length > 0 && lastOf(prevStack) != lca) { + while (prevStack.length > 0 && (!lca || lastOf(prevStack) !== lca)) { const closingFrame = prevStack.pop()! profile.leaveFrame(closingFrame, value) } // Open frames that are now becoming open - const toOpen: FrameInfo[] = [] - for (let i = stack.length - 1; i >= 0 && stack[i] != lca; i--) { - toOpen.push(stack[i]) - } - toOpen.reverse() - - for (let frame of toOpen) { + for (let i = lca ? stack.indexOf(lca) + 1 : 0; i < stack.length; i++) { + const frame = stack[i] profile.enterFrame(frame, value) } diff --git a/src/lib/__snapshots__/profile.test.ts.snap b/src/lib/__snapshots__/profile.test.ts.snap new file mode 100644 index 0000000..c08d9c8 --- /dev/null +++ b/src/lib/__snapshots__/profile.test.ts.snap @@ -0,0 +1,47 @@ +// Jest Snapshot v1, https://goo.gl/fbAQLP + +exports[`CallTreeProfileBuilder separates non-contiguous: append order 1`] = ` +"((speedscope root):0:5 + (a:1:5 + (b:1:2 + (c:1:1) + ) + (b:1:2 + (c:1:1) + ) + ) +)" +`; + +exports[`CallTreeProfileBuilder separates non-contiguous: grouped 1`] = ` +"((speedscope root):0:5 + (a:1:5 + (b:2:4 + (c:2:2) + ) + ) +)" +`; + +exports[`StackListProfileBuilder separates non-contiguous: append order 1`] = ` +"((speedscope root):4:0 + (a:1:5 + (b:1:2 + (c:1:1) + ) + (b:1:2 + (c:1:1) + ) + ) +)" +`; + +exports[`StackListProfileBuilder separates non-contiguous: grouped 1`] = ` +"((speedscope root):4:0 + (a:1:5 + (b:2:4 + (c:2:2) + ) + ) +)" +`; diff --git a/src/lib/profile.test.ts b/src/lib/profile.test.ts index 9467972..1e955dd 100644 --- a/src/lib/profile.test.ts +++ b/src/lib/profile.test.ts @@ -50,6 +50,30 @@ function toStackList(profile: Profile, grouped: boolean): string[] { return stackList } +function flatten(ts: T[][]): T[] { + let ret: T[] = [] + return ret.concat(...ts) +} + +function toTreeString(profile: Profile, grouped: boolean): string { + function visit(node: CallTreeNode): string[] { + const childLines = flatten(node.children.map(child => visit(child))).map(l => ` ${l}`) + const nodeStr = `${node.frame.key}:${node.getSelfWeight()}:${node.getTotalWeight()}` + + if (childLines.length > 0) { + return [`(${nodeStr}`].concat(childLines).concat(')') + } else { + return [`(${nodeStr})`] + } + } + + if (grouped) { + return visit(profile.getGroupedCalltreeRoot()).join('\n') + } else { + return visit(profile.getAppendOrderCalltreeRoot()).join('\n') + } +} + function verifyProfile(profile: Profile) { const allFrameKeys = new Set([fa, fb, fc, fd, fe].map(f => f.key)) const framesInProfile = new Set() @@ -133,6 +157,28 @@ test('StackListProfileBuilder', () => { verifyProfile(profile) }) +test('StackListProfileBuilder separates non-contiguous', () => { + const b = new StackListProfileBuilder() + + const samples = [ + // prettier-ignore + [fa, fb, fc], + [fa, fb], + [fa], + [fa, fb], + [fa, fb, fc], + ] + + samples.forEach(stack => { + b.appendSample(stack, 1) + }) + b.appendSample([], 4) + const profile = b.build() + + expect(toTreeString(profile, true)).toMatchSnapshot('grouped') + expect(toTreeString(profile, false)).toMatchSnapshot('append order') +}) + test('CallTreeProfileBuilder', () => { const b = new CallTreeProfileBuilder() @@ -167,6 +213,30 @@ test('CallTreeProfileBuilder', () => { verifyProfile(profile) }) +test('CallTreeProfileBuilder separates non-contiguous', () => { + const b = new CallTreeProfileBuilder() + + b.enterFrame(fa, 0) + b.enterFrame(fb, 0) + b.enterFrame(fc, 0) + + b.leaveFrame(fc, 1) + + b.leaveFrame(fb, 2) + + b.enterFrame(fb, 3) + + b.enterFrame(fc, 4) + + b.leaveFrame(fc, 5) + b.leaveFrame(fb, 5) + b.leaveFrame(fa, 5) + + const profile = b.build() + expect(toTreeString(profile, true)).toMatchSnapshot('grouped') + expect(toTreeString(profile, false)).toMatchSnapshot('append order') +}) + test('getInvertedProfileForCallersOf', () => { const b = new StackListProfileBuilder() diff --git a/src/lib/profile.ts b/src/lib/profile.ts index 324eabb..70c5cfd 100644 --- a/src/lib/profile.ts +++ b/src/lib/profile.ts @@ -89,6 +89,15 @@ export class CallTreeNode extends HasWeights { return this.frame === Frame.root } + // If a node is "frozen", it means it should no longer be mutated. + private frozen = false + isFrozen() { + return this.frozen + } + freeze() { + this.frozen = true + } + constructor(readonly frame: Frame, readonly parent: CallTreeNode | null) { super() } @@ -100,9 +109,17 @@ export class Profile { protected totalWeight: number protected frames = new KeyedSet() + protected appendOrderCalltreeRoot = new CallTreeNode(Frame.root, null) protected groupedCalltreeRoot = new CallTreeNode(Frame.root, null) + public getAppendOrderCalltreeRoot() { + return this.appendOrderCalltreeRoot + } + public getGroupedCalltreeRoot() { + return this.groupedCalltreeRoot + } + // List of references to CallTreeNodes at the top of the // stack at the time of the sample. protected samples: CallTreeNode[] = [] @@ -395,7 +412,7 @@ export class StackListProfileBuilder extends Profile { const last = useAppendOrder ? lastOf(node.children) : node.children.find(c => c.frame === frame) - if (last && last.frame == frame) { + if (last && !last.isFrozen() && last.frame == frame) { node = last } else { const parent = node @@ -414,6 +431,12 @@ export class StackListProfileBuilder extends Profile { } node.addToSelfWeight(weight) + if (useAppendOrder) { + for (let child of node.children) { + child.freeze() + } + } + if (useAppendOrder) { node.frame.addToSelfWeight(weight) @@ -493,7 +516,7 @@ export class CallTreeProfileBuilder extends Profile { ? lastOf(prevTop.children) : prevTop.children.find(c => c.frame === frame) let node: CallTreeNode - if (last && last.frame == frame) { + if (last && !last.isFrozen() && last.frame == frame) { node = last } else { node = new CallTreeNode(frame, prevTop) @@ -520,10 +543,17 @@ export class CallTreeProfileBuilder extends Profile { if (useAppendOrder) { const leavingStackTop = this.appendOrderStack.pop() - const delta = value - this.lastValue! + if (leavingStackTop == null) { + throw new Error(`Trying to leave ${frame.key} when stack is empty`) + } + if (this.lastValue == null) { + throw new Error(`Trying to leave a ${frame.key} before any have been entered`) + } + leavingStackTop.freeze() + const delta = value - this.lastValue if (delta > 0) { - this.samples.push(leavingStackTop!) - this.weights.push(value - this.lastValue!) + this.samples.push(leavingStackTop) + this.weights.push(value - this.lastValue) } else if (delta < 0) { throw new Error( `Samples must be provided in increasing order of cumulative value. Last sample was ${this