From 35b8b0b27afeb387bd76d41d75e0817f2e89e532 Mon Sep 17 00:00:00 2001 From: "adamk@chromium.org" Date: Mon, 19 May 2014 07:57:04 +0000 Subject: [PATCH] Move microtask queueing logic from JavaScript to C++ This avoids the appearence of a leak due to storing a JSObject as the microtask_state in the strong root list, and allows callers to call Isolate::RunMicrotasks() without having any v8::Context available (as at least Blink has interest in doing). The queue is now a strong root, represented as a FixedArray of JSFunctions (or empty_fixed_array, if it's empty); it doubles in size when it needs to grow. The number of elements in the queue is stored in Isolate::pending_microtask_count(). LOG=Y R=dcarney@chromium.org Review URL: https://codereview.chromium.org/290633010 git-svn-id: https://v8.googlecode.com/svn/branches/bleeding_edge@21356 ce2b1a6d-e550-0410-aec6-3dcde31c8c00 --- src/api.cc | 9 ++--- src/bootstrapper.cc | 3 -- src/contexts.h | 2 - src/execution.cc | 22 ----------- src/execution.h | 3 -- src/heap.cc | 6 +-- src/heap.h | 2 +- src/isolate.cc | 43 +++++++++++++++++++--- src/isolate.h | 3 +- src/object-observe.js | 4 +- src/promise.js | 2 +- src/runtime.cc | 20 +++------- src/runtime.h | 5 +-- src/v8natives.js | 34 ----------------- test/cctest/test-api.cc | 19 ++++++++++ .../{getmicrotaskstate.js => enqueuemicrotask.js} | 3 +- test/mjsunit/runtime-gen/setmicrotaskpending.js | 5 --- tools/generate-runtime-tests.py | 4 +- 18 files changed, 79 insertions(+), 110 deletions(-) rename test/mjsunit/runtime-gen/{getmicrotaskstate.js => enqueuemicrotask.js} (73%) delete mode 100644 test/mjsunit/runtime-gen/setmicrotaskpending.js diff --git a/src/api.cc b/src/api.cc index b559015..7d4ea16 100644 --- a/src/api.cc +++ b/src/api.cc @@ -6626,16 +6626,13 @@ void Isolate::RemoveCallCompletedCallback(CallCompletedCallback callback) { void Isolate::RunMicrotasks() { - i::Isolate* i_isolate = reinterpret_cast(this); - i::HandleScope scope(i_isolate); - i_isolate->RunMicrotasks(); + reinterpret_cast(this)->RunMicrotasks(); } void Isolate::EnqueueMicrotask(Handle microtask) { - i::Isolate* i_isolate = reinterpret_cast(this); - ENTER_V8(i_isolate); - i::Execution::EnqueueMicrotask(i_isolate, Utils::OpenHandle(*microtask)); + i::Isolate* isolate = reinterpret_cast(this); + isolate->EnqueueMicrotask(Utils::OpenHandle(*microtask)); } diff --git a/src/bootstrapper.cc b/src/bootstrapper.cc index 892f90e..fb882d3 100644 --- a/src/bootstrapper.cc +++ b/src/bootstrapper.cc @@ -1550,9 +1550,6 @@ void Genesis::InstallNativeFunctions() { void Genesis::InstallExperimentalNativeFunctions() { - INSTALL_NATIVE(JSFunction, "RunMicrotasksJS", run_microtasks); - INSTALL_NATIVE(JSFunction, "EnqueueMicrotask", enqueue_microtask); - if (FLAG_harmony_proxies) { INSTALL_NATIVE(JSFunction, "DerivedHasTrap", derived_has_trap); INSTALL_NATIVE(JSFunction, "DerivedGetTrap", derived_get_trap); diff --git a/src/contexts.h b/src/contexts.h index 5381337..978e582 100644 --- a/src/contexts.h +++ b/src/contexts.h @@ -156,8 +156,6 @@ enum BindingFlags { V(ALLOW_CODE_GEN_FROM_STRINGS_INDEX, Object, allow_code_gen_from_strings) \ V(ERROR_MESSAGE_FOR_CODE_GEN_FROM_STRINGS_INDEX, Object, \ error_message_for_code_gen_from_strings) \ - V(RUN_MICROTASKS_INDEX, JSFunction, run_microtasks) \ - V(ENQUEUE_MICROTASK_INDEX, JSFunction, enqueue_microtask) \ V(IS_PROMISE_INDEX, JSFunction, is_promise) \ V(PROMISE_CREATE_INDEX, JSFunction, promise_create) \ V(PROMISE_RESOLVE_INDEX, JSFunction, promise_resolve) \ diff --git a/src/execution.cc b/src/execution.cc index 8f8c153..b550415 100644 --- a/src/execution.cc +++ b/src/execution.cc @@ -307,28 +307,6 @@ MaybeHandle Execution::TryGetConstructorDelegate( } -void Execution::RunMicrotasks(Isolate* isolate) { - ASSERT(isolate->microtask_pending()); - Execution::Call( - isolate, - isolate->run_microtasks(), - isolate->factory()->undefined_value(), - 0, - NULL).Check(); -} - - -void Execution::EnqueueMicrotask(Isolate* isolate, Handle microtask) { - Handle args[] = { microtask }; - Execution::Call( - isolate, - isolate->enqueue_microtask(), - isolate->factory()->undefined_value(), - 1, - args).Check(); -} - - bool StackGuard::IsStackOverflow() { ExecutionAccess access(isolate_); return (thread_local_.jslimit_ != kInterruptLimit && diff --git a/src/execution.h b/src/execution.h index 563c496..513ec73 100644 --- a/src/execution.h +++ b/src/execution.h @@ -121,9 +121,6 @@ class Execution V8_FINAL : public AllStatic { Handle object); static MaybeHandle TryGetConstructorDelegate(Isolate* isolate, Handle object); - - static void RunMicrotasks(Isolate* isolate); - static void EnqueueMicrotask(Isolate* isolate, Handle microtask); }; diff --git a/src/heap.cc b/src/heap.cc index 5d338b2..bdfb677 100644 --- a/src/heap.cc +++ b/src/heap.cc @@ -2897,9 +2897,9 @@ void Heap::CreateInitialObjects() { set_observation_state(*factory->NewJSObjectFromMap( factory->NewMap(JS_OBJECT_TYPE, JSObject::kHeaderSize))); - // Allocate object to hold object microtask state. - set_microtask_state(*factory->NewJSObjectFromMap( - factory->NewMap(JS_OBJECT_TYPE, JSObject::kHeaderSize))); + // Microtask queue uses the empty fixed array as a sentinel for "empty". + // Number of queued microtasks stored in Isolate::pending_microtask_count(). + set_microtask_queue(empty_fixed_array()); set_frozen_symbol(*factory->NewPrivateSymbol()); set_nonexistent_symbol(*factory->NewPrivateSymbol()); diff --git a/src/heap.h b/src/heap.h index d0d3547..f561374 100644 --- a/src/heap.h +++ b/src/heap.h @@ -196,7 +196,7 @@ namespace internal { V(Symbol, megamorphic_symbol, MegamorphicSymbol) \ V(FixedArray, materialized_objects, MaterializedObjects) \ V(FixedArray, allocation_sites_scratchpad, AllocationSitesScratchpad) \ - V(JSObject, microtask_state, MicrotaskState) + V(FixedArray, microtask_queue, MicrotaskQueue) // Entries in this list are limited to Smis and are not visited during GC. #define SMI_ROOT_LIST(V) \ diff --git a/src/isolate.cc b/src/isolate.cc index 06df1f6..a07451a 100644 --- a/src/isolate.cc +++ b/src/isolate.cc @@ -2232,13 +2232,13 @@ void Isolate::RemoveCallCompletedCallback(CallCompletedCallback callback) { void Isolate::FireCallCompletedCallback() { bool has_call_completed_callbacks = !call_completed_callbacks_.is_empty(); - bool run_microtasks = autorun_microtasks() && microtask_pending(); + bool run_microtasks = autorun_microtasks() && pending_microtask_count(); if (!has_call_completed_callbacks && !run_microtasks) return; if (!handle_scope_implementer()->CallDepthIsZero()) return; + if (run_microtasks) RunMicrotasks(); // Fire callbacks. Increase call depth to prevent recursive callbacks. handle_scope_implementer()->IncrementCallDepth(); - if (run_microtasks) Execution::RunMicrotasks(this); for (int i = 0; i < call_completed_callbacks_.length(); i++) { call_completed_callbacks_.at(i)(); } @@ -2246,15 +2246,46 @@ void Isolate::FireCallCompletedCallback() { } -void Isolate::RunMicrotasks() { - if (!microtask_pending()) - return; +void Isolate::EnqueueMicrotask(Handle microtask) { + Handle queue(heap()->microtask_queue(), this); + int num_tasks = pending_microtask_count(); + ASSERT(num_tasks <= queue->length()); + if (num_tasks == 0) { + queue = factory()->NewFixedArray(8); + heap()->set_microtask_queue(*queue); + } else if (num_tasks == queue->length()) { + queue = FixedArray::CopySize(queue, num_tasks * 2); + heap()->set_microtask_queue(*queue); + } + ASSERT(queue->get(num_tasks)->IsUndefined()); + queue->set(num_tasks, *microtask); + set_pending_microtask_count(num_tasks + 1); +} + +void Isolate::RunMicrotasks() { ASSERT(handle_scope_implementer()->CallDepthIsZero()); // Increase call depth to prevent recursive callbacks. handle_scope_implementer()->IncrementCallDepth(); - Execution::RunMicrotasks(this); + + while (pending_microtask_count() > 0) { + HandleScope scope(this); + int num_tasks = pending_microtask_count(); + Handle queue(heap()->microtask_queue(), this); + ASSERT(num_tasks <= queue->length()); + set_pending_microtask_count(0); + heap()->set_microtask_queue(heap()->empty_fixed_array()); + + for (int i = 0; i < num_tasks; i++) { + HandleScope scope(this); + Handle microtask(JSFunction::cast(queue->get(i)), this); + // TODO(adamk): This should ignore/clear exceptions instead of Checking. + Execution::Call(this, microtask, factory()->undefined_value(), + 0, NULL).Check(); + } + } + handle_scope_implementer()->DecrementCallDepth(); } diff --git a/src/isolate.h b/src/isolate.h index 9b58321..1e955a9 100644 --- a/src/isolate.h +++ b/src/isolate.h @@ -349,7 +349,7 @@ typedef List DebugObjectCache; /* AstNode state. */ \ V(int, ast_node_id, 0) \ V(unsigned, ast_node_count, 0) \ - V(bool, microtask_pending, false) \ + V(int, pending_microtask_count, 0) \ V(bool, autorun_microtasks, true) \ V(HStatistics*, hstatistics, NULL) \ V(HTracer*, htracer, NULL) \ @@ -1067,6 +1067,7 @@ class Isolate { void RemoveCallCompletedCallback(CallCompletedCallback callback); void FireCallCompletedCallback(); + void EnqueueMicrotask(Handle microtask); void RunMicrotasks(); private: diff --git a/src/object-observe.js b/src/object-observe.js index 26a1e18..03afd3f 100644 --- a/src/object-observe.js +++ b/src/object-observe.js @@ -419,8 +419,8 @@ function ObserverEnqueueIfActive(observer, objectInfo, changeRecord) { var callbackInfo = CallbackInfoNormalize(callback); if (IS_NULL(GetPendingObservers())) { - SetPendingObservers(nullProtoObject()) - EnqueueMicrotask(ObserveMicrotaskRunner); + SetPendingObservers(nullProtoObject()); + %EnqueueMicrotask(ObserveMicrotaskRunner); } GetPendingObservers()[callbackInfo.priority] = callback; callbackInfo.push(changeRecord); diff --git a/src/promise.js b/src/promise.js index 959be51..d202f28 100644 --- a/src/promise.js +++ b/src/promise.js @@ -151,7 +151,7 @@ function PromiseCatch(onReject) { } function PromiseEnqueue(value, tasks) { - EnqueueMicrotask(function() { + %EnqueueMicrotask(function() { for (var i = 0; i < tasks.length; i += 2) { PromiseHandle(value, tasks[i], tasks[i + 1]) } diff --git a/src/runtime.cc b/src/runtime.cc index e53c4c1..d07d1a1 100644 --- a/src/runtime.cc +++ b/src/runtime.cc @@ -14880,31 +14880,23 @@ RUNTIME_FUNCTION(Runtime_SetIsObserved) { } -RUNTIME_FUNCTION(Runtime_SetMicrotaskPending) { - SealHandleScope shs(isolate); +RUNTIME_FUNCTION(Runtime_EnqueueMicrotask) { + HandleScope scope(isolate); ASSERT(args.length() == 1); - CONVERT_BOOLEAN_ARG_CHECKED(new_state, 0); - bool old_state = isolate->microtask_pending(); - isolate->set_microtask_pending(new_state); - return isolate->heap()->ToBoolean(old_state); + CONVERT_ARG_HANDLE_CHECKED(JSFunction, microtask, 0); + isolate->EnqueueMicrotask(microtask); + return isolate->heap()->undefined_value(); } RUNTIME_FUNCTION(Runtime_RunMicrotasks) { HandleScope scope(isolate); ASSERT(args.length() == 0); - if (isolate->microtask_pending()) Execution::RunMicrotasks(isolate); + isolate->RunMicrotasks(); return isolate->heap()->undefined_value(); } -RUNTIME_FUNCTION(Runtime_GetMicrotaskState) { - SealHandleScope shs(isolate); - ASSERT(args.length() == 0); - return isolate->heap()->microtask_state(); -} - - RUNTIME_FUNCTION(Runtime_GetObservationState) { SealHandleScope shs(isolate); ASSERT(args.length() == 0); diff --git a/src/runtime.h b/src/runtime.h index 39f6730..6565bf5 100644 --- a/src/runtime.h +++ b/src/runtime.h @@ -244,9 +244,6 @@ namespace internal { /* ES5 */ \ F(ObjectFreeze, 1, 1) \ \ - /* Harmony microtasks */ \ - F(GetMicrotaskState, 0, 1) \ - \ /* Harmony modules */ \ F(IsJSModule, 1, 1) \ \ @@ -302,7 +299,7 @@ namespace internal { F(WeakCollectionSet, 3, 1) \ \ /* Harmony events */ \ - F(SetMicrotaskPending, 1, 1) \ + F(EnqueueMicrotask, 1, 1) \ F(RunMicrotasks, 0, 1) \ \ /* Harmony observe */ \ diff --git a/src/v8natives.js b/src/v8natives.js index 2e76162..1a50a84 100644 --- a/src/v8natives.js +++ b/src/v8natives.js @@ -1862,37 +1862,3 @@ function SetUpFunction() { } SetUpFunction(); - - -//---------------------------------------------------------------------------- - -// TODO(rossberg): very simple abstraction for generic microtask queue. -// Eventually, we should move to a real event queue that allows to maintain -// relative ordering of different kinds of tasks. - -function RunMicrotasksJS() { - while (%SetMicrotaskPending(false)) { - var microtaskState = %GetMicrotaskState(); - if (IS_UNDEFINED(microtaskState.queue)) - return; - - var microtasks = microtaskState.queue; - microtaskState.queue = null; - - for (var i = 0; i < microtasks.length; i++) { - microtasks[i](); - } - } -} - -function EnqueueMicrotask(fn) { - if (!IS_FUNCTION(fn)) { - throw new $TypeError('Can only enqueue functions'); - } - var microtaskState = %GetMicrotaskState(); - if (IS_UNDEFINED(microtaskState.queue) || IS_NULL(microtaskState.queue)) { - microtaskState.queue = new InternalArray; - } - microtaskState.queue.push(fn); - %SetMicrotaskPending(true); -} diff --git a/test/cctest/test-api.cc b/test/cctest/test-api.cc index 93c8e41..cbead0f 100644 --- a/test/cctest/test-api.cc +++ b/test/cctest/test-api.cc @@ -20821,6 +20821,25 @@ TEST(SetAutorunMicrotasks) { } +TEST(RunMicrotasksWithoutEnteringContext) { + v8::Isolate* isolate = CcTest::isolate(); + HandleScope handle_scope(isolate); + isolate->SetAutorunMicrotasks(false); + Handle context = Context::New(isolate); + { + Context::Scope context_scope(context); + CompileRun("var ext1Calls = 0;"); + isolate->EnqueueMicrotask(Function::New(isolate, MicrotaskOne)); + } + isolate->RunMicrotasks(); + { + Context::Scope context_scope(context); + CHECK_EQ(1, CompileRun("ext1Calls")->Int32Value()); + } + isolate->SetAutorunMicrotasks(true); +} + + static int probes_counter = 0; static int misses_counter = 0; static int updates_counter = 0; diff --git a/test/mjsunit/runtime-gen/getmicrotaskstate.js b/test/mjsunit/runtime-gen/enqueuemicrotask.js similarity index 73% rename from test/mjsunit/runtime-gen/getmicrotaskstate.js rename to test/mjsunit/runtime-gen/enqueuemicrotask.js index e488205..94d7495 100644 --- a/test/mjsunit/runtime-gen/getmicrotaskstate.js +++ b/test/mjsunit/runtime-gen/enqueuemicrotask.js @@ -1,4 +1,5 @@ // Copyright 2014 the V8 project authors. All rights reserved. // AUTO-GENERATED BY tools/generate-runtime-tests.py, DO NOT MODIFY // Flags: --allow-natives-syntax --harmony -%GetMicrotaskState(); +var _microtask = function() {}; +%EnqueueMicrotask(_microtask); diff --git a/test/mjsunit/runtime-gen/setmicrotaskpending.js b/test/mjsunit/runtime-gen/setmicrotaskpending.js deleted file mode 100644 index 5844cc5..0000000 --- a/test/mjsunit/runtime-gen/setmicrotaskpending.js +++ /dev/null @@ -1,5 +0,0 @@ -// Copyright 2014 the V8 project authors. All rights reserved. -// AUTO-GENERATED BY tools/generate-runtime-tests.py, DO NOT MODIFY -// Flags: --allow-natives-syntax --harmony -var _new_state = true; -%SetMicrotaskPending(_new_state); diff --git a/tools/generate-runtime-tests.py b/tools/generate-runtime-tests.py index 487b797..95dd56f 100755 --- a/tools/generate-runtime-tests.py +++ b/tools/generate-runtime-tests.py @@ -47,11 +47,11 @@ EXPAND_MACROS = [ # that the parser doesn't bit-rot. Change the values as needed when you add, # remove or change runtime functions, but make sure we don't lose our ability # to parse them! -EXPECTED_FUNCTION_COUNT = 362 +EXPECTED_FUNCTION_COUNT = 361 EXPECTED_FUZZABLE_COUNT = 329 EXPECTED_CCTEST_COUNT = 6 EXPECTED_UNKNOWN_COUNT = 5 -EXPECTED_BUILTINS_COUNT = 826 +EXPECTED_BUILTINS_COUNT = 824 # Don't call these at all. -- 2.7.4