Revision: 12902
Author: [email protected]
Date: Thu Nov 8 05:44:59 2012
Log: Delivery logic for Object.observe
This CL has two parts: the first is the logic itself, whereby each observer
callback is assigned
a "priority" number the first time it's passed as an observer to
Object.observe(), and that
priority is used to determine the order of delivery.
The second part invokes the above logic as part of the API, when the JS
stack winds down to
zero.
Added several tests via the API, as the delivery logic isn't testable from
a JS test
(it runs after such a test would exit).
Review URL: https://codereview.chromium.org/11266011
Patch from Adam Klein <[email protected]>.
http://code.google.com/p/v8/source/detail?r=12902
Modified:
/branches/bleeding_edge/src/bootstrapper.cc
/branches/bleeding_edge/src/contexts.h
/branches/bleeding_edge/src/isolate.h
/branches/bleeding_edge/src/object-observe.js
/branches/bleeding_edge/src/objects.cc
/branches/bleeding_edge/src/objects.h
/branches/bleeding_edge/src/runtime.cc
/branches/bleeding_edge/src/runtime.h
/branches/bleeding_edge/src/v8.cc
/branches/bleeding_edge/test/cctest/test-object-observe.cc
=======================================
--- /branches/bleeding_edge/src/bootstrapper.cc Tue Nov 6 04:32:36 2012
+++ /branches/bleeding_edge/src/bootstrapper.cc Thu Nov 8 05:44:59 2012
@@ -1418,6 +1418,8 @@
}
if (FLAG_harmony_observation) {
INSTALL_NATIVE(JSFunction, "NotifyChange", observers_notify_change);
+ INSTALL_NATIVE(JSFunction, "DeliverChangeRecords",
+ observers_deliver_changes);
}
}
=======================================
--- /branches/bleeding_edge/src/contexts.h Tue Nov 6 04:32:36 2012
+++ /branches/bleeding_edge/src/contexts.h Thu Nov 8 05:44:59 2012
@@ -163,6 +163,7 @@
V(DERIVED_SET_TRAP_INDEX, JSFunction, derived_set_trap) \
V(PROXY_ENUMERATE_INDEX, JSFunction, proxy_enumerate) \
V(OBSERVERS_NOTIFY_CHANGE_INDEX, JSFunction, observers_notify_change) \
+ V(OBSERVERS_DELIVER_CHANGES_INDEX, JSFunction,
observers_deliver_changes) \
V(RANDOM_SEED_INDEX, ByteArray, random_seed)
// JSFunctions are pairs (context, function code), sometimes also called
@@ -291,6 +292,7 @@
DERIVED_SET_TRAP_INDEX,
PROXY_ENUMERATE_INDEX,
OBSERVERS_NOTIFY_CHANGE_INDEX,
+ OBSERVERS_DELIVER_CHANGES_INDEX,
RANDOM_SEED_INDEX,
// Properties from here are treated as weak references by the full GC.
=======================================
--- /branches/bleeding_edge/src/isolate.h Fri Sep 14 04:16:56 2012
+++ /branches/bleeding_edge/src/isolate.h Thu Nov 8 05:44:59 2012
@@ -354,6 +354,7 @@
V(uint64_t, enabled_cpu_features,
0) \
V(CpuProfiler*, cpu_profiler,
NULL) \
V(HeapProfiler*, heap_profiler,
NULL) \
+ V(bool, observer_delivery_pending,
false) \
ISOLATE_DEBUGGER_INIT_LIST(V)
class Isolate {
=======================================
--- /branches/bleeding_edge/src/object-observe.js Thu Nov 8 04:58:08 2012
+++ /branches/bleeding_edge/src/object-observe.js Thu Nov 8 05:44:59 2012
@@ -34,6 +34,8 @@
if (IS_UNDEFINED(observationState.observerInfoMap)) {
observationState.observerInfoMap = %CreateObjectHashTable();
observationState.objectInfoMap = %CreateObjectHashTable();
+ observationState.activeObservers = new InternalArray;
+ observationState.observerPriority = 0;
}
function InternalObjectHashTable(table) {
@@ -65,9 +67,9 @@
throw MakeTypeError("observe_callback_frozen");
if (!observerInfoMap.has(callback)) {
- // TODO: setup observerInfo.priority.
observerInfoMap.set(callback, {
- pendingChangeRecords: null
+ pendingChangeRecords: null,
+ priority: observationState.observerPriority++,
});
}
@@ -109,9 +111,8 @@
for (var i = 0; i < observers.length; i++) {
var observer = observers[i];
var observerInfo = observerInfoMap.get(observer);
-
- // TODO: "activate" the observer
-
+ observationState.activeObservers[observerInfo.priority] = observer;
+ %SetObserverDeliveryPending();
if (IS_NULL(observerInfo.pendingChangeRecords)) {
observerInfo.pendingChangeRecords = new InternalArray(changeRecord);
} else {
@@ -151,11 +152,8 @@
EnqueueChangeRecord(newRecord, objectInfo.changeObservers);
}
-function ObjectDeliverChangeRecords(callback) {
- if (!IS_SPEC_FUNCTION(callback))
- throw MakeTypeError("observe_non_function", ["deliverChangeRecords"]);
-
- var observerInfo = observerInfoMap.get(callback);
+function DeliverChangeRecordsForObserver(observer) {
+ var observerInfo = observerInfoMap.get(observer);
if (IS_UNDEFINED(observerInfo))
return;
@@ -167,9 +165,26 @@
var delivered = [];
%MoveArrayContents(pendingChangeRecords, delivered);
try {
- %Call(void 0, delivered, callback);
+ %Call(void 0, delivered, observer);
} catch (ex) {}
}
+
+function ObjectDeliverChangeRecords(callback) {
+ if (!IS_SPEC_FUNCTION(callback))
+ throw MakeTypeError("observe_non_function", ["deliverChangeRecords"]);
+
+ DeliverChangeRecordsForObserver(callback);
+}
+
+function DeliverChangeRecords() {
+ while (observationState.activeObservers.length) {
+ var activeObservers = observationState.activeObservers;
+ observationState.activeObservers = new InternalArray;
+ for (var i in activeObservers) {
+ DeliverChangeRecordsForObserver(activeObservers[i]);
+ }
+ }
+}
function SetupObjectObserve() {
%CheckIsBootstrapping();
=======================================
--- /branches/bleeding_edge/src/objects.cc Thu Nov 8 05:15:54 2012
+++ /branches/bleeding_edge/src/objects.cc Thu Nov 8 05:44:59 2012
@@ -1739,6 +1739,20 @@
&threw);
ASSERT(!threw);
}
+
+
+void JSObject::DeliverChangeRecords(Isolate* isolate) {
+ ASSERT(isolate->observer_delivery_pending());
+ bool threw = false;
+ Execution::Call(
+ isolate->observers_deliver_changes(),
+ isolate->factory()->undefined_value(),
+ 0,
+ NULL,
+ &threw);
+ ASSERT(!threw);
+ isolate->set_observer_delivery_pending(false);
+}
MaybeObject* JSObject::SetPropertyPostInterceptor(
=======================================
--- /branches/bleeding_edge/src/objects.h Thu Nov 8 04:58:08 2012
+++ /branches/bleeding_edge/src/objects.h Thu Nov 8 05:44:59 2012
@@ -2208,6 +2208,9 @@
static inline int SizeOf(Map* map, HeapObject* object);
};
+ // Deliver change records to observers. May cause GC.
+ static void DeliverChangeRecords(Isolate* isolate);
+
private:
friend class DictionaryElementsAccessor;
=======================================
--- /branches/bleeding_edge/src/runtime.cc Thu Nov 8 04:58:08 2012
+++ /branches/bleeding_edge/src/runtime.cc Thu Nov 8 05:44:59 2012
@@ -13261,6 +13261,13 @@
}
return isolate->heap()->undefined_value();
}
+
+
+RUNTIME_FUNCTION(MaybeObject*, Runtime_SetObserverDeliveryPending) {
+ ASSERT(args.length() == 0);
+ isolate->set_observer_delivery_pending(true);
+ return isolate->heap()->undefined_value();
+}
RUNTIME_FUNCTION(MaybeObject*, Runtime_GetObservationState) {
=======================================
--- /branches/bleeding_edge/src/runtime.h Tue Nov 6 10:14:45 2012
+++ /branches/bleeding_edge/src/runtime.h Thu Nov 8 05:44:59 2012
@@ -322,6 +322,7 @@
/* Harmony observe */ \
F(IsObserved, 1, 1) \
F(SetIsObserved, 2, 1) \
+ F(SetObserverDeliveryPending, 0, 1) \
F(GetObservationState, 0, 1) \
F(CreateObjectHashTable, 0, 1) \
F(ObjectHashTableGet, 2, 1) \
=======================================
--- /branches/bleeding_edge/src/v8.cc Fri Aug 17 02:03:08 2012
+++ /branches/bleeding_edge/src/v8.cc Thu Nov 8 05:44:59 2012
@@ -38,6 +38,7 @@
#include "hydrogen.h"
#include "lithium-allocator.h"
#include "log.h"
+#include "objects.h"
#include "once.h"
#include "platform.h"
#include "runtime-profiler.h"
@@ -216,14 +217,22 @@
void V8::FireCallCompletedCallback(Isolate* isolate) {
- if (call_completed_callbacks_ == NULL) return;
+ bool has_call_completed_callbacks = call_completed_callbacks_ != NULL;
+ bool observer_delivery_pending =
+ FLAG_harmony_observation && isolate->observer_delivery_pending();
+ if (!has_call_completed_callbacks && !observer_delivery_pending) return;
HandleScopeImplementer* handle_scope_implementer =
isolate->handle_scope_implementer();
if (!handle_scope_implementer->CallDepthIsZero()) return;
// Fire callbacks. Increase call depth to prevent recursive callbacks.
handle_scope_implementer->IncrementCallDepth();
- for (int i = 0; i < call_completed_callbacks_->length(); i++) {
- call_completed_callbacks_->at(i)();
+ if (observer_delivery_pending) {
+ JSObject::DeliverChangeRecords(isolate);
+ }
+ if (has_call_completed_callbacks) {
+ for (int i = 0; i < call_completed_callbacks_->length(); i++) {
+ call_completed_callbacks_->at(i)();
+ }
}
handle_scope_implementer->DecrementCallDepth();
}
=======================================
--- /branches/bleeding_edge/test/cctest/test-object-observe.cc Tue Nov 6
08:47:15 2012
+++ /branches/bleeding_edge/test/cctest/test-object-observe.cc Thu Nov 8
05:44:59 2012
@@ -60,25 +60,108 @@
"var calls = 0;"
"var observer = function(records) { count = records.length; calls++
};"
"var obj = {};"
- "Object.observe(obj, observer);"
- "Object.notify(obj, {type: 'a'});");
+ "Object.observe(obj, observer);");
Handle<Value> observer = CompileRun("observer");
Handle<Value> obj = CompileRun("obj");
+ Handle<Value> notify_fun1 = CompileRun(
+ "(function() { Object.notify(obj, {type: 'a'}); })");
+ Handle<Value> notify_fun2;
{
LocalContext context2;
context2->Global()->Set(String::New("obj"), obj);
- CompileRun("Object.notify(obj, {type: 'b'});");
+ notify_fun2 = CompileRun(
+ "(function() { Object.notify(obj, {type: 'b'}); })");
}
+ Handle<Value> notify_fun3;
{
LocalContext context3;
context3->Global()->Set(String::New("obj"), obj);
- CompileRun("Object.notify(obj, {type: 'c'});");
+ notify_fun3 = CompileRun(
+ "(function() { Object.notify(obj, {type: 'c'}); })");
}
{
LocalContext context4;
context4->Global()->Set(String::New("observer"), observer);
- CompileRun("Object.deliverChangeRecords(observer)");
+ context4->Global()->Set(String::New("fun1"), notify_fun1);
+ context4->Global()->Set(String::New("fun2"), notify_fun2);
+ context4->Global()->Set(String::New("fun3"), notify_fun3);
+ CompileRun("fun1(); fun2(); fun3();
Object.deliverChangeRecords(observer)");
}
CHECK_EQ(1, CompileRun("calls")->Int32Value());
CHECK_EQ(3, CompileRun("count")->Int32Value());
}
+
+TEST(EndOfMicrotaskDelivery) {
+ HarmonyIsolate isolate;
+ HandleScope scope;
+ LocalContext context;
+ CompileRun(
+ "var obj = {};"
+ "var count = 0;"
+ "var observer = function(records) { count = records.length };"
+ "Object.observe(obj, observer);"
+ "Object.notify(obj, {type: 'a'});");
+ CHECK_EQ(1, CompileRun("count")->Int32Value());
+}
+
+TEST(DeliveryOrdering) {
+ HarmonyIsolate isolate;
+ HandleScope scope;
+ LocalContext context;
+ CompileRun(
+ "var obj1 = {};"
+ "var obj2 = {};"
+ "var ordering = [];"
+ "function observer2() { ordering.push(2); };"
+ "function observer1() { ordering.push(1); };"
+ "function observer3() { ordering.push(3); };"
+ "Object.observe(obj1, observer1);"
+ "Object.observe(obj1, observer2);"
+ "Object.observe(obj1, observer3);"
+ "Object.notify(obj1, {type: 'a'});");
+ CHECK_EQ(3, CompileRun("ordering.length")->Int32Value());
+ CHECK_EQ(1, CompileRun("ordering[0]")->Int32Value());
+ CHECK_EQ(2, CompileRun("ordering[1]")->Int32Value());
+ CHECK_EQ(3, CompileRun("ordering[2]")->Int32Value());
+ CompileRun(
+ "ordering = [];"
+ "Object.observe(obj2, observer3);"
+ "Object.observe(obj2, observer2);"
+ "Object.observe(obj2, observer1);"
+ "Object.notify(obj2, {type: 'b'});");
+ CHECK_EQ(3, CompileRun("ordering.length")->Int32Value());
+ CHECK_EQ(1, CompileRun("ordering[0]")->Int32Value());
+ CHECK_EQ(2, CompileRun("ordering[1]")->Int32Value());
+ CHECK_EQ(3, CompileRun("ordering[2]")->Int32Value());
+}
+
+TEST(DeliveryOrderingReentrant) {
+ HarmonyIsolate isolate;
+ HandleScope scope;
+ LocalContext context;
+ CompileRun(
+ "var obj = {};"
+ "var reentered = false;"
+ "var ordering = [];"
+ "function observer1() { ordering.push(1); };"
+ "function observer2() {"
+ " if (!reentered) {"
+ " Object.notify(obj, {type: 'b'});"
+ " reentered = true;"
+ " }"
+ " ordering.push(2);"
+ "};"
+ "function observer3() { ordering.push(3); };"
+ "Object.observe(obj, observer1);"
+ "Object.observe(obj, observer2);"
+ "Object.observe(obj, observer3);"
+ "Object.notify(obj, {type: 'a'});");
+ CHECK_EQ(5, CompileRun("ordering.length")->Int32Value());
+ CHECK_EQ(1, CompileRun("ordering[0]")->Int32Value());
+ CHECK_EQ(2, CompileRun("ordering[1]")->Int32Value());
+ CHECK_EQ(3, CompileRun("ordering[2]")->Int32Value());
+ // Note that we re-deliver to observers 1 and 2, while observer3
+ // already received the second record during the first round.
+ CHECK_EQ(1, CompileRun("ordering[3]")->Int32Value());
+ CHECK_EQ(2, CompileRun("ordering[1]")->Int32Value());
+}
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
