From 972b4a8658fbe532627402c98e899fbf61c9d178 Mon Sep 17 00:00:00 2001
From: Rafael Weinstein <rafaelw@chromium.org>
Date: Fri, 3 Jan 2014 13:41:26 -0800
Subject: [PATCH] Decouple observedSet from PathObservers

The basic idea here is that the observedSet is basically maintaining a write-barrier on all objects which are deps for a set of Paths. If any changes, then all paths must be checked.

However, when paths are dirty-checked (because of deliver() or discardChanges()), it's not necessary to flush pending records.

Because any dependent object changing will only ever cause the paths to be checked for new values, false positives are filtered out.

This is a precursor to allowing multiple separate observers to use the same observedSet as a write-barrier, which will allow sharing of observation of common objects (Object.proto, etc...).
R=arv
BUG=

Review URL: https://codereview.appspot.com/47630043
---
 src/observe.js | 122 ++++++++++++++++++++++++++-----------------------
 1 file changed, 64 insertions(+), 58 deletions(-)

diff --git a/src/observe.js b/src/observe.js
index f66505e..d3235f1 100644
--- a/src/observe.js
+++ b/src/observe.js
@@ -180,7 +180,7 @@
       this.push(part);
     }, this);
 
-    if (hasEval && !hasObserve && this.length) {
+    if (hasEval && this.length) {
       this.getValueFrom = this.compiledGetValueFromFn();
     }
   }
@@ -222,13 +222,20 @@
       for (var i = 0; i < this.length; i++) {
         if (obj == null)
           return;
-        if (directObserver)
-          directObserver.observe(obj);
         obj = obj[this[i]];
       }
       return obj;
     },
 
+    iterateObjects: function(obj, observe) {
+      for (var i = 0; i < this.length; i++) {
+        if (obj == null)
+          return;
+        observe(obj);
+        obj = obj[this[i]];
+      }
+    },
+
     compiledGetValueFromFn: function() {
       var accessors = this.map(function(ident) {
         return isIndex(ident) ? '["' + ident + '"]' : '.' + ident;
@@ -400,51 +407,52 @@
     var toRemove = emptyArray;
     var discardRecords = false;
 
-    function callback(records) {
-      if (observer && observer.state_ == OPENED && !discardRecords)
-        observer.check_(records);
+    function observe(obj) {
+      if (!isObject(obj) || obj === objProto || obj === arrayProto)
+        return;
+
+      var index = toRemove.indexOf(obj);
+      if (index >= 0) {
+        toRemove[index] = undefined;
+        objects.push(obj);
+      } else if (objects.indexOf(obj) < 0) {
+        objects.push(obj);
+        Object.observe(obj, callback);
+      }
+
+      observe(Object.getPrototypeOf(obj));
     }
 
-    return {
-      open: function(obs) {
-        observer = obs;
-      },
-      observe: function(obj) {
-        if (!isObject(obj) || obj === objProto || obj === arrayProto)
-          return;
+    function reset() {
+      var objs = toRemove === emptyArray ? [] : toRemove;
+      toRemove = objects;
+      objects = objs;
+    }
 
-        var index = toRemove.indexOf(obj);
-        if (index >= 0) {
-          toRemove[index] = undefined;
-          objects.push(obj);
-        } else if (objects.indexOf(obj) < 0) {
-          objects.push(obj);
-          Object.observe(obj, callback);
-        }
+    function cleanup() {
+      for (var i = 0; i < toRemove.length; i++) {
+        var obj = toRemove[i];
+        if (obj)
+          Object.unobserve(obj, callback);
+      }
 
-        this.observe(Object.getPrototypeOf(obj));
-      },
-      deliver: function(discard) {
-        discardRecords = discard;
-        Object.deliverChangeRecords(callback);
-        discardRecords = false;
-      },
-      reset: function() {
-        if (!objects.length)
-          return;
+      toRemove.length = 0;
+    }
 
-        var objs = toRemove === emptyArray ? [] : toRemove;
-        toRemove = objects;
-        objects = objs;
-      },
-      cleanup: function() {
-        for (var i = 0; i < toRemove.length; i++) {
-          var obj = toRemove[i];
-          if (obj)
-            Object.unobserve(obj, callback);
-        }
+    function callback(records) {
+      if (!observer || observer.state_ != OPENED || discardRecords)
+        return;
 
-        toRemove.length = 0;
+      observer.check_(records);
+      reset();
+      observer.iterateObjects_(observe);
+      cleanup();
+    }
+
+    return {
+      open: function(obs) {
+        observer = obs;
+        observer.iterateObjects_(observe);
       },
       close: function() {
         for (var i = 0; i < objects.length; i++) {
@@ -766,18 +774,13 @@
       }
     },
 
-    check_: function(changeRecords, skipChanges) {
-      // Note: Extracting this to a member function for use here and below
-      // regresses dirty-checking path perf by about 25% =-(.
-      if (this.directObserver_)
-        this.directObserver_.reset();
+    iterateObjects_: function(observe) {
+      this.path_.iterateObjects(this.object_, observe);
+    },
 
+    check_: function(changeRecords, skipChanges) {
       var oldValue = this.value_;
-      this.value_ = this.path_.getValueFrom(this.object_, this.directObserver_);
-
-      if (this.directObserver_)
-        this.directObserver_.cleanup();
-
+      this.value_ = this.path_.getValueFrom(this.object_);
       if (skipChanges || areSameValue(this.value_, oldValue))
         return false;
 
@@ -842,10 +845,16 @@
       this.value_.push(value);
     },
 
-    check_: function(changeRecords, skipChanges) {
-      if (this.directObserver_)
-        this.directObserver_.reset();
+    iterateObjects_: function(observe) {
+      var object;
+      for (var i = 0; i < this.observed_.length; i += 2) {
+        object = this.observed_[i]
+        if (object !== observerSentinel)
+          this.observed_[i + 1].iterateObjects(object, observe)
+      }
+    },
 
+    check_: function(changeRecords, skipChanges) {
       var oldValues;
       for (var i = 0; i < this.observed_.length; i += 2) {
         var pathOrObserver = this.observed_[i+1];
@@ -867,9 +876,6 @@
         this.value_[i / 2] = value;
       }
 
-      if (this.directObserver_)
-        this.directObserver_.cleanup();
-
       if (!oldValues)
         return false;