Repository: facebook/react
Pull request: DevTools: Hook names optimizations

This PR dramatically improves the performance of the hook names feature by replacing the `source-map-js` integration with custom mapping code built on top of `sourcemap-codec`. Based on [my own benchmarking](https://github.com/bvaughn/source-map-parsing-benchmark), this makes parsing 3-4 times faster. (The bulk of these changes are in `SourceMapConsumer.js`.) For example:
![image](https://user-images.githubusercontent.com/29597/134433302-d73e7b5a-d688-40ce-813c-4e834aac02d8.png)

While implementing this code, I also uncovered a problem with the way we were caching source-map metadata that was causing us to potential parse the same source-map multiple times. (I addressed this in a separate commit for easier reviewing. The bulk of these changes are in `parseSourceAndMetadata.js`.)

Altogether these changes dramatically improve the performance of the hooks parsing code:
![before-and-after](https://user-images.githubusercontent.com/29597/134433357-b9fbe1ad-789c-407e-8998-a19fcc27faa8.png)

One additional thing we could look into if the source-map download still remains a large bottleneck would be to [stream it](https://developer.mozilla.org/en-US/docs/Web/API/Streams_API/Using_readable_streams) and decode the `mappings` array _while it streams in_ rather than in one synchronous chunk after the full source-map has been downloaded.

Changed files:
filename: packages/react-devtools-shared/src/hooks/SourceMapConsumer.js
patch: @@ -0,0 +1,241 @@
+/**
+ * Copyright (c) Facebook, Inc. and its affiliates.
+ *
+ * This source code is licensed under the MIT license found in the
+ * LICENSE file in the root directory of this source tree.
+ *
+ * @flow
+ */
+import {withSyncPerfMeasurements} from 'react-devtools-shared/src/PerformanceLoggingUtils';
+import {decode} from 'sourcemap-codec';
+
+import type {
+  IndexSourceMap,
+  BasicSourceMap,
+  MixedSourceMap,
+} from './SourceMapTypes';
+
+type SearchPosition = {|
+  columnNumber: number,
+  lineNumber: number,
+|};
+
+type ResultPosition = {|
+  column: number,
+  line: number,
+  sourceContent: string,
+  sourceURL: string,
+|};
+
+export type SourceMapConsumerType = {|
+  originalPositionFor: SearchPosition => ResultPosition,
+|};
+
+type Mappings = Array<Array<Array<number>>>;
+
+export default function SourceMapConsumer(
+  sourceMapJSON: MixedSourceMap,
+): SourceMapConsumerType {
+  if (sourceMapJSON.sections != null) {
+    return IndexedSourceMapConsumer(((sourceMapJSON: any): IndexSourceMap));
+  } else {
+    return BasicSourceMapConsumer(((sourceMapJSON: any): BasicSourceMap));
+  }
+}
+
+function BasicSourceMapConsumer(sourceMapJSON: BasicSourceMap) {
+  const decodedMappings: Mappings = withSyncPerfMeasurements(
+    'Decoding source map mappings with sourcemap-codec',
+    () => decode(sourceMapJSON.mappings),
+  );
+
+  function originalPositionFor({
+    columnNumber,
+    lineNumber,
+  }: SearchPosition): ResultPosition {
+    // Error.prototype.stack columns are 1-based (like most IDEs) but ASTs are 0-based.
+    const targetColumnNumber = columnNumber - 1;
+
+    const lineMappings = decodedMappings[lineNumber - 1];
+
+    let nearestEntry = null;
+
+    let startIndex = 0;
+    let stopIndex = lineMappings.length - 1;
+    let index = -1;
+    while (startIndex <= stopIndex) {
+      index = Math.floor((stopIndex + startIndex) / 2);
+      nearestEntry = lineMappings[index];
+
+      const currentColumn = nearestEntry[0];
+      if (currentColumn === targetColumnNumber) {
+        break;
+      } else {
+        if (currentColumn > targetColumnNumber) {
+          if (stopIndex - index > 0) {
+            stopIndex = index;
+          } else {
+            index = stopIndex;
+            break;
+          }
+        } else {
+          if (index - startIndex > 0) {
+            startIndex = index;
+          } else {
+            index = startIndex;
+            break;
+          }
+        }
+      }
+    }
+
+    // We have found either the exact element, or the next-closest element.
+    // However there may be more than one such element.
+    // Make sure we always return the smallest of these.
+    while (index > 0) {
+      const previousEntry = lineMappings[index - 1];
+      const currentColumn = previousEntry[0];
+      if (currentColumn !== targetColumnNumber) {
+        break;
+      }
+      index--;
+    }
+
+    if (nearestEntry == null) {
+      // TODO maybe fall back to the runtime source instead of throwing?
+      throw Error(
+        `Could not find runtime location for line:${lineNumber} and column:${columnNumber}`,
+      );
+    }
+
+    const sourceIndex = nearestEntry[1];
+    const sourceContent =
+      sourceMapJSON.sourcesContent != null
+        ? sourceMapJSON.sourcesContent[sourceIndex]
+        : null;
+    const sourceURL = sourceMapJSON.sources[sourceIndex] ?? null;
+    const line = nearestEntry[2] + 1;
+    const column = nearestEntry[3];
+
+    if (sourceContent === null || sourceURL === null) {
+      // TODO maybe fall back to the runtime source instead of throwing?
+      throw Error(
+        `Could not find original source for line:${lineNumber} and column:${columnNumber}`,
+      );
+    }
+
+    return {
+      column,
+      line,
+      sourceContent: ((sourceContent: any): string),
+      sourceURL: ((sourceURL: any): string),
+    };
+  }
+
+  return (({
+    originalPositionFor,
+  }: any): SourceMapConsumerType);
+}
+
+function IndexedSourceMapConsumer(sourceMapJSON: IndexSourceMap) {
+  let lastOffset = {
+    line: -1,
+    column: 0,
+  };
+
+  const sections = sourceMapJSON.sections.map(section => {
+    const offset = section.offset;
+    const offsetLine = offset.line;
+    const offsetColumn = offset.column;
+
+    if (
+      offsetLine < lastOffset.line ||
+      (offsetLine === lastOffset.line && offsetColumn < lastOffset.column)
+    ) {
+      throw new Error('Section offsets must be ordered and non-overlapping.');
+    }
+
+    lastOffset = offset;
+
+    return {
+      // The offset fields are 0-based, but we use 1-based indices when encoding/decoding from VLQ.
+      generatedLine: offsetLine + 1,
+      generatedColumn: offsetColumn + 1,
+      sourceMapConsumer: new SourceMapConsumer(section.map),
+    };
+  });
+
+  function originalPositionFor({
+    columnNumber,
+    lineNumber,
+  }: SearchPosition): ResultPosition {
+    // Error.prototype.stack columns are 1-based (like most IDEs) but ASTs are 0-based.
+    const targetColumnNumber = columnNumber - 1;
+
+    let section = null;
+
+    let startIndex = 0;
+    let stopIndex = sections.length - 1;
+    let index = -1;
+    while (startIndex <= stopIndex) {
+      index = Math.floor((stopIndex + startIndex) / 2);
+      section = sections[index];
+
+      const currentLine = section.generatedLine;
+      if (currentLine === lineNumber) {
+        const currentColumn = section.generatedColumn;
+        if (currentColumn === lineNumber) {
+          break;
+        } else {
+          if (currentColumn > targetColumnNumber) {
+            if (stopIndex - index > 0) {
+              stopIndex = index;
+            } else {
+              index = stopIndex;
+              break;
+            }
+          } else {
+            if (index - startIndex > 0) {
+              startIndex = index;
+            } else {
+              index = startIndex;
+              break;
+            }
+          }
+        }
+      } else {
+        if (currentLine > lineNumber) {
+          if (stopIndex - index > 0) {
+            stopIndex = index;
+          } else {
+            index = stopIndex;
+            break;
+          }
+        } else {
+          if (index - startIndex > 0) {
+            startIndex = index;
+          } else {
+            index = startIndex;
+            break;
+          }
+        }
+      }
+    }
+
+    if (section == null) {
+      // TODO maybe fall back to the runtime source instead of throwing?
+      throw Error(
+        `Could not find matching section for line:${lineNumber} and column:${columnNumber}`,
+      );
+    }
+
+    return section.sourceMapConsumer.originalPositionFor({
+      columnNumber,
+      lineNumber,
+    });
+  }
+
+  return (({
+    originalPositionFor,
+  }: any): SourceMapConsumerType);
+}

filename: packages/react-devtools-shared/src/hooks/astUtils.js
patch: @@ -18,8 +18,6 @@ export type Position = {|
   column: number,
 |};
 
-export type SourceConsumer = any;
-
 export type SourceFileASTWithHookDetails = {
   sourceFileAST: File,
   line: number,

filename: packages/react-devtools-shared/src/hooks/parseHookNames/parseSourceAndMetadata.js
patch: @@ -12,7 +12,6 @@
 
 import {parse} from '@babel/parser';
 import LRU from 'lru-cache';
-import {SourceMapConsumer} from 'source-map-js';
 import {getHookName} from '../astUtils';
 import {areSourceMapsAppliedToErrors} from '../ErrorTester';
 import {__DEBUG__} from 'react-devtools-shared/src/constants';
@@ -22,14 +21,15 @@ import {
   withAsyncPerfMeasurements,
   withSyncPerfMeasurements,
 } from 'react-devtools-shared/src/PerformanceLoggingUtils';
+import SourceMapConsumer from '../SourceMapConsumer';
 
+import type {SourceMapConsumerType} from '../SourceMapConsumer';
 import type {
   HooksList,
   LocationKeyToHookSourceAndMetadata,
 } from './loadSourceAndMetadata';
 import type {HookSource} from 'react-debug-tools/src/ReactDebugHooks';
 import type {HookNames, LRUCache} from 'react-devtools-shared/src/types';
-import type {SourceConsumer} from '../astUtils';
 
 type AST = mixed;
 
@@ -53,35 +53,21 @@ type HookParsedMetadata = {|
   // Column number in original source code.
   originalSourceColumnNumber: number | null,
 
-  // APIs from source-map for parsing source maps (if detected).
-  sourceConsumer: SourceConsumer | null,
+  // Alternate APIs from source-map for parsing source maps (if detected).
+  sourceMapConsumer: SourceMapConsumerType | null,
 |};
 
 type LocationKeyToHookParsedMetadata = Map<string, HookParsedMetadata>;
 
 type CachedRuntimeCodeMetadata = {|
-  sourceConsumer: SourceConsumer | null,
   metadataConsumer: SourceMapMetadataConsumer | null,
+  sourceMapConsumer: SourceMapConsumerType | null,
 |};
 
 const runtimeURLToMetadataCache: LRUCache<
   string,
   CachedRuntimeCodeMetadata,
-> = new LRU({
-  max: 50,
-  dispose: (runtimeSourceURL: string, metadata: CachedRuntimeCodeMetadata) => {
-    if (__DEBUG__) {
-      console.log(
-        `runtimeURLToMetadataCache.dispose() Evicting cached metadata for "${runtimeSourceURL}"`,
-      );
-    }
-
-    const sourceConsumer = metadata.sourceConsumer;
-    if (sourceConsumer !== null) {
-      sourceConsumer.destroy();
-    }
-  },
-});
+> = new LRU({max: 50});
 
 type CachedSourceCodeMetadata = {|
   originalSourceAST: AST,
@@ -220,27 +206,10 @@ function initializeHookParsedMetadata(
         originalSourceURL: null,
         originalSourceLineNumber: null,
         originalSourceColumnNumber: null,
-        sourceConsumer: null,
+        sourceMapConsumer: null,
       };
 
       locationKeyToHookParsedMetadata.set(locationKey, hookParsedMetadata);
-
-      const runtimeSourceURL = hookSourceAndMetadata.runtimeSourceURL;
-
-      // If we've already loaded the source map info for this file,
-      // we can skip reloading it (and more importantly, re-parsing it).
-      const runtimeMetadata = runtimeURLToMetadataCache.get(runtimeSourceURL);
-      if (runtimeMetadata != null) {
-        if (__DEBUG__) {
-          console.groupCollapsed(
-            `parseHookNames() Found cached runtime metadata for file "${runtimeSourceURL}"`,
-          );
-          console.log(runtimeMetadata);
-          console.groupEnd();
-        }
-        hookParsedMetadata.sourceConsumer = runtimeMetadata.sourceConsumer;
-        hookParsedMetadata.metadataConsumer = runtimeMetadata.metadataConsumer;
-      }
     },
   );
 
@@ -280,14 +249,14 @@ function parseSourceAST(
         throw Error('Hook source code location not found.');
       }
 
-      const {metadataConsumer, sourceConsumer} = hookParsedMetadata;
+      const {metadataConsumer, sourceMapConsumer} = hookParsedMetadata;
       const runtimeSourceCode = ((hookSourceAndMetadata.runtimeSourceCode: any): string);
       let hasHookMap = false;
       let originalSourceURL;
       let originalSourceCode;
       let originalSourceColumnNumber;
       let originalSourceLineNumber;
-      if (areSourceMapsAppliedToErrors() || sourceConsumer == null) {
+      if (areSourceMapsAppliedToErrors() || sourceMapConsumer === null) {
         // Either the current environment automatically applies source maps to errors,
         // or the current code had no source map to begin with.
         // Either way, we don't need to convert the Error stack frame locations.
@@ -299,55 +268,32 @@ function parseSourceAST(
         // Namespace them?
         originalSourceURL = hookSourceAndMetadata.runtimeSourceURL;
       } else {
-        // Parse and extract the AST from the source map.
-        // Now that the source map has been loaded,
-        // extract the original source for later.
-        // TODO (named hooks) Refactor this read, github.com/facebook/react/pull/22181
-        const {column, line, source} = withSyncPerfMeasurements(
-          'sourceConsumer.originalPositionFor()',
-          () =>
-            sourceConsumer.originalPositionFor({
-              line: lineNumber,
-
-              // Column numbers are represented differently between tools/engines.
-              // Error.prototype.stack columns are 1-based (like most IDEs) but ASTs are 0-based.
-              // For more info see https://github.com/facebook/react/issues/21792#issuecomment-873171991
-              column: columnNumber - 1,
-            }),
-        );
-
-        if (source == null) {
-          // TODO (named hooks) maybe fall back to the runtime source instead of throwing?
-          throw new Error(
-            'Could not map hook runtime location to original source location',
-          );
-        }
+        const {
+          column,
+          line,
+          sourceContent,
+          sourceURL,
+        } = sourceMapConsumer.originalPositionFor({
+          columnNumber,
+          lineNumber,
+        });
 
         originalSourceColumnNumber = column;
         originalSourceLineNumber = line;
-        // TODO (named hooks) maybe canonicalize this URL somehow?
-        // It can be relative if the source map specifies it that way,
-        // but we use it as a cache key across different source maps and there can be collisions.
-        originalSourceURL = (source: string);
-        originalSourceCode = withSyncPerfMeasurements(
-          'sourceConsumer.sourceContentFor()',
-          () => (sourceConsumer.sourceContentFor(source, true): string),
-        );
+        originalSourceCode = sourceContent;
+        originalSourceURL = sourceURL;
+      }
 
-        if (__DEBUG__) {
-          console.groupCollapsed(
-            `parseSourceAST() Extracted source code from source map for "${originalSourceURL}"`,
-          );
-          console.log(originalSourceCode);
-          console.groupEnd();
-        }
+      hookParsedMetadata.originalSourceCode = originalSourceCode;
+      hookParsedMetadata.originalSourceURL = originalSourceURL;
+      hookParsedMetadata.originalSourceLineNumber = originalSourceLineNumber;
+      hookParsedMetadata.originalSourceColumnNumber = originalSourceColumnNumber;
 
-        if (
-          metadataConsumer != null &&
-          metadataConsumer.hasHookMap(originalSourceURL)
-        ) {
-          hasHookMap = true;
-        }
+      if (
+        metadataConsumer != null &&
+        metadataConsumer.hasHookMap(originalSourceURL)
+      ) {
+        hasHookMap = true;
       }
 
       if (__DEBUG__) {
@@ -356,11 +302,6 @@ function parseSourceAST(
         );
       }
 
-      hookParsedMetadata.originalSourceCode = originalSourceCode;
-      hookParsedMetadata.originalSourceURL = originalSourceURL;
-      hookParsedMetadata.originalSourceLineNumber = originalSourceLineNumber;
-      hookParsedMetadata.originalSourceColumnNumber = originalSourceColumnNumber;
-
       if (hasHookMap) {
         if (__DEBUG__) {
           console.log(
@@ -447,30 +388,42 @@ function parseSourceMaps(
         throw Error(`Expected to find HookParsedMetadata for "${locationKey}"`);
       }
 
-      const sourceMapJSON = hookSourceAndMetadata.sourceMapJSON;
-      if (sourceMapJSON != null) {
-        hookParsedMetadata.metadataConsumer = withSyncPerfMeasurements(
-          'new SourceMapMetadataConsumer(sourceMapJSON)',
-          () => new SourceMapMetadataConsumer(sourceMapJSON),
-        );
-        hookParsedMetadata.sourceConsumer = withSyncPerfMeasurements(
-          'new SourceMapConsumer(sourceMapJSON)',
-          () => new SourceMapConsumer(sourceMapJSON),
-        );
+      const {runtimeSourceURL, sourceMapJSON} = hookSourceAndMetadata;
+
+      // If we've already loaded the source map info for this file,
+      // we can skip reloading it (and more importantly, re-parsing it).
+      const runtimeMetadata = runtimeURLToMetadataCache.get(runtimeSourceURL);
+      if (runtimeMetadata != null) {
+        if (__DEBUG__) {
+          console.groupCollapsed(
+            `parseHookNames() Found cached runtime metadata for file "${runtimeSourceURL}"`,
+          );
+          console.log(runtimeMetadata);
+          console.groupEnd();
+        }
 
-        const runtimeSourceURL = hookSourceAndMetadata.runtimeSourceURL;
+        hookParsedMetadata.metadataConsumer = runtimeMetadata.metadataConsumer;
+        hookParsedMetadata.sourceMapConsumer =
+          runtimeMetadata.sourceMapConsumer;
+      } else {
+        if (sourceMapJSON != null) {
+          const sourceMapConsumer = withSyncPerfMeasurements(
+            'new SourceMapConsumer(sourceMapJSON)',
+            () => SourceMapConsumer(sourceMapJSON),
+          );
 
-        // Only set once to avoid triggering eviction/cleanup code.
-        if (!runtimeURLToMetadataCache.has(runtimeSourceURL)) {
-          if (__DEBUG__) {
-            console.log(
-              `parseSourceMaps() Caching runtime metadata for "${runtimeSourceURL}"`,
-            );
-          }
+          const metadataConsumer = withSyncPerfMeasurements(
+            'new SourceMapMetadataConsumer(sourceMapJSON)',
+            () => new SourceMapMetadataConsumer(sourceMapJSON),
+          );
+
+          hookParsedMetadata.metadataConsumer = metadataConsumer;
+          hookParsedMetadata.sourceMapConsumer = sourceMapConsumer;
 
+          // Only set once to avoid triggering eviction/cleanup code.
           runtimeURLToMetadataCache.set(runtimeSourceURL, {
-            metadataConsumer: hookParsedMetadata.metadataConsumer,
-            sourceConsumer: hookParsedMetadata.sourceConsumer,
+            metadataConsumer: metadataConsumer,
+            sourceMapConsumer: sourceMapConsumer,
           });
         }
       }