Improve optimization of EJSON.clone calls in Minimongo take 3

Make it work with documents with Mongo IDs.

Also some more code reorg

History.md

@@ -1,5 +1,8 @@
 ## v.NEXT
 
+* Improve minimongo performance on updating documents when there are
+  many active observes. #5627
+
 * Split up `standard-minifiers` in separate CSS (`standard-minifiers-css`) and JS
   minifiers(`standard-minifiers-js`). `standard-minifiers` now acts as an umbrella package for these
   2 minifiers.
@@ -7,6 +10,9 @@
 * Move `DDPRateLimiter` to the server only, since it won't work if it is called from the client. It
   will now error if referenced from the client at all.
 
+Patches contributed by GitHub users vereed, ...
+
+
 ## v.1.2.1, 2015-Oct-26
 
 * `coll.insert()` now uses a faster (but cryptographically insecure)

packages/minimongo/minimongo.js

@@ -698,15 +698,18 @@ LocalCollection.prototype.update = function (selector, mod, options, callback) {
   // _recomputeResults.)
   var qidToOriginalResults = {};
   // We should only clone each document once, even if it appears in multiple queries
-  var docMap = {};
+  var docMap = new LocalCollection._IdMap;
   var idsMatchedBySelector = LocalCollection._idsMatchedBySelector(selector);
 
   _.each(self.queries, function (query, qid) {
     if ((query.cursor.skip || query.cursor.limit) && ! self.paused) {
       // Catch the case of a reactive `count()` on a cursor with skip
-      // or limit, which registers an unordered observe
+      // or limit, which registers an unordered observe. This is a
+      // pretty rare case, so we just clone the entire result set with
+      // no optimizations for documents that appear in these result
+      // sets and other queries.
       if (query.results instanceof LocalCollection._IdMap) {
-        qidToOriginalResults[qid] = EJSON.clone(query.results);
+        qidToOriginalResults[qid] = query.results.clone();
         return;
       }
 
@@ -714,22 +717,30 @@ LocalCollection.prototype.update = function (selector, mod, options, callback) {
         throw new Error("Assertion failed: query.results not an array");
       }
 
-      qidToOriginalResults[qid] = query.results.map(function(result) {
-        if (!_.has(docMap, result._id)) {
-          // Clone each document because it may be modified it before
-          // the new and old result sets are diffed. But if we know
-          // exactly which document IDs we're going to modify, then we
-          // only need to clone those.
-          docMap[result._id] =
-            (idsMatchedBySelector && !_.any(idsMatchedBySelector, function(id) {
-              return EJSON.equals(id, result._id);
-            }))
-            ? result
-            : EJSON.clone(result);
-        }
+      // Clones a document to be stored in `qidToOriginalResults`
+      // because it may be modified before the new and old result sets
+      // are diffed. But if we know exactly which document IDs we're
+      // going to modify, then we only need to clone those.
+      var memoizedCloneIfNeeded = function(doc) {
+        if (docMap.has(doc._id)) {
+          return docMap.get(doc._id);
+        } else {
+          var docToMemoize;
 
-        return docMap[result._id];
-      });
+          if (idsMatchedBySelector && !_.any(idsMatchedBySelector, function(id) {
+            return EJSON.equals(id, doc._id);
+          })) {
+            docToMemoize = doc;
+          } else {
+            docToMemoize = EJSON.clone(doc);
+          }
+
+          docMap.set(doc._id, docToMemoize);
+          return docToMemoize;
+        }
+      };
+
+      qidToOriginalResults[qid] = query.results.map(memoizedCloneIfNeeded);
     }
   });
   var recomputeQids = {};