From 038cd96142d384c0d2238973f1cb277725a62eba Mon Sep 17 00:00:00 2001 From: Clark DuVall Date: Thu, 4 Nov 2021 22:37:15 +0000 Subject: [PATCH] Fetch code cache earlier in NavigationBodyLoader The results from the NavigationThreadingOptimizations experiment have shown that decreasing the time we wait for the code cache can have a large affect on FCP/LCP. This change attempts to fetch the code cache much earlier in commit so it will hopefully be ready when needed. Note if the main contributor to the code cache response time is the queue time of the response task in the renderer, this probably won't help much. If experiment results don't show much of a speedup, that's the next place to optimize. Bug: 1266146 Change-Id: Ie8d702b467ce7455a65ade1ca70d30b2def22870 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3258419 Commit-Queue: Clark DuVall Reviewed-by: John Abd-El-Malek Reviewed-by: Jeremy Roman Cr-Commit-Position: refs/heads/main@{#938553} --- content/renderer/render_frame_impl.cc | 12 ++++- third_party/blink/common/features.cc | 4 ++ third_party/blink/public/common/features.h | 2 + .../platform/web_navigation_body_loader.h | 3 ++ .../renderer/core/loader/document_loader.cc | 22 +++++--- .../renderer/core/loader/document_loader.h | 3 ++ .../url_loader/navigation_body_loader.cc | 50 ++++++++++++++----- .../fetch/url_loader/navigation_body_loader.h | 22 ++++++-- .../static_data_navigation_body_loader.cc | 3 ++ .../static_data_navigation_body_loader.h | 1 + 10 files changed, 96 insertions(+), 26 deletions(-) diff --git a/content/renderer/render_frame_impl.cc b/content/renderer/render_frame_impl.cc index 8a887e8f90c6a4..af92416646911b 100644 --- a/content/renderer/render_frame_impl.cc +++ b/content/renderer/render_frame_impl.cc @@ -3737,6 +3737,12 @@ void RenderFrameImpl::DidCreateDocumentLoader( document_loader->SetServiceWorkerNetworkProvider( ServiceWorkerNetworkProviderForFrame::CreateInvalidInstance()); } + + // Set the code cache host earlier if the kEarlyCodeCache feature is enabled + // to allow fetching the code cache as soon as possible. + if (base::FeatureList::IsEnabled(blink::features::kEarlyCodeCache)) { + document_loader->SetCodeCacheHost(std::move(pending_code_cache_host_)); + } } void RenderFrameImpl::DidCommitNavigation( @@ -3853,8 +3859,10 @@ void RenderFrameImpl::DidCommitNavigation( // required. When pending_code_cache_host_ is nullptr this method just resets // any earlier code cache host interface. Since we are committing a new // navigation any interfaces requested prior to this point should not be used. - frame_->GetDocumentLoader()->SetCodeCacheHost( - std::move(pending_code_cache_host_)); + if (!base::FeatureList::IsEnabled(blink::features::kEarlyCodeCache)) { + frame_->GetDocumentLoader()->SetCodeCacheHost( + std::move(pending_code_cache_host_)); + } // TODO(crbug.com/888079): Turn this into a DCHECK for origin equality when // the linked bug is fixed. Currently sometimes the browser and renderer diff --git a/third_party/blink/common/features.cc b/third_party/blink/common/features.cc index c0636897ed0672..7fa83cb1265898 100644 --- a/third_party/blink/common/features.cc +++ b/third_party/blink/common/features.cc @@ -1141,5 +1141,9 @@ const base::Feature kLateFormNewlineNormalization{ const base::Feature kAutoExpandDetailsElement{"AutoExpandDetailsElement", base::FEATURE_ENABLED_BY_DEFAULT}; +// Enables fetching the code cache earlier in navigation. +const base::Feature kEarlyCodeCache{"EarlyCodeCache", + base::FEATURE_DISABLED_BY_DEFAULT}; + } // namespace features } // namespace blink diff --git a/third_party/blink/public/common/features.h b/third_party/blink/public/common/features.h index 9af81aa7fa5d6b..d63c5cc4d17f73 100644 --- a/third_party/blink/public/common/features.h +++ b/third_party/blink/public/common/features.h @@ -537,6 +537,8 @@ BLINK_COMMON_EXPORT extern const base::Feature kLateFormNewlineNormalization; // and released to stable with no issues. BLINK_COMMON_EXPORT extern const base::Feature kAutoExpandDetailsElement; +BLINK_COMMON_EXPORT extern const base::Feature kEarlyCodeCache; + } // namespace features } // namespace blink diff --git a/third_party/blink/public/platform/web_navigation_body_loader.h b/third_party/blink/public/platform/web_navigation_body_loader.h index f694c5999a3173..2595196f98dae0 100644 --- a/third_party/blink/public/platform/web_navigation_body_loader.h +++ b/third_party/blink/public/platform/web_navigation_body_loader.h @@ -76,6 +76,9 @@ class BLINK_EXPORT WebNavigationBodyLoader { // Starts loading the body. Client must be non-null, and will receive // the body, code cache and final result. virtual void StartLoadingBody(Client*, CodeCacheHost* code_cache_host) = 0; + + // Starts loading the code cache. + virtual void StartLoadingCodeCache(CodeCacheHost* code_cache_host) = 0; }; } // namespace blink diff --git a/third_party/blink/renderer/core/loader/document_loader.cc b/third_party/blink/renderer/core/loader/document_loader.cc index 81a1f827a6adbf..5de604c74205a4 100644 --- a/third_party/blink/renderer/core/loader/document_loader.cc +++ b/third_party/blink/renderer/core/loader/document_loader.cc @@ -1691,13 +1691,8 @@ void DocumentLoader::StartLoadingResponse() { return; } - bool use_isolated_code_cache = - RuntimeEnabledFeatures::CacheInlineScriptCodeEnabled() && - ShouldUseIsolatedCodeCache(mojom::blink::RequestContextType::HYPERLINK, - response_); - // The |cached_metadata_handler_| is created, even when - // |use_isolated_code_cache| is false to support the parts that don't + // |UseIsolatedCodeCache()| is false to support the parts that don't // go throught the site-isolated-code-cache. auto cached_metadata_sender = CachedMetadataSender::Create( response_, blink::mojom::CodeCacheType::kJavascript, requestor_origin_); @@ -1706,7 +1701,7 @@ void DocumentLoader::StartLoadingResponse() { WTF::TextEncoding(), std::move(cached_metadata_sender)); CodeCacheHost* code_cache_host = nullptr; - if (use_isolated_code_cache) { + if (UseIsolatedCodeCache()) { code_cache_host = GetCodeCacheHost(); DCHECK(code_cache_host); } @@ -2188,6 +2183,13 @@ void DocumentLoader::CommitNavigation() { DCHECK_EQ(frame_->Tree().ChildCount(), 0u); state_ = kCommitted; + if (body_loader_ && !loading_main_document_from_mhtml_archive_ && + !loading_url_as_empty_document_ && url_.ProtocolIsInHTTPFamily() && + UseIsolatedCodeCache() && + base::FeatureList::IsEnabled(features::kEarlyCodeCache)) { + body_loader_->StartLoadingCodeCache(GetCodeCacheHost()); + } + // Prepare a DocumentInit before clearing the frame, because it may need to // inherit an aliased security context. Document* owner_document = nullptr; @@ -2714,6 +2716,12 @@ ContentSecurityPolicy* DocumentLoader::CreateCSP() { return csp; } +bool DocumentLoader::UseIsolatedCodeCache() { + return RuntimeEnabledFeatures::CacheInlineScriptCodeEnabled() && + ShouldUseIsolatedCodeCache(mojom::blink::RequestContextType::HYPERLINK, + response_); +} + bool& GetDisableCodeCacheForTesting() { static bool disable_code_cache_for_testing = false; return disable_code_cache_for_testing; diff --git a/third_party/blink/renderer/core/loader/document_loader.h b/third_party/blink/renderer/core/loader/document_loader.h index e1d00ae8ba99a3..e5fc185d61bc1c 100644 --- a/third_party/blink/renderer/core/loader/document_loader.h +++ b/third_party/blink/renderer/core/loader/document_loader.h @@ -481,6 +481,9 @@ class CORE_EXPORT DocumentLoader : public GarbageCollected, // Computes and creates CSP for this document. ContentSecurityPolicy* CreateCSP(); + // Whether the isolated code cache should be used for this document. + bool UseIsolatedCodeCache(); + // Params are saved in constructor and are cleared after StartLoading(). // TODO(dgozman): remove once StartLoading is merged with constructor. std::unique_ptr params_; diff --git a/third_party/blink/renderer/platform/loader/fetch/url_loader/navigation_body_loader.cc b/third_party/blink/renderer/platform/loader/fetch/url_loader/navigation_body_loader.cc index d7033d0aaceee9..2241d95c7bbe4d 100644 --- a/third_party/blink/renderer/platform/loader/fetch/url_loader/navigation_body_loader.cc +++ b/third_party/blink/renderer/platform/loader/fetch/url_loader/navigation_body_loader.cc @@ -138,32 +138,58 @@ void NavigationBodyLoader::StartLoadingBody( std::move(response_head_), PreviewsTypes::PREVIEWS_OFF); if (code_cache_host) { - code_cache_loader_ = WebCodeCacheLoader::Create(code_cache_host); - code_cache_loader_->FetchFromCodeCache( - mojom::CodeCacheType::kJavascript, original_url_, - base::BindOnce(&NavigationBodyLoader::CodeCacheReceived, - weak_factory_.GetWeakPtr(), base::TimeTicks::Now(), - response_head_response_time)); + if (code_cache_data_) { + ContinueWithCodeCache(base::TimeTicks::Now(), + response_head_response_time); + return; + } + + // Save these for when the code cache is ready. + code_cache_wait_start_time_ = base::TimeTicks::Now(); + response_head_response_time_ = response_head_response_time; + + // If the code cache loader hasn't been created yet the request hasn't + // started, so start it now. + if (!code_cache_loader_) + StartLoadingCodeCache(code_cache_host); return; } BindURLLoaderAndStartLoadingResponseBodyIfPossible(); } -void NavigationBodyLoader::CodeCacheReceived( +void NavigationBodyLoader::StartLoadingCodeCache( + CodeCacheHost* code_cache_host) { + code_cache_loader_ = WebCodeCacheLoader::Create(code_cache_host); + code_cache_loader_->FetchFromCodeCache( + mojom::CodeCacheType::kJavascript, original_url_, + base::BindOnce(&NavigationBodyLoader::CodeCacheReceived, + weak_factory_.GetWeakPtr())); +} + +void NavigationBodyLoader::CodeCacheReceived(base::Time response_time, + mojo_base::BigBuffer data) { + code_cache_data_ = std::move(data); + code_cache_response_time_ = response_time; + if (!code_cache_wait_start_time_.is_null()) { + ContinueWithCodeCache(code_cache_wait_start_time_, + response_head_response_time_); + } +} + +void NavigationBodyLoader::ContinueWithCodeCache( base::TimeTicks start_time, - base::Time response_head_response_time, - base::Time response_time, - mojo_base::BigBuffer data) { + base::Time response_head_response_time) { base::UmaHistogramTimes( base::StrCat({"Navigation.CodeCacheTime.", is_main_frame_ ? "MainFrame" : "Subframe"}), base::TimeTicks::Now() - start_time); + // Check that the times match to ensure that the code cache data is for this // response. See https://crbug.com/1099587. - if (response_head_response_time == response_time && client_) { + if (response_head_response_time == code_cache_response_time_ && client_) { base::WeakPtr weak_self = weak_factory_.GetWeakPtr(); - client_->BodyCodeCacheReceived(std::move(data)); + client_->BodyCodeCacheReceived(std::move(*code_cache_data_)); if (!weak_self) return; } diff --git a/third_party/blink/renderer/platform/loader/fetch/url_loader/navigation_body_loader.h b/third_party/blink/renderer/platform/loader/fetch/url_loader/navigation_body_loader.h index 8ec7e90ec90502..327ea96d96a3b2 100644 --- a/third_party/blink/renderer/platform/loader/fetch/url_loader/navigation_body_loader.h +++ b/third_party/blink/renderer/platform/loader/fetch/url_loader/navigation_body_loader.h @@ -63,7 +63,7 @@ class NavigationBodyLoader : public WebNavigationBodyLoader, // // StartLoadingBody // request code cache - // CodeCacheReceived + // ContinueWithCodeCache // notify client about cache // BindURLLoaderAndContinue // OnStartLoadingResponseBody @@ -83,6 +83,7 @@ class NavigationBodyLoader : public WebNavigationBodyLoader, void SetDefersLoading(WebLoaderFreezeMode mode) override; void StartLoadingBody(WebNavigationBodyLoader::Client* client, CodeCacheHost* code_cache_host) override; + void StartLoadingCodeCache(CodeCacheHost* code_cache_host) override; // network::mojom::URLLoaderClient implementation. void OnReceiveEarlyHints(network::mojom::EarlyHintsPtr early_hints) override; @@ -100,10 +101,9 @@ class NavigationBodyLoader : public WebNavigationBodyLoader, mojo::ScopedDataPipeConsumerHandle handle) override; void OnComplete(const network::URLLoaderCompletionStatus& status) override; - void CodeCacheReceived(base::TimeTicks start_time, - base::Time response_head_response_time, - base::Time response_time, - mojo_base::BigBuffer data); + void CodeCacheReceived(base::Time response_time, mojo_base::BigBuffer data); + void ContinueWithCodeCache(base::TimeTicks start_time, + base::Time response_head_response_time); void BindURLLoaderAndContinue(); void OnConnectionClosed(); void OnReadable(MojoResult unused); @@ -163,6 +163,18 @@ class NavigationBodyLoader : public WebNavigationBodyLoader, const bool is_main_frame_; + // The time the loader started waiting for the code cache. If this is + // non-null, |ContinueWithCodeCache()| should be called when the cache is + // available. + base::TimeTicks code_cache_wait_start_time_; + // The response time from the response head. + base::Time response_head_response_time_; + + // These fields will be filled with the code cache response to be used when + // needed. + base::Time code_cache_response_time_; + absl::optional code_cache_data_; + base::WeakPtrFactory weak_factory_{this}; }; diff --git a/third_party/blink/renderer/platform/loader/static_data_navigation_body_loader.cc b/third_party/blink/renderer/platform/loader/static_data_navigation_body_loader.cc index 6bc3ac7e725a16..81dabf8d4cd993 100644 --- a/third_party/blink/renderer/platform/loader/static_data_navigation_body_loader.cc +++ b/third_party/blink/renderer/platform/loader/static_data_navigation_body_loader.cc @@ -47,6 +47,9 @@ void StaticDataNavigationBodyLoader::StartLoadingBody( Continue(); } +void StaticDataNavigationBodyLoader::StartLoadingCodeCache( + CodeCacheHost* code_cache_host) {} + void StaticDataNavigationBodyLoader::Continue() { if (freeze_mode_ != LoaderFreezeMode::kNone || !client_ || is_in_continue_) return; diff --git a/third_party/blink/renderer/platform/loader/static_data_navigation_body_loader.h b/third_party/blink/renderer/platform/loader/static_data_navigation_body_loader.h index 829ac63fd76b76..deb703251d8de6 100644 --- a/third_party/blink/renderer/platform/loader/static_data_navigation_body_loader.h +++ b/third_party/blink/renderer/platform/loader/static_data_navigation_body_loader.h @@ -29,6 +29,7 @@ class PLATFORM_EXPORT StaticDataNavigationBodyLoader void SetDefersLoading(LoaderFreezeMode) override; void StartLoadingBody(WebNavigationBodyLoader::Client*, CodeCacheHost* host) override; + void StartLoadingCodeCache(CodeCacheHost* code_cache_host) override; private: void Continue();