Bug 1541631 - Part 4: Use resource utility for SourceWithContent caching. r=jlast
authorLogan Smyth <loganfsmyth@gmail.com>
Thu, 15 Aug 2019 21:45:52 +0000
changeset 488391 c3131097b7c34fdb8606bab7d6d4972dca632249
parent 488390 4600340598aa1823ef007a8a6610404ecfac0c4b
child 488392 780781260eaa9a306db882eddaca6be1f5b72e43
push id36443
push userccoroiu@mozilla.com
push dateFri, 16 Aug 2019 09:48:15 +0000
treeherdermozilla-central@5d4cbfe103bb [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjlast
bugs1541631
milestone70.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 1541631 - Part 4: Use resource utility for SourceWithContent caching. r=jlast Differential Revision: https://phabricator.services.mozilla.com/D42029
devtools/client/debugger/src/components/SecondaryPanes/Frames/tests/Frames.spec.js
devtools/client/debugger/src/reducers/sources.js
--- a/devtools/client/debugger/src/components/SecondaryPanes/Frames/tests/Frames.spec.js
+++ b/devtools/client/debugger/src/components/SecondaryPanes/Frames/tests/Frames.spec.js
@@ -201,18 +201,18 @@ describe("Frames", () => {
       const frames = [
         makeMockFrame("1", source1),
         makeMockFrame("2", source2),
         makeMockFrame("3", source1),
         makeMockFrame("8", source2),
       ];
 
       const sources: SourceResourceState = insertResources(createInitial(), [
-        source1,
-        source2,
+        { ...source1, content: null },
+        { ...source2, content: null },
       ]);
 
       const processedFrames = formatCallStackFrames(frames, sources, source1);
       const selectedFrame = frames[0];
 
       const component = render({
         frames: processedFrames,
         frameworkGroupingOn: false,
--- a/devtools/client/debugger/src/reducers/sources.js
+++ b/devtools/client/debugger/src/reducers/sources.js
@@ -19,17 +19,19 @@ import {
   getPlainUrl,
 } from "../utils/source";
 import {
   createInitial,
   insertResources,
   updateResources,
   hasResource,
   getResource,
+  getMappedResource,
   getResourceIds,
+  memoizeResourceShallow,
   makeReduceQuery,
   makeReduceAllQuery,
   makeMapWithArgs,
   type Resource,
   type ResourceState,
   type ReduceQuery,
   type ReduceAllQuery,
 } from "../utils/resource";
@@ -88,27 +90,26 @@ export type SourceBase = {|
   +introductionType: ?string,
   +extensionName: ?string,
   +isExtension: boolean,
   +isWasm: boolean,
 |};
 
 type SourceResource = Resource<{
   ...SourceBase,
+  content: AsyncValue<SourceContent> | null,
 }>;
 export type SourceResourceState = ResourceState<SourceResource>;
 
 export type SourcesState = {
   epoch: number,
 
   // All known sources.
   sources: SourceResourceState,
 
-  content: SourcesContentMap,
-
   breakpointPositions: BreakpointPositionsMap,
   breakableLines: { [SourceId]: Array<number> },
 
   // A link between each source object and the source actor they wrap over.
   actors: SourceActorMap,
 
   // All sources associated with a given URL. When using source maps, multiple
   // sources can have the same URL.
@@ -249,46 +250,55 @@ function update(
 
     case "SET_FOCUSED_SOURCE_ITEM":
       return { ...state, focusedItem: action.item };
   }
 
   return state;
 }
 
-function resourceAsSource(r: SourceResource): Source {
-  return { ...r };
-}
+const resourceAsSourceBase = memoizeResourceShallow(
+  ({ content, ...source }: SourceResource): SourceBase => source
+);
+
+const resourceAsSourceWithContent = memoizeResourceShallow(
+  ({ content, ...source }: SourceResource): SourceWithContent => ({
+    ...source,
+    content: content && content.state !== "pending" ? content : null,
+  })
+);
 
 /*
  * Add sources to the sources store
  * - Add the source to the sources store
  * - Add the source URL to the urls map
  */
 function addSources(state: SourcesState, sources: SourceBase[]): SourcesState {
   state = {
     ...state,
-    content: { ...state.content },
     urls: { ...state.urls },
     plainUrls: { ...state.plainUrls },
   };
 
-  state.sources = insertResources(state.sources, sources);
+  state.sources = insertResources(
+    state.sources,
+    sources.map(source => ({
+      ...source,
+      content: null,
+    }))
+  );
 
   for (const source of sources) {
-    // 1. Add the source to the sources map
-    state.content[source.id] = null;
-
-    // 2. Update the source url map
+    // 1. Update the source url map
     const existing = state.urls[source.url] || [];
     if (!existing.includes(source.id)) {
       state.urls[source.url] = [...existing, source.id];
     }
 
-    // 3. Update the plain url map
+    // 2. Update the plain url map
     if (source.url) {
       const plainUrl = getPlainUrl(source.url);
       const existingPlainUrls = state.plainUrls[plainUrl] || [];
       if (!existingPlainUrls.includes(source.url)) {
         state.plainUrls[plainUrl] = [...existingPlainUrls, source.url];
       }
     }
   }
@@ -398,17 +408,17 @@ function updateRootRelativeValues(
 function updateLoadedState(
   state: SourcesState,
   action: LoadSourceAction
 ): SourcesState {
   const { sourceId } = action;
 
   // If there was a navigation between the time the action was started and
   // completed, we don't want to update the store.
-  if (action.epoch !== state.epoch || !(sourceId in state.content)) {
+  if (action.epoch !== state.epoch || !hasResource(state.sources, sourceId)) {
     return state;
   }
 
   let content;
   if (action.status === "start") {
     content = asyncValue.pending();
   } else if (action.status === "error") {
     content = asyncValue.rejected(action.error);
@@ -422,20 +432,22 @@ function updateLoadedState(
     content = asyncValue.fulfilled({
       type: "wasm",
       value: action.value.text,
     });
   }
 
   return {
     ...state,
-    content: {
-      ...state.content,
-      [sourceId]: content,
-    },
+    sources: updateResources(state.sources, [
+      {
+        id: sourceId,
+        content,
+      },
+    ]),
   };
 }
 
 function clearSourceMaps(
   state: SourcesState,
   sourceId: SourceId
 ): SourcesState {
   if (!hasResource(state.sources, sourceId)) {
@@ -523,17 +535,19 @@ export function getSourceThreads(
     )
   );
 }
 
 export function getSourceInSources(
   sources: SourceResourceState,
   id: string
 ): ?Source {
-  return hasResource(sources, id) ? getResource(sources, id) : null;
+  return hasResource(sources, id)
+    ? getMappedResource(sources, id, resourceAsSourceBase)
+    : null;
 }
 
 export function getSource(state: OuterState, id: SourceId): ?Source {
   return getSourceInSources(getSources(state), id);
 }
 
 export function getSourceFromId(state: OuterState, id: string): Source {
   const source = getSource(state, id);
@@ -557,17 +571,19 @@ export function getSourceByActorId(
 export function getSourcesByURLInSources(
   sources: SourceResourceState,
   urls: UrlsMap,
   url: string
 ): Source[] {
   if (!url || !urls[url]) {
     return [];
   }
-  return urls[url].map(id => getResource(sources, id));
+  return urls[url].map(id =>
+    getMappedResource(sources, id, resourceAsSourceBase)
+  );
 }
 
 export function getSourcesByURL(state: OuterState, url: string): Source[] {
   return getSourcesByURLInSources(getSources(state), getUrls(state), url);
 }
 
 export function getSourceByURL(state: OuterState, url: string): ?Source {
   const foundSources = getSourcesByURL(state, url);
@@ -676,17 +692,17 @@ export function getHasSiblingOfSameName(
   }
 
   return getSourcesUrlsInSources(state, source.url).length > 1;
 }
 
 const querySourceList: ReduceAllQuery<
   SourceResource,
   Array<Source>
-> = makeReduceAllQuery(resourceAsSource, sources => sources.slice());
+> = makeReduceAllQuery(resourceAsSourceBase, sources => sources.slice());
 
 export function getSources(state: OuterState): SourceResourceState {
   return state.sources.sources;
 }
 
 export function getSourcesEpoch(state: OuterState) {
   return state.sources.epoch;
 }
@@ -743,79 +759,51 @@ export const getSelectedSource: Selector
     return getSourceInSources(sources, selectedLocation.sourceId);
   }
 );
 
 type GSSWC = Selector<?SourceWithContent>;
 export const getSelectedSourceWithContent: GSSWC = createSelector(
   getSelectedLocation,
   getSources,
-  state => state.sources.content,
   (
     selectedLocation: ?SourceLocation,
-    sources: SourceResourceState,
-    content: SourcesContentMap
+    sources: SourceResourceState
   ): SourceWithContent | null => {
     const source =
       selectedLocation &&
       getSourceInSources(sources, selectedLocation.sourceId);
     return source
-      ? getSourceWithContentInner(sources, content, source.id)
+      ? getMappedResource(sources, source.id, resourceAsSourceWithContent)
       : null;
   }
 );
 export function getSourceWithContent(
   state: OuterState,
   id: SourceId
 ): SourceWithContent {
-  return getSourceWithContentInner(
+  return getMappedResource(
     state.sources.sources,
-    state.sources.content,
-    id
+    id,
+    resourceAsSourceWithContent
   );
 }
 export function getSourceContent(
   state: OuterState,
   id: SourceId
 ): SettledValue<SourceContent> | null {
-  // Assert the resource exists.
-  getResource(state.sources.sources, id);
-  const content = state.sources.content[id];
+  const { content } = getResource(state.sources.sources, id);
 
   if (!content || content.state === "pending") {
     return null;
   }
 
   return content;
 }
 
-const contentLookup: WeakMap<Source, SourceWithContent> = new WeakMap();
-function getSourceWithContentInner(
-  sources: SourceResourceState,
-  content: SourcesContentMap,
-  id: SourceId
-): SourceWithContent {
-  const source = getResource(sources, id);
-  let contentValue = content[source.id];
-
-  let result = contentLookup.get(source);
-  if (!result || result.content !== contentValue) {
-    if (contentValue && contentValue.state === "pending") {
-      contentValue = null;
-    }
-    result = {
-      ...source,
-      content: contentValue,
-    };
-    contentLookup.set(source, result);
-  }
-
-  return result;
-}
-
 export function getSelectedSourceId(state: OuterState) {
   const source = getSelectedSource((state: any));
   return source && source.id;
 }
 
 export function getProjectDirectoryRoot(state: OuterState): string {
   return state.sources.projectDirectoryRoot;
 }
@@ -996,13 +984,13 @@ export const getSelectedBreakableLines: 
   state => {
     const sourceId = getSelectedSourceId(state);
     return sourceId && state.sources.breakableLines[sourceId];
   },
   breakableLines => new Set(breakableLines || [])
 );
 
 export function isSourceLoadingOrLoaded(state: OuterState, sourceId: string) {
-  const content = state.sources.content[sourceId];
+  const { content } = getResource(state.sources.sources, sourceId);
   return content !== null;
 }
 
 export default update;