Add tests for Profile and value formatters (#54)

* Split profile building APIs into dedicated classes

* Split value formatters into their own file

* Add tests for formatters

* Add test for StackListProfileBuilder

* Add test for CallTreeProfileBuilder

import/bg-flamegraph.ts

@@ -1,6 +1,6 @@
 // https://github.com/brendangregg/FlameGraph#2-fold-stacks
 
-import {Profile, FrameInfo} from '../profile'
+import {Profile, FrameInfo, StackListProfileBuilder} from '../profile'
 
 interface BGSample {
   stack: FrameInfo[]
@@ -22,9 +22,9 @@ function parseBGFoldedStacks(contents: string): BGSample[] {
 export function importFromBGFlameGraph(contents: string): Profile {
   const parsed = parseBGFoldedStacks(contents)
   const duration = parsed.reduce((prev: number, cur: BGSample) => prev + cur.duration, 0)
-  const profile = new Profile(duration)
+  const profile = new StackListProfileBuilder(duration)
   for (let sample of parsed) {
     profile.appendSample(sample.stack, sample.duration)
   }
-  return profile
+  return profile.build()
 }

import/chrome.ts

@@ -1,5 +1,6 @@
-import {Profile, TimeFormatter, FrameInfo} from '../profile'
+import {Profile, FrameInfo, CallTreeProfileBuilder} from '../profile'
 import {getOrInsert, lastOf} from '../utils'
+import {TimeFormatter} from '../value-formatters'
 
 interface TimelineEvent {
   pid: number
@@ -44,7 +45,7 @@ interface CPUProfile {
   timeDeltas: number[]
 }
 
-export function importFromChromeTimeline(events: TimelineEvent[]) {
+export function importFromChromeTimeline(events: TimelineEvent[]): Profile {
   // It seems like sometimes Chrome timeline files contain multiple CpuProfiles?
   // For now, choose the first one in the list.
   for (let event of events) {
@@ -73,8 +74,8 @@ function frameInfoForCallFrame(callFrame: CPUProfileCallFrame) {
   })
 }
 
-export function importFromChromeCPUProfile(chromeProfile: CPUProfile) {
-  const profile = new Profile(chromeProfile.endTime - chromeProfile.startTime)
+export function importFromChromeCPUProfile(chromeProfile: CPUProfile): Profile {
+  const profile = new CallTreeProfileBuilder(chromeProfile.endTime - chromeProfile.startTime)
 
   const nodeById = new Map<number, CPUProfileNode>()
   for (let node of chromeProfile.nodes) {
@@ -172,5 +173,5 @@ export function importFromChromeCPUProfile(chromeProfile: CPUProfile) {
   }
 
   profile.setValueFormatter(new TimeFormatter('microseconds'))
-  return profile
+  return profile.build()
 }

import/firefox.ts

@@ -1,5 +1,6 @@
-import {Profile, FrameInfo, TimeFormatter} from '../profile'
+import {Profile, FrameInfo, CallTreeProfileBuilder} from '../profile'
 import {getOrInsert, lastOf} from '../utils'
+import {TimeFormatter} from '../value-formatters'
 
 interface Allocations {
   frames: any[]
@@ -194,7 +195,7 @@ export function importFromFirefox(firefoxProfile: FirefoxProfile): Profile {
       .filter(f => f != null) as FrameInfo[]
   }
 
-  const profile = new Profile(firefoxProfile.duration)
+  const profile = new CallTreeProfileBuilder(firefoxProfile.duration)
 
   let prevStack: FrameInfo[] = []
   for (let sample of thread.samples.data) {
@@ -229,5 +230,5 @@ export function importFromFirefox(firefoxProfile: FirefoxProfile): Profile {
   }
 
   profile.setValueFormatter(new TimeFormatter('milliseconds'))
-  return profile
+  return profile.build()
 }

import/instruments.ts

@@ -1,9 +1,10 @@
 // This file contains methods to import data from OS X Instruments.app
 // https://developer.apple.com/library/content/documentation/DeveloperTools/Conceptual/InstrumentsUserGuide/index.html
 
-import {Profile, FrameInfo, ByteFormatter, TimeFormatter} from '../profile'
+import {Profile, FrameInfo, CallTreeProfileBuilder, StackListProfileBuilder} from '../profile'
 import {sortBy, getOrThrow, getOrInsert, lastOf, getOrElse, zeroPad} from '../utils'
 import * as pako from 'pako'
+import {ByteFormatter, TimeFormatter} from '../value-formatters'
 
 function parseTSV<T>(contents: string): T[] {
   const lines = contents.split('\n').map(l => l.split('\t'))
@@ -87,7 +88,7 @@ function getWeight(deepCopyRow: any): number {
 
 // Import from a deep copy made of a profile
 export function importFromInstrumentsDeepCopy(contents: string): Profile {
-  const profile = new Profile()
+  const profile = new CallTreeProfileBuilder()
   const rows = parseTSV<PastedTimeProfileRow | PastedAllocationsProfileRow>(contents)
 
   const stack: FrameInfoWithWeight[] = []
@@ -138,7 +139,7 @@ export function importFromInstrumentsDeepCopy(contents: string): Profile {
     profile.setValueFormatter(new TimeFormatter('milliseconds'))
   }
 
-  return profile
+  return profile.build()
 }
 
 interface TraceDirectoryTree {
@@ -433,7 +434,7 @@ export async function importFromInstrumentsTrace(entry: WebKitEntry): Promise<Pr
 
   const backtraceIDtoStack = new Map<number, FrameInfo[]>()
 
-  const profile = new Profile(lastOf(samples)!.timestamp)
+  const profile = new StackListProfileBuilder(lastOf(samples)!.timestamp)
   profile.setName(entry.name)
 
   // For now, we can only display the flamechart for a single thread of execution,
@@ -495,7 +496,7 @@ export async function importFromInstrumentsTrace(entry: WebKitEntry): Promise<Pr
   }
 
   profile.setValueFormatter(new TimeFormatter('nanoseconds'))
-  return profile
+  return profile.build()
 }
 
 export function readInstrumentsKeyedArchive(buffer: ArrayBuffer): any {

import/stackprof.ts

@@ -1,6 +1,7 @@
 // https://github.com/tmm1/stackprof
 
-import {Profile, TimeFormatter, FrameInfo} from '../profile'
+import {Profile, FrameInfo, StackListProfileBuilder} from '../profile'
+import {TimeFormatter} from '../value-formatters'
 
 interface StackprofFrame {
   name: string
@@ -16,7 +17,7 @@ export interface StackprofProfile {
 
 export function importFromStackprof(stackprofProfile: StackprofProfile): Profile {
   const duration = stackprofProfile.raw_timestamp_deltas.reduce((a, b) => a + b, 0)
-  const profile = new Profile(duration)
+  const profile = new StackListProfileBuilder(duration)
 
   const {frames, raw, raw_timestamp_deltas} = stackprofProfile
   let sampleIndex = 0
@@ -42,5 +43,5 @@ export function importFromStackprof(stackprofProfile: StackprofProfile): Profile
   }
 
   profile.setValueFormatter(new TimeFormatter('microseconds'))
-  return profile
+  return profile.build()
 }

profile.test.ts

@@ -0,0 +1,140 @@
+import {
+  FrameInfo,
+  StackListProfileBuilder,
+  CallTreeNode,
+  CallTreeProfileBuilder,
+  Profile,
+} from './profile'
+
+function getFrameInfo(key: string): FrameInfo {
+  return {
+    key: key,
+    name: key,
+    file: `${key}.ts`,
+    line: key.length,
+  }
+}
+
+const fa = getFrameInfo('a')
+const fb = getFrameInfo('b')
+const fc = getFrameInfo('c')
+const fd = getFrameInfo('d')
+const fe = getFrameInfo('e')
+
+function verifyProfile(profile: Profile) {
+  const allFrameKeys = new Set([fa, fb, fc, fd, fe].map(f => f.key))
+  const framesInProfile = new Set<string | number>()
+  profile.forEachFrame(f => framesInProfile.add(f.key))
+  expect(allFrameKeys).toEqual(framesInProfile)
+
+  let stackList: string[] = []
+  const curStack: (number | string)[] = []
+
+  let lastValue = 0
+  function openFrame(node: CallTreeNode, value: number) {
+    if (lastValue != value) {
+      stackList.push(curStack.map(k => `${k}`).join(';'))
+      lastValue = value
+    }
+    curStack.push(node.frame.key)
+  }
+
+  function closeFrame(value: number) {
+    if (lastValue != value) {
+      stackList.push(curStack.map(k => `${k}`).join(';'))
+      lastValue = value
+    }
+    curStack.pop()
+  }
+
+  profile.forEachCall(openFrame, closeFrame)
+  expect(stackList).toEqual([
+    // prettier-ignore
+    'a',
+    'a;b',
+    'a;b;d',
+    'a;b;c',
+    '',
+    'a',
+    'a;b',
+    'a;b;b',
+    'a;b;e',
+    'a',
+  ])
+
+  stackList = []
+  profile.forEachCallGrouped(openFrame, closeFrame)
+  expect(stackList).toEqual([
+    // prettier-ignore
+    '',
+    'a;b;e',
+    'a;b;b',
+    'a;b;c',
+    'a;b;d',
+    'a;b',
+    'a',
+  ])
+}
+
+test('StackListProfileBuilder', () => {
+  const b = new StackListProfileBuilder()
+
+  const samples = [
+    // prettier-ignore
+    [fa],
+    [fa, fb],
+    [fa, fb],
+    [fa, fb, fd],
+    [fa, fb, fc],
+    [],
+    [fa],
+    [fa, fb],
+    [fa, fb, fb],
+    [fa, fb, fe],
+    [fa],
+  ]
+
+  samples.forEach(stack => {
+    b.appendSample(stack, 1)
+  })
+  b.appendSample([], 4)
+
+  const profile = b.build()
+  expect(profile.getTotalWeight()).toBe(samples.length + 4)
+  expect(profile.getTotalNonIdleWeight()).toBe(samples.length - 1)
+  verifyProfile(profile)
+})
+
+test('CallTreeProfileBuilder', () => {
+  const b = new CallTreeProfileBuilder()
+
+  b.enterFrame(fa, 0)
+
+  b.enterFrame(fb, 1)
+
+  b.enterFrame(fd, 3)
+
+  b.leaveFrame(fd, 4)
+  b.enterFrame(fc, 4)
+
+  b.leaveFrame(fc, 5)
+  b.leaveFrame(fb, 5)
+  b.leaveFrame(fa, 5)
+
+  b.enterFrame(fa, 6)
+
+  b.enterFrame(fb, 7)
+
+  b.enterFrame(fb, 8)
+
+  b.leaveFrame(fb, 9)
+  b.enterFrame(fe, 9)
+
+  b.leaveFrame(fe, 10)
+  b.leaveFrame(fb, 10)
+
+  b.leaveFrame(fa, 11)
+
+  const profile = b.build()
+  verifyProfile(profile)
+})

profile.ts

@@ -1,4 +1,5 @@
 import {lastOf, getOrInsert} from './utils'
+import {ValueFormatter, RawValueFormatter} from './value-formatters'
 const demangleCppModule = import('./demangle-cpp')
 
 // Force eager loading of the module
@@ -83,64 +84,21 @@ const rootFrame = new Frame({
   name: '(speedscope root)',
 })
 
-export interface ValueFormatter {
-  format(v: number): string
-}
-
-export class RawValueFormatter implements ValueFormatter {
-  format(v: number) {
-    return v.toLocaleString()
-  }
-}
-
-export class TimeFormatter implements ValueFormatter {
-  private multiplier: number
-
-  constructor(unit: 'nanoseconds' | 'microseconds' | 'milliseconds' | 'seconds') {
-    if (unit === 'nanoseconds') this.multiplier = 1e-9
-    else if (unit === 'microseconds') this.multiplier = 1e-6
-    else if (unit === 'milliseconds') this.multiplier = 1e-3
-    else this.multiplier = 1
-  }
-
-  format(v: number) {
-    const s = v * this.multiplier
-
-    if (s / 60 >= 1) return `${(s / 60).toFixed(2)}min`
-    if (s / 1 >= 1) return `${s.toFixed(2)}s`
-    if (s / 1e-3 >= 1) return `${(s / 1e-3).toFixed(2)}ms`
-    if (s / 1e-6 >= 1) return `${(s / 1e-6).toFixed(2)}µs`
-    else return `${(s / 1e-9).toFixed(2)}ms`
-  }
-}
-
-export class ByteFormatter implements ValueFormatter {
-  format(v: number) {
-    if (v < 1024) return `${v.toFixed(2)} B`
-    v /= 1024
-    if (v < 1024) return `${v.toFixed(2)} KB`
-    v /= 1024
-    if (v < 1024) return `${v.toFixed(2)} MB`
-    v /= 1024
-    return `${v.toFixed(2)} GB`
-  }
-}
-
 export class Profile {
-  private name: string = ''
+  protected name: string = ''
 
-  private totalWeight: number
+  protected totalWeight: number
 
-  private frames = new Map<string | number, Frame>()
-  private appendOrderCalltreeRoot = new CallTreeNode(rootFrame, null)
-  private groupedCalltreeRoot = new CallTreeNode(rootFrame, null)
+  protected frames = new Map<string | number, Frame>()
+  protected appendOrderCalltreeRoot = new CallTreeNode(rootFrame, null)
+  protected groupedCalltreeRoot = new CallTreeNode(rootFrame, null)
 
   // List of references to CallTreeNodes at the top of the
   // stack at the time of the sample.
-  private samples: CallTreeNode[] = []
-  private weights: number[] = []
+  protected samples: CallTreeNode[] = []
+  protected weights: number[] = []
 
-  private valueFormatter: ValueFormatter = new RawValueFormatter()
+  protected valueFormatter: ValueFormatter = new RawValueFormatter()
 
   constructor(totalWeight: number = 0) {
     this.totalWeight = totalWeight
@@ -248,6 +206,24 @@ export class Profile {
     this.frames.forEach(fn)
   }
 
+  // Demangle symbols for readability
+  async demangle() {
+    let demangleCpp: ((name: string) => string) | null = null
+
+    for (let frame of this.frames.values()) {
+      // This function converts a mangled C++ name such as "__ZNK7Support6ColorFeqERKS0_"
+      // into a human-readable symbol (in this case "Support::ColorF::==(Support::ColorF&)")
+      if (frame.name.startsWith('__Z')) {
+        if (!demangleCpp) {
+          demangleCpp = (await demangleCppModule).demangleCpp
+        }
+        frame.name = demangleCpp(frame.name)
+      }
+    }
+  }
+}
+
+export class StackListProfileBuilder extends Profile {
   _appendSample(stack: FrameInfo[], weight: number, useAppendOrder: boolean) {
     if (isNaN(weight)) throw new Error('invalid weight')
     let node = useAppendOrder ? this.appendOrderCalltreeRoot : this.groupedCalltreeRoot
@@ -295,9 +271,16 @@ export class Profile {
     this._appendSample(stack, weight, false)
   }
 
-  // As an alternative API for importing profiles more efficiently, provide a
-  // way to open & close frames directly without needing to construct tons of
-  // arrays as intermediaries.
+  build(): Profile {
+    this.totalWeight = Math.max(this.totalWeight, this.weights.reduce((a, b) => a + b, 0))
+    return this
+  }
+}
+
+// As an alternative API for importing profiles more efficiently, provide a
+// way to open & close frames directly without needing to construct tons of
+// arrays as intermediaries.
+export class CallTreeProfileBuilder extends Profile {
   private appendOrderStack: CallTreeNode[] = [this.appendOrderCalltreeRoot]
   private groupedOrderStack: CallTreeNode[] = [this.groupedCalltreeRoot]
   private framesInStack = new Map<Frame, number>()
@@ -402,19 +385,7 @@ export class Profile {
     this.totalWeight = Math.max(this.totalWeight, this.lastValue)
   }
 
-  // Demangle symbols for readability
-  async demangle() {
-    let demangleCpp: ((name: string) => string) | null = null
-
-    for (let frame of this.frames.values()) {
-      // This function converts a mangled C++ name such as "__ZNK7Support6ColorFeqERKS0_"
-      // into a human-readable symbol (in this case "Support::ColorF::==(Support::ColorF&)")
-      if (frame.name.startsWith('__Z')) {
-        if (!demangleCpp) {
-          demangleCpp = (await demangleCppModule).demangleCpp
-        }
-        frame.name = demangleCpp(frame.name)
-      }
-    }
+  build(): Profile {
+    return this
   }
 }

value-formatters.test.ts

@@ -0,0 +1,27 @@
+import {TimeFormatter, ByteFormatter} from './value-formatters'
+
+describe('TimeFormatter', () => {
+  test('input units milliseconds', () => {
+    const f = new TimeFormatter('milliseconds')
+    expect(f.format(0.04)).toEqual('40.00µs')
+    expect(f.format(3)).toEqual('3.00ms')
+    expect(f.format(2070)).toEqual('2.07s')
+    expect(f.format(1203123)).toEqual('20.05min')
+  })
+
+  test('input units seconds', () => {
+    const f = new TimeFormatter('seconds')
+    expect(f.format(0.00004)).toEqual('40.00µs')
+    expect(f.format(0.003)).toEqual('3.00ms')
+    expect(f.format(2.07)).toEqual('2.07s')
+    expect(f.format(1203.123)).toEqual('20.05min')
+  })
+})
+
+test('ByteFormatter', () => {
+  const f = new ByteFormatter()
+  expect(f.format(100)).toEqual('100 B')
+  expect(f.format(1024)).toEqual('1.00 KB')
+  expect(f.format(3.5 * 1024 * 1024)).toEqual('3.50 MB')
+  expect(f.format(4.32 * 1024 * 1024 * 1024)).toEqual('4.32 GB')
+})

value-formatters.ts

@@ -0,0 +1,42 @@
+export interface ValueFormatter {
+  format(v: number): string
+}
+
+export class RawValueFormatter implements ValueFormatter {
+  format(v: number) {
+    return v.toLocaleString()
+  }
+}
+
+export class TimeFormatter implements ValueFormatter {
+  private multiplier: number
+
+  constructor(unit: 'nanoseconds' | 'microseconds' | 'milliseconds' | 'seconds') {
+    if (unit === 'nanoseconds') this.multiplier = 1e-9
+    else if (unit === 'microseconds') this.multiplier = 1e-6
+    else if (unit === 'milliseconds') this.multiplier = 1e-3
+    else this.multiplier = 1
+  }
+
+  format(v: number) {
+    const s = v * this.multiplier
+
+    if (s / 60 >= 1) return `${(s / 60).toFixed(2)}min`
+    if (s / 1 >= 1) return `${s.toFixed(2)}s`
+    if (s / 1e-3 >= 1) return `${(s / 1e-3).toFixed(2)}ms`
+    if (s / 1e-6 >= 1) return `${(s / 1e-6).toFixed(2)}µs`
+    else return `${(s / 1e-9).toFixed(2)}ms`
+  }
+}
+
+export class ByteFormatter implements ValueFormatter {
+  format(v: number) {
+    if (v < 1024) return `${v.toFixed(0)} B`
+    v /= 1024
+    if (v < 1024) return `${v.toFixed(2)} KB`
+    v /= 1024
+    if (v < 1024) return `${v.toFixed(2)} MB`
+    v /= 1024
+    return `${v.toFixed(2)} GB`
+  }
+}