[Object.observe] Lazily allocate callbackInfo structure

This patch allows callbacks to lazily allocate the InternalArray which is used to store pendingChangeRecords. This moves some of the expense of observation to the case where changes actually occurred.

When there are no pendingChangeRecords, the callbackInfo structure is a number which is the callbacks priority. Whenever a changeRecord is enqueued to the callback, it "normalizes" to be an InternalArray with a priority property. Immediately before its changeRecords are delivered, it returns to its optimized state.

---
Note: Naming confusion resolved:

This patch corrects some naming confusion in object-observe.js. Previously, we used the terms "callback" and "observer" to mean roughly the same thing, and overloaded the term "observer" to be both the callback itself and the *registration* on a object to observe (which now includes an accept map).

This patch resolves this confusion:

"object" (objectInfo, objectInfoMap): This refers to the observed object and its structures

"callback" (callbackInfo, callbackInfoMap): This refers to the callback to whom change records may be delivered

"observer" (objectInfo.changeObservers): This refers to a registration to observe a given object by a given callback with the specified accept list.
---

R=rossberg@chromium.org

Review URL: https://codereview.chromium.org/19132002

Patch from Rafael Weinstein <rafaelw@chromium.org>.

git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@15682 ce2b1a6d-e550-0410-aec6-3dcde31c8c00

diff --git a/src/object-observe.js b/src/object-observe.js
index 90c9a69..1c147d9 100644 (file)
--- a/src/object-observe.js
+++ b/src/object-observe.js
@@ -28,12 +28,12 @@
 "use strict";
 
 var observationState = %GetObservationState();
-if (IS_UNDEFINED(observationState.observerInfoMap)) {
-  observationState.observerInfoMap = %ObservationWeakMapCreate();
+if (IS_UNDEFINED(observationState.callbackInfoMap)) {
+  observationState.callbackInfoMap = %ObservationWeakMapCreate();
   observationState.objectInfoMap = %ObservationWeakMapCreate();
   observationState.notifierTargetMap = %ObservationWeakMapCreate();
   observationState.pendingObservers = new InternalArray;
-  observationState.observerPriority = 0;
+  observationState.nextCallbackPriority = 0;
 }
 
 function ObservationWeakMap(map) {
@@ -56,8 +56,8 @@ ObservationWeakMap.prototype = {
   }
 };
 
-var observerInfoMap =
-    new ObservationWeakMap(observationState.observerInfoMap);
+var callbackInfoMap =
+    new ObservationWeakMap(observationState.callbackInfoMap);
 var objectInfoMap = new ObservationWeakMap(observationState.objectInfoMap);
 var notifierTargetMap =
     new ObservationWeakMap(observationState.notifierTargetMap);
@@ -198,6 +198,22 @@ function AcceptArgIsValid(arg) {
   return true;
 }
 
+function EnsureCallbackPriority(callback) {
+  if (!callbackInfoMap.has(callback))
+    callbackInfoMap.set(callback, observationState.nextCallbackPriority++);
+}
+
+function NormalizeCallbackInfo(callback) {
+  var callbackInfo = callbackInfoMap.get(callback);
+  if (IS_NUMBER(callbackInfo)) {
+    var priority = callbackInfo;
+    callbackInfo = new InternalArray;
+    callbackInfo.priority = priority;
+    callbackInfoMap.set(callback, callbackInfo);
+  }
+  return callbackInfo;
+}
+
 function ObjectObserve(object, callback, accept) {
   if (!IS_SPEC_OBJECT(object))
     throw MakeTypeError("observe_non_object", ["observe"]);
@@ -208,12 +224,7 @@ function ObjectObserve(object, callback, accept) {
   if (!AcceptArgIsValid(accept))
     throw MakeTypeError("observe_accept_invalid");
 
-  if (!observerInfoMap.has(callback)) {
-    observerInfoMap.set(callback, {
-      pendingChangeRecords: null,
-      priority: observationState.observerPriority++,
-    });
-  }
+  EnsureCallbackPriority(callback);
 
   var objectInfo = objectInfoMap.get(object);
   if (IS_UNDEFINED(objectInfo)) {
@@ -257,6 +268,13 @@ function ArrayUnobserve(object, callback) {
   return ObjectUnobserve(object, callback);
 }
 
+function EnqueueToCallback(callback, changeRecord) {
+  var callbackInfo = NormalizeCallbackInfo(callback);
+  observationState.pendingObservers[callbackInfo.priority] = callback;
+  callbackInfo.push(changeRecord);
+  %SetObserverDeliveryPending();
+}
+
 function EnqueueChangeRecord(changeRecord, observers) {
   // TODO(rossberg): adjust once there is a story for symbols vs proxies.
   if (IS_SYMBOL(changeRecord.name)) return;
@@ -266,15 +284,7 @@ function EnqueueChangeRecord(changeRecord, observers) {
     if (IS_UNDEFINED(observer.accept[changeRecord.type]))
       continue;
 
-    var callback = observer.callback;
-    var observerInfo = observerInfoMap.get(callback);
-    observationState.pendingObservers[observerInfo.priority] = callback;
-    %SetObserverDeliveryPending();
-    if (IS_NULL(observerInfo.pendingChangeRecords)) {
-      observerInfo.pendingChangeRecords = new InternalArray(changeRecord);
-    } else {
-      observerInfo.pendingChangeRecords.push(changeRecord);
-    }
+    EnqueueToCallback(observer.callback, changeRecord);
   }
 }
 
@@ -394,21 +404,22 @@ function ObjectGetNotifier(object) {
   return objectInfo.notifier;
 }
 
-function DeliverChangeRecordsForObserver(observer) {
-  var observerInfo = observerInfoMap.get(observer);
-  if (IS_UNDEFINED(observerInfo))
+function CallbackDeliverPending(callback) {
+  var callbackInfo = callbackInfoMap.get(callback);
+  if (IS_UNDEFINED(callbackInfo) || IS_NUMBER(callbackInfo))
     return false;
 
-  var pendingChangeRecords = observerInfo.pendingChangeRecords;
-  if (IS_NULL(pendingChangeRecords))
-    return false;
+  // Clear the pending change records from callback and return it to its
+  // "optimized" state.
+  var priority = callbackInfo.priority;
+  callbackInfoMap.set(callback, priority);
 
-  observerInfo.pendingChangeRecords = null;
-  delete observationState.pendingObservers[observerInfo.priority];
+  delete observationState.pendingObservers[priority];
   var delivered = [];
-  %MoveArrayContents(pendingChangeRecords, delivered);
+  %MoveArrayContents(callbackInfo, delivered);
+
   try {
-    %Call(void 0, delivered, observer);
+    %Call(void 0, delivered, callback);
   } catch (ex) {}
   return true;
 }
@@ -417,7 +428,7 @@ function ObjectDeliverChangeRecords(callback) {
   if (!IS_SPEC_FUNCTION(callback))
     throw MakeTypeError("observe_non_function", ["deliverChangeRecords"]);
 
-  while (DeliverChangeRecordsForObserver(callback)) {}
+  while (CallbackDeliverPending(callback)) {}
 }
 
 function DeliverChangeRecords() {
@@ -425,7 +436,7 @@ function DeliverChangeRecords() {
     var pendingObservers = observationState.pendingObservers;
     observationState.pendingObservers = new InternalArray;
     for (var i in pendingObservers) {
-      DeliverChangeRecordsForObserver(pendingObservers[i]);
+      CallbackDeliverPending(pendingObservers[i]);
     }
   }
 }

diff --git a/test/cctest/test-object-observe.cc b/test/cctest/test-object-observe.cc
index 2bbcaad..44ddb6f 100644 (file)
--- a/test/cctest/test-object-observe.cc
+++ b/test/cctest/test-object-observe.cc
@@ -429,20 +429,20 @@ TEST(ObservationWeakMap) {
       "obj = null;");
   i::Handle<i::JSObject> observation_state =
       i::Isolate::Current()->factory()->observation_state();
-  i::Handle<i::JSWeakMap> observerInfoMap =
+  i::Handle<i::JSWeakMap> callbackInfoMap =
       i::Handle<i::JSWeakMap>::cast(
-          i::GetProperty(observation_state, "observerInfoMap"));
+          i::GetProperty(observation_state, "callbackInfoMap"));
   i::Handle<i::JSWeakMap> objectInfoMap =
       i::Handle<i::JSWeakMap>::cast(
           i::GetProperty(observation_state, "objectInfoMap"));
   i::Handle<i::JSWeakMap> notifierTargetMap =
       i::Handle<i::JSWeakMap>::cast(
           i::GetProperty(observation_state, "notifierTargetMap"));
-  CHECK_EQ(1, NumberOfElements(observerInfoMap));
+  CHECK_EQ(1, NumberOfElements(callbackInfoMap));
   CHECK_EQ(1, NumberOfElements(objectInfoMap));
   CHECK_EQ(1, NumberOfElements(notifierTargetMap));
   HEAP->CollectAllGarbage(i::Heap::kAbortIncrementalMarkingMask);
-  CHECK_EQ(0, NumberOfElements(observerInfoMap));
+  CHECK_EQ(0, NumberOfElements(callbackInfoMap));
   CHECK_EQ(0, NumberOfElements(objectInfoMap));
   CHECK_EQ(0, NumberOfElements(notifierTargetMap));
 }