Commit 7aa44df4 authored by tbansal's avatar tbansal Committed by Commit bot

NQE: Use cached estimates

Read cached estimates from the network quality estimator (NQE) prefs.
The read estimates are notified to observers if the network ID of the
cached estimates matches the ID of the current network. Both writing
and readings of prefs is guarded behind field trial.

CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_cronet_tester

BUG=490870

Review-Url: https://codereview.chromium.org/2487883002
Cr-Commit-Position: refs/heads/master@{#439526}
parent 62379b0d
......@@ -39,10 +39,18 @@ std::string GetStringValueForVariationParamWithDefaultValue(
: it->second;
}
// Returns true if persistent caching has been enabled in the field trial.
bool persistent_caching_enabled() {
// Returns true if writing to the persistent cache has been enabled via field
// trial.
bool persistent_cache_writing_enabled() {
return GetStringValueForVariationParamWithDefaultValue(
"persistent_caching_enabled", "false") == "true";
"persistent_cache_writing_enabled", "false") == "true";
}
// Returns true if reading from the persistent cache has been enabled via field
// trial.
bool persistent_cache_reading_enabled() {
return GetStringValueForVariationParamWithDefaultValue(
"persistent_cache_reading_enabled", "false") == "true";
}
// PrefDelegateImpl writes the provided dictionary value to the network quality
......@@ -59,17 +67,19 @@ class PrefDelegateImpl
void SetDictionaryValue(const base::DictionaryValue& value) override {
DCHECK(thread_checker_.CalledOnValidThread());
if (!persistent_caching_enabled())
if (!persistent_cache_writing_enabled())
return;
pref_service_->Set(path_, value);
UMA_HISTOGRAM_COUNTS_1000("NQE.Prefs.WriteCount", 1);
}
const base::DictionaryValue& GetDictionaryValue() override {
std::unique_ptr<base::DictionaryValue> GetDictionaryValue() override {
DCHECK(thread_checker_.CalledOnValidThread());
if (!persistent_cache_reading_enabled())
return base::WrapUnique(new base::DictionaryValue());
UMA_HISTOGRAM_COUNTS_1000("NQE.Prefs.ReadCount", 1);
return *pref_service_->GetDictionary(path_);
return pref_service_->GetDictionary(path_)->CreateDeepCopy();
}
private:
......
......@@ -57,19 +57,21 @@ class UINetworkQualityEstimatorServiceBrowserTest
public:
UINetworkQualityEstimatorServiceBrowserTest() {}
// Enables persistent caching if |persistent_caching_enabled| is true.
// Verifies that the network quality prefs are written correctly, and that
// they are written only if the persistent caching was enabled.
void VerifyWritingReadingPrefs(bool persistent_caching_enabled) {
if (persistent_caching_enabled) {
std::map<std::string, std::string> variation_params;
variation_params["persistent_caching_enabled"] = "true";
variations::AssociateVariationParams("NetworkQualityEstimator", "Enabled",
variation_params);
base::FieldTrialList::CreateFieldTrial("NetworkQualityEstimator",
"Enabled");
}
// Verifies that the network quality prefs are written amd read correctly.
void VerifyWritingReadingPrefs(bool persistent_cache_writing_enabled,
bool persistent_cache_reading_enabled) {
std::map<std::string, std::string> variation_params;
if (persistent_cache_writing_enabled)
variation_params["persistent_cache_writing_enabled"] = "true";
if (persistent_cache_reading_enabled)
variation_params["persistent_cache_reading_enabled"] = "true";
variations::AssociateVariationParams("NetworkQualityEstimator", "Enabled",
variation_params);
base::FieldTrialList::CreateFieldTrial("NetworkQualityEstimator",
"Enabled");
// Verifies that NQE notifying EffectiveConnectionTypeObservers causes the
// UINetworkQualityEstimatorService to receive an updated
......@@ -101,7 +103,7 @@ class UINetworkQualityEstimatorServiceBrowserTest
// Prefs are written only if the network id was available, and persistent
// caching was enabled.
EXPECT_NE(network_id_available && persistent_caching_enabled,
EXPECT_NE(network_id_available && persistent_cache_writing_enabled,
histogram_tester.GetAllSamples("NQE.Prefs.WriteCount").empty());
histogram_tester.ExpectTotalCount("NQE.Prefs.ReadCount", 0);
......@@ -119,7 +121,7 @@ class UINetworkQualityEstimatorServiceBrowserTest
// Prefs are written only if the network id was available, and persistent
// caching was enabled.
EXPECT_NE(network_id_available && persistent_caching_enabled,
EXPECT_NE(network_id_available && persistent_cache_writing_enabled,
histogram_tester.GetAllSamples("NQE.Prefs.WriteCount").empty());
histogram_tester.ExpectTotalCount("NQE.Prefs.ReadCount", 0);
......@@ -131,9 +133,13 @@ class UINetworkQualityEstimatorServiceBrowserTest
std::map<net::nqe::internal::NetworkID,
net::nqe::internal::CachedNetworkQuality>
read_prefs = nqe_service->ForceReadPrefsForTesting();
EXPECT_EQ(network_id_available && persistent_caching_enabled ? 1u : 0u,
EXPECT_EQ(network_id_available && persistent_cache_writing_enabled &&
persistent_cache_reading_enabled
? 1u
: 0u,
read_prefs.size());
if (network_id_available && persistent_caching_enabled) {
if (network_id_available && persistent_cache_writing_enabled &&
persistent_cache_reading_enabled) {
// Verify that the cached network quality was written correctly.
EXPECT_EQ(net::EFFECTIVE_CONNECTION_TYPE_SLOW_2G,
read_prefs.begin()->second.effective_connection_type());
......@@ -214,12 +220,18 @@ IN_PROC_BROWSER_TEST_F(UINetworkQualityEstimatorServiceBrowserTest,
// via field trial.
IN_PROC_BROWSER_TEST_F(UINetworkQualityEstimatorServiceBrowserTest,
WritingToPrefsDisabled) {
VerifyWritingReadingPrefs(false);
VerifyWritingReadingPrefs(false, true);
}
// Verify that prefs are not read when reading of the prefs is not enabled
// via field trial.
IN_PROC_BROWSER_TEST_F(UINetworkQualityEstimatorServiceBrowserTest,
ReadingFromPrefsDisabled) {
VerifyWritingReadingPrefs(true, false);
}
// Verify that prefs are writen when writing of the prefs is enabled via field
// trial.
// Verify that prefs are writen and read correctly.
IN_PROC_BROWSER_TEST_F(UINetworkQualityEstimatorServiceBrowserTest,
WritingToPrefsEnabled) {
VerifyWritingReadingPrefs(true);
WritingReadingToPrefsEnabled) {
VerifyWritingReadingPrefs(true, true);
}
......@@ -8,6 +8,7 @@
#include <utility>
#include "base/bind.h"
#include "base/rand_util.h"
#include "base/sequenced_task_runner.h"
#include "base/threading/thread_checker.h"
#include "base/threading/thread_task_runner_handle.h"
......@@ -56,7 +57,7 @@ NetworkQualitiesPrefsManager::NetworkQualitiesPrefsManager(
std::unique_ptr<PrefDelegate> pref_delegate)
: pref_delegate_(std::move(pref_delegate)),
pref_task_runner_(base::ThreadTaskRunnerHandle::Get()),
prefs_(pref_delegate_->GetDictionaryValue().CreateDeepCopy()),
prefs_(pref_delegate_->GetDictionaryValue()),
network_quality_estimator_(nullptr),
read_prefs_startup_(ConvertDictionaryValueToMap(prefs_.get())),
pref_weak_ptr_factory_(this) {
......@@ -132,16 +133,26 @@ void NetworkQualitiesPrefsManager::OnChangeInCachedNetworkQualityOnPrefThread(
cached_network_quality.effective_connection_type()));
if (prefs_->size() > kMaxCacheSize) {
// Delete one value that has key different than |network_id|.
// Delete one randomly selected value that has a key that is different from
// |network_id|.
DCHECK_EQ(kMaxCacheSize + 1, prefs_->size());
// Generate a random number between 0 and |kMaxCacheSize| -1 (both
// inclusive) since the number of network IDs in |prefs_| other than
// |network_id| is |kMaxCacheSize|.
int index_to_delete = base::RandInt(0, kMaxCacheSize - 1);
for (base::DictionaryValue::Iterator it(*prefs_); !it.IsAtEnd();
it.Advance()) {
const nqe::internal::NetworkID it_network_id =
nqe::internal::NetworkID::FromString(it.key());
if (it_network_id != network_id) {
// Delete the kth element in the dictionary, not including the element
// that represents the current network. k == |index_to_delete|.
if (nqe::internal::NetworkID::FromString(it.key()) == network_id)
continue;
if (index_to_delete == 0) {
prefs_->RemovePath(it.key(), nullptr);
break;
}
index_to_delete--;
}
}
DCHECK_GE(kMaxCacheSize, prefs_->size());
......@@ -153,7 +164,7 @@ void NetworkQualitiesPrefsManager::OnChangeInCachedNetworkQualityOnPrefThread(
ParsedPrefs NetworkQualitiesPrefsManager::ForceReadPrefsForTesting() const {
DCHECK(pref_task_runner_->RunsTasksOnCurrentThread());
std::unique_ptr<base::DictionaryValue> value(
pref_delegate_->GetDictionaryValue().CreateDeepCopy());
pref_delegate_->GetDictionaryValue());
return ConvertDictionaryValueToMap(value.get());
}
......
......@@ -54,8 +54,8 @@ class NET_EXPORT NetworkQualitiesPrefsManager
// Sets the persistent pref to the given value.
virtual void SetDictionaryValue(const base::DictionaryValue& value) = 0;
// Returns the peristent prefs.
virtual const base::DictionaryValue& GetDictionaryValue() = 0;
// Returns a copy of the persistent prefs.
virtual std::unique_ptr<base::DictionaryValue> GetDictionaryValue() = 0;
};
// Creates an instance of the NetworkQualitiesPrefsManager. Ownership of
......
......@@ -9,6 +9,7 @@
#include "base/macros.h"
#include "base/run_loop.h"
#include "base/test/histogram_tester.h"
#include "base/threading/thread_checker.h"
#include "base/values.h"
#include "net/base/network_change_notifier.h"
......@@ -40,11 +41,11 @@ class TestPrefDelegate : public NetworkQualitiesPrefsManager::PrefDelegate {
ASSERT_EQ(value.size(), value_->size());
}
const base::DictionaryValue& GetDictionaryValue() override {
std::unique_ptr<base::DictionaryValue> GetDictionaryValue() override {
DCHECK(thread_checker_.CalledOnValidThread());
read_count_++;
return *(value_.get());
return value_->CreateDeepCopy();
}
size_t write_count() const {
......@@ -225,6 +226,11 @@ TEST(NetworkQualitiesPrefManager, WriteAndReadWithMultipleNetworkIDs) {
NOTREACHED();
}
}
base::HistogramTester histogram_tester;
estimator.OnPrefsRead(read_prefs);
histogram_tester.ExpectUniqueSample("NQE.Prefs.ReadSize", 3, 1);
manager.ShutdownOnPrefThread();
}
......
......@@ -788,6 +788,8 @@ void NetworkQualityEstimator::OnConnectionTypeChanged(
effective_connection_type_ = EFFECTIVE_CONNECTION_TYPE_UNKNOWN;
effective_connection_type_at_last_main_frame_ =
EFFECTIVE_CONNECTION_TYPE_UNKNOWN;
rtt_observations_size_at_last_ect_computation_ = 0;
throughput_observations_size_at_last_ect_computation_ = 0;
// Update the local state as part of preparation for the new connection.
current_network_id_ = GetCurrentNetworkID();
......@@ -1390,6 +1392,15 @@ bool NetworkQualityEstimator::ReadCachedNetworkQualityEstimate() {
rtt_observations_.AddObservation(rtt_observation);
NotifyObserversOfRTT(rtt_observation);
}
if (cached_network_quality.network_quality().transport_rtt() !=
nqe::internal::InvalidRTT()) {
RttObservation rtt_observation(
cached_network_quality.network_quality().transport_rtt(), now,
NETWORK_QUALITY_OBSERVATION_SOURCE_TRANSPORT_CACHED_ESTIMATE);
rtt_observations_.AddObservation(rtt_observation);
NotifyObserversOfRTT(rtt_observation);
}
return true;
}
......@@ -1600,9 +1611,74 @@ void NetworkQualityEstimator::OnPrefsRead(
const std::map<nqe::internal::NetworkID,
nqe::internal::CachedNetworkQuality> read_prefs) {
DCHECK(thread_checker_.CalledOnValidThread());
UMA_HISTOGRAM_COUNTS("NQE.Prefs.ReadSize", read_prefs.size());
// TODO(tbansal): crbug.com/490870. Incorporate the network quality into the
// current estimates.
for (auto& it : read_prefs) {
EffectiveConnectionType effective_connection_type =
it.second.effective_connection_type();
if (effective_connection_type == EFFECTIVE_CONNECTION_TYPE_UNKNOWN ||
effective_connection_type == EFFECTIVE_CONNECTION_TYPE_OFFLINE) {
continue;
}
// RTT and throughput values are not set in the prefs.
DCHECK_EQ(nqe::internal::InvalidRTT(),
it.second.network_quality().http_rtt());
DCHECK_EQ(nqe::internal::InvalidRTT(),
it.second.network_quality().transport_rtt());
DCHECK_EQ(nqe::internal::kInvalidThroughput,
it.second.network_quality().downstream_throughput_kbps());
nqe::internal::NetworkQuality network_quality(
typical_network_quality_[effective_connection_type].http_rtt(),
typical_network_quality_[effective_connection_type].transport_rtt(),
typical_network_quality_[effective_connection_type]
.downstream_throughput_kbps());
nqe::internal::CachedNetworkQuality cached_network_quality(
base::TimeTicks::Now(), network_quality, effective_connection_type);
network_quality_store_->Add(it.first, cached_network_quality);
MaybeUpdateNetworkQualityFromCache(it.first, cached_network_quality);
}
}
void NetworkQualityEstimator::MaybeUpdateNetworkQualityFromCache(
const nqe::internal::NetworkID& network_id,
const nqe::internal::CachedNetworkQuality& cached_network_quality) {
DCHECK(thread_checker_.CalledOnValidThread());
if (network_id != current_network_id_)
return;
// Since the cached network quality is for the current network, add it to
// the current observations.
RttObservation http_rtt_observation(
cached_network_quality.network_quality().http_rtt(),
base::TimeTicks::Now(),
NETWORK_QUALITY_OBSERVATION_SOURCE_HTTP_CACHED_ESTIMATE);
rtt_observations_.AddObservation(http_rtt_observation);
NotifyObserversOfRTT(http_rtt_observation);
RttObservation transport_rtt_observation(
cached_network_quality.network_quality().transport_rtt(),
base::TimeTicks::Now(),
NETWORK_QUALITY_OBSERVATION_SOURCE_TRANSPORT_CACHED_ESTIMATE);
rtt_observations_.AddObservation(transport_rtt_observation);
NotifyObserversOfRTT(transport_rtt_observation);
// TODO(tbansal): crbug.com/673977: Remove this check.
if (cached_network_quality.network_quality().downstream_throughput_kbps() !=
nqe::internal::kInvalidThroughput) {
ThroughputObservation throughput_observation(
cached_network_quality.network_quality().downstream_throughput_kbps(),
base::TimeTicks::Now(),
NETWORK_QUALITY_OBSERVATION_SOURCE_HTTP_CACHED_ESTIMATE);
downstream_throughput_kbps_observations_.AddObservation(
throughput_observation);
NotifyObserversOfThroughput(throughput_observation);
}
ComputeEffectiveConnectionType();
}
} // namespace net
......@@ -489,8 +489,12 @@ class NET_EXPORT NetworkQualityEstimator
// Virtualized for testing.
virtual nqe::internal::NetworkID GetCurrentNetworkID() const;
// Notifies RTT observers of |observation|. May also trigger recomputation
// of effective connection type.
void NotifyObserversOfRTT(const RttObservation& observation);
// Notifies throughput observers of |observation|. May also trigger
// recomputation of effective connection type.
void NotifyObserversOfThroughput(const ThroughputObservation& observation);
// Returns true only if the |request| can be used for RTT estimation.
......@@ -579,6 +583,13 @@ class NET_EXPORT NetworkQualityEstimator
// if there is a change in its value.
void ComputeEffectiveConnectionType();
// May update the network quality of the current network if |network_id|
// matches the ID of the current network. |cached_network_quality| is the
// cached network quality of the network with id |network_id|.
void MaybeUpdateNetworkQualityFromCache(
const nqe::internal::NetworkID& network_id,
const nqe::internal::CachedNetworkQuality& cached_network_quality);
// Determines if the requests to local host can be used in estimating the
// network quality. Set to true only for tests.
bool use_localhost_requests_;
......
......@@ -118,6 +118,17 @@ class TestRTTObserver : public NetworkQualityEstimator::RTTObserver {
observations_.push_back(Observation(rtt_ms, timestamp, source));
}
// Returns the last received RTT observation that has source set to |source|.
base::TimeDelta last_rtt(NetworkQualityObservationSource source) {
for (std::vector<Observation>::reverse_iterator i = observations_.rbegin();
i != observations_.rend(); ++i) {
Observation observation = *i;
if (observation.source == source)
return base::TimeDelta::FromMilliseconds(observation.rtt_ms);
}
return nqe::internal::InvalidRTT();
}
private:
std::vector<Observation> observations_;
};
......@@ -343,6 +354,10 @@ TEST(NetworkQualityEstimatorTest, Caching) {
TestThroughputObserver throughput_observer;
estimator.AddThroughputObserver(&throughput_observer);
// |observer| should be notified as soon as it is added.
base::RunLoop().RunUntilIdle();
EXPECT_EQ(1U, observer.effective_connection_types().size());
estimator.SimulateNetworkChange(
NetworkChangeNotifier::ConnectionType::CONNECTION_2G, "test");
histogram_tester.ExpectBucketCount("NQE.CachedNetworkQualityAvailable", true,
......@@ -353,7 +368,9 @@ TEST(NetworkQualityEstimatorTest, Caching) {
// Verify that the cached network quality was read, and observers were
// notified. |observer| must be notified once right after it was added, and
// once again after the cached network quality was read.
EXPECT_EQ(2U, observer.effective_connection_types().size());
EXPECT_LE(2U, observer.effective_connection_types().size());
EXPECT_EQ(estimator.GetEffectiveConnectionType(),
observer.effective_connection_types().back());
EXPECT_EQ(1U, rtt_observer.observations().size());
EXPECT_EQ(1U, throughput_observer.observations().size());
}
......@@ -2577,4 +2594,114 @@ TEST(NetworkQualityEstimatorTest, TypicalNetworkQualities) {
}
}
// Verify that the cached network qualities from the prefs are correctly used.
TEST(NetworkQualityEstimatorTest, OnPrefsRead) {
base::HistogramTester histogram_tester;
// Construct the read prefs.
std::map<nqe::internal::NetworkID, nqe::internal::CachedNetworkQuality>
read_prefs;
read_prefs[nqe::internal::NetworkID(NetworkChangeNotifier::CONNECTION_WIFI,
"test_ect_2g")] =
nqe::internal::CachedNetworkQuality(EFFECTIVE_CONNECTION_TYPE_2G);
read_prefs[nqe::internal::NetworkID(NetworkChangeNotifier::CONNECTION_WIFI,
"test_ect_slow_2g")] =
nqe::internal::CachedNetworkQuality(EFFECTIVE_CONNECTION_TYPE_SLOW_2G);
read_prefs[nqe::internal::NetworkID(NetworkChangeNotifier::CONNECTION_4G,
"test_ect_4g")] =
nqe::internal::CachedNetworkQuality(EFFECTIVE_CONNECTION_TYPE_4G);
std::map<std::string, std::string> variation_params;
variation_params["effective_connection_type_algorithm"] =
"TransportRTTOrDownstreamThroughput";
// Disable default platform values so that the effect of cached estimates
// at the time of startup can be studied in isolation.
TestNetworkQualityEstimator estimator(
std::unique_ptr<net::ExternalEstimateProvider>(), variation_params, true,
true, false /* use_default_platform_values */);
// Add observers.
TestRTTObserver rtt_observer;
TestThroughputObserver throughput_observer;
TestRTTAndThroughputEstimatesObserver rtt_throughput_observer;
TestEffectiveConnectionTypeObserver effective_connection_type_observer;
estimator.AddRTTObserver(&rtt_observer);
estimator.AddThroughputObserver(&throughput_observer);
estimator.AddRTTAndThroughputEstimatesObserver(&rtt_throughput_observer);
estimator.AddEffectiveConnectionTypeObserver(
&effective_connection_type_observer);
std::string network_name("test_ect_2g");
estimator.SimulateNetworkChange(
NetworkChangeNotifier::ConnectionType::CONNECTION_WIFI, network_name);
EXPECT_EQ(0u, rtt_observer.observations().size());
EXPECT_EQ(0u, throughput_observer.observations().size());
EXPECT_LE(0, rtt_throughput_observer.notifications_received());
// Simulate reading of prefs.
estimator.OnPrefsRead(read_prefs);
histogram_tester.ExpectUniqueSample("NQE.Prefs.ReadSize", read_prefs.size(),
1);
// Taken from network_quality_estimator_params.cc.
EXPECT_EQ(base::TimeDelta::FromMilliseconds(1800),
rtt_observer.last_rtt(
NETWORK_QUALITY_OBSERVATION_SOURCE_HTTP_CACHED_ESTIMATE));
EXPECT_EQ(base::TimeDelta::FromMilliseconds(1500),
rtt_observer.last_rtt(
NETWORK_QUALITY_OBSERVATION_SOURCE_TRANSPORT_CACHED_ESTIMATE));
EXPECT_EQ(0u, throughput_observer.observations().size());
EXPECT_EQ(base::TimeDelta::FromMilliseconds(1800),
rtt_throughput_observer.http_rtt());
EXPECT_EQ(base::TimeDelta::FromMilliseconds(1500),
rtt_throughput_observer.transport_rtt());
EXPECT_EQ(nqe::internal::kInvalidThroughput,
rtt_throughput_observer.downstream_throughput_kbps());
EXPECT_LE(
1u,
effective_connection_type_observer.effective_connection_types().size());
// Compare the ECT stored in prefs with the observer's last entry.
EXPECT_EQ(
read_prefs[nqe::internal::NetworkID(
NetworkChangeNotifier::CONNECTION_WIFI, network_name)]
.effective_connection_type(),
effective_connection_type_observer.effective_connection_types().back());
// Change to a different connection type.
network_name = "test_ect_slow_2g";
estimator.SimulateNetworkChange(
NetworkChangeNotifier::ConnectionType::CONNECTION_WIFI, network_name);
EXPECT_EQ(base::TimeDelta::FromMilliseconds(3600),
rtt_observer.last_rtt(
NETWORK_QUALITY_OBSERVATION_SOURCE_HTTP_CACHED_ESTIMATE));
EXPECT_EQ(base::TimeDelta::FromMilliseconds(3000),
rtt_observer.last_rtt(
NETWORK_QUALITY_OBSERVATION_SOURCE_TRANSPORT_CACHED_ESTIMATE));
EXPECT_EQ(0u, throughput_observer.observations().size());
EXPECT_EQ(base::TimeDelta::FromMilliseconds(3600),
rtt_throughput_observer.http_rtt());
EXPECT_EQ(base::TimeDelta::FromMilliseconds(3000),
rtt_throughput_observer.transport_rtt());
EXPECT_EQ(nqe::internal::kInvalidThroughput,
rtt_throughput_observer.downstream_throughput_kbps());
EXPECT_LE(
2u,
effective_connection_type_observer.effective_connection_types().size());
// Compare with the last entry.
EXPECT_EQ(
read_prefs[nqe::internal::NetworkID(
NetworkChangeNotifier::CONNECTION_WIFI, network_name)]
.effective_connection_type(),
effective_connection_type_observer.effective_connection_types().back());
// Cleanup.
estimator.RemoveRTTObserver(&rtt_observer);
estimator.RemoveThroughputObserver(&throughput_observer);
estimator.RemoveRTTAndThroughputEstimatesObserver(&rtt_throughput_observer);
estimator.RemoveEffectiveConnectionTypeObserver(
&effective_connection_type_observer);
}
} // namespace net
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