Bug 1533813 - getOriginalLocation can be expensive. r=loganfsmyth
☠☠ backed out by 02b387ef172e ☠ ☠
authorLogan Smyth <loganfsmyth@gmail.com>
Mon, 11 Mar 2019 23:23:27 +0000
changeset 521452 985e05a4b54024ae40be6d94aaf8d9fb3d66f9b2
parent 521451 a7dee08698fc6aa4a15cafb3052e4f4c0d41ed0a
child 521453 f9de9668b5e87be6b0461ef8c0295f48bc71dcd1
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)
reviewersloganfsmyth
bugs1533813
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 1533813 - getOriginalLocation can be expensive. r=loganfsmyth Differential Revision: https://phabricator.services.mozilla.com/D22760
devtools/client/debugger/new/packages/devtools-source-map/src/index.js
devtools/client/debugger/new/packages/devtools-source-map/src/source-map.js
devtools/client/debugger/new/packages/devtools-source-map/src/worker.js
devtools/client/debugger/new/src/actions/breakpoints/breakpointPositions.js
devtools/client/debugger/new/src/actions/pause/tests/pause.spec.js
devtools/client/debugger/new/src/actions/sources/tests/select.spec.js
devtools/client/debugger/new/src/actions/tests/ast.spec.js
devtools/client/debugger/new/src/actions/tests/pending-breakpoints.spec.js
devtools/client/debugger/new/src/actions/tests/project-text-search.spec.js
devtools/client/shared/source-map/index.js
devtools/client/shared/source-map/worker.js
--- a/devtools/client/debugger/new/packages/devtools-source-map/src/index.js
+++ b/devtools/client/debugger/new/packages/devtools-source-map/src/index.js
@@ -72,16 +72,22 @@ export const getAllGeneratedLocations = 
 ): Promise<Array<SourceLocation>> =>
   _getAllGeneratedLocations(location, originalSource);
 
 export const getOriginalLocation = async (
   location: SourceLocation,
   options: locationOptions = {}
 ): Promise<SourceLocation> => _getOriginalLocation(location, options);
 
+export const getOriginalLocations = async (
+  locations: SourceLocation[],
+  options: locationOptions = {}
+): Promise<SourceLocation[]> =>
+  dispatcher.invoke("getOriginalLocations", locations, options);
+
 export const getGeneratedRangesForOriginal = async (
   sourceId: SourceId,
   url: string,
   mergeUnmappedRegions?: boolean
 ): Promise<
   Array<{
     start: {
       line: number,
--- a/devtools/client/debugger/new/packages/devtools-source-map/src/source-map.js
+++ b/devtools/client/debugger/new/packages/devtools-source-map/src/source-map.js
@@ -41,16 +41,20 @@ type Range = {
     column: number
   },
   end: {
     line: number,
     column: number
   }
 };
 
+export type locationOptions = {
+  search?: "LEAST_UPPER_BOUND" | "GREATEST_LOWER_BOUND"
+};
+
 async function getOriginalURLs(
   generatedSource: Source
 ): Promise<SourceMapConsumer> {
   const map = await fetchSourceMap(generatedSource);
   return map && map.sources;
 }
 
 const COMPUTED_SPANS = new WeakSet();
@@ -252,19 +256,25 @@ async function getAllGeneratedLocations(
 
   return positions.map(({ line, column }) => ({
     sourceId: generatedSourceId,
     line,
     column
   }));
 }
 
-export type locationOptions = {
-  search?: "LEAST_UPPER_BOUND" | "GREATEST_LOWER_BOUND"
-};
+function getOriginalLocations(
+  locations: SourceLocation[],
+  options: locationOptions = {}
+) {
+  return Promise.all(
+    locations.map(location => getOriginalLocation(location, options))
+  );
+}
+
 async function getOriginalLocation(
   location: SourceLocation,
   { search }: locationOptions = {}
 ): Promise<SourceLocation> {
   if (!isGeneratedId(location.sourceId)) {
     return location;
   }
 
@@ -541,15 +551,16 @@ function clearSourceMaps() {
 module.exports = {
   getOriginalURLs,
   hasOriginalURL,
   getOriginalRanges,
   getGeneratedRanges,
   getGeneratedLocation,
   getAllGeneratedLocations,
   getOriginalLocation,
+  getOriginalLocations,
   getOriginalSourceText,
   getGeneratedRangesForOriginal,
   getFileGeneratedRange,
   applySourceMap,
   clearSourceMaps,
   hasMappedSource
 };
--- a/devtools/client/debugger/new/packages/devtools-source-map/src/worker.js
+++ b/devtools/client/debugger/new/packages/devtools-source-map/src/worker.js
@@ -6,16 +6,17 @@
 const {
   getOriginalURLs,
   hasOriginalURL,
   getOriginalRanges,
   getGeneratedRanges,
   getGeneratedLocation,
   getAllGeneratedLocations,
   getOriginalLocation,
+  getOriginalLocations,
   getOriginalSourceText,
   getGeneratedRangesForOriginal,
   getFileGeneratedRange,
   hasMappedSource,
   clearSourceMaps,
   applySourceMap
 } = require("./source-map");
 
@@ -32,16 +33,17 @@ self.onmessage = workerHandler({
   setAssetRootURL,
   getOriginalURLs,
   hasOriginalURL,
   getOriginalRanges,
   getGeneratedRanges,
   getGeneratedLocation,
   getAllGeneratedLocations,
   getOriginalLocation,
+  getOriginalLocations,
   getOriginalSourceText,
   getOriginalStackFrames,
   getGeneratedRangesForOriginal,
   getFileGeneratedRange,
   hasMappedSource,
   applySourceMap,
   clearSourceMaps
 });
--- a/devtools/client/debugger/new/src/actions/breakpoints/breakpointPositions.js
+++ b/devtools/client/debugger/new/src/actions/breakpoints/breakpointPositions.js
@@ -1,42 +1,41 @@
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at <http://mozilla.org/MPL/2.0/>. */
 
 // @flow
 
 import { isOriginalId, originalToGeneratedId } from "devtools-source-map";
-import { uniqBy } from "lodash";
+import { uniqBy, zip } from "lodash";
 
 import {
   getSource,
   getSourceFromId,
   hasBreakpointPositions,
   getBreakpointPositionsForSource
 } from "../../selectors";
 
 import type { MappedLocation, SourceLocation } from "../../types";
 import type { ThunkArgs } from "../../actions/types";
-import { getOriginalLocation } from "../../utils/source-maps";
 import { makeBreakpointId } from "../../utils/breakpoint";
 import typeof SourceMaps from "../../../packages/devtools-source-map/src";
 
 const requests = new Map();
 
 async function mapLocations(
   generatedLocations: SourceLocation[],
   { sourceMaps }: { sourceMaps: SourceMaps }
 ) {
-  return Promise.all(
-    (generatedLocations: any).map(async (generatedLocation: SourceLocation) => {
-      const location = await getOriginalLocation(generatedLocation, sourceMaps);
+  const originalLocations = await sourceMaps.getOriginalLocations(
+    generatedLocations
+  );
 
-      return { location, generatedLocation };
-    })
+  return zip(originalLocations, generatedLocations).map(
+    ([location, generatedLocation]) => ({ location, generatedLocation })
   );
 }
 
 function filterByUniqLocation(positions: MappedLocation[]) {
   return uniqBy(positions, ({ location }) => makeBreakpointId(location));
 }
 
 function convertToList(results, source) {
--- a/devtools/client/debugger/new/src/actions/pause/tests/pause.spec.js
+++ b/devtools/client/debugger/new/src/actions/pause/tests/pause.spec.js
@@ -251,16 +251,17 @@ describe("pause", () => {
       const originalLocation = {
         sourceId: "foo-original",
         line: 3,
         column: 0
       };
 
       const sourceMapsMock = {
         getOriginalLocation: () => Promise.resolve(originalLocation),
+        getOriginalLocations: async items => items,
         getOriginalSourceText: async () => ({
           source: "\n\nfunction fooOriginal() {\n  return -5;\n}",
           contentType: "text/javascript"
         }),
         getGeneratedLocation: async location => location
       };
 
       const store = createStore(mockThreadClient, {}, sourceMapsMock);
@@ -313,16 +314,17 @@ describe("pause", () => {
           displayName: "barZoo",
           location: originalLocation2
         }
       ];
 
       const sourceMapsMock = {
         getOriginalStackFrames: loc => Promise.resolve(originStackFrames),
         getOriginalLocation: () => Promise.resolve(originalLocation),
+        getOriginalLocations: async items => items,
         getOriginalSourceText: async () => ({
           source: "fn fooBar() {}\nfn barZoo() { fooBar() }",
           contentType: "text/rust"
         }),
         getGeneratedRangesForOriginal: async () => []
       };
 
       const store = createStore(mockThreadClient, {}, sourceMapsMock);
--- a/devtools/client/debugger/new/src/actions/sources/tests/select.spec.js
+++ b/devtools/client/debugger/new/src/actions/sources/tests/select.spec.js
@@ -224,16 +224,17 @@ describe("sources", () => {
   });
 
   it("should keep the original the viewing context", async () => {
     const { dispatch, getState } = createStore(
       sourceThreadClient,
       {},
       {
         getOriginalLocation: async location => ({ ...location, line: 12 }),
+        getOriginalLocations: async items => items,
         getGeneratedLocation: async location => ({ ...location, line: 12 }),
         getOriginalSourceText: async () => ({ source: "" }),
         getGeneratedRangesForOriginal: async () => []
       }
     );
 
     const baseSource = makeSource("base.js");
     await dispatch(actions.newSource(baseSource));
@@ -252,16 +253,17 @@ describe("sources", () => {
   });
 
   it("should change the original the viewing context", async () => {
     const { dispatch, getState } = createStore(
       sourceThreadClient,
       {},
       {
         getOriginalLocation: async location => ({ ...location, line: 12 }),
+        getOriginalLocations: async items => items,
         getGeneratedRangesForOriginal: async () => [],
         getOriginalSourceText: async () => ({ source: "" })
       }
     );
 
     await dispatch(actions.newSource(makeSource("base.js")));
     const baseSource = makeOriginalSource("base.js");
 
--- a/devtools/client/debugger/new/src/actions/tests/ast.spec.js
+++ b/devtools/client/debugger/new/src/actions/tests/ast.spec.js
@@ -40,17 +40,18 @@ const threadClient = {
 };
 
 const sourceMaps = {
   getOriginalSourceText: async ({ id }) => ({
     id,
     text: sourceTexts[id],
     contentType: "text/javascript"
   }),
-  getGeneratedRangesForOriginal: async () => []
+  getGeneratedRangesForOriginal: async () => [],
+  getOriginalLocations: async items => items
 };
 
 const sourceTexts = {
   "base.js": "function base(boo) {}",
   "foo.js": "function base(boo) { return this.bazz; } outOfScope",
   "scopes.js": readFixture("scopes.js"),
   "reactComponent.js/originalSource": readFixture("reactComponent.js"),
   "reactFuncComponent.js/originalSource": readFixture("reactFuncComponent.js")
--- a/devtools/client/debugger/new/src/actions/tests/pending-breakpoints.spec.js
+++ b/devtools/client/debugger/new/src/actions/tests/pending-breakpoints.spec.js
@@ -63,17 +63,18 @@ function mockSourceMaps() {
     ...sourceMaps,
     getOriginalSourceText: async source => ({
       id: source.id,
       text: "",
       contentType: "text/javascript"
     }),
     getGeneratedRangesForOriginal: async () => [
       { start: { line: 0, column: 0 }, end: { line: 10, column: 10 } }
-    ]
+    ],
+    getOriginalLocations: async items => items
   };
 }
 
 describe("when adding breakpoints", () => {
   it("a corresponding pending breakpoint should be added", async () => {
     const { dispatch, getState } = createStore(
       mockClient({ "5": [1] }),
       loadInitialState(),
@@ -374,17 +375,18 @@ describe("adding sources", () => {
       getGeneratedLocation: async (location, _source) => ({
         line: location.line,
         column: location.column,
         sourceId: _source.id
       }),
       getOriginalLocation: async location => location,
       getGeneratedRangesForOriginal: async () => [
         { start: { line: 0, column: 0 }, end: { line: 10, column: 10 } }
-      ]
+      ],
+      getOriginalLocations: async items => items
     });
 
     const { getState, dispatch } = store;
 
     expect(selectors.getBreakpointCount(getState())).toEqual(0);
 
     await dispatch(actions.newSource(makeSource("bar.js")));
     await dispatch(actions.newSource(source));
--- a/devtools/client/debugger/new/src/actions/tests/project-text-search.spec.js
+++ b/devtools/client/debugger/new/src/actions/tests/project-text-search.spec.js
@@ -72,17 +72,18 @@ describe("project text search", () => {
     const source2 = makeSource("bar:formatted");
 
     const mockMaps = {
       getOriginalSourceText: async () => ({
         source: "function bla(x, y) {\n const bar = 4; return 2;\n}",
         contentType: "text/javascript"
       }),
       getOriginalURLs: async () => [source2.url],
-      getGeneratedRangesForOriginal: async () => []
+      getGeneratedRangesForOriginal: async () => [],
+      getOriginalLocations: async items => items
     };
 
     const { dispatch, getState } = createStore(threadClient, {}, mockMaps);
     const mockQuery = "bla";
 
     await dispatch(actions.newSource(source1));
     await dispatch(actions.newSource(source2));
 
--- a/devtools/client/shared/source-map/index.js
+++ b/devtools/client/shared/source-map/index.js
@@ -154,16 +154,18 @@ const getOriginalRanges = exports.getOri
 const getGeneratedRanges = exports.getGeneratedRanges = async (location, originalSource) => _getGeneratedRanges(location, originalSource);
 
 const getGeneratedLocation = exports.getGeneratedLocation = async (location, originalSource) => _getGeneratedLocation(location, originalSource);
 
 const getAllGeneratedLocations = exports.getAllGeneratedLocations = async (location, originalSource) => _getAllGeneratedLocations(location, originalSource);
 
 const getOriginalLocation = exports.getOriginalLocation = async (location, options = {}) => _getOriginalLocation(location, options);
 
+const getOriginalLocations = exports.getOriginalLocations = async (locations, options = {}) => dispatcher.invoke("getOriginalLocations", locations, options);
+
 const getGeneratedRangesForOriginal = exports.getGeneratedRangesForOriginal = async (sourceId, url, mergeUnmappedRegions) => dispatcher.invoke("getGeneratedRangesForOriginal", sourceId, url, mergeUnmappedRegions);
 
 const getFileGeneratedRange = exports.getFileGeneratedRange = async originalSource => dispatcher.invoke("getFileGeneratedRange", originalSource);
 
 const getLocationScopes = exports.getLocationScopes = dispatcher.task("getLocationScopes");
 
 const getOriginalSourceText = exports.getOriginalSourceText = async originalSource => dispatcher.invoke("getOriginalSourceText", originalSource);
 
--- a/devtools/client/shared/source-map/worker.js
+++ b/devtools/client/shared/source-map/worker.js
@@ -6906,16 +6906,17 @@ module.exports = __webpack_require__(740
 const {
   getOriginalURLs,
   hasOriginalURL,
   getOriginalRanges,
   getGeneratedRanges,
   getGeneratedLocation,
   getAllGeneratedLocations,
   getOriginalLocation,
+  getOriginalLocations,
   getOriginalSourceText,
   getGeneratedRangesForOriginal,
   getFileGeneratedRange,
   hasMappedSource,
   clearSourceMaps,
   applySourceMap
 } = __webpack_require__(741);
 
@@ -6932,16 +6933,17 @@ self.onmessage = workerHandler({
   setAssetRootURL,
   getOriginalURLs,
   hasOriginalURL,
   getOriginalRanges,
   getGeneratedRanges,
   getGeneratedLocation,
   getAllGeneratedLocations,
   getOriginalLocation,
+  getOriginalLocations,
   getOriginalSourceText,
   getOriginalStackFrames,
   getGeneratedRangesForOriginal,
   getFileGeneratedRange,
   hasMappedSource,
   applySourceMap,
   clearSourceMaps
 });
@@ -7159,16 +7161,20 @@ async function getAllGeneratedLocations(
 
   return positions.map(({ line, column }) => ({
     sourceId: generatedSourceId,
     line,
     column
   }));
 }
 
+function getOriginalLocations(locations, options = {}) {
+  return Promise.all(locations.map(location => getOriginalLocation(location, options)));
+}
+
 async function getOriginalLocation(location, { search } = {}) {
   if (!isGeneratedId(location.sourceId)) {
     return location;
   }
 
   const map = await getSourceMap(location.sourceId);
   if (!map) {
     return location;
@@ -7405,16 +7411,17 @@ function clearSourceMaps() {
 module.exports = {
   getOriginalURLs,
   hasOriginalURL,
   getOriginalRanges,
   getGeneratedRanges,
   getGeneratedLocation,
   getAllGeneratedLocations,
   getOriginalLocation,
+  getOriginalLocations,
   getOriginalSourceText,
   getGeneratedRangesForOriginal,
   getFileGeneratedRange,
   applySourceMap,
   clearSourceMaps,
   hasMappedSource
 };