Script streaming: Add an option to make the main thread block (wait for parsing)

Source/bindings/core/v8/ScriptStreamer.cpp

 // Copyright 2014 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "config.h"
 #include "bindings/core/v8/ScriptStreamer.h"
 
 #include "bindings/core/v8/ScriptStreamerThread.h"
 #include "bindings/core/v8/V8ScriptRunner.h"
 #include "core/dom/Document.h"
 #include "core/dom/Element.h"
 #include "core/dom/PendingScript.h"
 #include "core/fetch/ScriptResource.h"
 #include "core/frame/Settings.h"
 #include "platform/SharedBuffer.h"
+#include "platform/TraceEvent.h"
 #include "public/platform/Platform.h"
 #include "wtf/MainThread.h"
 #include "wtf/text/TextEncodingRegistry.h"
 
 namespace blink {
 
 // For passing data between the main thread (producer) and the streamer thread
 // (consumer). The main thread prepares the data (copies it from Resource) and
 // the streamer thread feeds it to V8.
 class SourceStreamDataQueue {
@@ skipping 191 matching lines @@
 void ScriptStreamer::startStreaming(PendingScript& script, Settings* settings, ScriptState* scriptState, PendingScript::Type scriptType)
 {
     // We don't yet know whether the script will really be streamed. E.g.,
     // suppressing streaming for short scripts is done later. Record only the
     // sure negative cases here.
     bool startedStreaming = startStreamingInternal(script, settings, scriptState, scriptType);
     if (!startedStreaming)
         blink::Platform::current()->histogramEnumeration(startedStreamingHistogramName(scriptType), 0, 2);
 }
 
-void ScriptStreamer::streamingComplete()
+void ScriptStreamer::streamingCompleteOnBackgroundThread()
 {
-    ASSERT(isMainThread());
-    // It's possible that the corresponding Resource was deleted before V8
-    // finished streaming. In that case, the data or the notification is not
-    // needed. In addition, if the streaming is suppressed, the non-streaming
-    // code path will resume after the resource has loaded, before the
-    // background task finishes.
-    if (m_detached || m_streamingSuppressed) {
-        deref();
-        return;
+    ASSERT(!isMainThread());
+    MutexLocker locker(m_mutex);

[haraken, 2014/10/23 15:32:09]

Let's add ASSERT(!isFinished()).

[marja, 2014/10/24 13:04:41]

I don't think that's true; the streaming can still be suppressed (if we started
the parsing task but then got the code cache).

But, oops, the code doesn't handle that case gracefully. Fixing that. If the
streaming is suppressed, the main thread won't wait and it shouldn't deref()
either because it's too early (the task is still running). Fixed that. So, now
do the cleanup here.

The decision of whether to do the cleanup in the main thread or here was very
subtle - way too subtle. And error prone.

So I made it explicit: if the main thread is waiting right now, then let the
main thread do the cleanup afterwards, otherwise do the cleanup here. I added an
extra variable to track exactly that, so we don't need this "if (foo && bar &&
(!baz || quux)" mess for deciding.

+    m_parsingFinished = true;
+    if (shouldBlockMainThread()) {
+        // Normally, the main thread is waiting at this point, but it can also
+        // happen that the load is not yet finished (e.g., a parse error). In
+        // that case, notifyFinished will be called eventually and it will not
+        // wait on m_parsingFinishedCondition.
+        m_parsingFinishedCondition.signal();
+    } else {
+        callOnMainThread(WTF::bind(&ScriptStreamer::streamingComplete, this));
     }
-
-    // We have now streamed the whole script to V8 and it has parsed the
-    // script. We're ready for the next step: compiling and executing the
-    // script.
-    m_parsingFinished = true;
-
-    notifyFinishedToClient();
-
-    // The background thread no longer holds an implicit reference.
-    deref();
 }
 
 void ScriptStreamer::cancel()
 {
     ASSERT(isMainThread());
     // The upper layer doesn't need the script any more, but streaming might
     // still be ongoing. Tell SourceStream to try to cancel it whenever it gets
     // the control the next time. It can also be that V8 has already completed
     // its operations and streamingComplete will be called soon.
     m_detached = true;
     m_resource = 0;
     m_stream->cancel();
 }
 
 void ScriptStreamer::suppressStreaming()
 {
+    MutexLocker locker(m_mutex);

[haraken, 2014/10/23 15:32:09]

This lock is a bit too wide. Shall we add the MutexLocker into isFinished() and
use ASSERT(!isFinished()) here?

[marja, 2014/10/24 13:04:41]

Hmm, in notifyFinished() I want to call isFinished and
m_parsingFinishedCondition.wait without releasing the mutex inbetween. (If I
release it, the background thread can go and do m_parsingFinished = true;
m_parsingFinishedCondition.signal(); before the main thread has a chance to
wait().

IMO, not worth micro-optimizing here.

     ASSERT(!m_parsingFinished);
     ASSERT(!m_loadingFinished);
     m_streamingSuppressed = true;
 }
 
 void ScriptStreamer::notifyAppendData(ScriptResource* resource)
 {
     ASSERT(isMainThread());
     ASSERT(m_resource == resource);
     if (m_streamingSuppressed)
@@ skipping 12 matching lines @@
             // new task shouldn't be queued before the running task completes,
             // because the running task can block and wait for data from the
             // network.
             suppressStreaming();
             blink::Platform::current()->histogramEnumeration(histogramName, 0, 2);
             return;
         }
         ASSERT(m_task);
         // ScriptStreamer needs to stay alive as long as the background task is
         // running. This is taken care of with a manual ref() & deref() pair;
-        // the corresponding deref() is in streamingComplete.
+        // the corresponding deref() is in streamingComplete or in
+        // notifyFinished.
         ref();
         ScriptStreamingTask* task = new ScriptStreamingTask(m_task.release(), this);
         ScriptStreamerThread::shared()->postTask(task);
         blink::Platform::current()->histogramEnumeration(histogramName, 1, 2);
     }
     m_stream->didReceiveData();
 }
 
 void ScriptStreamer::notifyFinished(Resource* resource)
 {
     ASSERT(isMainThread());
     ASSERT(m_resource == resource);
     // A special case: empty and small scripts. We didn't receive enough data to
     // start the streaming before this notification. In that case, there won't
     // be a "parsing complete" notification either, and we should not wait for
     // it.
     if (!m_haveEnoughDataForStreaming) {
         const char* histogramName = startedStreamingHistogramName(m_scriptType);
         blink::Platform::current()->histogramEnumeration(histogramName, 0, 2);
         suppressStreaming();
     }
     m_stream->didFinishLoading();
     m_loadingFinished = true;
+
+    if (shouldBlockMainThread()) {
+        // Make the main thead wait until the streaming is complete, to make
+        // sure that the script gets the main thread's attention as early as
+        // possible (for possible compiling, if the client wants to do it
+        // right away). Note that blocking here is not any worse than the
+        // non-streaming code path where the main thread eventually blocks
+        // to parse the script.
+        TRACE_EVENT0("v8", "v8.mainThreadWaitingForParserThread");
+        MutexLocker locker(m_mutex);
+        if (!isFinished()) {
+            m_parsingFinishedCondition.wait(m_mutex);
+        }
+        ASSERT(isFinished());
+    }
+
     notifyFinishedToClient();
+
+    if (shouldBlockMainThread() && m_parsingFinished) {

[haraken, 2014/10/23 15:32:09]

Do we need the '&& m_parsingFinished' check? I guess it is always true.

[marja, 2014/10/24 13:04:40]

No, it's false when we didn't even start the parsing task (e.g., if the script
was too small). The comment tried to explain it :)

However, this is too subtle, made it explicit.

+        // streamingComplete won't be called, so do the ramp-down work
+        // here. Since m_parsingFinished is true, we know that there was a
+        // background task and we need to deref().
+        deref();
+    }
 }
 
-ScriptStreamer::ScriptStreamer(ScriptResource* resource, v8::ScriptCompiler::StreamedSource::Encoding encoding, PendingScript::Type scriptType)
+ScriptStreamer::ScriptStreamer(ScriptResource* resource, v8::ScriptCompiler::StreamedSource::Encoding encoding, PendingScript::Type scriptType, ScriptStreamingMode mode)
     : m_resource(resource)
     , m_detached(false)
     , m_stream(new SourceStream(this))
     , m_source(m_stream, encoding) // m_source takes ownership of m_stream.
     , m_client(0)
     , m_loadingFinished(false)
     , m_parsingFinished(false)
     , m_haveEnoughDataForStreaming(false)
     , m_streamingSuppressed(false)
     , m_scriptType(scriptType)
+    , m_scriptStreamingMode(mode)
 {
 }
 
+void ScriptStreamer::streamingComplete()
+{
+    // The background task is completed; do the necessary ramp-down in the main
+    // thread.
+    ASSERT(isMainThread());
+    // In the blocking mode, the ramp-down is done in notifyFinished.
+    ASSERT(!shouldBlockMainThread());
+
+    // It's possible that the corresponding Resource was deleted before V8
+    // finished streaming. In that case, the data or the notification is not
+    // needed. In addition, if the streaming is suppressed, the non-streaming
+    // code path will resume after the resource has loaded, before the
+    // background task finishes.
+    if (m_detached || m_streamingSuppressed) {
+        deref();
+        return;
+    }
+
+    // We have now streamed the whole script to V8 and it has parsed the
+    // script. We're ready for the next step: compiling and executing the
+    // script.
+    notifyFinishedToClient();
+
+    // The background thread no longer holds an implicit reference.
+    deref();
+}
+
 void ScriptStreamer::notifyFinishedToClient()
 {
     ASSERT(isMainThread());
     // Usually, the loading will be finished first, and V8 will still need some
     // time to catch up. But the other way is possible too: if V8 detects a
     // parse error, the V8 side can complete before loading has finished. Send
     // the notification after both loading and V8 side operations have
     // completed. Here we also check that we have a client: it can happen that a
     // function calling notifyFinishedToClient was already scheduled in the task
     // queue and the upper layer decided that it's not interested in the script
     // and called removeClient.
+    MutexLocker locker(m_mutex);

[haraken, 2014/10/23 15:32:09]

This lock is too wide. We don't need to hold a lock while calling
notifyFinished(). We can add the MutexLocker to isFinished().

[marja, 2014/10/24 13:04:41]

Made this code release the lock before calling client.

     if (isFinished() && m_client)
         m_client->notifyFinished(m_resource);
 }
 
 const char* ScriptStreamer::startedStreamingHistogramName(PendingScript::Type scriptType)
 {
     switch (scriptType) {
     case PendingScript::ParsingBlocking:
         return "WebCore.Scripts.ParsingBlocking.StartedStreaming";
         break;
     case PendingScript::Deferred:
         return "WebCore.Scripts.Deferred.StartedStreaming";
         break;
     case PendingScript::Async:
         return "WebCore.Scripts.Async.StartedStreaming";
         break;
     default:
         ASSERT_NOT_REACHED();
         break;
     }
     return 0;
 }
 
 bool ScriptStreamer::startStreamingInternal(PendingScript& script, Settings* settings, ScriptState* scriptState, PendingScript::Type scriptType)
 {
     ASSERT(isMainThread());
     if (!settings || !settings->v8ScriptStreamingEnabled())
         return false;
+    if (settings->v8ScriptStreamingMode() == ScriptStreamingModeOnlyAsyncAndDefer
+        && scriptType == PendingScript::ParsingBlocking)
+        return false;
+
     ScriptResource* resource = script.resource();
     ASSERT(!resource->isLoaded());
     if (!resource->url().protocolIsInHTTPFamily())
         return false;
     if (resource->resourceToRevalidate()) {
         // This happens e.g., during reloads. We're actually not going to load
         // the current Resource of the PendingScript but switch to another
         // Resource -> don't stream.
         return false;
     }
@@ skipping 21 matching lines @@
         return false;
     }
 
     if (!scriptState->contextIsValid())
         return false;
     ScriptState::Scope scope(scriptState);
 
     // The Resource might go out of scope if the script is no longer needed. We
     // will soon call PendingScript::setStreamer, which makes the PendingScript
     // notify the ScriptStreamer when it is destroyed.
-    RefPtr<ScriptStreamer> streamer = adoptRef(new ScriptStreamer(resource, encoding, scriptType));
+    RefPtr<ScriptStreamer> streamer = adoptRef(new ScriptStreamer(resource, encoding, scriptType, settings->v8ScriptStreamingMode()));
 
     // Decide what kind of cached data we should produce while streaming. By
     // default, we generate the parser cache for streamed scripts, to emulate
     // the non-streaming behavior (see V8ScriptRunner::compileScript).
     v8::ScriptCompiler::CompileOptions compileOption = v8::ScriptCompiler::kProduceParserCache;
     if (settings->v8CacheOptions() == V8CacheOptionsCode)
         compileOption = v8::ScriptCompiler::kProduceCodeCache;
     v8::ScriptCompiler::ScriptStreamingTask* scriptStreamingTask = v8::ScriptCompiler::StartStreamingScript(scriptState->isolate(), &(streamer->m_source), compileOption);
     if (scriptStreamingTask) {
         streamer->m_task = adoptPtr(scriptStreamingTask);
         script.setStreamer(streamer.release());
         return true;
     }
     // Otherwise, V8 cannot stream the script.
     return false;
 }
 
 } // namespace blink