Bug 1534342 - Fix O(n^2) perf issues in getSymbols. r=jlast
authorLogan Smyth <loganfsmyth@gmail.com>
Mon, 11 Mar 2019 22:34:39 +0000
changeset 521451 a7dee08698fc6aa4a15cafb3052e4f4c0d41ed0a
parent 521450 95274fd66e5bea637cc26d7047aef7f96d5e19a7
child 521452 985e05a4b54024ae40be6d94aaf8d9fb3d66f9b2
push id10866
push usernerli@mozilla.com
push dateTue, 12 Mar 2019 18:59:09 +0000
treeherdermozilla-beta@445c24a51727 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjlast
bugs1534342
milestone67.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1534342 - Fix O(n^2) perf issues in getSymbols. r=jlast Differential Revision: https://phabricator.services.mozilla.com/D22997
devtools/client/debugger/new/dist/parser-worker.js
devtools/client/debugger/new/panel.js
devtools/client/debugger/new/src/actions/sources/loadSourceText.js
devtools/client/debugger/new/src/workers/parser/getSymbols.js
devtools/client/shared/view-source.js
--- a/devtools/client/debugger/new/dist/parser-worker.js
+++ b/devtools/client/debugger/new/dist/parser-worker.js
@@ -11402,29 +11402,35 @@ function getUniqueIdentifiers(identifier
       newIdentifiers.push(newId);
     }
   }
 
   return newIdentifiers;
 }
 
 /* eslint-disable complexity */
-function extractSymbol(path, symbols) {
+function extractSymbol(path, symbols, state) {
   if ((0, _helpers.isFunction)(path)) {
     const name = (0, _getFunctionName2.default)(path.node, path.parent);
+
+    if (!state.fnCounts[name]) {
+      state.fnCounts[name] = 0;
+    }
+    const index = state.fnCounts[name]++;
+
     symbols.functions.push({
       name,
       klass: (0, _inferClassName.inferClassName)(path),
       location: path.node.loc,
       parameterNames: (0, _helpers.getFunctionParameterNames)(path),
       identifier: path.node.id,
       // indicates the occurence of the function in a file
       // e.g { name: foo, ... index: 4 } is the 4th foo function
       // in the file
-      index: symbols.functions.filter(f => f.name === name).length
+      index
     });
   }
 
   if (t.isJSXElement(path)) {
     symbols.hasJsx = true;
   }
 
   if (t.isGenericTypeAnnotation(path)) {
@@ -11546,18 +11552,17 @@ function extractSymbol(path, symbols) {
       location: { start, end },
       expression: "this"
     });
   }
 
   if (t.isVariableDeclarator(path)) {
     const nodeId = path.node.id;
 
-    const ids = (0, _helpers.getPatternIdentifiers)(nodeId);
-    symbols.identifiers = [...symbols.identifiers, ...ids];
+    symbols.identifiers.push(...(0, _helpers.getPatternIdentifiers)(nodeId));
   }
 }
 
 /* eslint-enable complexity */
 
 function extractSymbols(sourceId) {
   const symbols = {
     functions: [],
@@ -11569,22 +11574,26 @@ function extractSymbols(sourceId) {
     classes: [],
     imports: [],
     literals: [],
     hasJsx: false,
     hasTypes: false,
     loading: false
   };
 
+  const state = {
+    fnCounts: Object.create(null)
+  };
+
   const ast = (0, _ast.traverseAst)(sourceId, {
     enter(node, ancestors) {
       try {
         const path = (0, _simplePath2.default)(ancestors);
         if (path) {
-          extractSymbol(path, symbols);
+          extractSymbol(path, symbols, state);
         }
       } catch (e) {
         console.error(e);
       }
     }
   });
 
   // comments are extracted separately from the AST
--- a/devtools/client/debugger/new/panel.js
+++ b/devtools/client/debugger/new/panel.js
@@ -124,21 +124,21 @@ DebuggerPanel.prototype = {
     return this._actions.getMappedExpression(expression);
   },
 
   isPaused() {
     return this._selectors.isPaused(this._getState());
   },
 
   selectSourceURL(url, line) {
-    this._actions.selectSourceURL(url, { line });
+    return this._actions.selectSourceURL(url, { line });
   },
 
   selectSource(sourceId, line) {
-    this._actions.selectSource(sourceId, { line });
+    return this._actions.selectSource(sourceId, { line });
   },
 
   getSourceByActorId(sourceId) {
     return this._selectors.getSourceByActorId(this._getState(), sourceId);
   },
 
   getSourceByURL(sourceURL) {
     return this._selectors.getSourceByURL(this._getState(), sourceURL);
--- a/devtools/client/debugger/new/src/actions/sources/loadSourceText.js
+++ b/devtools/client/debugger/new/src/actions/sources/loadSourceText.js
@@ -79,17 +79,17 @@ export function loadSourceText(source: ?
     }
 
     const newSource = getSource(getState(), source.id);
     if (!newSource) {
       return;
     }
 
     if (!newSource.isWasm && isLoaded(newSource)) {
-      await parser.setSource(newSource);
+      parser.setSource(newSource);
       await dispatch(setBreakpointPositions(newSource.id));
     }
 
     // signal that the action is finished
     deferred.resolve();
     requests.delete(id);
 
     return source;
--- a/devtools/client/debugger/new/src/workers/parser/getSymbols.js
+++ b/devtools/client/debugger/new/src/workers/parser/getSymbols.js
@@ -95,29 +95,35 @@ function getUniqueIdentifiers(identifier
       newIdentifiers.push(newId);
     }
   }
 
   return newIdentifiers;
 }
 
 /* eslint-disable complexity */
-function extractSymbol(path: SimplePath, symbols) {
+function extractSymbol(path: SimplePath, symbols, state) {
   if (isFunction(path)) {
     const name = getFunctionName(path.node, path.parent);
+
+    if (!state.fnCounts[name]) {
+      state.fnCounts[name] = 0;
+    }
+    const index = state.fnCounts[name]++;
+
     symbols.functions.push({
       name,
       klass: inferClassName(path),
       location: path.node.loc,
       parameterNames: getFunctionParameterNames(path),
       identifier: path.node.id,
       // indicates the occurence of the function in a file
       // e.g { name: foo, ... index: 4 } is the 4th foo function
       // in the file
-      index: symbols.functions.filter(f => f.name === name).length
+      index
     });
   }
 
   if (t.isJSXElement(path)) {
     symbols.hasJsx = true;
   }
 
   if (t.isGenericTypeAnnotation(path)) {
@@ -246,18 +252,17 @@ function extractSymbol(path: SimplePath,
       location: { start, end },
       expression: "this"
     });
   }
 
   if (t.isVariableDeclarator(path)) {
     const nodeId = path.node.id;
 
-    const ids = getPatternIdentifiers(nodeId);
-    symbols.identifiers = [...symbols.identifiers, ...ids];
+    symbols.identifiers.push(...getPatternIdentifiers(nodeId));
   }
 }
 
 /* eslint-enable complexity */
 
 function extractSymbols(sourceId): SymbolDeclarations {
   const symbols = {
     functions: [],
@@ -269,22 +274,26 @@ function extractSymbols(sourceId): Symbo
     classes: [],
     imports: [],
     literals: [],
     hasJsx: false,
     hasTypes: false,
     loading: false
   };
 
+  const state = {
+    fnCounts: Object.create(null)
+  };
+
   const ast = traverseAst(sourceId, {
     enter(node: Node, ancestors: TraversalAncestors) {
       try {
         const path = createSimplePath(ancestors);
         if (path) {
-          extractSymbol(path, symbols);
+          extractSymbol(path, symbols, state);
         }
       } catch (e) {
         console.error(e);
       }
     }
   });
 
   // comments are extracted separately from the AST
--- a/devtools/client/shared/view-source.js
+++ b/devtools/client/shared/view-source.js
@@ -51,23 +51,33 @@ exports.viewSourceInStyleEditor = async 
  */
 exports.viewSourceInDebugger = async function(toolbox, sourceURL, sourceLine, sourceId,
                                               reason = "unknown") {
   const dbg = await toolbox.loadTool("jsdebugger");
   const source =
     sourceId ? dbg.getSourceByActorId(sourceId) : dbg.getSourceByURL(sourceURL);
   if (source) {
     await toolbox.selectTool("jsdebugger", reason);
-    dbg.selectSource(source.id, sourceLine);
+    try {
+      await dbg.selectSource(source.id, sourceLine);
+    } catch (err) {
+      console.error("Failed to view source in debugger", err);
+      return false;
+    }
     return true;
   } else if (await toolbox.sourceMapService.hasOriginalURL(sourceURL)) {
     // We have seen a source map for the URL but no source. The debugger will
     // still be able to load the source.
     await toolbox.selectTool("jsdebugger", reason);
-    dbg.selectSourceURL(sourceURL, sourceLine);
+    try {
+      await dbg.selectSourceURL(sourceURL, sourceLine);
+    } catch (err) {
+      console.error("Failed to view source in debugger", err);
+      return false;
+    }
     return true;
   }
 
   exports.viewSource(toolbox, sourceURL, sourceLine);
   return false;
 };
 
 /**