From bb638b7694f7cb6f8739d0bc6d258dbdbbadba5d Mon Sep 17 00:00:00 2001
From: David Glasser <glasser@meteor.com>
Date: Mon, 2 Mar 2015 16:54:22 -0800
Subject: [PATCH] minimongo: Find paused flag in the right place

The `paused` flag is stored on LocalCollection, but two places in
minimongo looked for it on the `query` object (which represents an
active observeChanges call).  In both cases, the bug had no correctness
impact but could have a performance impact.

Bug 1:

`_recomputeResults` (used to calculate changes to skip and limit
queries) tried to avoid calculating the diff if the collection had been
paused (by pauseObservers as part of latency compensation), but it
looked for the `paused` flag in the wrong place and always ran the diff.

This didn't have an effect on correctness (because the wrapped callbacks
on `query` are no-ops when `paused` is set) but did waste time on
unnecessary diffs.

Bug 2:

In `update`, we tried to avoid saving original results for skip/limit
queries if the collection was paused, because we don't actually run the
diff for paused queries.  (Well, except for the fact that Bug 1 made us
run the diffs anyway...)  But since we checked `query.paused` instead of
`self.paused`, we wasted time cloning the results even if we were
paused.

Reviewed at https://rbcommons.com/s/meteor/r/4/
---
 History.md                      |  2 ++
 packages/minimongo/minimongo.js | 18 ++++++++++--------
 2 files changed, 12 insertions(+), 8 deletions(-)

diff --git a/History.md b/History.md
index d7ea37e7d2..b354610520 100644
--- a/History.md
+++ b/History.md
@@ -108,6 +108,8 @@
 * Fix crash in `meteor publish` in some cases when the package is inside an
   app. #3676
 
+* Avoid unnecessary work while paused in minimongo.
+
 * Upgraded dependencies:
 
   - node: 0.10.36 (from 0.10.33)
diff --git a/packages/minimongo/minimongo.js b/packages/minimongo/minimongo.js
index 9b43174751..89c0dc0e1e 100644
--- a/packages/minimongo/minimongo.js
+++ b/packages/minimongo/minimongo.js
@@ -582,7 +582,7 @@ LocalCollection.prototype.insert = function (doc, callback) {
 
   _.each(queriesToRecompute, function (qid) {
     if (self.queries[qid])
-      LocalCollection._recomputeResults(self.queries[qid]);
+      self._recomputeResults(self.queries[qid]);
   });
   self._observeQueue.drain();
 
@@ -676,7 +676,7 @@ LocalCollection.prototype.remove = function (selector, callback) {
   _.each(queriesToRecompute, function (qid) {
     var query = self.queries[qid];
     if (query)
-      LocalCollection._recomputeResults(query);
+      self._recomputeResults(query);
   });
   self._observeQueue.drain();
   result = remove.length;
@@ -708,7 +708,7 @@ LocalCollection.prototype.update = function (selector, mod, options, callback) {
   _.each(self.queries, function (query, qid) {
     // XXX for now, skip/limit implies ordered observe, so query.results is
     // always an array
-    if ((query.cursor.skip || query.cursor.limit) && !query.paused)
+    if ((query.cursor.skip || query.cursor.limit) && ! self.paused)
       qidToOriginalResults[qid] = EJSON.clone(query.results);
   });
   var recomputeQids = {};
@@ -731,8 +731,7 @@ LocalCollection.prototype.update = function (selector, mod, options, callback) {
   _.each(recomputeQids, function (dummy, qid) {
     var query = self.queries[qid];
     if (query)
-      LocalCollection._recomputeResults(query,
-                                        qidToOriginalResults[qid]);
+      self._recomputeResults(query, qidToOriginalResults[qid]);
   });
   self._observeQueue.drain();
 
@@ -919,15 +918,18 @@ LocalCollection._updateInResults = function (query, doc, old_doc) {
 // old results (and there's no need to pass in oldResults), because these
 // operations don't mutate the documents in the collection. Update needs to pass
 // in an oldResults which was deep-copied before the modifier was applied.
-LocalCollection._recomputeResults = function (query, oldResults) {
-  if (!oldResults)
+//
+// oldResults is guaranteed to be ignored if the query is not paused.
+LocalCollection.prototype._recomputeResults = function (query, oldResults) {
+  var self = this;
+  if (! self.paused && ! oldResults)
     oldResults = query.results;
   if (query.distances)
     query.distances.clear();
   query.results = query.cursor._getRawObjects({
     ordered: query.ordered, distances: query.distances});
 
-  if (!query.paused) {
+  if (! self.paused) {
     LocalCollection._diffQueryChanges(
       query.ordered, oldResults, query.results, query);
   }