From 41039c4f13473f445e229ed39b55afea93c16a5b Mon Sep 17 00:00:00 2001 From: "rafaelw@chromium.org" Date: Fri, 7 Feb 2014 01:08:50 +0000 Subject: [PATCH] Revert "Implement Microtask Delivery Queue" TBR=adamk,rossberg BUG= Review URL: https://codereview.chromium.org/150103012 git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@19176 ce2b1a6d-e550-0410-aec6-3dcde31c8c00 --- src/object-observe.js | 14 +-- src/promise.js | 44 ++++---- src/runtime.cc | 8 -- src/runtime.h | 1 - src/v8natives.js | 9 +- test/cctest/cctest.gyp | 1 - test/cctest/test-microtask-delivery.cc | 93 ---------------- test/mjsunit/fuzz-natives-part3.js | 5 +- test/mjsunit/harmony/microtask-delivery.js | 168 ----------------------------- 9 files changed, 31 insertions(+), 312 deletions(-) delete mode 100644 test/cctest/test-microtask-delivery.cc delete mode 100644 test/mjsunit/harmony/microtask-delivery.js diff --git a/src/object-observe.js b/src/object-observe.js index 7bed924..499b27e 100644 --- a/src/object-observe.js +++ b/src/object-observe.js @@ -63,7 +63,6 @@ if (IS_UNDEFINED(observationState.callbackInfoMap)) { observationState.notifierObjectInfoMap = %ObservationWeakMapCreate(); observationState.pendingObservers = null; observationState.nextCallbackPriority = 0; - observationState.microtaskScheduled = false; } function ObservationWeakMap(map) { @@ -395,7 +394,7 @@ function ObserverEnqueueIfActive(observer, objectInfo, changeRecord, observationState.pendingObservers = nullProtoObject(); observationState.pendingObservers[callbackInfo.priority] = callback; callbackInfo.push(changeRecord); - EnqueueObserveMicrotask(); + %SetMicrotaskPending(true); } function ObjectInfoEnqueueExternalChangeRecord(objectInfo, changeRecord, type) { @@ -576,7 +575,6 @@ function ObjectDeliverChangeRecords(callback) { } function ObserveMicrotaskRunner() { - observationState.microtaskScheduled = false; var pendingObservers = observationState.pendingObservers; if (pendingObservers) { observationState.pendingObservers = null; @@ -585,15 +583,7 @@ function ObserveMicrotaskRunner() { } } } - -function EnqueueObserveMicrotask() { - if (observationState.microtaskScheduled) - return; - - RunMicrotasks.queue.push(ObserveMicrotaskRunner); - %SetMicrotaskPending(true); - observationState.microtaskScheduled = true; -} +RunMicrotasks.runners.push(ObserveMicrotaskRunner); function SetupObjectObserve() { %CheckIsBootstrapping(); diff --git a/src/promise.js b/src/promise.js index 552ce3d..db7863f 100644 --- a/src/promise.js +++ b/src/promise.js @@ -173,29 +173,37 @@ function PromiseCatch(onReject) { } function PromiseEnqueue(value, tasks) { - RunMicrotasks.queue.push(function() { - for (var i = 0; i < tasks.length; i += 2) { - PromiseHandle(value, tasks[i], tasks[i + 1]) - } - }); - + promiseEvents.push(value, tasks); %SetMicrotaskPending(true); } -function PromiseHandle(value, handler, deferred) { - try { - var result = handler(value); - if (result === deferred.promise) - throw MakeTypeError('promise_cyclic', [result]); - else if (IsPromise(result)) - result.chain(deferred.resolve, deferred.reject); - else - deferred.resolve(result); - } catch(e) { - // TODO(rossberg): perhaps log uncaught exceptions below. - try { deferred.reject(e) } catch(e) {} +function PromiseMicrotaskRunner() { + var events = promiseEvents; + if (events.length > 0) { + promiseEvents = new InternalArray; + for (var i = 0; i < events.length; i += 2) { + var value = events[i]; + var tasks = events[i + 1]; + for (var j = 0; j < tasks.length; j += 2) { + var handler = tasks[j]; + var deferred = tasks[j + 1]; + try { + var result = handler(value); + if (result === deferred.promise) + throw MakeTypeError('promise_cyclic', [result]); + else if (IsPromise(result)) + result.chain(deferred.resolve, deferred.reject); + else + deferred.resolve(result); + } catch(e) { + // TODO(rossberg): perhaps log uncaught exceptions below. + try { deferred.reject(e) } catch(e) {} + } + } + } } } +RunMicrotasks.runners.push(PromiseMicrotaskRunner); // Multi-unwrapped chaining with thenable coercion. diff --git a/src/runtime.cc b/src/runtime.cc index 372d74b..3596add 100644 --- a/src/runtime.cc +++ b/src/runtime.cc @@ -14597,14 +14597,6 @@ RUNTIME_FUNCTION(MaybeObject*, Runtime_SetMicrotaskPending) { } -RUNTIME_FUNCTION(MaybeObject*, Runtime_RunMicrotasks) { - HandleScope scope(isolate); - ASSERT(args.length() == 0); - Execution::RunMicrotasks(isolate); - return isolate->heap()->undefined_value(); -} - - RUNTIME_FUNCTION(MaybeObject*, Runtime_GetObservationState) { SealHandleScope shs(isolate); ASSERT(args.length() == 0); diff --git a/src/runtime.h b/src/runtime.h index 90fc5be..61c019c 100644 --- a/src/runtime.h +++ b/src/runtime.h @@ -351,7 +351,6 @@ namespace internal { \ /* Harmony events */ \ F(SetMicrotaskPending, 1, 1) \ - F(RunMicrotasks, 0, 1) \ \ /* Harmony observe */ \ F(IsObserved, 1, 1) \ diff --git a/src/v8natives.js b/src/v8natives.js index 24b0c51..df663c0 100644 --- a/src/v8natives.js +++ b/src/v8natives.js @@ -1889,15 +1889,10 @@ SetUpFunction(); // Eventually, we should move to a real event queue that allows to maintain // relative ordering of different kinds of tasks. -RunMicrotasks.queue = new InternalArray; +RunMicrotasks.runners = new InternalArray; function RunMicrotasks() { while (%SetMicrotaskPending(false)) { - var microtasks = RunMicrotasks.queue; - RunMicrotasks.queue = new InternalArray; - - for (var i = 0; i < microtasks.length; i++) { - microtasks[i](); - } + for (var i in RunMicrotasks.runners) RunMicrotasks.runners[i](); } } diff --git a/test/cctest/cctest.gyp b/test/cctest/cctest.gyp index 24bb22f..996db3e 100644 --- a/test/cctest/cctest.gyp +++ b/test/cctest/cctest.gyp @@ -88,7 +88,6 @@ 'test-liveedit.cc', 'test-lockers.cc', 'test-log.cc', - 'test-microtask-delivery.cc', 'test-mark-compact.cc', 'test-mementos.cc', 'test-mutex.cc', diff --git a/test/cctest/test-microtask-delivery.cc b/test/cctest/test-microtask-delivery.cc deleted file mode 100644 index 97d5cf3..0000000 --- a/test/cctest/test-microtask-delivery.cc +++ /dev/null @@ -1,93 +0,0 @@ -// Copyright 2012 the V8 project authors. All rights reserved. -// Redistribution and use in source and binary forms, with or without -// modification, are permitted provided that the following conditions are -// met: -// -// * Redistributions of source code must retain the above copyright -// notice, this list of conditions and the following disclaimer. -// * Redistributions in binary form must reproduce the above -// copyright notice, this list of conditions and the following -// disclaimer in the documentation and/or other materials provided -// with the distribution. -// * Neither the name of Google Inc. nor the names of its -// contributors may be used to endorse or promote products derived -// from this software without specific prior written permission. -// -// THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS -// "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT -// LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR -// A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT -// OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, -// SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT -// LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, -// DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY -// THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT -// (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE -// OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. - -#include "v8.h" - -#include "cctest.h" - -using namespace v8; -namespace i = v8::internal; - -namespace { -class HarmonyIsolate { - public: - HarmonyIsolate() { - i::FLAG_harmony_observation = true; - i::FLAG_harmony_promises = true; - isolate_ = Isolate::New(); - isolate_->Enter(); - } - - ~HarmonyIsolate() { - isolate_->Exit(); - isolate_->Dispose(); - } - - Isolate* GetIsolate() const { return isolate_; } - - private: - Isolate* isolate_; -}; -} - - -TEST(MicrotaskDeliverySimple) { - HarmonyIsolate isolate; - HandleScope scope(isolate.GetIsolate()); - LocalContext context(isolate.GetIsolate()); - CompileRun( - "var ordering = [];" - "var resolver = {};" - "function handler(resolve) { resolver.resolve = resolve; }" - "var obj = {};" - "var observeOrders = [1, 4];" - "function observer() {" - "ordering.push(observeOrders.shift());" - "resolver.resolve();" - "}" - "var p = new Promise(handler);" - "p.then(function() {" - "ordering.push(2);" - "}).then(function() {" - "ordering.push(3);" - "obj.id++;" - "return new Promise(handler);" - "}).then(function() {" - "ordering.push(5);" - "}).then(function() {" - "ordering.push(6);" - "});" - "Object.observe(obj, observer);" - "obj.id = 1;"); - CHECK_EQ(6, 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()); - CHECK_EQ(4, CompileRun("ordering[3]")->Int32Value()); - CHECK_EQ(5, CompileRun("ordering[4]")->Int32Value()); - CHECK_EQ(6, CompileRun("ordering[5]")->Int32Value()); -} diff --git a/test/mjsunit/fuzz-natives-part3.js b/test/mjsunit/fuzz-natives-part3.js index b3a1fb6..ba51b3d 100644 --- a/test/mjsunit/fuzz-natives-part3.js +++ b/test/mjsunit/fuzz-natives-part3.js @@ -216,10 +216,7 @@ var knownProblems = { "DataViewInitialize":true, "DataViewGetBuffer": true, "DataViewGetByteLength": true, - "DataViewGetByteOffset": true, - - // Only ever called internally. - "RunMicrotasks": true + "DataViewGetByteOffset": true }; var currentlyUncallable = { diff --git a/test/mjsunit/harmony/microtask-delivery.js b/test/mjsunit/harmony/microtask-delivery.js deleted file mode 100644 index 566a39d..0000000 --- a/test/mjsunit/harmony/microtask-delivery.js +++ /dev/null @@ -1,168 +0,0 @@ -// Copyright 2012 the V8 project authors. All rights reserved. -// Redistribution and use in source and binary forms, with or without -// modification, are permitted provided that the following conditions are -// met: -// -// * Redistributions of source code must retain the above copyright -// notice, this list of conditions and the following disclaimer. -// * Redistributions in binary form must reproduce the above -// copyright notice, this list of conditions and the following -// disclaimer in the documentation and/or other materials provided -// with the distribution. -// * Neither the name of Google Inc. nor the names of its -// contributors may be used to endorse or promote products derived -// from this software without specific prior written permission. -// -// THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS -// "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT -// LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR -// A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT -// OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, -// SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT -// LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, -// DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY -// THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT -// (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE -// OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. - -// Flags: --harmony-observation --harmony-promises --allow-natives-syntax - -var ordering = []; -function reset() { - ordering = []; -} - -function assertArrayValues(expected, actual) { - assertEquals(expected.length, actual.length); - for (var i = 0; i < expected.length; i++) { - assertEquals(expected[i], actual[i]); - } -} - -function assertOrdering(expected) { - %RunMicrotasks(); - assertArrayValues(expected, ordering); -} - -function newPromise(id, fn) { - var r; - var t = 1; - var promise = new Promise(function(resolve) { - r = resolve; - if (fn) fn(); - }); - - var next = promise.then(function(value) { - ordering.push('p' + id); - return value; - }); - - return { - resolve: r, - then: function(fn) { - next = next.then(function(value) { - ordering.push('p' + id + ':' + t++); - return fn ? fn(value) : value; - }); - - return this; - } - }; -} - -function newObserver(id, fn, obj) { - var observer = { - value: 1, - recordCounts: [] - }; - - Object.observe(observer, function(records) { - ordering.push('o' + id); - observer.recordCounts.push(records.length); - if (fn) fn(); - }); - - return observer; -} - - -(function PromiseThens() { - reset(); - - var p1 = newPromise(1).then(); - var p2 = newPromise(2).then(); - - p1.resolve(); - p2.resolve(); - - assertOrdering(['p1', 'p2', 'p1:1', 'p2:1']); -})(); - - -(function ObserversBatch() { - reset(); - - var p1 = newPromise(1); - var p2 = newPromise(2); - var p3 = newPromise(3); - - var ob1 = newObserver(1); - var ob2 = newObserver(2, function() { - ob3.value++; - p3.resolve(); - ob1.value++; - }); - var ob3 = newObserver(3); - - p1.resolve(); - ob1.value++; - p2.resolve(); - ob2.value++; - - assertOrdering(['p1', 'o1', 'o2', 'p2', 'o1', 'o3', 'p3']); - assertArrayValues([1, 1], ob1.recordCounts); - assertArrayValues([1], ob2.recordCounts); - assertArrayValues([1], ob3.recordCounts); -})(); - - -(function ObserversGetAllRecords() { - reset(); - - var p1 = newPromise(1); - var p2 = newPromise(2); - var ob1 = newObserver(1, function() { - ob2.value++; - }); - var ob2 = newObserver(2); - - p1.resolve(); - ob1.value++; - p2.resolve(); - ob2.value++; - - assertOrdering(['p1', 'o1', 'o2', 'p2']); - assertArrayValues([1], ob1.recordCounts); - assertArrayValues([2], ob2.recordCounts); -})(); - - -(function NewObserverDeliveryGetsNewMicrotask() { - reset(); - - var p1 = newPromise(1); - var p2 = newPromise(2); - var ob1 = newObserver(1); - var ob2 = newObserver(2, function() { - ob1.value++; - }); - - p1.resolve(); - ob1.value++; - p2.resolve(); - ob2.value++; - - assertOrdering(['p1', 'o1', 'o2', 'p2', 'o1']); - assertArrayValues([1, 1], ob1.recordCounts); - assertArrayValues([1], ob2.recordCounts); -})(); -- 2.7.4