Title: [214970] trunk/Source/_javascript_Core
Revision
214970
Author
[email protected]
Date
2017-04-05 14:19:37 -0700 (Wed, 05 Apr 2017)

Log Message

WebAssembly: Plans should be able to have more than one completion task.
https://bugs.webkit.org/show_bug.cgi?id=170516

Reviewed by Saam Barati.

This patch also eliminates the need for blocked tasks on the
PromiseDeferredTimer and pendingPromise on Wasm::Plan.

* runtime/PromiseDeferredTimer.cpp:
(JSC::PromiseDeferredTimer::doWork):
(JSC::PromiseDeferredTimer::cancelPendingPromise):
(JSC::PromiseDeferredTimer::scheduleBlockedTask): Deleted.
* runtime/PromiseDeferredTimer.h:
* wasm/WasmPlan.cpp:
(JSC::Wasm::Plan::Plan):
(JSC::Wasm::Plan::addCompletionTask):
(JSC::Wasm::Plan::complete):
* wasm/WasmPlan.h:
(JSC::Wasm::Plan::setMode):
(JSC::Wasm::Plan::mode):
(JSC::Wasm::Plan::setModeAndPromise): Deleted.
(JSC::Wasm::Plan::pendingPromise): Deleted.
* wasm/WasmWorklist.cpp:
(JSC::Wasm::Worklist::enqueue):
* wasm/js/WebAssemblyInstanceConstructor.cpp:
(JSC::constructJSWebAssemblyInstance):
* wasm/js/WebAssemblyPrototype.cpp:
(JSC::instantiate):

Modified Paths

Diff

Modified: trunk/Source/_javascript_Core/ChangeLog (214969 => 214970)


--- trunk/Source/_javascript_Core/ChangeLog	2017-04-05 21:00:17 UTC (rev 214969)
+++ trunk/Source/_javascript_Core/ChangeLog	2017-04-05 21:19:37 UTC (rev 214970)
@@ -1,3 +1,34 @@
+2017-04-05  Keith Miller  <[email protected]>
+
+        WebAssembly: Plans should be able to have more than one completion task.
+        https://bugs.webkit.org/show_bug.cgi?id=170516
+
+        Reviewed by Saam Barati.
+
+        This patch also eliminates the need for blocked tasks on the
+        PromiseDeferredTimer and pendingPromise on Wasm::Plan.
+
+        * runtime/PromiseDeferredTimer.cpp:
+        (JSC::PromiseDeferredTimer::doWork):
+        (JSC::PromiseDeferredTimer::cancelPendingPromise):
+        (JSC::PromiseDeferredTimer::scheduleBlockedTask): Deleted.
+        * runtime/PromiseDeferredTimer.h:
+        * wasm/WasmPlan.cpp:
+        (JSC::Wasm::Plan::Plan):
+        (JSC::Wasm::Plan::addCompletionTask):
+        (JSC::Wasm::Plan::complete):
+        * wasm/WasmPlan.h:
+        (JSC::Wasm::Plan::setMode):
+        (JSC::Wasm::Plan::mode):
+        (JSC::Wasm::Plan::setModeAndPromise): Deleted.
+        (JSC::Wasm::Plan::pendingPromise): Deleted.
+        * wasm/WasmWorklist.cpp:
+        (JSC::Wasm::Worklist::enqueue):
+        * wasm/js/WebAssemblyInstanceConstructor.cpp:
+        (JSC::constructJSWebAssemblyInstance):
+        * wasm/js/WebAssemblyPrototype.cpp:
+        (JSC::instantiate):
+
 2017-04-05  Guilherme Iscaro  <[email protected]>
 
         Do not use BLX for immediates (ARM-32)

Modified: trunk/Source/_javascript_Core/runtime/PromiseDeferredTimer.cpp (214969 => 214970)


--- trunk/Source/_javascript_Core/runtime/PromiseDeferredTimer.cpp	2017-04-05 21:00:17 UTC (rev 214969)
+++ trunk/Source/_javascript_Core/runtime/PromiseDeferredTimer.cpp	2017-04-05 21:19:37 UTC (rev 214970)
@@ -60,12 +60,6 @@
             task();
             m_vm->drainMicrotasks();
         }
-
-        auto waitingTasks = m_blockedTasks.take(ticket);
-        for (const Task& unblockedTask : waitingTasks) {
-            unblockedTask();
-            m_vm->drainMicrotasks();
-        }
     }
 
     if (m_pendingPromises.isEmpty() && m_shouldStopRunLoopWhenAllPromisesFinish)
@@ -120,9 +114,6 @@
     if (result)
         dataLogLnIf(verbose, "Canceling promise: ", RawPointer(ticket));
 
-    auto blockedTasks = m_blockedTasks.take(ticket);
-    for (const Task& task : blockedTasks)
-        task();
     return result;
 }
 
@@ -134,12 +125,4 @@
         scheduleTimer(0);
 }
 
-void PromiseDeferredTimer::scheduleBlockedTask(JSPromiseDeferred* blockingTicket, Task&& task)
-{
-    ASSERT(m_vm->currentThreadIsHoldingAPILock());
-    ASSERT(m_pendingPromises.contains(blockingTicket));
-    auto result = m_blockedTasks.add(blockingTicket, Vector<Task>());
-    result.iterator->value.append(WTFMove(task));
-}
-
 } // namespace JSC

Modified: trunk/Source/_javascript_Core/runtime/PromiseDeferredTimer.h (214969 => 214970)


--- trunk/Source/_javascript_Core/runtime/PromiseDeferredTimer.h	2017-04-05 21:00:17 UTC (rev 214969)
+++ trunk/Source/_javascript_Core/runtime/PromiseDeferredTimer.h	2017-04-05 21:19:37 UTC (rev 214970)
@@ -55,11 +55,6 @@
     typedef std::function<void()> Task;
     void scheduleWorkSoon(JSPromiseDeferred*, Task&&);
 
-    // Blocked tasks should only be registered while holding the JS API lock. If we didn't require holding the
-    // JS API lock then there might be a race where the promise you are waiting on is run before your task is
-    // registered.
-    void scheduleBlockedTask(JSPromiseDeferred*, Task&&);
-
     void stopRunningTasks() { m_runTasks = false; }
 
     JS_EXPORT_PRIVATE void runRunLoop();
@@ -70,7 +65,6 @@
     bool m_runTasks { true };
     bool m_shouldStopRunLoopWhenAllPromisesFinish { false };
     Vector<std::tuple<JSPromiseDeferred*, Task>> m_tasks;
-    HashMap<JSPromiseDeferred*, Vector<Task>> m_blockedTasks;
 };
 
 } // namespace JSC

Modified: trunk/Source/_javascript_Core/wasm/WasmPlan.cpp (214969 => 214970)


--- trunk/Source/_javascript_Core/wasm/WasmPlan.cpp	2017-04-05 21:00:17 UTC (rev 214969)
+++ trunk/Source/_javascript_Core/wasm/WasmPlan.cpp	2017-04-05 21:19:37 UTC (rev 214970)
@@ -53,12 +53,12 @@
 Plan::Plan(VM& vm, Ref<ModuleInformation> info, AsyncWork work, CompletionTask&& task)
     : m_moduleInformation(WTFMove(info))
     , m_vm(vm)
-    , m_completionTask(task)
     , m_source(m_moduleInformation->source.data())
     , m_sourceLength(m_moduleInformation->source.size())
     , m_state(State::Validated)
     , m_asyncWork(work)
 {
+    m_completionTasks.append(WTFMove(task));
 }
 
 Plan::Plan(VM& vm, Vector<uint8_t>&& source, AsyncWork work, CompletionTask&& task)
@@ -70,12 +70,12 @@
 Plan::Plan(VM& vm, const uint8_t* source, size_t sourceLength, AsyncWork work, CompletionTask&& task)
     : m_moduleInformation(makeRef(*new ModuleInformation(Vector<uint8_t>())))
     , m_vm(vm)
-    , m_completionTask(task)
     , m_source(source)
     , m_sourceLength(sourceLength)
     , m_state(State::Initial)
     , m_asyncWork(work)
 {
+    m_completionTasks.append(WTFMove(task));
 }
 
 const char* Plan::stateString(State state)
@@ -104,6 +104,15 @@
     complete(locker);
 }
 
+void Plan::addCompletionTask(CompletionTask&& task)
+{
+    LockHolder locker(m_lock);
+    if (m_state != State::Completed)
+        m_completionTasks.append(WTFMove(task));
+    else
+        task(*this);
+}
+
 bool Plan::parseAndValidateModule()
 {
     if (m_state != State::Initial)
@@ -317,7 +326,8 @@
 
     if (m_state != State::Completed) {
         moveToState(State::Completed);
-        m_completionTask(*this);
+        for (auto& task : m_completionTasks)
+            task(*this);
         m_completed.notifyAll();
     }
 }

Modified: trunk/Source/_javascript_Core/wasm/WasmPlan.h (214969 => 214970)


--- trunk/Source/_javascript_Core/wasm/WasmPlan.h	2017-04-05 21:00:17 UTC (rev 214969)
+++ trunk/Source/_javascript_Core/wasm/WasmPlan.h	2017-04-05 21:19:37 UTC (rev 214970)
@@ -56,6 +56,8 @@
     JS_EXPORT_PRIVATE Plan(VM&, const uint8_t*, size_t, AsyncWork, CompletionTask&&);
     JS_EXPORT_PRIVATE ~Plan();
 
+    void addCompletionTask(CompletionTask&&);
+
     bool parseAndValidateModule();
 
     JS_EXPORT_PRIVATE void prepare();
@@ -95,13 +97,8 @@
         return WTFMove(m_wasmExitStubs);
     }
 
-    void setModeAndPromise(MemoryMode mode, JSPromiseDeferred* promise)
-    {
-        m_mode = mode;
-        m_pendingPromise = promise;
-    }
+    void setMode(MemoryMode mode) { m_mode = mode; }
     MemoryMode mode() const { return m_mode; }
-    JSPromiseDeferred* pendingPromise() { return m_pendingPromise; }
     VM& vm() const { return m_vm; }
 
     enum class State : uint8_t {
@@ -139,8 +136,7 @@
     Vector<CompilationContext> m_compilationContexts;
 
     VM& m_vm;
-    JSPromiseDeferred* m_pendingPromise { nullptr };
-    CompletionTask m_completionTask;
+    Vector<CompletionTask, 1> m_completionTasks;
 
     Vector<Vector<UnlinkedWasmToWasmCall>> m_unlinkedWasmToWasmCalls;
     const uint8_t* m_source;

Modified: trunk/Source/_javascript_Core/wasm/WasmWorklist.cpp (214969 => 214970)


--- trunk/Source/_javascript_Core/wasm/WasmWorklist.cpp	2017-04-05 21:00:17 UTC (rev 214969)
+++ trunk/Source/_javascript_Core/wasm/WasmWorklist.cpp	2017-04-05 21:19:37 UTC (rev 214970)
@@ -187,10 +187,8 @@
         });
     }
 
-    // If we don't have a promise it must be synchronous so we should boost the priority.
-    Priority priority = plan->pendingPromise() ? Priority::Preparation : Priority::Synchronous;
-    dataLogLnIf(verbose, "Enqueuing plan with priority: ", priorityString(priority));
-    m_queue.push({ priority, nextTicket(),  WTFMove(plan) });
+    dataLogLnIf(verbose, "Enqueuing plan");
+    m_queue.push({ Priority::Preparation, nextTicket(),  WTFMove(plan) });
     m_planEnqueued->notifyOne(locker);
 }
 

Modified: trunk/Source/_javascript_Core/wasm/js/WebAssemblyInstanceConstructor.cpp (214969 => 214970)


--- trunk/Source/_javascript_Core/wasm/js/WebAssemblyInstanceConstructor.cpp	2017-04-05 21:00:17 UTC (rev 214969)
+++ trunk/Source/_javascript_Core/wasm/js/WebAssemblyInstanceConstructor.cpp	2017-04-05 21:19:37 UTC (rev 214970)
@@ -88,7 +88,7 @@
             Wasm::ensureWorklist().completePlanSynchronously(instance->codeBlock()->plan());
         else {
             Ref<Wasm::Plan> plan = adoptRef(*new Plan(vm, makeRef(const_cast<Wasm::ModuleInformation&>(module->moduleInformation())), Plan::FullCompile, Plan::dontFinalize));
-            plan->setModeAndPromise(instance->memoryMode(), nullptr);
+            plan->setMode(instance->memoryMode());
             instance->addUnitializedCodeBlock(vm, plan.copyRef());
 
             auto& worklist = Wasm::ensureWorklist();

Modified: trunk/Source/_javascript_Core/wasm/js/WebAssemblyPrototype.cpp (214969 => 214970)


--- trunk/Source/_javascript_Core/wasm/js/WebAssemblyPrototype.cpp	2017-04-05 21:00:17 UTC (rev 214969)
+++ trunk/Source/_javascript_Core/wasm/js/WebAssemblyPrototype.cpp	2017-04-05 21:19:37 UTC (rev 214970)
@@ -156,10 +156,12 @@
     vm.promiseDeferredTimer->addPendingPromise(promise, WTFMove(dependencies));
 
     if (instance->codeBlock()) {
-        vm.promiseDeferredTimer->scheduleBlockedTask(instance->codeBlock()->plan().pendingPromise(), [&vm, promise, instance, module, entries] () {
-            auto* globalObject = instance->globalObject();
-            ExecState* exec = globalObject->globalExec();
-            resolve(vm, exec, promise, instance, module, entries);
+        instance->codeBlock()->plan().addCompletionTask([promise, instance, module, entries] (Plan& p) {
+            RefPtr<Plan> plan = makeRef(p);
+            p.vm().promiseDeferredTimer->scheduleWorkSoon(promise, [promise, instance, module, entries, plan = WTFMove(plan)] () {
+                ExecState* exec = instance->globalObject()->globalExec();
+                resolve(plan->vm(), exec, promise, instance, module, entries);
+            });
         });
         return;
     }
@@ -177,7 +179,7 @@
     }));
 
     instance->addUnitializedCodeBlock(vm, plan.copyRef());
-    plan->setModeAndPromise(instance->memoryMode(), promise);
+    plan->setMode(instance->memoryMode());
     Wasm::ensureWorklist().enqueue(WTFMove(plan));
 }
 
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to