Bug 1362786 - (3) Run CppFiltParser in worker r?kmag draft
authorDoug Thayer <dothayer@mozilla.com>
Fri, 21 Jul 2017 14:13:37 -0700
changeset 613395 5c49e0133e908a77004806f77495d2992c579f18
parent 613393 e1f1be5e49efedf1e78a5813af8d8789774deb08
child 638661 453bba0ea02a12615853ddd2807ceefc65d17b5c
push id69781
push userbmo:dothayer@mozilla.com
push dateFri, 21 Jul 2017 21:24:31 +0000
reviewerskmag
bugs1362786
milestone56.0a1
Bug 1362786 - (3) Run CppFiltParser in worker r?kmag Pulls out the CppFiltParser work into a worker to run in the background and avoid the jank induced by the final convertSymsMapToExpectedSymFormat call. Profiling shows that after this change there is virtually no jank when asking the geckoProfiler API for symbols. MozReview-Commit-ID: CAKMvjLa4dl
browser/components/extensions/ParseCppFiltSymbols-worker.js
browser/components/extensions/ParseSymbols.jsm
browser/components/extensions/ext-geckoProfiler.js
browser/components/extensions/moz.build
new file mode 100644
--- /dev/null
+++ b/browser/components/extensions/ParseCppFiltSymbols-worker.js
@@ -0,0 +1,64 @@
+/* -*- Mode: indent-tabs-mode: nil; js-indent-level: 2 -*- */
+/* vim: set sts=2 sw=2 et tw=80: */
+/* eslint-env worker */
+
+"use strict";
+
+importScripts("resource:///modules/ParseSymbols.jsm");
+
+class WorkerCppFiltParser {
+  constructor(length) {
+    this._decoder = new TextDecoder();
+    this._index = 0;
+    this._approximateLength = 0;
+    this._results = new Array(length);
+  }
+
+  consume(arrayBuffer) {
+    const data = this._decoder.decode(arrayBuffer, {stream: true});
+    const lineRegex = /.*\n?/g;
+    const buffer = this._currentLine + data;
+
+    let match;
+    while ((match = lineRegex.exec(buffer))) {
+      let [line] = match;
+      if (line[line.length - 1] === "\n") {
+        this._processLine(line);
+      } else {
+        this._currentLine = line;
+        break;
+      }
+    }
+  }
+
+  finish() {
+    this._processLine(this._currentLine);
+    return {symsArray: this._results, approximateLength: this._approximateLength};
+  }
+
+  _processLine(line) {
+    const trimmed = line.trimRight();
+    this._approximateLength += trimmed.length;
+    this._results[this._index++] = trimmed;
+  }
+}
+
+let cppFiltParser;
+onmessage = async e => {
+  try {
+    if (!cppFiltParser) {
+      cppFiltParser = new WorkerCppFiltParser();
+    }
+    if (e.data.finish) {
+      const {symsArray, approximateLength} = cppFiltParser.finish();
+      const result = ParseSymbols.convertSymsArrayToExpectedSymFormat(symsArray, approximateLength);
+
+      postMessage({result}, result.map(r => r.buffer));
+      close();
+    } else {
+      cppFiltParser.consume(e.data.buffer);
+    }
+  } catch (error) {
+    postMessage({error: error.toString()});
+  }
+};
--- a/browser/components/extensions/ParseSymbols.jsm
+++ b/browser/components/extensions/ParseSymbols.jsm
@@ -39,23 +39,30 @@ function convertSymsMapToExpectedSymForm
 
   const symsArray = addresses.map(addr => syms.get(addr));
   const {index, buffer} =
     convertStringArrayToUint8BufferWithIndex(symsArray, approximateSymLength);
 
   return [new Uint32Array(addresses), index, buffer];
 }
 
+function convertSymsArrayToExpectedSymFormat(symsArray, approximateSymLength) {
+  const {index, buffer} =
+    convertStringArrayToUint8BufferWithIndex(symsArray, approximateSymLength);
+  return [index, buffer];
+}
+
 function convertSymsMapToDemanglerFormat(syms) {
   const addresses = Array.from(syms.keys());
   addresses.sort((a, b) => a - b);
 
   const symsArray = addresses.map(addr => syms.get(addr));
   const textEncoder = new TextEncoder();
   const buffer = textEncoder.encode(symsArray.join("\n"));
 
   return [new Uint32Array(addresses), buffer];
 }
 
 var ParseSymbols = {
   convertSymsMapToExpectedSymFormat,
+  convertSymsArrayToExpectedSymFormat,
   convertSymsMapToDemanglerFormat,
 };
--- a/browser/components/extensions/ext-geckoProfiler.js
+++ b/browser/components/extensions/ext-geckoProfiler.js
@@ -60,46 +60,38 @@ class NMParser {
       finish: true,
       isDarwin: Services.appinfo.OS === "Darwin",
     });
     return promise;
   }
 }
 
 class CppFiltParser {
-  constructor(length) {
-    this._decoder = new TextDecoder();
-    this._index = 0;
-    this._results = new Array(length);
+  constructor() {
+    this._worker = new ChromeWorker("resource://app/modules/ParseCppFiltSymbols-worker.js");
   }
 
-  consume(arrayBuffer) {
-    const data = this._decoder.decode(arrayBuffer, {stream: true});
-    const lineRegex = /.*\n?/g;
-    const buffer = this._currentLine + data;
-
-    let match;
-    while ((match = lineRegex.exec(buffer))) {
-      let [line] = match;
-      if (line[line.length - 1] === "\n") {
-        this._processLine(line);
-      } else {
-        this._currentLine = line;
-        break;
-      }
-    }
+  consume(buffer) {
+    this._worker.postMessage({buffer}, [buffer]);
   }
 
   finish() {
-    this._processLine(this._currentLine);
-    return this._results;
-  }
-
-  _processLine(line) {
-    this._results[this._index++] = line.trimRight();
+    const promise = new Promise((resolve, reject) => {
+      this._worker.onmessage = (e) => {
+        if (e.data.error) {
+          reject(e.data.error);
+        } else {
+          resolve(e.data.result);
+        }
+      };
+    });
+    this._worker.postMessage({
+      finish: true,
+    });
+    return promise;
   }
 }
 
 const readAllData = async function(pipe, processData) {
   let data;
   while ((data = await pipe.read()) && data.byteLength) {
     processData(data);
   }
@@ -135,30 +127,23 @@ const getSymbolsFromNM = async function(
 
   await spawnProcess("nm", args, data => parser.consume(data));
   await spawnProcess("nm", ["-D", ...args], data => parser.consume(data));
   let result = await parser.finish();
   if (Services.appinfo.OS !== "Darwin") {
     return result;
   }
 
-  const syms = new Map();
   const [addresses, symbolsJoinedBuffer] = result;
   const decoder = new TextDecoder();
   const symbolsJoined = decoder.decode(symbolsJoinedBuffer);
   const demangler = new CppFiltParser(addresses.length);
   await spawnProcess("c++filt", [], data => demangler.consume(data), symbolsJoined);
-  const newSymbols = demangler.finish();
-  let approximateLength = 0;
-  for (let [i, symbol] of newSymbols.entries()) {
-    approximateLength += symbol.length;
-    syms.set(addresses[i], symbol);
-  }
-
-  return ParseSymbols.convertSymsMapToExpectedSymFormat(syms, approximateLength);
+  const [newIndex, newBuffer] = await demangler.finish();
+  return [addresses, newIndex, newBuffer];
 };
 
 const pathComponentsForSymbolFile = (debugName, breakpadId) => {
   const symName = debugName.replace(/(\.pdb)?$/, ".sym");
   return [debugName, breakpadId, symName];
 };
 
 const urlForSymFile = (debugName, breakpadId) => {
--- a/browser/components/extensions/moz.build
+++ b/browser/components/extensions/moz.build
@@ -11,16 +11,17 @@ JAR_MANIFESTS += ['jar.mn']
 
 EXTRA_COMPONENTS += [
     'extensions-browser.manifest',
 ]
 
 EXTRA_JS_MODULES += [
     'ExtensionPopups.jsm',
     'ParseBreakpadSymbols-worker.js',
+    'ParseCppFiltSymbols-worker.js',
     'ParseNMSymbols-worker.js',
     'ParseSymbols.jsm',
 ]
 
 DIRS += ['schemas']
 
 BROWSER_CHROME_MANIFESTS += [
     'test/browser/browser-remote.ini',