Repository: jlfwong/speedscope
Pull request #424 (2023-06-05 21:30:37)

Fix pprof defaultSampleType
Fixes #415

* Interprets pprof's defaultSampleType as an index into the string table [as documented in the proto](https://github.com/jlfwong/speedscope/blob/0414c2f617742e7fb0cf31a66ac0f77c2f5c0540/src/import/profile.proto#L85), not as an index into the sample types repeated-field. This allows parsing to succeed when the string-table index is not a valid sampleTypes index, which is common on allocation profiles.
* Update the pprof snapshot. This is necessarily because we were previously interpreting an empty defaultSampleType as truthy-but-zero when long.js is present, ie: the first sample-type. But Speedscope-in-the-browser doesn't seem to include long.js, so our tests were disagreeing with in-browser behavior. With this PR, that should be fixed.

coveralls: 
[![Coverage Status](https://coveralls.io/builds/60539121/badge)](https://coveralls.io/builds/60539121)

coverage: 42.119% (+0.09%) from 42.032% when pulling **9fe218085db2ca8704ae9481b2522b0d9c3bc8b9 on vasi-stripe:vasi-pprof-default** into **0414c2f617742e7fb0cf31a66ac0f77c2f5c0540 on jlfwong:main**.


Patch:
diff --git a/src/import/pprof.ts b/src/import/pprof.ts
index 1f02f79..eb271eb 100644
--- a/src/import/pprof.ts
+++ b/src/import/pprof.ts
@@ -8,6 +8,25 @@ interface SampleType {
   unit: string
 }
 
+// Find the index of the SampleType which should be used as our default
+function getSampleTypeIndex(profile: perftools.profiles.Profile): number {
+  const dflt = profile.defaultSampleType
+  const sampleTypes = profile.sampleType
+  const fallback = sampleTypes.length - 1
+
+  // string_table[0] will always be empty-string, so we can assume dflt === 0 is just the proto
+  // empty-value, and means no defaultSampleType was specified.
+  if (!dflt || !+dflt) {
+    return fallback
+  }
+
+  const idx = sampleTypes.findIndex(e => e.type === dflt)
+  if (idx === -1) {
+    return fallback
+  }
+  return idx
+}
+
 export function importAsPprofProfile(rawProfile: ArrayBuffer): Profile | null {
   if (rawProfile.byteLength === 0) return null
 
@@ -103,9 +122,7 @@ export function importAsPprofProfile(rawProfile: ArrayBuffer): Profile | null {
     unit: (type.unit && stringVal(type.unit)) || 'count',
   }))
 
-  const sampleTypeIndex = protoProfile.defaultSampleType
-    ? +protoProfile.defaultSampleType
-    : sampleTypes.length - 1
+  const sampleTypeIndex = getSampleTypeIndex(protoProfile)
   const sampleType = sampleTypes[sampleTypeIndex]
 
   const profileBuilder = new StackListProfileBuilder()


Test patch:
diff --git a/src/import/__snapshots__/pprof.test.ts.snap b/src/import/__snapshots__/pprof.test.ts.snap
index 7039b84..69517d5 100644
--- a/src/import/__snapshots__/pprof.test.ts.snap
+++ b/src/import/__snapshots__/pprof.test.ts.snap
@@ -10,7 +10,7 @@ Object {
       "line": 0,
       "name": "runtime.main",
       "selfWeight": 0,
-      "totalWeight": 136,
+      "totalWeight": 1360000000,
     },
     Frame {
       "col": undefined,
@@ -19,7 +19,7 @@ Object {
       "line": 0,
       "name": "main.main",
       "selfWeight": 0,
-      "totalWeight": 136,
+      "totalWeight": 1360000000,
     },
     Frame {
       "col": undefined,
@@ -27,8 +27,8 @@ Object {
       "key": "main.delta:/Users/jlfwong/code/speedscope/sample/programs/go/simple.go:0",
       "line": 0,
       "name": "main.delta",
-      "selfWeight": 22,
-      "totalWeight": 58,
+      "selfWeight": 220000000,
+      "totalWeight": 580000000,
     },
     Frame {
       "col": undefined,
@@ -36,8 +36,8 @@ Object {
       "key": "main.beta:/Users/jlfwong/code/speedscope/sample/programs/go/simple.go:0",
       "line": 0,
       "name": "main.beta",
-      "selfWeight": 39,
-      "totalWeight": 39,
+      "selfWeight": 390000000,
+      "totalWeight": 390000000,
     },
     Frame {
       "col": undefined,
@@ -45,8 +45,8 @@ Object {
       "key": "main.alpha:/Users/jlfwong/code/speedscope/sample/programs/go/simple.go:0",
       "line": 0,
       "name": "main.alpha",
-      "selfWeight": 48,
-      "totalWeight": 48,
+      "selfWeight": 480000000,
+      "totalWeight": 480000000,
     },
     Frame {
       "col": undefined,
@@ -54,8 +54,8 @@ Object {
       "key": "main.gamma:/Users/jlfwong/code/speedscope/sample/programs/go/simple.go:0",
       "line": 0,
       "name": "main.gamma",
-      "selfWeight": 27,
-      "totalWeight": 27,
+      "selfWeight": 270000000,
+      "totalWeight": 270000000,
     },
     Frame {
       "col": undefined,
@@ -64,7 +64,7 @@ Object {
       "line": 0,
       "name": "runtime.morestack",
       "selfWeight": 0,
-      "totalWeight": 11,
+      "totalWeight": 110000000,
     },
     Frame {
       "col": undefined,
@@ -73,7 +73,7 @@ Object {
       "line": 0,
       "name": "runtime.newstack",
       "selfWeight": 0,
-      "totalWeight": 11,
+      "totalWeight": 110000000,
     },
     Frame {
       "col": undefined,
@@ -81,8 +81,8 @@ Object {
       "key": "runtime.duffcopy:/usr/local/Cellar/go/1.10.1/libexec/src/runtime/duff_amd64.s:0",
       "line": 0,
       "name": "runtime.duffcopy",
-      "selfWeight": 11,
-      "totalWeight": 11,
+      "selfWeight": 110000000,
+      "totalWeight": 110000000,
     },
     Frame {
       "col": undefined,
@@ -91,7 +91,7 @@ Object {
       "line": 0,
       "name": "runtime.timerproc",
       "selfWeight": 0,
-      "totalWeight": 1,
+      "totalWeight": 10000000,
     },
     Frame {
       "col": undefined,
@@ -100,7 +100,7 @@ Object {
       "line": 0,
       "name": "runtime.goroutineReady",
       "selfWeight": 0,
-      "totalWeight": 1,
+      "totalWeight": 10000000,
     },
     Frame {
       "col": undefined,
@@ -109,7 +109,7 @@ Object {
       "line": 0,
       "name": "runtime.goready",
       "selfWeight": 0,
-      "totalWeight": 1,
+      "totalWeight": 10000000,
     },
     Frame {
       "col": undefined,
@@ -118,7 +118,7 @@ Object {
       "line": 0,
       "name": "runtime.systemstack",
       "selfWeight": 0,
-      "totalWeight": 1,
+      "totalWeight": 10000000,
     },
     Frame {
       "col": undefined,
@@ -127,7 +127,7 @@ Object {
       "line": 0,
       "name": "runtime.goready.func1",
       "selfWeight": 0,
-      "totalWeight": 1,
+      "totalWeight": 10000000,
     },
     Frame {
       "col": undefined,
@@ -136,7 +136,7 @@ Object {
       "line": 0,
       "name": "runtime.ready",
       "selfWeight": 0,
-      "totalWeight": 1,
+      "totalWeight": 10000000,
     },
     Frame {
       "col": undefined,
@@ -145,7 +145,7 @@ Object {
       "line": 0,
       "name": "runtime.wakep",
       "selfWeight": 0,
-      "totalWeight": 1,
+      "totalWeight": 10000000,
     },
     Frame {
       "col": undefined,
@@ -154,7 +154,7 @@ Object {
       "line": 0,
       "name": "runtime.startm",
       "selfWeight": 0,
-      "totalWeight": 1,
+      "totalWeight": 10000000,
     },
     Frame {
       "col": undefined,
@@ -163,7 +163,7 @@ Object {
       "line": 0,
       "name": "runtime.notewakeup",
       "selfWeight": 0,
-      "totalWeight": 1,
+      "totalWeight": 10000000,
     },
     Frame {
       "col": undefined,
@@ -172,7 +172,7 @@ Object {
       "line": 0,
       "name": "runtime.semawakeup",
       "selfWeight": 0,
-      "totalWeight": 1,
+      "totalWeight": 10000000,
     },
     Frame {
       "col": undefined,
@@ -181,7 +181,7 @@ Object {
       "line": 0,
       "name": "runtime.mach_semrelease",
       "selfWeight": 0,
-      "totalWeight": 1,
+      "totalWeight": 10000000,
     },
     Frame {
       "col": undefined,
@@ -189,8 +189,8 @@ Object {
       "key": "runtime.mach_semaphore_signal:/usr/local/Cellar/go/1.10.1/libexec/src/runtime/sys_darwin_amd64.s:0",
       "line": 0,
       "name": "runtime.mach_semaphore_signal",
-      "selfWeight": 1,
-      "totalWeight": 1,
+      "selfWeight": 10000000,
+      "totalWeight": 10000000,
     },
     Frame {
       "col": undefined,
@@ -199,7 +199,7 @@ Object {
       "line": 0,
       "name": "runtime.mstart",
       "selfWeight": 0,
-      "totalWeight": 2,
+      "totalWeight": 20000000,
     },
     Frame {
       "col": undefined,
@@ -208,7 +208,7 @@ Object {
       "line": 0,
       "name": "runtime.mstart1",
       "selfWeight": 0,
-      "totalWeight": 2,
+      "totalWeight": 20000000,
     },
     Frame {
       "col": undefined,
@@ -217,7 +217,7 @@ Object {
       "line": 0,
       "name": "runtime.sysmon",
       "selfWeight": 0,
-      "totalWeight": 2,
+      "totalWeight": 20000000,
     },
     Frame {
       "col": undefined,
@@ -225,21 +225,21 @@ Object {
       "key": "runtime.usleep:/usr/local/Cellar/go/1.10.1/libexec/src/runtime/sys_darwin_amd64.s:0",
       "line": 0,
       "name": "runtime.usleep",
-      "selfWeight": 2,
-      "totalWeight": 2,
+      "selfWeight": 20000000,
+      "totalWeight": 20000000,
     },
   ],
   "name": "simple.prof",
   "stacks": Array [
-    "runtime.main;main.main;main.delta;main.beta 14",
-    "runtime.main;main.main;main.alpha 26",
-    "runtime.main;main.main;main.gamma 27",
-    "runtime.morestack;runtime.newstack;runtime.duffcopy 11",
-    "runtime.main;main.main;main.beta 25",
-    "runtime.main;main.main;main.delta 22",
-    "runtime.main;main.main;main.delta;main.alpha 22",
-    "runtime.timerproc;runtime.goroutineReady;runtime.goready;runtime.systemstack;runtime.goready.func1;runtime.ready;runtime.wakep;runtime.startm;runtime.notewakeup;runtime.semawakeup;runtime.mach_semrelease;runtime.mach_semaphore_signal 1",
-    "runtime.mstart;runtime.mstart1;runtime.sysmon;runtime.usleep 2",
+    "runtime.main;main.main;main.delta;main.beta 140.00ms",
+    "runtime.main;main.main;main.alpha 260.00ms",
+    "runtime.main;main.main;main.gamma 270.00ms",
+    "runtime.morestack;runtime.newstack;runtime.duffcopy 110.00ms",
+    "runtime.main;main.main;main.beta 250.00ms",
+    "runtime.main;main.main;main.delta 220.00ms",
+    "runtime.main;main.main;main.delta;main.alpha 220.00ms",
+    "runtime.timerproc;runtime.goroutineReady;runtime.goready;runtime.systemstack;runtime.goready.func1;runtime.ready;runtime.wakep;runtime.startm;runtime.notewakeup;runtime.semawakeup;runtime.mach_semrelease;runtime.mach_semaphore_signal 10.00ms",
+    "runtime.mstart;runtime.mstart1;runtime.sysmon;runtime.usleep 20.00ms",
   ],
 }
 `;