From 36516d9a3e364de4498cc78d76a2d82b1ae5d83d Mon Sep 17 00:00:00 2001 From: Tim Griesser Date: Fri, 5 Oct 2012 08:00:34 -0400 Subject: [PATCH 1/4] Overhauling set/change for speed improvements --- backbone.js | 143 ++++++++++++++++++++++++++++---------------------- test/model.js | 14 +++++ test/speed.js | 10 ++++ 3 files changed, 105 insertions(+), 62 deletions(-) diff --git a/backbone.js b/backbone.js index 3457b0f4..1dd79d80 100644 --- a/backbone.js +++ b/backbone.js @@ -189,17 +189,15 @@ if (defaults = _.result(this, 'defaults')) { attrs = _.extend({}, defaults, attrs); } - this.attributes = {}; - this._escapedAttributes = {}; this.cid = _.uniqueId('c'); this.changed = {}; - this._changes = {}; - this._pending = {}; + this.attributes = {}; + this._escapedAttributes = {}; + this._modelState = []; this.set(attrs, {silent: true}); - // Reset change tracking. - this.changed = {}; - this._changes = {}; - this._pending = {}; + this._cleanChange = true; + this._modelState = []; + this._currentState = _.clone(this.attributes); this._previousAttributes = _.clone(this.attributes); this.initialize.apply(this, arguments); }; @@ -210,17 +208,26 @@ // A hash of attributes whose current and previous value differ. changed: null, - // A hash of attributes that have changed since the last time `change` - // was called. - _changes: null, + // Whether there is a pending request to fire in the final `change` loop. + _pending: false, - // A hash of attributes that have changed since the last `change` event - // began. - _pending: null, + // Whether the model is in the midst of a change cycle. + _changing: false, - // A hash of attributes with the current model state to determine if - // a `change` should be recorded within a nested `change` block. - _changing : null, + // Whether there has been a `set` call since the last + // calculation of the changed hash, for efficiency. + _cleanChange: true, + + // The model state used for comparison in determining if a + // change should be fired. + _currentState: null, + + // An array queue of all changes attributed to a model + // on the next non-silent change event. + _modelState: null, + + // A hash of the model's attributes when the last `change` occured. + _previousAttributes: null, // The default name for the JSON `id` attribute is `"id"`. MongoDB and // CouchDB users may want to set this to `"_id"`. @@ -275,6 +282,7 @@ // Extract attributes and options. var silent = options && options.silent; var unset = options && options.unset; + if (attrs instanceof Model) attrs = attrs.attributes; if (unset) for (attr in attrs) attrs[attr] = void 0; @@ -284,38 +292,26 @@ // Check for changes of `id`. if (this.idAttribute in attrs) this.id = attrs[this.idAttribute]; - var changing = this._changing; var now = this.attributes; - var escaped = this._escapedAttributes; - var prev = this._previousAttributes || {}; + var esc = this._escapedAttributes; // For each `set` attribute... for (attr in attrs) { val = attrs[attr]; - // If the new and current value differ, record the change. - if (!_.isEqual(now[attr], val) || (unset && _.has(now, attr))) { - delete escaped[attr]; - this._changes[attr] = true; - } + // If an escaped attr exists, and the new and current value differ, remove the escaped attr. + if (esc[attr] && !_.isEqual(now[attr], val) || (unset && _.has(now, attr))) delete esc[attr]; // Update or delete the current value. unset ? delete now[attr] : now[attr] = val; - // If the new and previous value differ, record the change. If not, - // then remove changes for this attribute. - if (!_.isEqual(prev[attr], val) || (_.has(now, attr) !== _.has(prev, attr))) { - this.changed[attr] = val; - if (!silent) this._pending[attr] = true; - } else { - delete this.changed[attr]; - delete this._pending[attr]; - if (!changing) delete this._changes[attr]; - } - - if (changing && _.isEqual(now[attr], changing[attr])) delete this._changes[attr]; + // Track any action on the attribute. + this._modelState.push(attr, val, unset); } + // Signal that the model's state has potentially changed. + this._cleanChange = false; + // Fire the `"change"` events. if (!silent) this.change(options); return this; @@ -460,36 +456,18 @@ // Calling this will cause all objects observing the model to update. change: function(options) { var changing = this._changing; - var current = this._changing = {}; + this._changing = true; + this.changed = {}; - // Silent changes become pending changes. - for (var attr in this._changes) this._pending[attr] = true; + // The number of changes triggered on the model. + this._pending = this._changeCenter(this._modelState, this._currentState, (options || {})); - // Trigger 'change:attr' for any new or silent changes. - var changes = this._changes; - this._changes = {}; - - // Set the correct state for this._changing values - var triggers = []; - for (var attr in changes) { - current[attr] = this.get(attr); - triggers.push(attr); - } - - for (var i=0, l=triggers.length; i < l; i++) { - this.trigger('change:' + triggers[i], this, current[triggers[i]], options); - } if (changing) return this; - // Continue firing `"change"` events while there are pending changes. - while (!_.isEmpty(this._pending)) { - this._pending = {}; + // Trigger a `change` while there have been changes. + while (this._pending) { + this._pending = false; this.trigger('change', this, options); - // Pending and silent changes still remain. - for (var attr in this.changed) { - if (this._pending[attr] || this._changes[attr]) continue; - delete this.changed[attr]; - } this._previousAttributes = _.clone(this.attributes); } @@ -500,6 +478,7 @@ // Determine if the model has changed since the last `"change"` event. // If you specify an attribute name, determine if that attribute has changed. hasChanged: function(attr) { + if (!this._cleanChange) this._changeCenter(_.clone(this._modelState), _.clone(this._currentState)); if (attr == null) return !_.isEmpty(this.changed); return _.has(this.changed, attr); }, @@ -520,6 +499,46 @@ return changed; }, + // Calculates and handles any changes for a model. + // Checks against `this._currentState` (in a change block) + // or a clone of `this._currentState` in a `changedAttributes` call, firing the changes. + _changeCenter: function (modelState, currentState, options) { + var key, val, unset, local = {}, triggers = []; + + // Loop through the current queue of potential model changes. + for (var i = modelState.length - 3; i >= 0; i -= 3) { + key = modelState[i]; + val = modelState[i + 1]; + unset = modelState[i + 2]; + + // If the item hasn't been set locally this round, proceed. + if (!local[key]) { + local[key] = val; + + // Check if the attribute has been modified since the last change, + // and update `currentState` and `this.changed` accordingly. + if (currentState[key] !== val || (_.has(currentState, key) && unset)) { + triggers.push(key, val); + (!unset) ? currentState[key] = val : delete currentState[key]; + this.changed[key] = val; + } else { + delete this.changed[key]; + } + } + modelState.splice(i,3); + } + + // Trigger change events if this function is called from a `this.change` call. + // In either case, record the changed attribute on `this.changed`. + for (i = triggers.length - 2; i >= 0; i -= 2) { + if (options) this.trigger('change:' + triggers[i], this, triggers[i + 1], options); + } + + // Signals `this.changed` is current to prevent duplicate calls from `this.hasChanged`. + this._cleanChange = true; + return triggers.length; + }, + // Get the previous value of an attribute, recorded at the time the last // `"change"` event was fired. previous: function(attr) { diff --git a/test/model.js b/test/model.js index 6e819ad4..087e5fa7 100644 --- a/test/model.js +++ b/test/model.js @@ -878,4 +878,18 @@ $(document).ready(function() { deepEqual(changes, ['a',1,'item']); }); + test("silent changes in last `change` event back to original does not trigger change", 2, function() { + var changes = []; + var model = new Backbone.Model(); + model.on('change:a change:b change:c', function(model, val) { changes.push(val); }); + model.on('change', function() { + model.set({a:'c'}, {silent:true}); + }); + model.set({a:'a'}); + deepEqual(changes, ['a']); + model.set({a:'a'}, {silent:true}); + model.change(); + deepEqual(changes, ['a']); + }); + }); diff --git a/test/speed.js b/test/speed.js index db0a2c8d..3581b5d2 100644 --- a/test/speed.js +++ b/test/speed.js @@ -42,4 +42,14 @@ keyModel.set({number: Math.random()}); }); + var silentModel = new Backbone.Model; + silentModel.on('change', fn); + + JSLitmus.test('Model: silent sets with rand(), with an attribute observer', function () { + for (var i=0;i<10;i++) { + silentModel.set({number:Math.random()}, {silent:true}); + } + silentModel.set({number:'one'}); + }); + })(); From a5a2036fe62939c3d477571965f9d9e841b9629d Mon Sep 17 00:00:00 2001 From: Tim Griesser Date: Sun, 7 Oct 2012 09:17:13 -0400 Subject: [PATCH 2/4] moving trigger loop into 'change', this.changed into 'changeCenter', more simplification of '_changeCenter' --- backbone.js | 48 ++++++++++++++++++++++++------------------------ 1 file changed, 24 insertions(+), 24 deletions(-) diff --git a/backbone.js b/backbone.js index 1dd79d80..760a93e2 100644 --- a/backbone.js +++ b/backbone.js @@ -457,10 +457,14 @@ change: function(options) { var changing = this._changing; this._changing = true; - this.changed = {}; - // The number of changes triggered on the model. - this._pending = this._changeCenter(this._modelState, this._currentState, (options || {})); + // Generate the changes to be triggered on the model. + var triggers = this._changeCenter(true); + this._pending = triggers.length; + + for (i = triggers.length - 2; i >= 0; i -= 2) { + this.trigger('change:' + triggers[i], this, triggers[i + 1], options); + } if (changing) return this; @@ -478,7 +482,7 @@ // Determine if the model has changed since the last `"change"` event. // If you specify an attribute name, determine if that attribute has changed. hasChanged: function(attr) { - if (!this._cleanChange) this._changeCenter(_.clone(this._modelState), _.clone(this._currentState)); + if (!this._cleanChange) this._changeCenter(); if (attr == null) return !_.isEmpty(this.changed); return _.has(this.changed, attr); }, @@ -499,44 +503,40 @@ return changed; }, - // Calculates and handles any changes for a model. - // Checks against `this._currentState` (in a change block) - // or a clone of `this._currentState` in a `changedAttributes` call, firing the changes. - _changeCenter: function (modelState, currentState, options) { - var key, val, unset, local = {}, triggers = []; + // Calculates and handles any changes in `this._modelState`, + // checking against `this._currentState` to determine current changes. + _changeCenter: function (change) { + this.changed = {}; + var local = {}; + var triggers = []; + var modelState = this._modelState; + var currentState = this._currentState; // Loop through the current queue of potential model changes. for (var i = modelState.length - 3; i >= 0; i -= 3) { - key = modelState[i]; - val = modelState[i + 1]; - unset = modelState[i + 2]; - + var key = modelState[i], val = modelState[i + 1], unset = modelState[i + 2]; + // If the item hasn't been set locally this round, proceed. if (!local[key]) { local[key] = val; // Check if the attribute has been modified since the last change, - // and update `currentState` and `this.changed` accordingly. + // and update `this.changed` accordingly. if (currentState[key] !== val || (_.has(currentState, key) && unset)) { + this.changed[key] = val; + + // Triggers & modifications are only created inside a `change` call. + if (!change) continue; triggers.push(key, val); (!unset) ? currentState[key] = val : delete currentState[key]; - this.changed[key] = val; - } else { - delete this.changed[key]; } } modelState.splice(i,3); } - // Trigger change events if this function is called from a `this.change` call. - // In either case, record the changed attribute on `this.changed`. - for (i = triggers.length - 2; i >= 0; i -= 2) { - if (options) this.trigger('change:' + triggers[i], this, triggers[i + 1], options); - } - // Signals `this.changed` is current to prevent duplicate calls from `this.hasChanged`. this._cleanChange = true; - return triggers.length; + return triggers; }, // Get the previous value of an attribute, recorded at the time the last From 68d1e70a4aaff1942088bf614a5c9bc77c431fdf Mon Sep 17 00:00:00 2001 From: Casey Foster Date: Mon, 22 Oct 2012 20:22:58 -0700 Subject: [PATCH 3/4] Refactor `set` to avoid unnecessary `arguments` object creation --- backbone.js | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/backbone.js b/backbone.js index 760a93e2..91094a50 100644 --- a/backbone.js +++ b/backbone.js @@ -268,15 +268,16 @@ // Set a hash of model attributes on the object, firing `"change"` unless // you choose to silence it. - set: function(attrs, options) { - var attr, key, val; - if (attrs == null) return this; + set: function(key, val, options) { + var attr, attrs; + if (key == null) return this; // Handle both `"key", value` and `{key: value}` -style arguments. - if (!_.isObject(attrs)) { - key = attrs; - (attrs = {})[key] = options; - options = arguments[2]; + if (_.isObject(key)) { + attrs = key; + options = val; + } else { + (attrs = {})[key] = val; } // Extract attributes and options. From 33a5ea3310f6e883e30acc8e8ac4d0f08b451ce2 Mon Sep 17 00:00:00 2001 From: Casey Foster Date: Tue, 23 Oct 2012 07:44:17 -0700 Subject: [PATCH 4/4] Match `save` to new `set` signature --- backbone.js | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/backbone.js b/backbone.js index 91094a50..40532787 100644 --- a/backbone.js +++ b/backbone.js @@ -274,10 +274,10 @@ // Handle both `"key", value` and `{key: value}` -style arguments. if (_.isObject(key)) { - attrs = key; - options = val; + attrs = key; + options = val; } else { - (attrs = {})[key] = val; + (attrs = {})[key] = val; } // Extract attributes and options. @@ -349,14 +349,15 @@ // Set a hash of model attributes, and sync the model to the server. // If the server returns an attributes hash that differs, the model's // state will be `set` again. - save: function(attrs, options) { - var key, current, done; + save: function(key, val, options) { + var attrs, current, done; // Handle both `"key", value` and `{key: value}` -style arguments. - if (attrs != null && !_.isObject(attrs)) { - key = attrs; - (attrs = {})[key] = options; - options = arguments[2]; + if (key == null || _.isObject(key)) { + attrs = key; + options = val; + } else if (key != null) { + (attrs = {})[key] = val; } options = options ? _.clone(options) : {};