Flame chart review comments

m1rp · Feb 13, 2017 · b224e93 · b224e93
1 parent 89f381d
commit b224e93
Show file tree

Hide file tree

Showing 7 changed files with 22 additions and 17 deletions.
diff --git a/.eslintrc.js b/.eslintrc.js
@@ -24,6 +24,7 @@ module.exports = {
         "flowtype/generic-spacing": [2, "never"],
         "flowtype/space-before-type-colon": [ 2, "never" ],
         "flowtype/space-after-type-colon": [ 2, "always" ],
+        "flowtype/delimiter-dangle": [ 2, "always-multiline" ],
 
         // JS Rules:
         "indent": [

diff --git a/src/common/summarize-profile.js b/src/common/summarize-profile.js
@@ -7,7 +7,7 @@ type Summary = { [id: string]: number };
 type StacksInCategory = { [id: string]: { [id: string]: number } }
 type SummarySegment = {
   percentage: {[id: string]: number},
-  samples: {[id: string]: number}
+  samples: {[id: string]: number},
 }
 type RollingSummary = SummarySegment[];
 type Categories = Array<(string|null)>;
@@ -229,10 +229,13 @@ function calculateSummaryPercentages(summary: Summary) {
       const percentage = samples / sampleCount;
       return { category, samples, percentage };
     })
-    // Sort by name first so that the results are deterministic.
-    .sort((a, b) => a.category > b.category ? -1 : 1)
-    // Sort by sample count second.
-    .sort((a, b) => b.samples - a.samples);
+    // Sort by sample count, then by name so that the results are deterministic.
+    .sort((a, b) => {
+      if (a.samples === b.samples) {
+        return a.category.localeCompare(b.category);
+      }
+      return b.samples - a.samples;
+    });
 }
 
 function logStacks(stacksInCategory: StacksInCategory, maxLogLength = 10) {

diff --git a/src/common/test/summarize-profile.js b/src/common/test/summarize-profile.js
@@ -31,10 +31,10 @@ describe('summarize-profile', function () {
       { category: 'script.compile', samples: 16 },
       { category: 'script.compile.baseline', samples: 14 },
       { category: 'frameconstruction', samples: 6 },
-      { category: 'script.parse', samples: 4 },
       { category: 'network', samples: 4 },
-      { category: 'script.compile.ion', samples: 2 },
+      { category: 'script.parse', samples: 4 },
       { category: 'dom.wait', samples: 2 },
+      { category: 'script.compile.ion', samples: 2 },
       { category: 'script.icupdate', samples: 2 },
       { category: 'CC', samples: 1 },
       { category: 'CC.wait', samples: 1 },

diff --git a/src/common/types/profile-derived.js b/src/common/types/profile-derived.js
@@ -10,5 +10,5 @@ export type FuncStackTable = {
 
 export type FuncStackInfo = {
   funcStackTable: FuncStackTable,
-  stackIndexToFuncStackIndex: Uint32Array
+  stackIndexToFuncStackIndex: Uint32Array,
 }
diff --git a/src/common/types/profile.js b/src/common/types/profile.js
@@ -9,6 +9,7 @@ export type IndexIntoResourceTable = number;
 export type IndexIntoLibs = number;
 export type categoryBitMask = number;
 export type resourceTypeEnum = number;
+export type MemoryOffset = number;
 
 export type StackTable = {
   frame: number[],
@@ -33,7 +34,7 @@ export type Marker = {
   dur?: number,
   title?: string,
   start?: number,
-  name?: string
+  name?: string,
 };
 
 export type MarkersTable = {
@@ -50,7 +51,7 @@ export type FrameTable = {
   implementation: (IndexIntoStringTable | null)[],
   line: (number | null)[],
   optimizations: ({} | null)[],
-  length: number
+  length: number,
 };
 
 export type StringTable = {
@@ -62,7 +63,7 @@ export type StringTable = {
 };
 
 export type FuncTable = {
-  address: IndexIntoStringTable[],
+  address: MemoryOffset[],
   libs: {
     breakpadId: string,
     end: number,
@@ -83,7 +84,7 @@ export type ResourceTable = {
   length: number,
   lib: IndexIntoLibs[],
   name: IndexIntoStringTable,
-  type: resourceTypeEnum
+  type: resourceTypeEnum,
 }
 
 export type Thread = {
@@ -97,11 +98,11 @@ export type Thread = {
   stringTable: StringTable,
   libs: [],
   funcTable: FuncTable,
-  resourceTable: {}
+  resourceTable: {},
 };
 
 export type ProfileMeta = {
-  interval: number
+  interval: number,
 };
 
 export type TaskTracer = {};

diff --git a/src/content/profile-data.js b/src/content/profile-data.js
@@ -212,7 +212,7 @@ export function filterThreadToJSOnly(thread: Thread) {
               const newFuncIndex = newFuncTable.length++;
               newFuncTable.name.push(stringTable.indexForString('Platform'));
               newFuncTable.resource.push(null);
-              newFuncTable.address.push(stringTable.indexForString(''));
+              newFuncTable.address.push(-1);
               newFuncTable.isJS.push(false);
 
               newFrameTable.implementation.push(null);

diff --git a/src/content/stack-timing.js b/src/content/stack-timing.js
@@ -33,13 +33,13 @@ import type { FuncStackInfo } from '../common/types/profile-derived';
  *   {start: [20, 40, 60], end: [40, 60, 80], stack: [1, 2, 3]}
  *   {start: [20, 40, 60], end: [40, 60, 80], stack: [34, 59, 72]}
  *   ...
- *   {start: [25, 45], end: [35, 55], stack: [123, 159, 160]}
+ *   {start: [25, 45], end: [35, 55], stack: [123, 159]}
  * ]
  */
 export type StackTimingByDepth = Array<{
   // Start time of stack in milliseconds.
   start: number[],
-  // Start time of stack in milliseconds.
+  // End time of stack in milliseconds.
   end: number[],
   stack: IndexIntoStackTable[],
   length: number,