From ceb32e65819b215c31c885d3fd8db0eb6b315a58 Mon Sep 17 00:00:00 2001 From: Jarda Snajdr Date: Thu, 5 Jan 2017 21:42:04 +0100 Subject: [PATCH] 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 | 2 +- .../client/netmonitor/reducers/requests.js | 35 ++++------ .../client/netmonitor/selectors/requests.js | 63 ++++++++--------- devtools/client/netmonitor/sort-predicates.js | 69 +++++++++---------- 4 files changed, 76 insertions(+), 93 deletions(-) diff --git a/devtools/client/netmonitor/netmonitor-view.js b/devtools/client/netmonitor/netmonitor-view.js index 7825522adab0..368c4629942a 100644 --- a/devtools/client/netmonitor/netmonitor-view.js +++ b/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); }); diff --git a/devtools/client/netmonitor/reducers/requests.js b/devtools/client/netmonitor/reducers/requests.js index 1dfbf92063c0..eee34cdc5a07 100644 --- a/devtools/client/netmonitor/reducers/requests.js +++ b/devtools/client/netmonitor/reducers/requests.js @@ -55,8 +55,8 @@ const Request = I.Record({ }); const Requests = I.Record({ - // The request list - requests: I.List(), + // The collection of requests (keyed by id) + 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); - if (updateIdx === -1) { + let updatedRequest = requests.get(action.id); + 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); - if (clonedIdx === -1) { + let clonedRequest = requests.get(selectedId); + 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; diff --git a/devtools/client/netmonitor/selectors/requests.js b/devtools/client/netmonitor/selectors/requests.js index ba61f6848df4..208d74ad7711 100644 --- a/devtools/client/netmonitor/selectors/requests.js +++ b/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. - * Cloned requests are sorted by comparing the original ones. + * Take clones into account when sorting. + * 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) { - if (!req.id.endsWith("-clone")) { - return req; +function sortWithClones(requests, sorter, a, b) { + const aId = a.id, bId = b.id; + + if (aId.endsWith("-clone")) { + const aOrigId = aId.replace(/-clone$/, ""); + if (aOrigId === bId) { + return +1; + } + a = requests.get(aOrigId); } - const origId = req.id.replace(/-clone$/, ""); - return requests.find(r => r.id === origId); + if (bId.endsWith("-clone")) { + 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"]; - - 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 sorter = Sorters[sort.type || "waterfall"]; 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 => { - if (!requests.selectedId) { - return null; - } - - return requests.requests.find(r => r.id === requests.selectedId); - } + ({ selectedId, requests }) => selectedId ? requests.get(selectedId) : null ); function getRequestById(state, id) { - return state.requests.requests.find(r => r.id === id); + return state.requests.requests.get(id); } function getDisplayedRequestById(state, id) { diff --git a/devtools/client/netmonitor/sort-predicates.js b/devtools/client/netmonitor/sort-predicates.js index 935ce540bfb8..9a66dfc16165 100644 --- a/devtools/client/netmonitor/sort-predicates.js +++ b/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 - ? first.startedMillis - second.startedMillis - : first.status - second.status; + const result = compareValues(first.status, second.status); + return result || waterfall(first, second); } function method(first, second) { - if (first.method == second.method) { - return first.startedMillis - second.startedMillis; - } - return first.method > second.method ? 1 : -1; + const result = compareValues(first.method, second.method); + return result || waterfall(first, second); } function file(first, second) { - let firstUrl = getUrlBaseNameWithQuery(first.url).toLowerCase(); - let secondUrl = getUrlBaseNameWithQuery(second.url).toLowerCase(); - if (firstUrl == secondUrl) { - return first.startedMillis - second.startedMillis; - } - return firstUrl > secondUrl ? 1 : -1; + const firstUrl = first.urlDetails.baseNameWithQuery.toLowerCase(); + const secondUrl = second.urlDetails.baseNameWithQuery.toLowerCase(); + const result = compareValues(firstUrl, secondUrl); + return result || waterfall(first, second); } function domain(first, second) { - let firstDomain = getUrlHost(first.url).toLowerCase(); - let secondDomain = getUrlHost(second.url).toLowerCase(); - if (firstDomain == secondDomain) { - return first.startedMillis - second.startedMillis; - } - return firstDomain > secondDomain ? 1 : -1; + const firstDomain = first.urlDetails.host.toLowerCase(); + const secondDomain = second.urlDetails.host.toLowerCase(); + const result = compareValues(firstDomain, secondDomain); + return result || waterfall(first, second); } function cause(first, second) { - let firstCause = first.cause.type; - let secondCause = second.cause.type; - if (firstCause == secondCause) { - return first.startedMillis - second.startedMillis; - } - return firstCause > secondCause ? 1 : -1; + const firstCause = first.cause.type; + const secondCause = second.cause.type; + const result = compareValues(firstCause, secondCause); + return result || waterfall(first, second); } function type(first, second) { - let firstType = getAbbreviatedMimeType(first.mimeType).toLowerCase(); - let secondType = getAbbreviatedMimeType(second.mimeType).toLowerCase(); - if (firstType == secondType) { - return first.startedMillis - second.startedMillis; - } - return firstType > secondType ? 1 : -1; + const firstType = getAbbreviatedMimeType(first.mimeType).toLowerCase(); + const secondType = getAbbreviatedMimeType(second.mimeType).toLowerCase(); + const result = compareValues(firstType, secondType); + return result || waterfall(first, second); } 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 = {