Merge d0b5d60 into 01ae4a2

ampersand-state.js

@@ -116,7 +116,7 @@ assign(Base.prototype, Events, {
     set: function (key, value, options) {
         var self = this;
         var extraProperties = this.extraProperties;
-        var changing, changes, newType, newVal, def, cast, err, attr,
+        var wasChanging, changeEvents, newType, newVal, def, cast, err, attr,
             attrs, dataType, silent, unset, currentVal, initial, hasChanged, isEqual;
 
         // Handle both `"key", value` and `{key: value}` -style arguments.
@@ -137,18 +137,22 @@ assign(Base.prototype, Events, {
         silent = options.silent;
         initial = options.initial;
 
-        changes = [];
-        changing = this._changing;
+        // Initialize change tracking.
+        wasChanging = this._changing;
         this._changing = true;
+        changeEvents = [];
 
         // if not already changing, store previous
-        if (!changing) {
+        if (initial) {
+            this._previousAttributes = {};
+        } else if (!wasChanging) {
             this._previousAttributes = this.attributes;
             this._changed = {};
         }
 
         // For each `set` attribute...
-        for (attr in attrs) {
+        for (var i = 0, keys = Object.keys(attrs), len = keys.length; i < len; i++) {
+            attr = keys[i];
             newVal = attrs[attr];
             newType = typeof newVal;
             currentVal = this._values[attr];
@@ -211,48 +215,52 @@ assign(Base.prototype, Events, {
                 }
             }
 
-            hasChanged = !isEqual(currentVal, newVal, attr);
+            // We know this has 'changed' if it's the initial set, so skip a potentially expensive isEqual check.
+            hasChanged = initial || !isEqual(currentVal, newVal, attr);
 
             // enforce `setOnce` for properties if set
-            if (def.setOnce && currentVal !== undefined && hasChanged && !initial) {
+            if (def.setOnce && currentVal !== undefined && hasChanged) {
                 throw new TypeError('Property \'' + attr + '\' can only be set once.');
             }
 
-            // keep track of changed attributes
-            // and push to changes array
+            // set/unset attributes.
+            // If this is not the initial set, keep track of changed attributes
+            // and push to changeEvents array so we can fire events.
             if (hasChanged) {
-                changes.push({prev: currentVal, val: newVal, key: attr});
-                self._changed[attr] = newVal;
+                if (!initial) {
+                    this._changed[attr] = newVal;
+                    this._previousAttributes[attr] = currentVal;
+                    if (unset) {
+                        delete this._values[attr];
+                    }
+                    if (!silent) {
+                        changeEvents.push({prev: currentVal, val: newVal, key: attr});
+                    }
+                }
+                if (!unset) {
+                    this._values[attr] = newVal;
+                    if (newType === 'state') {
+                        this.listenTo(newVal, 'all', this._getEventBubblingHandler(attr));
+                    }
+                }
             } else {
-                delete self._changed[attr];
+                // Not changed
+                delete this._changed[attr];
             }
         }
 
-        // actually update our values
-        changes.forEach(function (change) {
-            self._previousAttributes[change.key] = change.prev;
-            if (unset) {
-                delete self._values[change.key];
-            } else {
-                self._values[change.key] = change.val;
-            }
+        // Fire events. This array is not populated if we are told to be silent.
+        if (changeEvents.length) this._pending = true;
+        changeEvents.forEach(function (change) {
+            self.trigger('change:' + change.key, self, change.val, options);
         });
 
-        if (!silent && changes.length) self._pending = true;
-        if (!silent) {
-            changes.forEach(function (change) {
-                self.trigger('change:' + change.key, self, change.val, options);
-            });
-        }
-
         // You might be wondering why there's a `while` loop here. Changes can
         // be recursively nested within `"change"` events.
-        if (changing) return this;
-        if (!silent) {
-            while (this._pending) {
-                this._pending = false;
-                this.trigger('change', this, options);
-            }
+        if (wasChanging) return this;
+        while (this._pending) {
+            this._pending = false;
+            this.trigger('change', this, options);
         }
         this._pending = false;
         this._changing = false;
@@ -394,8 +402,8 @@ assign(Base.prototype, Events, {
             derived: false
         }, options || {});
         var res = {};
-        var val, item, def;
-        for (item in this._definition) {
+        var val, def;
+        for (var item in this._definition) {
             def = this._definition[item];
             if ((options.session && def.session) || (options.props && !def.session)) {
                 val = raw ? this._values[item] : this[item];
@@ -405,7 +413,7 @@ assign(Base.prototype, Events, {
             }
         }
         if (options.derived) {
-            for (item in this._derived) res[item] = this[item];
+            for (var derivedItem in this._derived) res[derivedItem] = this[derivedItem];
         }
         return res;
     },
@@ -716,7 +724,7 @@ var dataTypes = {
                 };
             }
         },
-        compare: function (currentVal, newVal, attributeName) {
+        compare: function (currentVal, newVal) {
             var isSame = currentVal === newVal;
 
             // if this has changed we want to also handle
@@ -725,10 +733,6 @@ var dataTypes = {
                 if (currentVal) {
                     this.stopListening(currentVal);
                 }
-
-                if (newVal != null) {
-                    this.listenTo(newVal, 'all', this._getEventBubblingHandler(attributeName));
-                }
             }
 
             return isSame;

benchmark/manyProperties.js

@@ -0,0 +1,65 @@
+var State = require('../ampersand-state');
+
+// State slows down massively with many properties.
+// We benchmark this here.
+function createModel() {
+  var aModel = State.extend({
+    derived: {
+      foo: {
+        deps: ['a', 'b'],
+        fn: function derived () {
+          return this.a + this.b;
+        }
+      }
+    }
+  });
+  return aModel;
+}
+
+// Go through most of the normal ascii range.
+// Adjust I and J for larger objects.
+var properties = {};
+for (var i = 0; i < 1; i++) {
+  for (var j = 0; j < 83; j++) {
+    properties[String.fromCharCode(j + 33) + i] = j;
+  }
+}
+
+//Function that contains the pattern to be inspected
+var aModel = createModel();
+function benchFn() {
+  return new aModel(properties).foo;
+}
+
+function printStatus(fn) {
+    switch(%GetOptimizationStatus(fn)) {
+        case 1: console.log("Function is optimized"); break;
+        case 2: console.log("Function is not optimized"); break;
+        case 3: console.log("Function is always optimized"); break;
+        case 4: console.log("Function is never optimized"); break;
+        case 6: console.log("Function is maybe deoptimized"); break;
+    }
+}
+
+//Fill type-info
+benchFn();
+// 2 calls are needed to go from uninitialized -> pre-monomorphic -> monomorphic
+benchFn();
+benchFn();
+benchFn();
+benchFn();
+benchFn();
+
+%OptimizeFunctionOnNextCall(benchFn);
+//The next call
+benchFn();
+
+//Check
+printStatus(benchFn);
+
+
+console.time('manyProperties');
+for (var i = 0; i < 10000;i++) {
+  benchFn();
+}
+console.timeEnd('manyProperties');

package.json

@@ -68,7 +68,7 @@
     "validate": "npm ls",
     "start": "run-browser test/index.js",
     "lint": "jshint ampersand-state.js ./test/*",
-    "benchmark": "node --allow-natives-syntax benchmark/massCreate.js",
+    "benchmark": "for f in benchmark/*.js; do node --allow-natives-syntax --trace-deopt $f; done",
     "preversion": "git checkout master && git pull && npm ls",
     "publish-patch": "npm run preversion && npm version patch && git push origin master --tags && npm publish",
     "publish-minor": "npm run preversion && npm version minor && git push origin master --tags && npm publish",

test/full.js

@@ -664,9 +664,7 @@ test('Uses dataType compare', function (t) {
 
     compareRun = false;
     var foo = new Foo({ silliness: 'you' });
-    t.assert(compareRun);
 
-    compareRun = false;
     foo.silliness = 'they';
     t.assert(compareRun);
     t.end();