Bug 1329165 - Use Immutable.Map for request list, optimize sorting r=Honza

MozReview-Commit-ID: L31hiD5CETE

--HG--
extra : rebase_source : c1382f9c8f044d69099c1e5315c09c2ac0df357c

devtools/client/netmonitor/netmonitor-view.js

@@ -203,7 +203,7 @@ var NetMonitorView = {
         console.error(ex);
       }
 
-      const requests = requestsView.store.getState().requests.requests;
+      const requests = requestsView.store.getState().requests.requests.valueSeq();
       statisticsView.createPrimedCacheChart(requests);
       statisticsView.createEmptyCacheChart(requests);
     });

devtools/client/netmonitor/reducers/requests.js

@@ -55,8 +55,8 @@ const Request = I.Record({
 });
 
 const Requests = I.Record({
-  // The request list
+  // The collection of requests (keyed by id)
-  requests: I.List(),
+  requests: I.Map(),
   // Selection state
   selectedId: null,
   preselectedId: null,
@@ -101,7 +101,7 @@ function requestsReducer(state = new Requests(), action) {
           action.data,
           { urlDetails: getUrlDetails(action.data.url) }
         ));
-        st.requests = st.requests.push(newRequest);
+        st.requests = st.requests.set(newRequest.id, newRequest);
 
         // Update the started/ended timestamps
         let { startedMillis } = action.data;
@@ -123,12 +123,12 @@ function requestsReducer(state = new Requests(), action) {
     case UPDATE_REQUEST: {
       let { requests, lastEndedMillis } = state;
 
-      let updateIdx = requests.findIndex(r => r.id === action.id);
+      let updatedRequest = requests.get(action.id);
-      if (updateIdx === -1) {
+      if (!updatedRequest) {
         return state;
       }
 
-      requests = requests.update(updateIdx, r => r.withMutations(request => {
+      updatedRequest = updatedRequest.withMutations(request => {
         for (let [key, value] of Object.entries(action.data)) {
           if (!UPDATE_PROPS.includes(key)) {
             continue;
@@ -153,10 +153,10 @@ function requestsReducer(state = new Requests(), action) {
               break;
           }
         }
-      }));
+      });
 
       return state.withMutations(st => {
-        st.requests = requests;
+        st.requests = requests.set(updatedRequest.id, updatedRequest);
         st.lastEndedMillis = lastEndedMillis;
       });
     }
@@ -176,12 +176,11 @@ function requestsReducer(state = new Requests(), action) {
         return state;
       }
 
-      let clonedIdx = requests.findIndex(r => r.id === selectedId);
+      let clonedRequest = requests.get(selectedId);
-      if (clonedIdx === -1) {
+      if (!clonedRequest) {
         return state;
       }
 
-      let clonedRequest = requests.get(clonedIdx);
       let newRequest = new Request({
         id: clonedRequest.id + "-clone",
         method: clonedRequest.method,
@@ -192,13 +191,8 @@ function requestsReducer(state = new Requests(), action) {
         isCustom: true
       });
 
-      // Insert the clone right after the original. This ensures that the requests
-      // are always sorted next to each other, even when multiple requests are
-      // equal according to the sorting criteria.
-      requests = requests.insert(clonedIdx + 1, newRequest);
-
       return state.withMutations(st => {
-        st.requests = requests;
+        st.requests = requests.set(newRequest.id, newRequest);
         st.selectedId = newRequest.id;
       });
     }
@@ -209,15 +203,14 @@ function requestsReducer(state = new Requests(), action) {
         return state;
       }
 
-      let removedRequest = requests.find(r => r.id === selectedId);
-
       // Only custom requests can be removed
+      let removedRequest = requests.get(selectedId);
       if (!removedRequest || !removedRequest.isCustom) {
         return state;
       }
 
       return state.withMutations(st => {
-        st.requests = requests.filter(r => r !== removedRequest);
+        st.requests = requests.delete(selectedId);
         st.selectedId = null;
       });
     }
@@ -227,7 +220,7 @@ function requestsReducer(state = new Requests(), action) {
       }
 
       if (!state.selectedId && !state.requests.isEmpty()) {
-        return state.set("selectedId", state.requests.get(0).id);
+        return state.set("selectedId", state.requests.first().id);
       }
 
       return state;

devtools/client/netmonitor/selectors/requests.js

@@ -9,16 +9,30 @@ const { Filters, isFreetextMatch } = require("../filter-predicates");
 const { Sorters } = require("../sort-predicates");
 
 /**
- * Check if the given requests is a clone, find and return the original request if it is.
+ * Take clones into account when sorting.
- * Cloned requests are sorted by comparing the original ones.
+ * If a request is a clone, use the original request for comparison.
+ * If one of the compared request is a clone of the other, sort them next to each other.
  */
-function getOrigRequest(requests, req) {
+function sortWithClones(requests, sorter, a, b) {
-  if (!req.id.endsWith("-clone")) {
+  const aId = a.id, bId = b.id;
-    return req;
+
+  if (aId.endsWith("-clone")) {
+    const aOrigId = aId.replace(/-clone$/, "");
+    if (aOrigId === bId) {
+      return +1;
+    }
+    a = requests.get(aOrigId);
   }
 
-  const origId = req.id.replace(/-clone$/, "");
+  if (bId.endsWith("-clone")) {
-  return requests.find(r => r.id === origId);
+    const bOrigId = bId.replace(/-clone$/, "");
+    if (bOrigId === aId) {
+      return -1;
+    }
+    b = requests.get(bOrigId);
+  }
+
+  return sorter(a, b);
 }
 
 const getFilterFn = createSelector(
@@ -35,39 +49,24 @@ const getSortFn = createSelector(
   state => state.requests.requests,
   state => state.sort,
   (requests, sort) => {
-    let dataSorter = Sorters[sort.type || "waterfall"];
+    const sorter = Sorters[sort.type || "waterfall"];
-
-    function sortWithClones(a, b) {
-      // If one request is a clone of the other, sort them next to each other
-      if (a.id == b.id + "-clone") {
-        return +1;
-      } else if (a.id + "-clone" == b.id) {
-        return -1;
-      }
-
-      // Otherwise, get the original requests and compare them
-      return dataSorter(
-        getOrigRequest(requests, a),
-        getOrigRequest(requests, b)
-      );
-    }
-
     const ascending = sort.ascending ? +1 : -1;
-    return (a, b) => ascending * sortWithClones(a, b, dataSorter);
+    return (a, b) => ascending * sortWithClones(requests, sorter, a, b);
   }
 );
 
 const getSortedRequests = createSelector(
   state => state.requests.requests,
   getSortFn,
-  (requests, sortFn) => requests.sort(sortFn)
+  (requests, sortFn) => requests.valueSeq().sort(sortFn).toList()
 );
 
 const getDisplayedRequests = createSelector(
   state => state.requests.requests,
   getFilterFn,
   getSortFn,
-  (requests, filterFn, sortFn) => requests.filter(filterFn).sort(sortFn)
+  (requests, filterFn, sortFn) => requests.valueSeq()
+    .filter(filterFn).sort(sortFn).toList()
 );
 
 const getDisplayedRequestsSummary = createSelector(
@@ -95,17 +94,11 @@ const getDisplayedRequestsSummary = createSelector(
 
 const getSelectedRequest = createSelector(
   state => state.requests,
-  requests => {
+  ({ selectedId, requests }) => selectedId ? requests.get(selectedId) : null
-    if (!requests.selectedId) {
-      return null;
-    }
-
-    return requests.requests.find(r => r.id === requests.selectedId);
-  }
 );
 
 function getRequestById(state, id) {
-  return state.requests.requests.find(r => r.id === id);
+  return state.requests.requests.get(id);
 }
 
 function getDisplayedRequestById(state, id) {

devtools/client/netmonitor/sort-predicates.js

@@ -6,8 +6,6 @@
 
 const {
   getAbbreviatedMimeType,
-  getUrlBaseNameWithQuery,
-  getUrlHost,
 } = require("./request-utils");
 
 /**
@@ -23,65 +21,64 @@ const {
  *         >0 to sort second to a lower index than first
  */
 
+function compareValues(first, second) {
+  if (first === second) {
+    return 0;
+  }
+  return first > second ? 1 : -1;
+}
+
 function waterfall(first, second) {
-  return first.startedMillis - second.startedMillis;
+  const result = compareValues(first.startedMillis, second.startedMillis);
+  return result || compareValues(first.id, second.id);
 }
 
 function status(first, second) {
-  return first.status == second.status
+  const result = compareValues(first.status, second.status);
-         ? first.startedMillis - second.startedMillis
+  return result || waterfall(first, second);
-         : first.status - second.status;
 }
 
 function method(first, second) {
-  if (first.method == second.method) {
+  const result = compareValues(first.method, second.method);
-    return first.startedMillis - second.startedMillis;
+  return result || waterfall(first, second);
-  }
-  return first.method > second.method ? 1 : -1;
 }
 
 function file(first, second) {
-  let firstUrl = getUrlBaseNameWithQuery(first.url).toLowerCase();
+  const firstUrl = first.urlDetails.baseNameWithQuery.toLowerCase();
-  let secondUrl = getUrlBaseNameWithQuery(second.url).toLowerCase();
+  const secondUrl = second.urlDetails.baseNameWithQuery.toLowerCase();
-  if (firstUrl == secondUrl) {
+  const result = compareValues(firstUrl, secondUrl);
-    return first.startedMillis - second.startedMillis;
+  return result || waterfall(first, second);
-  }
-  return firstUrl > secondUrl ? 1 : -1;
 }
 
 function domain(first, second) {
-  let firstDomain = getUrlHost(first.url).toLowerCase();
+  const firstDomain = first.urlDetails.host.toLowerCase();
-  let secondDomain = getUrlHost(second.url).toLowerCase();
+  const secondDomain = second.urlDetails.host.toLowerCase();
-  if (firstDomain == secondDomain) {
+  const result = compareValues(firstDomain, secondDomain);
-    return first.startedMillis - second.startedMillis;
+  return result || waterfall(first, second);
-  }
-  return firstDomain > secondDomain ? 1 : -1;
 }
 
 function cause(first, second) {
-  let firstCause = first.cause.type;
+  const firstCause = first.cause.type;
-  let secondCause = second.cause.type;
+  const secondCause = second.cause.type;
-  if (firstCause == secondCause) {
+  const result = compareValues(firstCause, secondCause);
-    return first.startedMillis - second.startedMillis;
+  return result || waterfall(first, second);
-  }
-  return firstCause > secondCause ? 1 : -1;
 }
 
 function type(first, second) {
-  let firstType = getAbbreviatedMimeType(first.mimeType).toLowerCase();
+  const firstType = getAbbreviatedMimeType(first.mimeType).toLowerCase();
-  let secondType = getAbbreviatedMimeType(second.mimeType).toLowerCase();
+  const secondType = getAbbreviatedMimeType(second.mimeType).toLowerCase();
-  if (firstType == secondType) {
+  const result = compareValues(firstType, secondType);
-    return first.startedMillis - second.startedMillis;
+  return result || waterfall(first, second);
-  }
-  return firstType > secondType ? 1 : -1;
 }
 
 function transferred(first, second) {
-  return first.transferredSize - second.transferredSize;
+  const result = compareValues(first.transferredSize, second.transferredSize);
+  return result || waterfall(first, second);
 }
 
 function size(first, second) {
-  return first.contentSize - second.contentSize;
+  const result = compareValues(first.contentSize, second.contentSize);
+  return result || waterfall(first, second);
 }
 
 exports.Sorters = {