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/
This commit is contained in:
David Glasser
2015-03-02 16:54:22 -08:00
parent b595c5027f
commit bb638b7694
2 changed files with 12 additions and 8 deletions

View File

@@ -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)

View File

@@ -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);
}