Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(187)

Unified Diff: Source/bindings/core/v8/ScriptStreamer.cpp

Issue 674133002: Reland: Script streaming: Add an option to make the main thread block (wait for parsing) (Closed) Base URL: https://chromium.googlesource.com/chromium/blink.git@master
Patch Set: code review (haraken@) Created 6 years, 2 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « Source/bindings/core/v8/ScriptStreamer.h ('k') | Source/bindings/core/v8/ScriptStreamerTest.cpp » ('j') | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: Source/bindings/core/v8/ScriptStreamer.cpp
diff --git a/Source/bindings/core/v8/ScriptStreamer.cpp b/Source/bindings/core/v8/ScriptStreamer.cpp
index efbe8a5cba741a17b9c1f5df61ce4d09c83f8a4a..c1cea1802f0d321dda9bca29f9196c6d450b2495 100644
--- a/Source/bindings/core/v8/ScriptStreamer.cpp
+++ b/Source/bindings/core/v8/ScriptStreamer.cpp
@@ -13,6 +13,7 @@
#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"
@@ -224,28 +225,23 @@ void ScriptStreamer::startStreaming(PendingScript& script, Settings* settings, S
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;
- }
-
- // 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.
+ ASSERT(!isMainThread());
+ MutexLocker locker(m_mutex);
m_parsingFinished = true;
-
- notifyFinishedToClient();
-
- // The background thread no longer holds an implicit reference.
- deref();
+ // In the blocking case, the main thread is normally 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.
+
+ // In the non-blocking case, notifyFinished might already be called, or it
+ // might be called in the future. In any case, do the cleanup here.
+ if (m_mainThreadWaitingForParserThread) {
+ m_parsingFinishedCondition.signal();
+ } else {
+ callOnMainThread(WTF::bind(&ScriptStreamer::streamingComplete, this));
+ }
}
void ScriptStreamer::cancel()
@@ -262,8 +258,10 @@ void ScriptStreamer::cancel()
void ScriptStreamer::suppressStreaming()
{
- ASSERT(!m_parsingFinished);
+ MutexLocker locker(m_mutex);
ASSERT(!m_loadingFinished);
+ // It can be that the parsing task has already finished (e.g., if there was
+ // a parse error).
m_streamingSuppressed = true;
}
@@ -271,8 +269,11 @@ void ScriptStreamer::notifyAppendData(ScriptResource* resource)
{
ASSERT(isMainThread());
ASSERT(m_resource == resource);
- if (m_streamingSuppressed)
- return;
+ {
+ MutexLocker locker(m_mutex);
+ if (m_streamingSuppressed)
+ return;
+ }
if (!m_haveEnoughDataForStreaming) {
// Even if the first data chunk is small, the script can still be big
// enough - wait until the next data chunk comes before deciding whether
@@ -294,7 +295,8 @@ void ScriptStreamer::notifyAppendData(ScriptResource* resource)
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);
@@ -318,10 +320,40 @@ void ScriptStreamer::notifyFinished(Resource* resource)
}
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);
+ while (!isFinished()) {
+ ASSERT(!m_parsingFinished);
+ ASSERT(!m_streamingSuppressed);
+ m_mainThreadWaitingForParserThread = true;
+ m_parsingFinishedCondition.wait(m_mutex);
+ }
+ }
+
+ // Calling notifyFinishedToClient can result into the upper layers dropping
+ // references to ScriptStreamer. Keep it alive until this function ends.
+ RefPtr<ScriptStreamer> protect(this);
+
notifyFinishedToClient();
+
+ if (m_mainThreadWaitingForParserThread) {
+ ASSERT(m_parsingFinished);
+ ASSERT(!m_streamingSuppressed);
+ // streamingComplete won't be called, so do the ramp-down work
+ // here.
+ 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))
@@ -332,7 +364,34 @@ ScriptStreamer::ScriptStreamer(ScriptResource* resource, v8::ScriptCompiler::Str
, m_haveEnoughDataForStreaming(false)
, m_streamingSuppressed(false)
, m_scriptType(scriptType)
+ , m_scriptStreamingMode(mode)
+ , m_mainThreadWaitingForParserThread(false)
+{
+}
+
+void ScriptStreamer::streamingComplete()
{
+ // The background task is completed; do the necessary ramp-down in the main
+ // thread.
+ 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;
+ }
+
+ // 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()
@@ -346,7 +405,12 @@ void ScriptStreamer::notifyFinishedToClient()
// 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.
- if (isFinished() && m_client)
+ {
+ MutexLocker locker(m_mutex);
+ if (!isFinished())
+ return;
+ }
+ if (m_client)
m_client->notifyFinished(m_resource);
}
@@ -374,6 +438,10 @@ bool ScriptStreamer::startStreamingInternal(PendingScript& script, Settings* set
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())
@@ -415,7 +483,7 @@ bool ScriptStreamer::startStreamingInternal(PendingScript& script, Settings* set
// 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
« no previous file with comments | « Source/bindings/core/v8/ScriptStreamer.h ('k') | Source/bindings/core/v8/ScriptStreamerTest.cpp » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698