Bug 1857214 - [devtools] Avoid creating duplicated identifiers. r=devtools-reviewers,bomsy

We were creating them and later filtering them out. Also `getUniqueIdentifiers` was duplicating a possibly 100M+ large array... Differential Revision: https://phabricator.services.mozilla.com/D190177
2023-10-18 08:00:11 +00:00 · 2023-10-18 08:00:11 +00:00 · 17baf2bc60
--- a/devtools/client/debugger/bin/module-manifest.json
+++ b/devtools/client/debugger/bin/module-manifest.json
@ -2536,7 +2536,7 @@
        "byName": {},
        "byBlocks": {},
        "usedIds": {
-          "0": 0
+          "1": 1
        }
      }
    }
@ -2557,7 +2557,7 @@
        "byName": {},
        "byBlocks": {},
        "usedIds": {
-          "0": 0
+          "1": 1
        }
      }
    }
--- a/devtools/client/debugger/dist/parser-worker.js
+++ b/devtools/client/debugger/dist/parser-worker.js
@ -41349,9 +41349,11 @@
     * @param {Array.<Object>} identifiers
     *        the current list of identifiers where to push the new identifiers
     *        related to this path.
+     * @param {Set<String>} identifiersKeys
+     *        List of currently registered identifier location key.
     * @param {Object} pattern
     */
-    function addPatternIdentifiers(identifiers, pattern) {
+    function addPatternIdentifiers(identifiers, identifiersKeys, pattern) {
      let items;
      if (lib$3.isObjectPattern(pattern)) {
        items = pattern.properties.map(({ value }) => value);
@ -41362,21 +41364,22 @@
      }

      if (items) {
-        addIdentifiers(identifiers, items);
+        addIdentifiers(identifiers, identifiersKeys, items);
      }
    }

-    function addIdentifiers(identifiers, items) {
+    function addIdentifiers(identifiers, identifiersKeys, items) {
      for (const item of items) {
        if (lib$3.isObjectPattern(item) || lib$3.isArrayPattern(item)) {
-          addPatternIdentifiers(identifiers, item);
+          addPatternIdentifiers(identifiers, identifiersKeys, item);
        } else if (lib$3.isIdentifier(item)) {
-          const { start, end } = item.loc;
-          identifiers.push({
-            name: item.name,
-            expression: item.name,
-            location: { start, end },
-          });
+          if (!identifiersKeys.has(nodeLocationKey(item.loc))) {
+            identifiers.push({
+              name: item.name,
+              expression: item.name,
+              location: item.loc,
+            });
+          }
        }
      }
    }
@ -41387,8 +41390,7 @@
      return ancestors.filter(ancestor => ancestor.key == "body").length == 1;
    }

-    function nodeLocationKey(a) {
-      const { start, end } = a.location;
+    function nodeLocationKey({ start, end }) {
      return `${start.line}:${start.column}:${end.line}:${end.column}`;
    }

@ -41730,7 +41732,7 @@
        });
      }

-      getIdentifierSymbols(symbols.identifiers, path);
+      getIdentifierSymbols(symbols.identifiers, symbols.identifiersKeys, path);
    }

    function extractSymbols(sourceId) {
@ -41739,6 +41741,9 @@
        memberExpressions: [],
        comments: [],
        identifiers: [],
+        // This holds a set of unique identifier location key (string)
+        // It helps registering only the first identifier when there is duplicated ones for the same location.
+        identifiersKeys: new Set(),
        classes: [],
        literals: [],
        hasJsx: false,
@ -41766,7 +41771,6 @@

      // comments are extracted separately from the AST
      symbols.comments = getComments(ast);
-      symbols.identifiers = getUniqueIdentifiers(symbols.identifiers);
      symbols.framework = getFramework(symbols);

      return symbols;
@ -42014,20 +42018,6 @@
      };
    }

-    function getUniqueIdentifiers(identifiers) {
-      const newIdentifiers = [];
-      const locationKeys = new Set();
-      for (const newId of identifiers) {
-        const key = nodeLocationKey(newId);
-        if (!locationKeys.has(key)) {
-          locationKeys.add(key);
-          newIdentifiers.push(newId);
-        }
-      }
-
-      return newIdentifiers;
-    }
-
    function getMemberExpressionSymbol(path) {
      const { start, end } = path.node.property.loc;
      return {
@ -42083,16 +42073,19 @@
     * @param {Array.<Object>} identifiers
     *        the current list of identifiers where to push the new identifiers
     *        related to this path.
+     * @param {Set<String>} identifiersKeys
+     *        List of currently registered identifier location key.
     * @param {Object} path
     */
-    function getIdentifierSymbols(identifiers, path) {
+    function getIdentifierSymbols(identifiers, identifiersKeys, path) {
      if (lib$3.isStringLiteral(path) && lib$3.isProperty(path.parentPath)) {
-        const { start, end } = path.node.loc;
-        identifiers.push({
-          name: path.node.value,
-          expression: getObjectExpressionValue(path.parent),
-          location: { start, end },
-        });
+        if (!identifiersKeys.has(nodeLocationKey(path.node.loc))) {
+          identifiers.push({
+            name: path.node.value,
+            expression: getObjectExpressionValue(path.parent),
+            location: path.node.loc,
+          });
+        }
        return;
      }

@ -42103,12 +42096,13 @@
        }

        if (lib$3.isProperty(path.parentPath) && !isObjectShorthand(path.parent)) {
-          const { start, end } = path.node.loc;
-          identifiers.push({
-            name: path.node.name,
-            expression: getObjectExpressionValue(path.parent),
-            location: { start, end },
-          });
+          if (!identifiersKeys.has(nodeLocationKey(path.node.loc))) {
+            identifiers.push({
+              name: path.node.name,
+              expression: getObjectExpressionValue(path.parent),
+              location: path.node.loc,
+            });
+          }
          return;
        }

@ -42118,26 +42112,29 @@
          end = { ...end, column };
        }

-        identifiers.push({
-          name: path.node.name,
-          expression: path.node.name,
-          location: { start, end },
-        });
+        if (!identifiersKeys.has(nodeLocationKey({ start, end }))) {
+          identifiers.push({
+            name: path.node.name,
+            expression: path.node.name,
+            location: { start, end },
+          });
+        }
      }

      if (lib$3.isThisExpression(path.node)) {
-        const { start, end } = path.node.loc;
-        identifiers.push({
-          name: "this",
-          location: { start, end },
-          expression: "this",
-        });
+        if (!identifiersKeys.has(nodeLocationKey(path.node.loc))) {
+          identifiers.push({
+            name: "this",
+            location: path.node.loc,
+            expression: "this",
+          });
+        }
      }

      if (lib$3.isVariableDeclarator(path)) {
        const nodeId = path.node.id;

-        addPatternIdentifiers(identifiers, nodeId);
+        addPatternIdentifiers(identifiers, identifiersKeys, nodeId);
      }
    }

--- a/devtools/client/debugger/src/workers/parser/getSymbols.js
+++ b/devtools/client/debugger/src/workers/parser/getSymbols.js
@ -85,7 +85,7 @@ function extractSymbol(path, symbols, state) {
    });
  }

-  getIdentifierSymbols(symbols.identifiers, path);
+  getIdentifierSymbols(symbols.identifiers, symbols.identifiersKeys, path);
 }

 function extractSymbols(sourceId) {
@ -94,6 +94,9 @@ function extractSymbols(sourceId) {
    memberExpressions: [],
    comments: [],
    identifiers: [],
+    // This holds a set of unique identifier location key (string)
+    // It helps registering only the first identifier when there is duplicated ones for the same location.
+    identifiersKeys: new Set(),
    classes: [],
    literals: [],
    hasJsx: false,
@ -121,7 +124,6 @@ function extractSymbols(sourceId) {

  // comments are extracted separately from the AST
  symbols.comments = getComments(ast);
-  symbols.identifiers = getUniqueIdentifiers(symbols.identifiers);
  symbols.framework = getFramework(symbols);

  return symbols;
@ -369,20 +371,6 @@ export function getSymbols(sourceId) {
  };
 }

-function getUniqueIdentifiers(identifiers) {
-  const newIdentifiers = [];
-  const locationKeys = new Set();
-  for (const newId of identifiers) {
-    const key = nodeLocationKey(newId);
-    if (!locationKeys.has(key)) {
-      locationKeys.add(key);
-      newIdentifiers.push(newId);
-    }
-  }
-
-  return newIdentifiers;
-}
-
 function getMemberExpressionSymbol(path) {
  const { start, end } = path.node.property.loc;
  return {
@ -438,16 +426,19 @@ function getClassDeclarationSymbol(node) {
 * @param {Array.<Object>} identifiers
 *        the current list of identifiers where to push the new identifiers
 *        related to this path.
+ * @param {Set<String>} identifiersKeys
+ *        List of currently registered identifier location key.
 * @param {Object} path
 */
-function getIdentifierSymbols(identifiers, path) {
+function getIdentifierSymbols(identifiers, identifiersKeys, path) {
  if (t.isStringLiteral(path) && t.isProperty(path.parentPath)) {
-    const { start, end } = path.node.loc;
-    identifiers.push({
-      name: path.node.value,
-      expression: getObjectExpressionValue(path.parent),
-      location: { start, end },
-    });
+    if (!identifiersKeys.has(nodeLocationKey(path.node.loc))) {
+      identifiers.push({
+        name: path.node.value,
+        expression: getObjectExpressionValue(path.parent),
+        location: path.node.loc,
+      });
+    }
    return;
  }

@ -458,12 +449,13 @@ function getIdentifierSymbols(identifiers, path) {
    }

    if (t.isProperty(path.parentPath) && !isObjectShorthand(path.parent)) {
-      const { start, end } = path.node.loc;
-      identifiers.push({
-        name: path.node.name,
-        expression: getObjectExpressionValue(path.parent),
-        location: { start, end },
-      });
+      if (!identifiersKeys.has(nodeLocationKey(path.node.loc))) {
+        identifiers.push({
+          name: path.node.name,
+          expression: getObjectExpressionValue(path.parent),
+          location: path.node.loc,
+        });
+      }
      return;
    }

@ -473,25 +465,28 @@ function getIdentifierSymbols(identifiers, path) {
      end = { ...end, column };
    }

-    identifiers.push({
-      name: path.node.name,
-      expression: path.node.name,
-      location: { start, end },
-    });
+    if (!identifiersKeys.has(nodeLocationKey({ start, end }))) {
+      identifiers.push({
+        name: path.node.name,
+        expression: path.node.name,
+        location: { start, end },
+      });
+    }
  }

  if (t.isThisExpression(path.node)) {
-    const { start, end } = path.node.loc;
-    identifiers.push({
-      name: "this",
-      location: { start, end },
-      expression: "this",
-    });
+    if (!identifiersKeys.has(nodeLocationKey(path.node.loc))) {
+      identifiers.push({
+        name: "this",
+        location: path.node.loc,
+        expression: "this",
+      });
+    }
  }

  if (t.isVariableDeclarator(path)) {
    const nodeId = path.node.id;

-    addPatternIdentifiers(identifiers, nodeId);
+    addPatternIdentifiers(identifiers, identifiersKeys, nodeId);
  }
 }
--- a/devtools/client/debugger/src/workers/parser/utils/helpers.js
+++ b/devtools/client/debugger/src/workers/parser/utils/helpers.js
@ -145,9 +145,11 @@ export function getVariables(dec) {
 * @param {Array.<Object>} identifiers
 *        the current list of identifiers where to push the new identifiers
 *        related to this path.
+ * @param {Set<String>} identifiersKeys
+ *        List of currently registered identifier location key.
 * @param {Object} pattern
 */
-export function addPatternIdentifiers(identifiers, pattern) {
+export function addPatternIdentifiers(identifiers, identifiersKeys, pattern) {
  let items;
  if (t.isObjectPattern(pattern)) {
    items = pattern.properties.map(({ value }) => value);
@ -158,21 +160,22 @@ export function addPatternIdentifiers(identifiers, pattern) {
  }

  if (items) {
-    addIdentifiers(identifiers, items);
+    addIdentifiers(identifiers, identifiersKeys, items);
  }
 }

-function addIdentifiers(identifiers, items) {
+function addIdentifiers(identifiers, identifiersKeys, items) {
  for (const item of items) {
    if (t.isObjectPattern(item) || t.isArrayPattern(item)) {
-      addPatternIdentifiers(identifiers, item);
+      addPatternIdentifiers(identifiers, identifiersKeys, item);
    } else if (t.isIdentifier(item)) {
-      const { start, end } = item.loc;
-      identifiers.push({
-        name: item.name,
-        expression: item.name,
-        location: { start, end },
-      });
+      if (!identifiersKeys.has(nodeLocationKey(item.loc))) {
+        identifiers.push({
+          name: item.name,
+          expression: item.name,
+          location: item.loc,
+        });
+      }
    }
  }
 }
@ -183,8 +186,7 @@ export function isTopLevel(ancestors) {
  return ancestors.filter(ancestor => ancestor.key == "body").length == 1;
 }

-export function nodeLocationKey(a) {
-  const { start, end } = a.location;
+export function nodeLocationKey({ start, end }) {
  return `${start.line}:${start.column}:${end.line}:${end.column}`;
 }