[v8] Re-post non-blocking ScriptStreamer tasks (reland)

Posted blocking ScriptStreamer tasks can end up in a queue, and have
their Resource complete loading before the task even starts. When this
happens, these tasks will no longer block on network input, but they
will still be posted to the blocking task runner, which does not spin up
new threads if the running task is not blocked. When there are a lot of
small functions posted for streaming, this results in a slightly
paradoxical starvation, where the blocking task runner is processing
lots of no-longer-blocking tasks all in one thread, as it has no reason
to spin up new threads.

To avoid this, if a streaming task isn't started before the resource
finishes loading, we can cancel this posted task, and instead post a new
non-blocking task to the non-blocking thread pool. This frees up the
blocking task runner to process only blocking tasks (and thus allows it
to spin up new threads for them).

TBR=kouhei@chromium.org

Bug: chromium:865098
Bug: chromium:866868
Change-Id: I930b10d23fd538b3529ab3a31f6caba13a8661fb
Reviewed-on: https://chromium-review.googlesource.com/1179744Reviewed-by: Ross McIlroy <rmcilroy@chromium.org>
Commit-Queue: Leszek Swirski <leszeks@chromium.org>
Cr-Commit-Position: refs/heads/master@{#584032}

third_party/blink/renderer/bindings/core/v8/script_streamer.cc

@@ -308,7 +308,9 @@ void ScriptStreamer::SuppressStreaming(NotStreamingReason reason) {
   suppressed_reason_ = reason;
 }
 
-static void RunScriptStreamingTask(
+namespace {
+
+void RunScriptStreamingTask(
     std::unique_ptr<v8::ScriptCompiler::ScriptStreamingTask> task,
     ScriptStreamer* streamer) {
   TRACE_EVENT1(
@@ -321,6 +323,23 @@ static void RunScriptStreamingTask(
   streamer->StreamingCompleteOnBackgroundThread();
 }
 
+void RunBlockingScriptStreamingTask(
+    std::unique_ptr<v8::ScriptCompiler::ScriptStreamingTask> task,
+    ScriptStreamer* streamer,
+    std::atomic_flag* blocking_task_started_or_cancelled) {
+  if (blocking_task_started_or_cancelled->test_and_set())
+    return;
+  RunScriptStreamingTask(std::move(task), streamer);
+}
+
+void RunNonBlockingScriptStreamingTask(
+    std::unique_ptr<v8::ScriptCompiler::ScriptStreamingTask> task,
+    ScriptStreamer* streamer) {
+  RunScriptStreamingTask(std::move(task), streamer);
+}
+
+}  // namespace
+
 bool ScriptStreamer::HasEnoughDataForStreaming(size_t resource_buffer_size) {
   if (RuntimeEnabledFeatures::ScheduledScriptStreamingEnabled()) {
     // Enable streaming for small scripts, but we must still check the BOM
@@ -408,17 +427,24 @@ void ScriptStreamer::NotifyAppendData(ScriptResource* resource) {
     }
 
     if (RuntimeEnabledFeatures::ScheduledScriptStreamingEnabled()) {
-      // Script streaming tasks are high priority, as they can block the
-      // parser, and they can (and probably will) block during their own
-      // execution as they wait for more input.
+      // Script streaming tasks are high priority, as they can block the parser,
+      // and they can (and probably will) block during their own execution as
+      // they wait for more input.
+      //
+      // Pass through the atomic cancellation token which is set to true by the
+      // task when it is started, or set to true by the streamer if it wants to
+      // cancel the task.
       //
       // TODO(leszeks): Decrease the priority of these tasks where possible.
       BackgroundScheduler::PostOnBackgroundThreadWithTraits(
           FROM_HERE, {base::TaskPriority::USER_BLOCKING, base::MayBlock()},
-          CrossThreadBind(RunScriptStreamingTask,
+          CrossThreadBind(RunBlockingScriptStreamingTask,
                           WTF::Passed(std::move(script_streaming_task)),
-                          WrapCrossThreadPersistent(this)));
+                          WrapCrossThreadPersistent(this),
+                          WTF::CrossThreadUnretained(
+                              &blocking_task_started_or_cancelled_)));
     } else {
+      blocking_task_started_or_cancelled_.test_and_set();
       ScriptStreamerThread::Shared()->PostTask(
           CrossThreadBind(&ScriptStreamerThread::RunScriptStreamingTask,
                           WTF::Passed(std::move(script_streaming_task)),
@@ -440,8 +466,34 @@ void ScriptStreamer::NotifyFinished() {
     SuppressStreaming(kScriptTooSmall);
   }
 
-  if (stream_)
+  if (stream_) {
+    // If the corresponding blocking task hasn't started yet, cancel it and post
+    // a non-blocking task, since we know now that all the data is received and
+    // we will no longer block.
+    //
+    // TODO(874080): Once we have mutable task traits, simply unmark the
+    // existing task as no longer MayBlock.
+    if (RuntimeEnabledFeatures::ScheduledScriptStreamingEnabled() &&
+        !blocking_task_started_or_cancelled_.test_and_set()) {
+      ScriptState::Scope scope(script_state_);
+      std::unique_ptr<v8::ScriptCompiler::ScriptStreamingTask>
+          script_streaming_task(
+              base::WrapUnique(v8::ScriptCompiler::StartStreamingScript(
+                  script_state_->GetIsolate(), source_.get(),
+                  compile_options_)));
+
+      // The task creation shouldn't fail, since it didn't fail before during
+      // NotifyAppendData.
+      CHECK(script_streaming_task);
+      BackgroundScheduler::PostOnBackgroundThreadWithTraits(
+          FROM_HERE, {base::TaskPriority::USER_BLOCKING},
+          CrossThreadBind(RunNonBlockingScriptStreamingTask,
+                          WTF::Passed(std::move(script_streaming_task)),
+                          WrapCrossThreadPersistent(this)));
+    }
+
     stream_->DidFinishLoading();
+  }
   loading_finished_ = true;
 
   NotifyFinishedToClient();

third_party/blink/renderer/bindings/core/v8/script_streamer.h

@@ -5,6 +5,7 @@
 #ifndef THIRD_PARTY_BLINK_RENDERER_BINDINGS_CORE_V8_SCRIPT_STREAMER_H_
 #define THIRD_PARTY_BLINK_RENDERER_BINDINGS_CORE_V8_SCRIPT_STREAMER_H_
 
+#include <atomic>
 #include <memory>
 
 #include "base/single_thread_task_runner.h"
@@ -146,6 +147,11 @@ class CORE_EXPORT ScriptStreamer final
   // Whether we have received enough data to start the streaming.
   bool have_enough_data_for_streaming_;
 
+  // Flag used to allow atomic cancelling and reposting of the streaming task
+  // when the load completes without the task yet starting.
+  // TODO(874080): Remove once we can mutate task traits.
+  std::atomic_flag blocking_task_started_or_cancelled_ = ATOMIC_FLAG_INIT;
+
   // Whether the script source code should be retrieved from the Resource
   // instead of the ScriptStreamer.
   bool streaming_suppressed_;