Skip to content

Follow up on Meteors ticket "Reduce EJSON.clone's when observing query changes" #351

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Follow up on Meteors ticket "Reduce EJSON.clone's when observing query changes" #351

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
26f9b0d
Copied changes of https://github.com/meteor/meteor/pull/10864
SimonSimCity Mar 5, 2020
0a9a5c1
Merge remote-tracking branch 'cult-of-coders/master' into follow-up-m…
SimonSimCity Mar 5, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 10 additions & 10 deletions lib/mongo/ObserveMultiplex.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -189,13 +189,9 @@ _.extend(ObserveMultiplexer.prototype, {
if (!self._handles) return;

// First, apply the change to the cache.
// XXX We could make applyChange callbacks promise not to hang on to any
// state from their arguments (assuming that their supplied callbacks
// don't) and skip this clone. Currently 'changed' hangs on to state
// though.
self._cache.applyChange[callbackName].apply(
null,
EJSON.clone(args)
args
);

// If we haven't finished the initial adds, then we should only be getting
Expand All @@ -221,15 +217,16 @@ _.extend(ObserveMultiplexer.prototype, {
// We silence out removed exceptions
if (callback === 'removed') {
try {
callback.apply(null, EJSON.clone(args));
callback.apply(null, handle.nonMutatingCallbacks ? args : EJSON.clone(args));
} catch (e) {
// Supressing `removed non-existent exceptions`
if (!isRemovedNonExistent(e)) {
throw e;
}
}
} else {
callback && callback.apply(null, EJSON.clone(args));
callback && callback.apply(null,
handle.nonMutatingCallbacks ? args : EJSON.clone(args));
}
});
});
Expand All @@ -249,8 +246,8 @@ _.extend(ObserveMultiplexer.prototype, {
self._cache.docs.forEach(function(doc, id) {
if (!_.has(self._handles, handle._id))
throw Error('handle got removed before sending initial adds!');
var fields = EJSON.clone(doc);
delete fields._id;
const { _id, ...fields } = handle.nonMutatingCallbacks ? doc
: EJSON.clone(doc);
if (self._ordered) add(id, fields, null);
// we're going in order, so add at end
else add(id, fields);
Expand All @@ -259,7 +256,9 @@ _.extend(ObserveMultiplexer.prototype, {
});

var nextObserveHandleId = 1;
export function ObserveHandle(multiplexer, callbacks) {

// When the callbacks do not mutate the arguments, we can skip a lot of data clones
export function ObserveHandle(multiplexer, callbacks, nonMutatingCallbacks = false) {
var self = this;
// The end user is only supposed to call stop(). The other fields are
// accessible to the multiplexer, though.
Expand All @@ -279,6 +278,7 @@ export function ObserveHandle(multiplexer, callbacks) {
});
self._stopped = false;
self._id = nextObserveHandleId++;
self.nonMutatingCallbacks = nonMutatingCallbacks;
}

ObserveHandle.prototype.stop = function() {
Expand Down