From cf3ffa88a9b81154dc8f46db9246c573942406f1 Mon Sep 17 00:00:00 2001 From: David Glasser Date: Tue, 28 Jan 2014 15:12:14 -0800 Subject: [PATCH 1/7] check for paused earlier --- packages/minimongo/minimongo.js | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/packages/minimongo/minimongo.js b/packages/minimongo/minimongo.js index 34b5efa035..5bc4857f6c 100644 --- a/packages/minimongo/minimongo.js +++ b/packages/minimongo/minimongo.js @@ -335,17 +335,18 @@ _.extend(LocalCollection.Cursor.prototype, { var context = this; var args = arguments; + if (self.collection.paused) + return; + if (fieldsIndex !== undefined && self.projection_f) { args[fieldsIndex] = self.projection_f(args[fieldsIndex]); if (ignoreEmptyFields && _.isEmpty(args[fieldsIndex])) return; } - if (!self.collection.paused) { - self.collection._observeQueue.queueTask(function () { - f.apply(context, args); - }); - } + self.collection._observeQueue.queueTask(function () { + f.apply(context, args); + }); }; }; query.added = wrapCallback(options.added, 1); From c8a1df81a7517b0fbc856f002feb55638be35350 Mon Sep 17 00:00:00 2001 From: David Glasser Date: Tue, 28 Jan 2014 15:21:40 -0800 Subject: [PATCH 2/7] optimize paused remove({}) which oplog uses --- packages/minimongo/minimongo.js | 27 ++++++++++++++++++++++++--- packages/minimongo/minimongo_tests.js | 8 ++++++++ 2 files changed, 32 insertions(+), 3 deletions(-) diff --git a/packages/minimongo/minimongo.js b/packages/minimongo/minimongo.js index 5bc4857f6c..46d0bf17c0 100644 --- a/packages/minimongo/minimongo.js +++ b/packages/minimongo/minimongo.js @@ -563,10 +563,31 @@ LocalCollection.prototype.insert = function (doc, callback) { LocalCollection.prototype.remove = function (selector, callback) { var self = this; - var remove = []; - var queriesToRecompute = []; + // Easy special case: if we're not calling observeChanges callbacks and we're + // not saving originals and we got asked to remove everything, then just empty + // everything directly. + if (self.paused && !self._savedOriginals && EJSON.equals(selector, {})) { + var result = self._docs.size(); + self._docs.clear(); + _.each(self.queries, function (query) { + if (query.ordered) { + query.results = []; + } else { + query.results.clear(); + } + }); + if (callback) { + Meteor.defer(function () { + callback(null, result); + }); + } + return result; + } + var matcher = new Minimongo.Matcher(selector, self); + var queriesToRecompute = []; + var remove = []; // Avoid O(n) for "remove a single doc by ID". var specificIds = LocalCollection._idsMatchedBySelector(selector); @@ -616,7 +637,7 @@ LocalCollection.prototype.remove = function (selector, callback) { LocalCollection._recomputeResults(query); }); self._observeQueue.drain(); - var result = remove.length; + result = remove.length; if (callback) Meteor.defer(function () { callback(null, result); diff --git a/packages/minimongo/minimongo_tests.js b/packages/minimongo/minimongo_tests.js index 68c6b0203d..3ace1c931e 100644 --- a/packages/minimongo/minimongo_tests.js +++ b/packages/minimongo/minimongo_tests.js @@ -2565,6 +2565,14 @@ Tinytest.add("minimongo - pause", function (test) { test.equal(operations.shift(), ['changed', {a:3}, 0, {a:1}]); test.length(operations, 0); + // test special case for remove({}) + c.pauseObservers(); + test.equal(c.remove({}), 1); + test.length(operations, 0); + c.resumeObservers(); + test.equal(operations.shift(), ['removed', 1, 0, {a:3}]); + test.length(operations, 0); + h.stop(); }); From 4352b79502e12db36c1bec7c35d62478fe4480fb Mon Sep 17 00:00:00 2001 From: David Glasser Date: Tue, 28 Jan 2014 15:28:26 -0800 Subject: [PATCH 3/7] clean up minimongo 'query' object delete unused fields, rename underscored fields --- packages/minimongo/minimongo.js | 28 +++++++++++++--------------- 1 file changed, 13 insertions(+), 15 deletions(-) diff --git a/packages/minimongo/minimongo.js b/packages/minimongo/minimongo.js index 46d0bf17c0..db01a9bde1 100644 --- a/packages/minimongo/minimongo.js +++ b/packages/minimongo/minimongo.js @@ -42,7 +42,7 @@ LocalCollection = function (options) { // ordered: bool. ordered queries have addedBefore/movedBefore callbacks. // results: array (ordered) or object (unordered) of current results // (aliased with self._docs!) - // results_snapshot: snapshot of results. null if not paused. + // resultsSnapshot: snapshot of results. null if not paused. // cursor: Cursor object for the query. // selector, sorter, (callbacks): functions self.queries = {}; @@ -128,7 +128,7 @@ LocalCollection.Cursor = function (collection, selector, options) { self.fields = options.fields; if (self.fields) - self.projection_f = LocalCollection._compileProjection(self.fields); + self.projectionFn = LocalCollection._compileProjection(self.fields); self._transform = LocalCollection.wrapTransform(options.transform); @@ -181,8 +181,8 @@ LocalCollection.Cursor.prototype.forEach = function (callback, thisArg) { while (self.cursor_pos < self.db_objects.length) { var elt = EJSON.clone(self.db_objects[self.cursor_pos]); - if (self.projection_f) - elt = self.projection_f(elt); + if (self.projectionFn) + elt = self.projectionFn(elt); if (self._transform) elt = self._transform(elt); callback.call(thisArg, elt, self.cursor_pos, self); @@ -302,12 +302,10 @@ _.extend(LocalCollection.Cursor.prototype, { sorter: ordered && self.sorter, distances: ( self.matcher.hasGeoQuery() && ordered && new LocalCollection._IdMap), - results_snapshot: null, + resultsSnapshot: null, ordered: ordered, cursor: self, - observeChanges: options.observeChanges, - fields: self.fields, - projection_f: self.projection_f + projectionFn: self.projectionFn }; var qid; @@ -319,7 +317,7 @@ _.extend(LocalCollection.Cursor.prototype, { } query.results = self._getRawObjects(ordered, query.distances); if (self.collection.paused) - query.results_snapshot = (ordered ? [] : new LocalCollection._IdMap); + query.resultsSnapshot = (ordered ? [] : new LocalCollection._IdMap); // wrap callbacks we were passed. callbacks only fire when not paused and // are never undefined @@ -338,8 +336,8 @@ _.extend(LocalCollection.Cursor.prototype, { if (self.collection.paused) return; - if (fieldsIndex !== undefined && self.projection_f) { - args[fieldsIndex] = self.projection_f(args[fieldsIndex]); + if (fieldsIndex !== undefined && self.projectionFn) { + args[fieldsIndex] = self.projectionFn(args[fieldsIndex]); if (ignoreEmptyFields && _.isEmpty(args[fieldsIndex])) return; } @@ -660,7 +658,7 @@ LocalCollection.prototype.update = function (selector, mod, options, callback) { // Save the original results of any query that we might need to // _recomputeResults on, because _modifyAndNotify will mutate the objects in // it. (We don't need to save the original results of paused queries because - // they already have a results_snapshot and we won't be diffing in + // they already have a resultsSnapshot and we won't be diffing in // _recomputeResults.) var qidToOriginalResults = {}; _.each(self.queries, function (query, qid) { @@ -978,7 +976,7 @@ LocalCollection.prototype.pauseObservers = function () { for (var qid in this.queries) { var query = this.queries[qid]; - query.results_snapshot = EJSON.clone(query.results); + query.resultsSnapshot = EJSON.clone(query.results); } }; @@ -1001,8 +999,8 @@ LocalCollection.prototype.resumeObservers = function () { // Diff the current results against the snapshot and send to observers. // pass the query object for its observer callbacks. LocalCollection._diffQueryChanges( - query.ordered, query.results_snapshot, query.results, query); - query.results_snapshot = null; + query.ordered, query.resultsSnapshot, query.results, query); + query.resultsSnapshot = null; } self._observeQueue.drain(); }; From 9f253e6a0bcb9058c89c1c66ac755e092c8834cc Mon Sep 17 00:00:00 2001 From: David Glasser Date: Tue, 28 Jan 2014 16:06:07 -0800 Subject: [PATCH 4/7] minimongo: Make update-by-id O(1) --- packages/minimongo/minimongo.js | 48 +++++++++++++++++++-------------- 1 file changed, 28 insertions(+), 20 deletions(-) diff --git a/packages/minimongo/minimongo.js b/packages/minimongo/minimongo.js index db01a9bde1..ab74f42ddf 100644 --- a/packages/minimongo/minimongo.js +++ b/packages/minimongo/minimongo.js @@ -559,6 +559,28 @@ LocalCollection.prototype.insert = function (doc, callback) { return id; }; +// Iterates over a subset of documents that could match selector; calls +// f(doc, id) on each of them. Specifically, if selector specifies +// specific _id's, it only looks at those. doc is *not* cloned: it is the +// same object that is in _docs. +LocalCollection.prototype._eachPossiblyMatchingDoc = function (selector, f) { + var self = this; + var specificIds = LocalCollection._idsMatchedBySelector(selector); + if (specificIds) { + for (var i = 0; i < specificIds.length; ++i) { + var id = specificIds[i]; + var doc = self._docs.get(id); + if (doc) { + var breakIfFalse = f(doc, id); + if (breakIfFalse === false) + break; + } + } + } else { + self._docs.forEach(f); + } +}; + LocalCollection.prototype.remove = function (selector, callback) { var self = this; @@ -584,27 +606,13 @@ LocalCollection.prototype.remove = function (selector, callback) { } var matcher = new Minimongo.Matcher(selector, self); - var queriesToRecompute = []; var remove = []; + self._eachPossiblyMatchingDoc(selector, function (doc, id) { + if (matcher.documentMatches(doc).result) + remove.push(id); + }); - // Avoid O(n) for "remove a single doc by ID". - var specificIds = LocalCollection._idsMatchedBySelector(selector); - if (specificIds) { - _.each(specificIds, function (id) { - // We still have to run matcher, in case it's something like - // {_id: "X", a: 42} - var doc = self._docs.get(id); - if (doc && matcher.documentMatches(doc).result) - remove.push(id); - }); - } else { - self._docs.forEach(function (doc, id) { - if (matcher.documentMatches(doc).result) { - remove.push(id); - } - }); - } - + var queriesToRecompute = []; var queryRemove = []; for (var i = 0; i < remove.length; i++) { var removeId = remove[i]; @@ -671,7 +679,7 @@ LocalCollection.prototype.update = function (selector, mod, options, callback) { var updateCount = 0; - self._docs.forEach(function (doc, id) { + self._eachPossiblyMatchingDoc(selector, function (doc, id) { var queryResult = matcher.documentMatches(doc); if (queryResult.result) { // XXX Should we save the original even if mod ends up being a no-op? From 25fd3a344cfc0264f0362cf23ef51bb042230966 Mon Sep 17 00:00:00 2001 From: David Glasser Date: Tue, 28 Jan 2014 16:48:13 -0800 Subject: [PATCH 5/7] optionify LocalCollection._getRawObjects --- packages/minimongo/minimongo.js | 31 ++++++++++++++++++------------- 1 file changed, 18 insertions(+), 13 deletions(-) diff --git a/packages/minimongo/minimongo.js b/packages/minimongo/minimongo.js index ab74f42ddf..71977c86bf 100644 --- a/packages/minimongo/minimongo.js +++ b/packages/minimongo/minimongo.js @@ -170,7 +170,7 @@ LocalCollection.Cursor.prototype.forEach = function (callback, thisArg) { var self = this; if (self.db_objects === null) - self.db_objects = self._getRawObjects(true); + self.db_objects = self._getRawObjects({ordered: true}); if (self.reactive) self._depend({ @@ -220,7 +220,7 @@ LocalCollection.Cursor.prototype.count = function () { true /* allow the observe to be unordered */); if (self.db_objects === null) - self.db_objects = self._getRawObjects(true); + self.db_objects = self._getRawObjects({ordered: true}); return self.db_objects.length; }; @@ -315,7 +315,8 @@ _.extend(LocalCollection.Cursor.prototype, { qid = self.collection.next_qid++; self.collection.queries[qid] = query; } - query.results = self._getRawObjects(ordered, query.distances); + query.results = self._getRawObjects({ + ordered: ordered, distances: query.distances}); if (self.collection.paused) query.resultsSnapshot = (ordered ? [] : new LocalCollection._IdMap); @@ -412,13 +413,13 @@ _.extend(LocalCollection.Cursor.prototype, { // argument, this function will clear it and use it for this purpose (otherwise // it will just create its own _IdMap). The observeChanges implementation uses // this to remember the distances after this function returns. -LocalCollection.Cursor.prototype._getRawObjects = function (ordered, - distances) { +LocalCollection.Cursor.prototype._getRawObjects = function (options) { var self = this; + options = options || {}; // XXX use OrderedDict instead of array, and make IdMap and OrderedDict // compatible - var results = ordered ? [] : new LocalCollection._IdMap; + var results = options.ordered ? [] : new LocalCollection._IdMap; // fast path for single ID value if (self._selectorId !== undefined) { @@ -430,7 +431,7 @@ LocalCollection.Cursor.prototype._getRawObjects = function (ordered, var selectedDoc = self.collection._docs.get(self._selectorId); if (selectedDoc) { - if (ordered) + if (options.ordered) results.push(selectedDoc); else results.set(self._selectorId, selectedDoc); @@ -443,17 +444,20 @@ LocalCollection.Cursor.prototype._getRawObjects = function (ordered, // in the observeChanges case, distances is actually part of the "query" (ie, // live results set) object. in other cases, distances is only used inside // this function. - if (self.matcher.hasGeoQuery() && ordered) { - if (distances) + var distances; + if (self.matcher.hasGeoQuery() && options.ordered) { + if (options.distances) { + distances = options.distances; distances.clear(); - else + } else { distances = new LocalCollection._IdMap(); + } } self.collection._docs.forEach(function (doc, id) { var matchResult = self.matcher.documentMatches(doc); if (matchResult.result) { - if (ordered) { + if (options.ordered) { results.push(doc); if (distances && matchResult.distance !== undefined) distances.set(id, matchResult.distance); @@ -469,7 +473,7 @@ LocalCollection.Cursor.prototype._getRawObjects = function (ordered, return true; // continue }); - if (!ordered) + if (!options.ordered) return results; if (self.sorter) { @@ -888,7 +892,8 @@ LocalCollection._recomputeResults = function (query, oldResults) { oldResults = query.results; if (query.distances) query.distances.clear(); - query.results = query.cursor._getRawObjects(query.ordered, query.distances); + query.results = query.cursor._getRawObjects({ + ordered: query.ordered, distances: query.distances}); if (!query.paused) { LocalCollection._diffQueryChanges( From 764ceb6077d7aa76f11868beb9134522263fda8a Mon Sep 17 00:00:00 2001 From: David Glasser Date: Tue, 28 Jan 2014 17:00:00 -0800 Subject: [PATCH 6/7] add a test for observeChanges/limit/initial --- packages/minimongo/minimongo_tests.js | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/packages/minimongo/minimongo_tests.js b/packages/minimongo/minimongo_tests.js index 3ace1c931e..4bb58d2b94 100644 --- a/packages/minimongo/minimongo_tests.js +++ b/packages/minimongo/minimongo_tests.js @@ -2170,7 +2170,8 @@ Tinytest.add("minimongo - observe ordered", function (test) { handle.stop(); // test _suppress_initial - handle = c.find({}, {sort: {a: -1}}).observe(_.extend(cbs, {_suppress_initial: true})); + handle = c.find({}, {sort: {a: -1}}).observe(_.extend({ + _suppress_initial: true}, cbs)); test.equal(operations.shift(), undefined); c.insert({a:100}); test.equal(operations.shift(), ['added', {a:100}, 0, idA2]); @@ -2197,6 +2198,21 @@ Tinytest.add("minimongo - observe ordered", function (test) { test.equal(operations.shift(), ['changed', {a:3.5}, 0, {a:3}]); handle.stop(); + // test observe limit with pre-existing docs + c.remove({}); + c.insert({a: 1}); + c.insert({_id: 'two', a: 2}); + c.insert({a: 3}); + handle = c.find({}, {sort: {a: 1}, limit: 2}).observe(cbs); + test.equal(operations.shift(), ['added', {a:1}, 0, null]); + test.equal(operations.shift(), ['added', {a:2}, 1, null]); + test.equal(operations.shift(), undefined); + c.remove({a: 2}); + test.equal(operations.shift(), ['removed', 'two', 1, {a:2}]); + test.equal(operations.shift(), ['added', {a:3}, 1, null]); + test.equal(operations.shift(), undefined); + handle.stop(); + // test _no_indices c.remove({}); From a4b9dd52f30eac37bb597aba97a40e5e7596192c Mon Sep 17 00:00:00 2001 From: Avital Oliver Date: Wed, 29 Jan 2014 13:16:06 -0800 Subject: [PATCH 7/7] Fix _.each patch on IE. The patch was originally introuduced in bab936eac99004b6cd14b4ed8b279862e2d7e606 --- packages/underscore/underscore.js | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/packages/underscore/underscore.js b/packages/underscore/underscore.js index 12a74d5f55..3ffa3c111f 100644 --- a/packages/underscore/underscore.js +++ b/packages/underscore/underscore.js @@ -70,6 +70,21 @@ // Collection Functions // -------------------- + // METEOR CHANGE: Define _isArguments instead of depending on + // _.isArguments which is defined using each. In looksLikeArray + // (which each depends on), we then use _isArguments instead of + // _.isArguments. + var _isArguments = function (obj) { + return toString.call(obj) === '[object Arguments]'; + }; + // Define a fallback version of the method in browsers (ahem, IE), where + // there isn't any inspectable "Arguments" type. + if (!_isArguments(arguments)) { + _isArguments = function (obj) { + return !!(obj && hasOwnProperty.call(obj, 'callee')); + }; + } + // METEOR CHANGE: _.each({length: 5}) should be treated like an object, not an // array. This looksLikeArray function is introduced by Meteor, and replaces // all instances of `obj.length === +obj.length`. @@ -77,7 +92,8 @@ // https://github.com/jashkenas/underscore/issues/770 var looksLikeArray = function (obj) { return (obj.length === +obj.length - && (_.isArguments(obj) || obj.constructor !== Object)); + // _.isArguments not yet necessarily defined here + && (_isArguments(obj) || obj.constructor !== Object)); }; // The cornerstone, an `each` implementation, aka `forEach`.