Commit 45203ab0 authored by Sophie Chang's avatar Sophie Chang Committed by Commit Bot

Allow for number of inflight pre*s to be experimented with

Bug: 1108891
Change-Id: I627b0ca7aa08dbe3717654ff6ba1cb9233237540
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2316324Reviewed-by: default avatarTarun Bansal <tbansal@chromium.org>
Commit-Queue: Sophie Chang <sophiechang@chromium.org>
Cr-Commit-Position: refs/heads/master@{#791913}
parent 344f12fa
...@@ -8,6 +8,7 @@ ...@@ -8,6 +8,7 @@
#include "base/bind.h" #include "base/bind.h"
#include "base/trace_event/trace_event.h" #include "base/trace_event/trace_event.h"
#include "chrome/browser/predictors/predictors_features.h"
#include "chrome/browser/predictors/resource_prefetch_predictor.h" #include "chrome/browser/predictors/resource_prefetch_predictor.h"
#include "chrome/browser/profiles/profile.h" #include "chrome/browser/profiles/profile.h"
#include "content/public/browser/browser_task_traits.h" #include "content/public/browser/browser_task_traits.h"
...@@ -214,7 +215,7 @@ void PreconnectManager::TryToLaunchPreresolveJobs() { ...@@ -214,7 +215,7 @@ void PreconnectManager::TryToLaunchPreresolveJobs() {
DCHECK_CURRENTLY_ON(content::BrowserThread::UI); DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
while (!queued_jobs_.empty() && while (!queued_jobs_.empty() &&
inflight_preresolves_count_ < kMaxInflightPreresolves) { inflight_preresolves_count_ < features::GetMaxInflightPreresolves()) {
auto job_id = queued_jobs_.front(); auto job_id = queued_jobs_.front();
queued_jobs_.pop_front(); queued_jobs_.pop_front();
PreresolveJob* job = preresolve_jobs_.Lookup(job_id); PreresolveJob* job = preresolve_jobs_.Lookup(job_id);
......
...@@ -145,8 +145,6 @@ class PreconnectManager { ...@@ -145,8 +145,6 @@ class PreconnectManager {
bool success) {} bool success) {}
}; };
static const size_t kMaxInflightPreresolves = 3;
PreconnectManager(base::WeakPtr<Delegate> delegate, Profile* profile); PreconnectManager(base::WeakPtr<Delegate> delegate, Profile* profile);
virtual ~PreconnectManager(); virtual ~PreconnectManager();
......
...@@ -14,6 +14,7 @@ ...@@ -14,6 +14,7 @@
#include "base/strings/stringprintf.h" #include "base/strings/stringprintf.h"
#include "base/threading/thread_task_runner_handle.h" #include "base/threading/thread_task_runner_handle.h"
#include "chrome/browser/predictors/loading_test_util.h" #include "chrome/browser/predictors/loading_test_util.h"
#include "chrome/browser/predictors/predictors_features.h"
#include "chrome/browser/predictors/proxy_lookup_client_impl.h" #include "chrome/browser/predictors/proxy_lookup_client_impl.h"
#include "chrome/browser/predictors/resolve_host_client_impl.h" #include "chrome/browser/predictors/resolve_host_client_impl.h"
#include "chrome/test/base/testing_profile.h" #include "chrome/test/base/testing_profile.h"
...@@ -291,16 +292,16 @@ TEST_F(PreconnectManagerTest, TestStartOneUrlPreconnect_MultipleTimes) { ...@@ -291,16 +292,16 @@ TEST_F(PreconnectManagerTest, TestStartOneUrlPreconnect_MultipleTimes) {
GURL main_frame_url("http://google.com"); GURL main_frame_url("http://google.com");
net::NetworkIsolationKey network_isolation_key = net::NetworkIsolationKey network_isolation_key =
CreateNetworkIsolationKey(main_frame_url); CreateNetworkIsolationKey(main_frame_url);
size_t count = PreconnectManager::kMaxInflightPreresolves; size_t count = features::GetMaxInflightPreresolves();
std::vector<PreconnectRequest> requests; std::vector<PreconnectRequest> requests;
for (size_t i = 0; i < count + 1; ++i) { for (size_t i = 0; i < count + 1; ++i) {
// Exactly PreconnectManager::kMaxInflightPreresolves should be preresolved. // Exactly features::GetMaxInflightPreresolves() should be preresolved.
std::string url = base::StringPrintf("http://cdn%" PRIuS ".google.com", i); std::string url = base::StringPrintf("http://cdn%" PRIuS ".google.com", i);
requests.emplace_back(url::Origin::Create(GURL(url)), 1, requests.emplace_back(url::Origin::Create(GURL(url)), 1,
network_isolation_key); network_isolation_key);
} }
for (size_t i = 0; i < count; ++i) { for (size_t i = 0; i < count; ++i) {
// Exactly PreconnectManager::kMaxInflightPreresolves should be initiated // Exactly features::GetMaxInflightPreresolves() should be initiated
// and preresolved. // and preresolved.
EXPECT_CALL( EXPECT_CALL(
*mock_delegate_, *mock_delegate_,
...@@ -356,7 +357,7 @@ TEST_F(PreconnectManagerTest, TestTwoConcurrentMainFrameUrls_MultipleTimes) { ...@@ -356,7 +357,7 @@ TEST_F(PreconnectManagerTest, TestTwoConcurrentMainFrameUrls_MultipleTimes) {
GURL main_frame_url_1("http://google.com"); GURL main_frame_url_1("http://google.com");
net::NetworkIsolationKey network_isolation_key_1 = net::NetworkIsolationKey network_isolation_key_1 =
CreateNetworkIsolationKey(main_frame_url_1); CreateNetworkIsolationKey(main_frame_url_1);
size_t count = PreconnectManager::kMaxInflightPreresolves; size_t count = features::GetMaxInflightPreresolves();
std::vector<PreconnectRequest> requests; std::vector<PreconnectRequest> requests;
for (size_t i = 0; i < count + 1; ++i) { for (size_t i = 0; i < count + 1; ++i) {
std::string url = base::StringPrintf("http://cdn%" PRIuS ".google.com", i); std::string url = base::StringPrintf("http://cdn%" PRIuS ".google.com", i);
...@@ -455,7 +456,7 @@ TEST_F(PreconnectManagerTest, ...@@ -455,7 +456,7 @@ TEST_F(PreconnectManagerTest,
GURL main_frame_url_1("http://google1.com"); GURL main_frame_url_1("http://google1.com");
net::NetworkIsolationKey network_isolation_key_1 = net::NetworkIsolationKey network_isolation_key_1 =
CreateNetworkIsolationKey(main_frame_url_1); CreateNetworkIsolationKey(main_frame_url_1);
size_t count = PreconnectManager::kMaxInflightPreresolves; size_t count = features::GetMaxInflightPreresolves();
std::vector<PreconnectRequest> requests; std::vector<PreconnectRequest> requests;
for (size_t i = 0; i < count - 1; ++i) { for (size_t i = 0; i < count - 1; ++i) {
std::string url = std::string url =
...@@ -648,10 +649,10 @@ TEST_F(PreconnectManagerTest, TestUnqueuedPreresolvesCanceled) { ...@@ -648,10 +649,10 @@ TEST_F(PreconnectManagerTest, TestUnqueuedPreresolvesCanceled) {
GURL main_frame_url("http://google.com"); GURL main_frame_url("http://google.com");
net::NetworkIsolationKey network_isolation_key = net::NetworkIsolationKey network_isolation_key =
CreateNetworkIsolationKey(main_frame_url); CreateNetworkIsolationKey(main_frame_url);
size_t count = PreconnectManager::kMaxInflightPreresolves; size_t count = features::GetMaxInflightPreresolves();
std::vector<PreconnectRequest> requests; std::vector<PreconnectRequest> requests;
for (size_t i = 0; i < count; ++i) { for (size_t i = 0; i < count; ++i) {
// Exactly PreconnectManager::kMaxInflightPreresolves should be preresolved. // Exactly features::GetMaxInflightPreresolves() should be preresolved.
std::string url = base::StringPrintf("http://cdn%" PRIuS ".google.com", i); std::string url = base::StringPrintf("http://cdn%" PRIuS ".google.com", i);
requests.emplace_back(url::Origin::Create(GURL(url)), 1, requests.emplace_back(url::Origin::Create(GURL(url)), 1,
network_isolation_key); network_isolation_key);
...@@ -843,7 +844,7 @@ TEST_F(PreconnectManagerTest, TestDetachedRequestHasHigherPriority) { ...@@ -843,7 +844,7 @@ TEST_F(PreconnectManagerTest, TestDetachedRequestHasHigherPriority) {
GURL main_frame_url("http://google.com"); GURL main_frame_url("http://google.com");
net::NetworkIsolationKey network_isolation_key = net::NetworkIsolationKey network_isolation_key =
CreateNetworkIsolationKey(main_frame_url); CreateNetworkIsolationKey(main_frame_url);
size_t count = PreconnectManager::kMaxInflightPreresolves; size_t count = features::GetMaxInflightPreresolves();
std::vector<PreconnectRequest> requests; std::vector<PreconnectRequest> requests;
// Create enough asynchronous jobs to leave the last one in the queue. // Create enough asynchronous jobs to leave the last one in the queue.
for (size_t i = 0; i < count; ++i) { for (size_t i = 0; i < count; ++i) {
......
...@@ -53,6 +53,10 @@ const base::FeatureParam<PrefetchSubresourceType> ...@@ -53,6 +53,10 @@ const base::FeatureParam<PrefetchSubresourceType>
&kLoadingPredictorPrefetch, "subresource_type", &kLoadingPredictorPrefetch, "subresource_type",
PrefetchSubresourceType::kAll, &kPrefetchSubresourceTypeParamOptions}; PrefetchSubresourceType::kAll, &kPrefetchSubresourceTypeParamOptions};
const base::Feature kLoadingPredictorInflightPredictiveActions{
"kLoadingPredictorInflightPredictiveActions",
base::FEATURE_ENABLED_BY_DEFAULT};
bool ShouldUseLocalPredictions() { bool ShouldUseLocalPredictions() {
return base::FeatureList::IsEnabled(kLoadingPredictorUseLocalPredictions); return base::FeatureList::IsEnabled(kLoadingPredictorUseLocalPredictions);
} }
...@@ -65,4 +69,16 @@ bool ShouldUseOptimizationGuidePredictions() { ...@@ -65,4 +69,16 @@ bool ShouldUseOptimizationGuidePredictions() {
kLoadingPredictorUseOptimizationGuide, "use_predictions", true); kLoadingPredictorUseOptimizationGuide, "use_predictions", true);
} }
size_t GetMaxInflightPreresolves() {
return static_cast<size_t>(base::GetFieldTrialParamByFeatureAsInt(
kLoadingPredictorInflightPredictiveActions, "max_inflight_preresolves",
3));
}
size_t GetMaxInflightPrefetches() {
return static_cast<size_t>(base::GetFieldTrialParamByFeatureAsInt(
kLoadingPredictorInflightPredictiveActions, "max_inflight_prefetches",
3));
}
} // namespace features } // namespace features
...@@ -31,6 +31,8 @@ enum class PrefetchSubresourceType { kAll, kCss, kJsAndCss }; ...@@ -31,6 +31,8 @@ enum class PrefetchSubresourceType { kAll, kCss, kJsAndCss };
extern const base::FeatureParam<PrefetchSubresourceType> extern const base::FeatureParam<PrefetchSubresourceType>
kLoadingPredictorPrefetchSubresourceType; kLoadingPredictorPrefetchSubresourceType;
extern const base::Feature kLoadingPredictorInflightPredictiveActions;
// Returns whether local predictions should be used to make preconnect // Returns whether local predictions should be used to make preconnect
// predictions. // predictions.
bool ShouldUseLocalPredictions(); bool ShouldUseLocalPredictions();
...@@ -43,6 +45,14 @@ bool ShouldUseLocalPredictions(); ...@@ -43,6 +45,14 @@ bool ShouldUseLocalPredictions();
// predictions should actually be used. // predictions should actually be used.
bool ShouldUseOptimizationGuidePredictions(); bool ShouldUseOptimizationGuidePredictions();
// Returns the maximum number of preresolves that can be inflight at any given
// time.
size_t GetMaxInflightPreresolves();
// Returns the maximum number of prefetches that can be inflight at any given
// time.
size_t GetMaxInflightPrefetches();
} // namespace features } // namespace features
#endif // CHROME_BROWSER_PREDICTORS_PREDICTORS_FEATURES_H_ #endif // CHROME_BROWSER_PREDICTORS_PREDICTORS_FEATURES_H_
...@@ -285,8 +285,10 @@ void PrefetchManager::OnPrefetchFinished( ...@@ -285,8 +285,10 @@ void PrefetchManager::OnPrefetchFinished(
void PrefetchManager::TryToLaunchPrefetchJobs() { void PrefetchManager::TryToLaunchPrefetchJobs() {
DCHECK_CURRENTLY_ON(content::BrowserThread::UI); DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
if (queued_jobs_.empty() || inflight_jobs_count_ >= kMaxInflightJobs) if (queued_jobs_.empty() ||
inflight_jobs_count_ >= features::GetMaxInflightPrefetches()) {
return; return;
}
// TODO(falken): Is it ok to assume the default partition? Try to plumb the // TODO(falken): Is it ok to assume the default partition? Try to plumb the
// partition here, e.g., from WebContentsObserver. And make a similar change // partition here, e.g., from WebContentsObserver. And make a similar change
...@@ -296,7 +298,8 @@ void PrefetchManager::TryToLaunchPrefetchJobs() { ...@@ -296,7 +298,8 @@ void PrefetchManager::TryToLaunchPrefetchJobs() {
scoped_refptr<network::SharedURLLoaderFactory> factory = scoped_refptr<network::SharedURLLoaderFactory> factory =
storage_partition->GetURLLoaderFactoryForBrowserProcess(); storage_partition->GetURLLoaderFactoryForBrowserProcess();
while (!queued_jobs_.empty() && inflight_jobs_count_ < kMaxInflightJobs) { while (!queued_jobs_.empty() &&
inflight_jobs_count_ < features::GetMaxInflightPrefetches()) {
std::unique_ptr<PrefetchJob> job = std::move(queued_jobs_.front()); std::unique_ptr<PrefetchJob> job = std::move(queued_jobs_.front());
queued_jobs_.pop_front(); queued_jobs_.pop_front();
base::WeakPtr<PrefetchInfo> info = job->info; base::WeakPtr<PrefetchInfo> info = job->info;
......
...@@ -74,8 +74,6 @@ class PrefetchManager { ...@@ -74,8 +74,6 @@ class PrefetchManager {
virtual void PrefetchFinished(std::unique_ptr<PrefetchStats> stats) = 0; virtual void PrefetchFinished(std::unique_ptr<PrefetchStats> stats) = 0;
}; };
static const size_t kMaxInflightJobs = 3;
PrefetchManager(base::WeakPtr<Delegate> delegate, Profile* profile); PrefetchManager(base::WeakPtr<Delegate> delegate, Profile* profile);
~PrefetchManager(); ~PrefetchManager();
......
...@@ -182,7 +182,7 @@ TEST_F(PrefetchManagerTest, OneMainFrameUrlMultiplePrefetch) { ...@@ -182,7 +182,7 @@ TEST_F(PrefetchManagerTest, OneMainFrameUrlMultiplePrefetch) {
// The ControllableHttpResponses must be made before the test server // The ControllableHttpResponses must be made before the test server
// is started. // is started.
for (size_t i = 0; i < PrefetchManager::kMaxInflightJobs + 1; i++) { for (size_t i = 0; i < features::GetMaxInflightPrefetches() + 1; i++) {
std::string path = base::StringPrintf("/script%" PRIuS ".js", i); std::string path = base::StringPrintf("/script%" PRIuS ".js", i);
paths.push_back(path); paths.push_back(path);
responses.push_back( responses.push_back(
...@@ -251,7 +251,7 @@ TEST_F(PrefetchManagerTest, MultipleMainFrameUrlMultiplePrefetch) { ...@@ -251,7 +251,7 @@ TEST_F(PrefetchManagerTest, MultipleMainFrameUrlMultiplePrefetch) {
GURL main_frame_url2("https://def.invalid"); GURL main_frame_url2("https://def.invalid");
// Set up prefetches one more than the inflight limit. // Set up prefetches one more than the inflight limit.
size_t count = PrefetchManager::kMaxInflightJobs; size_t count = features::GetMaxInflightPrefetches();
// The ControllableHttpResponses must be made before the test server // The ControllableHttpResponses must be made before the test server
// is started. // is started.
...@@ -335,7 +335,7 @@ TEST_F(PrefetchManagerTest, Stop) { ...@@ -335,7 +335,7 @@ TEST_F(PrefetchManagerTest, Stop) {
net::test_server::EmbeddedTestServer test_server; net::test_server::EmbeddedTestServer test_server;
// Set up prefetches (limit + 1 for URL1, and 1 for URL2) // Set up prefetches (limit + 1 for URL1, and 1 for URL2)
size_t limit = PrefetchManager::kMaxInflightJobs; size_t limit = features::GetMaxInflightPrefetches();
GURL main_frame_url("https://abc.invalid"); GURL main_frame_url("https://abc.invalid");
std::vector<std::string> paths; std::vector<std::string> paths;
...@@ -423,7 +423,7 @@ TEST_F(PrefetchManagerTest, StopAndStart) { ...@@ -423,7 +423,7 @@ TEST_F(PrefetchManagerTest, StopAndStart) {
net::test_server::EmbeddedTestServer test_server; net::test_server::EmbeddedTestServer test_server;
// Set up prefetches (limit + 1). // Set up prefetches (limit + 1).
size_t limit = PrefetchManager::kMaxInflightJobs; size_t limit = features::GetMaxInflightPrefetches();
GURL main_frame_url("https://abc.invalid"); GURL main_frame_url("https://abc.invalid");
std::vector<std::string> paths; std::vector<std::string> paths;
......
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