Bug 1363678 - Refactor how we do the messages limit on the reducer. r=Honza
authorNicolas Chevobbe <nchevobbe@mozilla.com>
Wed, 31 May 2017 14:00:15 +0200
changeset 412304 e9f6b74293361a45b8ac2342e7a727e93e9bf7de
parent 412303 617099302c92ebfe8fc2a34dffb0dd8d9755fc9f
child 412305 f481ef5c3bf0496828b1c645cfdf9f029c895f2d
push id1490
push usermtabara@mozilla.com
push dateMon, 31 Jul 2017 14:08:16 +0000
treeherdermozilla-release@70e32e6bf15e [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersHonza
bugs1363678
milestone55.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 1363678 - Refactor how we do the messages limit on the reducer. r=Honza The parent commit did some changes to the architecture of the store that needed to be handled on the limit messages function. We take advantage of this to rewrite the functions involved to be as efficient as possible. To do that, we limit the work done by Immutable structures by doing changes in them only once per added messages. MozReview-Commit-ID: 6VzobhWzK40
devtools/client/webconsole/new-console-output/reducers/messages.js
--- a/devtools/client/webconsole/new-console-output/reducers/messages.js
+++ b/devtools/client/webconsole/new-console-output/reducers/messages.js
@@ -236,95 +236,106 @@ function getParentGroups(currentGroup, g
     }
   }
 
   return groups;
 }
 
 /**
  * Remove all top level messages that exceeds message limit.
- * Also release all backend actors associated with these
- * messages.
+ * Also populate an array of all backend actors associated with these
+ * messages so they can be released.
  */
 function limitTopLevelMessageCount(state, record) {
-  let tempRecord = {
-    messagesById: record.messagesById,
-    messagesUiById: record.messagesUiById,
-    messagesTableDataById: record.messagesTableDataById,
-    groupsById: record.groupsById,
-  };
+  let topLevelCount = record.groupsById.size === 0
+    ? record.messagesById.size
+    : getToplevelMessageCount(record);
 
-  let removedMessages = state.removedMessages;
-
-  // Remove top level messages logged over the limit.
-  let topLevelCount = getToplevelMessageCount(tempRecord);
-  while (topLevelCount > logLimit) {
-    removedMessages.push(...removeFirstMessage(tempRecord));
-    topLevelCount--;
+  if (topLevelCount <= logLimit) {
+    return record;
   }
 
-  // Filter out messages with no arguments. Only actual arguments
-  // can be associated with backend actors.
-  removedMessages = state.removedMessages.filter(msg => msg.parameters);
+  const removedMessagesId = [];
+  const removedMessages = [];
+  let visibleMessages = [...record.visibleMessages];
+
+  let cleaningGroup = false;
+  record.messagesById.forEach((message, id) => {
+    // If we were cleaning a group and the current message does not have
+    // a groupId, we're done cleaning.
+    if (cleaningGroup === true && !message.groupId) {
+      cleaningGroup = false;
+    }
+
+    // If we're not cleaning a group and the message count is below the logLimit,
+    // we exit the forEach iteration.
+    if (cleaningGroup === false && topLevelCount <= logLimit) {
+      return false;
+    }
+
+    // If we're not currently cleaning a group, and the current message is identified
+    // as a group, set the cleaning flag to true.
+    if (cleaningGroup === false && record.groupsById.has(id)) {
+      cleaningGroup = true;
+    }
+
+    if (!message.groupId) {
+      topLevelCount--;
+    }
+
+    removedMessagesId.push(id);
 
-  // Update original record object
-  record.set("messagesById", tempRecord.messagesById);
-  record.set("messagesUiById", tempRecord.messagesUiById);
-  record.set("messagesTableDataById", tempRecord.messagesTableDataById);
-  record.set("groupsById", tempRecord.groupsById);
-  record.set("removedMessages", removedMessages);
+    // Filter out messages with no arguments. Only actual arguments
+    // can be associated with backend actors.
+    if (message && message.parameters) {
+      removedMessages.push(message);
+    }
+
+    const index = visibleMessages.indexOf(id);
+    if (index > -1) {
+      visibleMessages.splice(index, 1);
+    }
+
+    return true;
+  });
+
+  if (removedMessages.length > 0) {
+    record.set("removedMessages", record.removedMessages.concat(removedMessages));
+  }
+
+  if (record.visibleMessages.length > visibleMessages.length) {
+    record.set("visibleMessages", visibleMessages);
+  }
+
+  const isInRemovedId = id => removedMessagesId.includes(id);
+  const mapHasRemovedIdKey = map => map.findKey((value, id) => isInRemovedId(id));
+  const cleanUpCollection = map => removedMessagesId.forEach(id => map.remove(id));
+
+  record.set("messagesById", record.messagesById.withMutations(cleanUpCollection));
+
+  if (record.messagesUiById.find(isInRemovedId)) {
+    record.set("messagesUiById", record.messagesUiById.withMutations(cleanUpCollection));
+  }
+  if (mapHasRemovedIdKey(record.messagesTableDataById)) {
+    record.set("messagesTableDataById",
+      record.messagesTableDataById.withMutations(cleanUpCollection));
+  }
+  if (mapHasRemovedIdKey(record.groupsById)) {
+    record.set("groupsById", record.groupsById.withMutations(cleanUpCollection));
+  }
+
+  return record;
 }
 
 /**
  * Returns total count of top level messages (those which are not
  * within a group).
  */
 function getToplevelMessageCount(record) {
-  return [...record.messagesById].filter(message => !message.groupId).length;
-}
-
-/**
- * Remove first (the oldest) message from the store. The methods removes
- * also all its references and children from the store.
- *
- * @return {Array} Flat array of removed messages.
- */
-function removeFirstMessage(record) {
-  let firstMessage = record.messagesById.first();
-  record.messagesById = record.messagesById.shift();
-
-  // Remove from list of opened groups.
-  let uiIndex = record.messagesUiById.indexOf(firstMessage);
-  if (uiIndex >= 0) {
-    record.messagesUiById = record.messagesUiById.delete(uiIndex);
-  }
-
-  // Remove from list of tables.
-  if (record.messagesTableDataById.has(firstMessage.id)) {
-    record.messagesTableDataById = record.messagesTableDataById.delete(firstMessage.id);
-  }
-
-  // Remove from list of parent groups.
-  if (record.groupsById.has(firstMessage.id)) {
-    record.groupsById = record.groupsById.delete(firstMessage.id);
-  }
-
-  let removedMessages = [firstMessage];
-
-  // Remove all children. This loop assumes that children of removed
-  // group immediately follows the group. We use recursion since
-  // there might be inner groups.
-  let message = record.messagesById.first();
-  while (message.groupId == firstMessage.id) {
-    removedMessages.push(...removeFirstMessage(record));
-    message = record.messagesById.first();
-  }
-
-  // Return array with all removed messages.
-  return removedMessages;
+  return record.messagesById.count(message => !message.groupId);
 }
 
 function shouldMessageBeVisible(message, messagesState, filtersState, checkGroup = true) {
   return (
     (
       checkGroup === false
       || isInOpenedGroup(message, messagesState.groupsById, messagesState.messagesUiById)
     )