From 0d2aca012664d3ba22ed109f8b3cd8920ed0988c Mon Sep 17 00:00:00 2001 From: Ross McIlroy Date: Wed, 25 Jul 2018 05:29:58 +0000 Subject: [PATCH] [ScriptStreamer] Propagate NotStreamingReason to v8.compile trace event. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Enables tracking of why a particular script was not parsed / compiled by the ScriptStreamer. BUG=865098 Change-Id: I1a1b033689d0f12df4f28ff5c4bb21243cbec197 Reviewed-on: https://chromium-review.googlesource.com/1143762 Commit-Queue: Ross McIlroy Reviewed-by: Marja Hölttä Reviewed-by: Hiroshige Hayashizaki Reviewed-by: Kentaro Hara Reviewed-by: Dmitry Gozman Reviewed-by: Kouhei Ueno Cr-Commit-Position: refs/heads/master@{#577808} --- .../service-worker-v8-cache-expected.txt | 4 ++ .../timeline-js/compile-script-expected.txt | 2 + .../bindings/core/v8/script_source_code.cc | 9 +++- .../bindings/core/v8/script_source_code.h | 8 +++- .../bindings/core/v8/v8_code_cache.cc | 4 +- .../bindings/core/v8/v8_script_runner.cc | 8 ++-- .../bindings/core/v8/v8_script_runner_test.cc | 9 ++-- .../core/inspector/inspector_trace_events.cc | 42 ++++++++++++++++++- .../core/inspector/inspector_trace_events.h | 4 +- .../core/script/classic_pending_script.cc | 3 +- 10 files changed, 78 insertions(+), 15 deletions(-) diff --git a/third_party/WebKit/LayoutTests/http/tests/devtools/service-workers/service-worker-v8-cache-expected.txt b/third_party/WebKit/LayoutTests/http/tests/devtools/service-workers/service-worker-v8-cache-expected.txt index d712dee8c224e3..35124379ac2425 100644 --- a/third_party/WebKit/LayoutTests/http/tests/devtools/service-workers/service-worker-v8-cache-expected.txt +++ b/third_party/WebKit/LayoutTests/http/tests/devtools/service-workers/service-worker-v8-cache-expected.txt @@ -6,6 +6,7 @@ v8.compile Properties: data : { columnNumber : 0 lineNumber : 0 + notStreamedReason : "inline script" streamed : url : .../devtools/service-workers/resources/v8-cache-worker.js } @@ -20,6 +21,7 @@ v8.compile Properties: cacheProduceOptions : "full code" columnNumber : 0 lineNumber : 0 + notStreamedReason : "script has code-cache available" producedCacheSize : streamed : url : .../devtools/resources/v8-cache-script.js @@ -39,6 +41,7 @@ v8.compile Properties: columnNumber : 0 consumedCacheSize : lineNumber : 0 + notStreamedReason : "script too small" streamed : url : .../devtools/resources/v8-cache-script.js } @@ -55,6 +58,7 @@ v8.compile Properties: columnNumber : 0 consumedCacheSize : lineNumber : 0 + notStreamedReason : "resource no longer alive" streamed : url : .../devtools/resources/v8-cache-script.js } diff --git a/third_party/WebKit/LayoutTests/http/tests/devtools/tracing/timeline-js/compile-script-expected.txt b/third_party/WebKit/LayoutTests/http/tests/devtools/tracing/timeline-js/compile-script-expected.txt index 3d3de97f3ce489..b373d486316fce 100644 --- a/third_party/WebKit/LayoutTests/http/tests/devtools/tracing/timeline-js/compile-script-expected.txt +++ b/third_party/WebKit/LayoutTests/http/tests/devtools/tracing/timeline-js/compile-script-expected.txt @@ -5,6 +5,7 @@ v8.compile Properties: data : { columnNumber : 0 lineNumber : 0 + notStreamedReason : "inline script" streamed : url : } @@ -18,6 +19,7 @@ v8.compile Properties: data : { columnNumber : 0 lineNumber : 0 + notStreamedReason : "script too small" streamed : url : .../devtools/tracing/timeline-js/resources/timeline-script-tag-2.js } diff --git a/third_party/blink/renderer/bindings/core/v8/script_source_code.cc b/third_party/blink/renderer/bindings/core/v8/script_source_code.cc index 7b5bd83192613e..cf0b33274f96d0 100644 --- a/third_party/blink/renderer/bindings/core/v8/script_source_code.cc +++ b/third_party/blink/renderer/bindings/core/v8/script_source_code.cc @@ -56,6 +56,7 @@ ScriptSourceCode::ScriptSourceCode( const TextPosition& start_position) : source_(TreatNullSourceAsEmpty(source)), cache_handler_(cache_handler), + not_streaming_reason_(ScriptStreamer::kInlineScript), url_(StripFragmentIdentifier(url)), start_position_(start_position), source_location_type_(source_location_type) { @@ -76,14 +77,18 @@ ScriptSourceCode::ScriptSourceCode( start_position) {} ScriptSourceCode::ScriptSourceCode(ScriptStreamer* streamer, - ScriptResource* resource) + ScriptResource* resource, + ScriptStreamer::NotStreamingReason reason) : source_(TreatNullSourceAsEmpty(resource->SourceText())), cache_handler_(resource->CacheHandler()), streamer_(streamer), + not_streaming_reason_(reason), url_(StripFragmentIdentifier(resource->GetResponse().Url())), source_map_url_(SourceMapUrlFromResponse(resource->GetResponse())), start_position_(TextPosition::MinimumPosition()), - source_location_type_(ScriptSourceLocationType::kExternalFile) {} + source_location_type_(ScriptSourceLocationType::kExternalFile) { + DCHECK_EQ(!streamer, reason != ScriptStreamer::NotStreamingReason::kInvalid); +} ScriptSourceCode::~ScriptSourceCode() = default; diff --git a/third_party/blink/renderer/bindings/core/v8/script_source_code.h b/third_party/blink/renderer/bindings/core/v8/script_source_code.h index 0dde36ca293497..8530b1221b7725 100644 --- a/third_party/blink/renderer/bindings/core/v8/script_source_code.h +++ b/third_party/blink/renderer/bindings/core/v8/script_source_code.h @@ -68,7 +68,9 @@ class CORE_EXPORT ScriptSourceCode final { // // We lose the encoding information from ScriptResource. // Not sure if that matters. - ScriptSourceCode(ScriptStreamer*, ScriptResource*); + ScriptSourceCode(ScriptStreamer*, + ScriptResource*, + ScriptStreamer::NotStreamingReason); ~ScriptSourceCode(); void Trace(blink::Visitor*); @@ -83,11 +85,15 @@ class CORE_EXPORT ScriptSourceCode final { const String& SourceMapUrl() const { return source_map_url_; } ScriptStreamer* Streamer() const { return streamer_; } + ScriptStreamer::NotStreamingReason NotStreamingReason() const { + return not_streaming_reason_; + } private: const MovableString source_; Member cache_handler_; Member streamer_; + ScriptStreamer::NotStreamingReason not_streaming_reason_; // The URL of the source code, which is primarily intended for DevTools // javascript debugger. diff --git a/third_party/blink/renderer/bindings/core/v8/v8_code_cache.cc b/third_party/blink/renderer/bindings/core/v8/v8_code_cache.cc index ad3eca49af98e6..548294f3c077b5 100644 --- a/third_party/blink/renderer/bindings/core/v8/v8_code_cache.cc +++ b/third_party/blink/renderer/bindings/core/v8/v8_code_cache.cc @@ -225,7 +225,7 @@ void V8CodeCache::ProduceCache( compile_options, cached_data ? cached_data->length : 0), base::Optional()), - source.Streamer())); + source.Streamer(), source.NotStreamingReason())); break; } case ProduceCacheOptions::kNoProduceCache: @@ -314,7 +314,7 @@ scoped_refptr V8CodeCache::GenerateFullCodeCache( cached_data ? cached_data->length : 0), base::Optional< InspectorCompileScriptEvent::V8CacheResult::ConsumeResult>()), - false)); + false, ScriptStreamer::kHasCodeCache)); return cached_metadata; } diff --git a/third_party/blink/renderer/bindings/core/v8/v8_script_runner.cc b/third_party/blink/renderer/bindings/core/v8/v8_script_runner.cc index 9d3e53b8f2b517..4388d8fc3ccd1d 100644 --- a/third_party/blink/renderer/bindings/core/v8/v8_script_runner.cc +++ b/third_party/blink/renderer/bindings/core/v8/v8_script_runner.cc @@ -215,10 +215,10 @@ v8::MaybeLocal V8ScriptRunner::CompileScript( v8::MaybeLocal script = CompileScriptInternal(isolate, execution_context, source, origin, compile_options, no_cache_reason, &cache_result); - TRACE_EVENT_END1( - kTraceEventCategoryGroup, "v8.compile", "data", - InspectorCompileScriptEvent::Data(file_name, script_start_position, - cache_result, source.Streamer())); + TRACE_EVENT_END1(kTraceEventCategoryGroup, "v8.compile", "data", + InspectorCompileScriptEvent::Data( + file_name, script_start_position, cache_result, + source.Streamer(), source.NotStreamingReason())); return script; } diff --git a/third_party/blink/renderer/bindings/core/v8/v8_script_runner_test.cc b/third_party/blink/renderer/bindings/core/v8/v8_script_runner_test.cc index 8e87ed69554022..0f989bb09ec1cd 100644 --- a/third_party/blink/renderer/bindings/core/v8/v8_script_runner_test.cc +++ b/third_party/blink/renderer/bindings/core/v8/v8_script_runner_test.cc @@ -130,7 +130,8 @@ TEST_F(V8ScriptRunnerTest, emptyResourceDoesNotHaveCacheHandler) { TEST_F(V8ScriptRunnerTest, codeOption) { V8TestingScope scope; - ScriptSourceCode source_code(nullptr, CreateResource(UTF8Encoding())); + ScriptSourceCode source_code(nullptr, CreateResource(UTF8Encoding()), + ScriptStreamer::kScriptTooSmall); SingleCachedMetadataHandler* cache_handler = source_code.CacheHandler(); SetCacheTimeStamp(cache_handler); @@ -147,7 +148,8 @@ TEST_F(V8ScriptRunnerTest, codeOption) { TEST_F(V8ScriptRunnerTest, consumeCodeOption) { V8TestingScope scope; - ScriptSourceCode source_code(nullptr, CreateResource(UTF8Encoding())); + ScriptSourceCode source_code(nullptr, CreateResource(UTF8Encoding()), + ScriptStreamer::kScriptTooSmall); // Set timestamp to simulate a warm run. SingleCachedMetadataHandler* cache_handler = source_code.CacheHandler(); SetCacheTimeStamp(cache_handler); @@ -177,7 +179,8 @@ TEST_F(V8ScriptRunnerTest, consumeCodeOption) { TEST_F(V8ScriptRunnerTest, produceAndConsumeCodeOption) { V8TestingScope scope; - ScriptSourceCode source_code(nullptr, CreateResource(UTF8Encoding())); + ScriptSourceCode source_code(nullptr, CreateResource(UTF8Encoding()), + ScriptStreamer::kScriptTooSmall); SingleCachedMetadataHandler* cache_handler = source_code.CacheHandler(); // Cold run - should set the timestamp diff --git a/third_party/blink/renderer/core/inspector/inspector_trace_events.cc b/third_party/blink/renderer/core/inspector/inspector_trace_events.cc index e22bd9a857fcec..444a6d42e74cee 100644 --- a/third_party/blink/renderer/core/inspector/inspector_trace_events.cc +++ b/third_party/blink/renderer/core/inspector/inspector_trace_events.cc @@ -358,6 +358,41 @@ const char* CompileOptionsString(v8::ScriptCompiler::CompileOptions options) { return ""; } +const char* NotStreamedReasonString(ScriptStreamer::NotStreamingReason reason) { + switch (reason) { + case ScriptStreamer::kNotHTTP: + return "not http/https protocol"; + case ScriptStreamer::kReload: + return "reload event"; + case ScriptStreamer::kContextNotValid: + return "script context not valid"; + case ScriptStreamer::kEncodingNotSupported: + return "encoding not supported"; + case ScriptStreamer::kThreadBusy: + return "script streamer thread busy"; + case ScriptStreamer::kV8CannotStream: + return "V8 cannot stream script"; + case ScriptStreamer::kScriptTooSmall: + return "script too small"; + case ScriptStreamer::kNoResourceBuffer: + return "resource no longer alive"; + case ScriptStreamer::kHasCodeCache: + return "script has code-cache available"; + case ScriptStreamer::kStreamerNotReadyOnGetSource: + return "streamer not ready"; + case ScriptStreamer::kInlineScript: + return "inline script"; + case ScriptStreamer::kDidntTryToStartStreaming: + return "start streaming not called"; + case ScriptStreamer::kAlreadyLoaded: + case ScriptStreamer::kCount: + case ScriptStreamer::kInvalid: + NOTREACHED(); + } + NOTREACHED(); + return ""; +} + } // namespace namespace InspectorScheduleStyleInvalidationTrackingEvent { @@ -1105,7 +1140,8 @@ std::unique_ptr InspectorCompileScriptEvent::Data( const String& url, const TextPosition& text_position, const V8CacheResult& cache_result, - bool streamed) { + bool streamed, + ScriptStreamer::NotStreamingReason not_streaming_reason) { std::unique_ptr value = FillLocation(url, text_position); if (cache_result.produce_result) { @@ -1125,6 +1161,10 @@ std::unique_ptr InspectorCompileScriptEvent::Data( value->SetBoolean("cacheRejected", cache_result.consume_result->rejected); } value->SetBoolean("streamed", streamed); + if (!streamed) { + value->SetString("notStreamedReason", + NotStreamedReasonString(not_streaming_reason)); + } return value; } diff --git a/third_party/blink/renderer/core/inspector/inspector_trace_events.h b/third_party/blink/renderer/core/inspector/inspector_trace_events.h index cd8112d1558af8..6812d634ddac3c 100644 --- a/third_party/blink/renderer/core/inspector/inspector_trace_events.h +++ b/third_party/blink/renderer/core/inspector/inspector_trace_events.h @@ -9,6 +9,7 @@ #include "base/macros.h" #include "base/optional.h" +#include "third_party/blink/renderer/bindings/core/v8/script_streamer.h" #include "third_party/blink/renderer/core/core_export.h" #include "third_party/blink/renderer/core/css/css_selector.h" #include "third_party/blink/renderer/core/loader/frame_loader_types.h" @@ -425,7 +426,8 @@ struct V8CacheResult { std::unique_ptr Data(const String& url, const WTF::TextPosition&, const V8CacheResult&, - bool streamed); + bool streamed, + ScriptStreamer::NotStreamingReason); } // namespace InspectorCompileScriptEvent namespace InspectorFunctionCallEvent { diff --git a/third_party/blink/renderer/core/script/classic_pending_script.cc b/third_party/blink/renderer/core/script/classic_pending_script.cc index acf3b26e885293..a08c15bb869a8a 100644 --- a/third_party/blink/renderer/core/script/classic_pending_script.cc +++ b/third_party/blink/renderer/core/script/classic_pending_script.cc @@ -391,7 +391,8 @@ ClassicScript* ClassicPendingScript::GetSource(const KURL& document_url, RecordStreamingHistogram(GetSchedulingType(), streamer_ready, not_streamed_reason); - ScriptSourceCode source_code(streamer_ready ? streamer_ : nullptr, resource); + ScriptSourceCode source_code(streamer_ready ? streamer_ : nullptr, resource, + not_streamed_reason); // The base URL for external classic script is // "the URL from which the script was obtained" [spec text] // https://html.spec.whatwg.org/multipage/webappapis.html#concept-script-base-url