Commit d468bddc authored by Camillo Bruni's avatar Camillo Bruni Committed by Chromium LUCI CQ

[blink] Enable ModuleScript streaming compilation

- Bail out in ScriptStreamer::TryStartStreamingTask if the response
  is not a JavaScript mime type (for CSS and JSON modules)
- Only initiate script streaming from the main thread (for now this
  prevents streaming compilation for workers and worklets)
- Fix trace event ending for streaming compiled module scripts
- Add WPT test with a large CSS module that would previously trigger
  script streaming

Bug: 1061857
Change-Id: I5100e4c12aa3504ea4132b344f7910041270318a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2595299
Commit-Queue: Camillo Bruni <cbruni@chromium.org>
Reviewed-by: default avatarHiroshige Hayashizaki <hiroshige@chromium.org>
Reviewed-by: default avatarKouhei Ueno <kouhei@chromium.org>
Reviewed-by: default avatarHiroki Nakagawa <nhiroki@chromium.org>
Cr-Commit-Position: refs/heads/master@{#839447}
parent 9ffd9fee
...@@ -24,6 +24,7 @@ ...@@ -24,6 +24,7 @@
#include "third_party/blink/renderer/platform/loader/fetch/cached_metadata.h" #include "third_party/blink/renderer/platform/loader/fetch/cached_metadata.h"
#include "third_party/blink/renderer/platform/loader/fetch/resource.h" #include "third_party/blink/renderer/platform/loader/fetch/resource.h"
#include "third_party/blink/renderer/platform/loader/fetch/response_body_loader.h" #include "third_party/blink/renderer/platform/loader/fetch/response_body_loader.h"
#include "third_party/blink/renderer/platform/network/mime/mime_type_registry.h"
#include "third_party/blink/renderer/platform/scheduler/public/post_cross_thread_task.h" #include "third_party/blink/renderer/platform/scheduler/public/post_cross_thread_task.h"
#include "third_party/blink/renderer/platform/scheduler/public/thread_scheduler.h" #include "third_party/blink/renderer/platform/scheduler/public/thread_scheduler.h"
#include "third_party/blink/renderer/platform/scheduler/public/worker_pool.h" #include "third_party/blink/renderer/platform/scheduler/public/worker_pool.h"
...@@ -524,6 +525,16 @@ bool ScriptStreamer::TryStartStreamingTask() { ...@@ -524,6 +525,16 @@ bool ScriptStreamer::TryStartStreamingTask() {
if (!CanStartStreaming()) if (!CanStartStreaming())
return false; return false;
// Skip non-JS modules based on the mime-type.
// TODO(crbug/1132413),TODO(crbug/1061857): Disable streaming for non-JS
// based the specific import statements.
if (script_resource_->GetScriptType() == mojom::blink::ScriptType::kModule &&
!MIMETypeRegistry::IsSupportedJavaScriptMIMEType(
script_resource_->GetResponse().HttpContentType())) {
SuppressStreaming(NotStreamingReason::kNonJavascriptModule);
return false;
}
// Even if the first data chunk is small, the script can still be big enough - // 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 to start the // wait until the next data chunk comes before deciding whether to start the
// streaming. // streaming.
......
...@@ -60,6 +60,7 @@ class CORE_EXPORT ScriptStreamer final ...@@ -60,6 +60,7 @@ class CORE_EXPORT ScriptStreamer final
kModuleScript, kModuleScript,
kNoDataPipe, kNoDataPipe,
kLoadingCancelled, kLoadingCancelled,
kNonJavascriptModule,
kDisabledByFeatureList, kDisabledByFeatureList,
// Pseudo values that should never be seen in reported metrics // Pseudo values that should never be seen in reported metrics
......
...@@ -272,58 +272,58 @@ v8::MaybeLocal<v8::Module> V8ScriptRunner::CompileModule( ...@@ -272,58 +272,58 @@ v8::MaybeLocal<v8::Module> V8ScriptRunner::CompileModule(
referrer_info.ToV8HostDefinedOptions(isolate)); referrer_info.ToV8HostDefinedOptions(isolate));
v8::Local<v8::String> code = V8String(isolate, params.GetSourceText()); v8::Local<v8::String> code = V8String(isolate, params.GetSourceText());
if (ScriptStreamer* streamer = params.GetScriptStreamer()) { inspector_compile_script_event::V8CacheResult cache_result;
v8::MaybeLocal<v8::Module> script;
ScriptStreamer* streamer = params.GetScriptStreamer();
if (streamer) {
// Final compile call for a streamed compilation. // Final compile call for a streamed compilation.
// Streaming compilation may involve use of code cache. // Streaming compilation may involve use of code cache.
// TODO(leszeks): Add compile timer to streaming compilation. // TODO(leszeks): Add compile timer to streaming compilation.
DCHECK(streamer->IsFinished()); DCHECK(streamer->IsFinished());
DCHECK(!streamer->IsStreamingSuppressed()); DCHECK(!streamer->IsStreamingSuppressed());
return v8::ScriptCompiler::CompileModule(isolate->GetCurrentContext(), script = v8::ScriptCompiler::CompileModule(
streamer->Source(), code, origin); isolate->GetCurrentContext(), streamer->Source(), code, origin);
} } else {
switch (compile_options) {
v8::MaybeLocal<v8::Module> script; case v8::ScriptCompiler::kNoCompileOptions:
inspector_compile_script_event::V8CacheResult cache_result; case v8::ScriptCompiler::kEagerCompile: {
v8::ScriptCompiler::Source source(code, origin);
switch (compile_options) { script = v8::ScriptCompiler::CompileModule(
case v8::ScriptCompiler::kNoCompileOptions: isolate, &source, compile_options, no_cache_reason);
case v8::ScriptCompiler::kEagerCompile: { break;
v8::ScriptCompiler::Source source(code, origin); }
script = v8::ScriptCompiler::CompileModule(
isolate, &source, compile_options, no_cache_reason);
break;
}
case v8::ScriptCompiler::kConsumeCodeCache: { case v8::ScriptCompiler::kConsumeCodeCache: {
// Compile a script, and consume a V8 cache that was generated previously. // Compile a script, and consume a V8 cache that was generated
SingleCachedMetadataHandler* cache_handler = params.CacheHandler(); // previously.
DCHECK(cache_handler); SingleCachedMetadataHandler* cache_handler = params.CacheHandler();
v8::ScriptCompiler::CachedData* cached_data = DCHECK(cache_handler);
V8CodeCache::CreateCachedData(cache_handler); v8::ScriptCompiler::CachedData* cached_data =
v8::ScriptCompiler::Source source(code, origin, cached_data); V8CodeCache::CreateCachedData(cache_handler);
script = v8::ScriptCompiler::CompileModule( v8::ScriptCompiler::Source source(code, origin, cached_data);
isolate, &source, compile_options, no_cache_reason); script = v8::ScriptCompiler::CompileModule(
if (cached_data->rejected) { isolate, &source, compile_options, no_cache_reason);
cache_handler->ClearCachedMetadata( if (cached_data->rejected) {
CachedMetadataHandler::kClearPersistentStorage); cache_handler->ClearCachedMetadata(
} else if (InDiscardExperiment()) { CachedMetadataHandler::kClearPersistentStorage);
// Experimentally free code cache from memory after first use. See } else if (InDiscardExperiment()) {
// http://crbug.com/1045052. // Experimentally free code cache from memory after first use. See
cache_handler->ClearCachedMetadata( // http://crbug.com/1045052.
CachedMetadataHandler::kDiscardLocally); cache_handler->ClearCachedMetadata(
CachedMetadataHandler::kDiscardLocally);
}
cache_result.consume_result = base::make_optional(
inspector_compile_script_event::V8CacheResult::ConsumeResult(
compile_options, cached_data->length, cached_data->rejected));
break;
} }
cache_result.consume_result = base::make_optional(
inspector_compile_script_event::V8CacheResult::ConsumeResult(
compile_options, cached_data->length, cached_data->rejected));
break;
} }
} }
TRACE_EVENT_END1(kTraceEventCategoryGroup, "v8.compileModule", "data", TRACE_EVENT_END1(kTraceEventCategoryGroup, "v8.compileModule", "data",
inspector_compile_script_event::Data( inspector_compile_script_event::Data(
file_name, start_position, cache_result, false, file_name, start_position, cache_result, streamer,
ScriptStreamer::NotStreamingReason::kModuleScript)); params.NotStreamingReason()));
return script; return script;
} }
......
...@@ -433,6 +433,8 @@ const char* NotStreamedReasonString(ScriptStreamer::NotStreamingReason reason) { ...@@ -433,6 +433,8 @@ const char* NotStreamedReasonString(ScriptStreamer::NotStreamingReason reason) {
return "no data pipe received"; return "no data pipe received";
case ScriptStreamer::NotStreamingReason::kDisabledByFeatureList: case ScriptStreamer::NotStreamingReason::kDisabledByFeatureList:
return "streaming disabled from the feature list"; return "streaming disabled from the feature list";
case ScriptStreamer::NotStreamingReason::kNonJavascriptModule:
return "not a javascript module";
case ScriptStreamer::NotStreamingReason::kLoadingCancelled: case ScriptStreamer::NotStreamingReason::kLoadingCancelled:
return "loading was cancelled"; return "loading was cancelled";
case ScriptStreamer::NotStreamingReason::kDidntTryToStartStreaming: case ScriptStreamer::NotStreamingReason::kDidntTryToStartStreaming:
......
...@@ -28,9 +28,13 @@ void DocumentModuleScriptFetcher::Fetch( ...@@ -28,9 +28,13 @@ void DocumentModuleScriptFetcher::Fetch(
DCHECK(fetch_client_settings_object_fetcher); DCHECK(fetch_client_settings_object_fetcher);
DCHECK(!client_); DCHECK(!client_);
client_ = client; client_ = client;
// TODO(crbug.com/1061857): Enable streaming. // Streaming can currently only be triggered from the main thread. This
// currently happens only for dynamic imports in worker modules.
ScriptResource::StreamingAllowed streaming_allowed =
IsMainThread() ? ScriptResource::kAllowStreaming
: ScriptResource::kNoStreaming;
ScriptResource::Fetch(fetch_params, fetch_client_settings_object_fetcher, ScriptResource::Fetch(fetch_params, fetch_client_settings_object_fetcher,
this, ScriptResource::kNoStreaming); this, streaming_allowed);
} }
void DocumentModuleScriptFetcher::NotifyFinished(Resource* resource) { void DocumentModuleScriptFetcher::NotifyFinished(Resource* resource) {
......
...@@ -110,6 +110,9 @@ class ModuleScriptCreationParams { ...@@ -110,6 +110,9 @@ class ModuleScriptCreationParams {
} }
ScriptStreamer* GetScriptStreamer() const { return script_streamer_; } ScriptStreamer* GetScriptStreamer() const { return script_streamer_; }
ScriptStreamer::NotStreamingReason NotStreamingReason() const {
return not_streaming_reason_;
}
private: private:
// Creates an isolated copy. // Creates an isolated copy.
......
...@@ -18,6 +18,20 @@ ...@@ -18,6 +18,20 @@
document.body.appendChild(iframe); document.body.appendChild(iframe);
}, "A CSS Module should load"); }, "A CSS Module should load");
async_test(function (test) {
// This tests potential streaming compilation of modules in
// Chromium that is triggered only for large (32>KiB) files in older
// versions.
const iframe = document.createElement("iframe");
iframe.src = "resources/css-module-basic-large-iframe.html";
iframe.onload = test.step_func_done(function () {
assert_equals(getComputedStyle(iframe.contentDocument.querySelector('#test'))
.backgroundColor, "rgb(255, 0, 0)",
"CSS module import should succeed");
});
document.body.appendChild(iframe);
}, "A large CSS Module should load");
async_test(function (test) { async_test(function (test) {
const iframe = document.createElement("iframe"); const iframe = document.createElement("iframe");
iframe.src = "resources/css-module-at-import-iframe.html"; iframe.src = "resources/css-module-at-import-iframe.html";
...@@ -41,4 +55,4 @@ ...@@ -41,4 +55,4 @@
document.body.appendChild(iframe); document.body.appendChild(iframe);
}, "Malformed CSS should not load"); }, "Malformed CSS should not load");
</script> </script>
</body> </body>
\ No newline at end of file
<!DOCTYPE html>
<body>
<script>
window.onerror = function (errorMsg, url, lineNumber, column, errorObj)
{
document.load_error = errorObj.name;
return true;
};
</script>
<script type="module">
import v from "./basic-large.css";
document.adoptedStyleSheets = [v];
</script>
<div id="test">
I am a test div.
</div>
</body>
...@@ -6,7 +6,7 @@ v8.compileModule Properties: ...@@ -6,7 +6,7 @@ v8.compileModule Properties:
data : { data : {
columnNumber : 1 columnNumber : 1
lineNumber : 1 lineNumber : 1
notStreamedReason : "module script" notStreamedReason : "script too small"
streamed : <boolean> streamed : <boolean>
url : .../devtools/resources/v8-cache-script.cgi url : .../devtools/resources/v8-cache-script.cgi
} }
...@@ -20,7 +20,7 @@ v8.compileModule Properties: ...@@ -20,7 +20,7 @@ v8.compileModule Properties:
data : { data : {
columnNumber : 1 columnNumber : 1
lineNumber : 1 lineNumber : 1
notStreamedReason : "module script" notStreamedReason : "already used streamed data"
streamed : <boolean> streamed : <boolean>
url : .../devtools/resources/v8-cache-script.cgi url : .../devtools/resources/v8-cache-script.cgi
} }
...@@ -52,7 +52,7 @@ v8.compileModule Properties: ...@@ -52,7 +52,7 @@ v8.compileModule Properties:
columnNumber : 1 columnNumber : 1
consumedCacheSize : <number> consumedCacheSize : <number>
lineNumber : 1 lineNumber : 1
notStreamedReason : "module script" notStreamedReason : "already used streamed data"
streamed : <boolean> streamed : <boolean>
url : .../devtools/resources/v8-cache-script.cgi url : .../devtools/resources/v8-cache-script.cgi
} }
...@@ -66,7 +66,7 @@ v8.compileModule Properties: ...@@ -66,7 +66,7 @@ v8.compileModule Properties:
data : { data : {
columnNumber : 1 columnNumber : 1
lineNumber : 1 lineNumber : 1
notStreamedReason : "module script" notStreamedReason : "script too small"
streamed : <boolean> streamed : <boolean>
url : .../devtools/resources/v8-cache-script.cgi url : .../devtools/resources/v8-cache-script.cgi
} }
...@@ -83,7 +83,7 @@ v8.compileModule Properties: ...@@ -83,7 +83,7 @@ v8.compileModule Properties:
columnNumber : 1 columnNumber : 1
consumedCacheSize : <number> consumedCacheSize : <number>
lineNumber : 1 lineNumber : 1
notStreamedReason : "module script" notStreamedReason : "already used streamed data"
streamed : <boolean> streamed : <boolean>
url : .../devtools/resources/v8-cache-script.cgi url : .../devtools/resources/v8-cache-script.cgi
} }
...@@ -100,7 +100,7 @@ v8.compileModule Properties: ...@@ -100,7 +100,7 @@ v8.compileModule Properties:
columnNumber : 1 columnNumber : 1
consumedCacheSize : <number> consumedCacheSize : <number>
lineNumber : 1 lineNumber : 1
notStreamedReason : "module script" notStreamedReason : "script too small"
streamed : <boolean> streamed : <boolean>
url : .../devtools/resources/v8-cache-script.cgi url : .../devtools/resources/v8-cache-script.cgi
} }
......
...@@ -5,7 +5,7 @@ v8.compileModule Properties: ...@@ -5,7 +5,7 @@ v8.compileModule Properties:
data : { data : {
columnNumber : 1 columnNumber : 1
lineNumber : 1 lineNumber : 1
notStreamedReason : "module script" notStreamedReason : "already disabled streaming"
streamed : <boolean> streamed : <boolean>
url : .../devtools/resources/inspected-page.html url : .../devtools/resources/inspected-page.html
} }
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment