From d1853b975434d1b31a2143fa3a25af48fafda7a0 Mon Sep 17 00:00:00 2001 From: Zhongyi Shi Date: Tue, 25 Jul 2017 20:14:23 +0000 Subject: [PATCH] Default packet reader yield time to 2ms as stable experimental results shows performance improvement. Bug: Change-Id: I7f40d51e15b19dd5ae6d1b56b9a0167aa7f80b11 Reviewed-on: https://chromium-review.googlesource.com/582327 Commit-Queue: Zhongyi Shi Reviewed-by: Ryan Hamilton Cr-Commit-Position: refs/heads/master@{#489413} --- .../browser/network_session_configurator.cc | 18 ------------------ .../network_session_configurator_unittest.cc | 14 -------------- net/http/http_network_session.cc | 7 ------- net/http/http_network_session.h | 3 --- .../chromium/quic_chromium_packet_reader.h | 2 +- net/quic/chromium/quic_stream_factory.cc | 3 +-- net/quic/chromium/quic_stream_factory.h | 1 - net/quic/chromium/quic_stream_factory_test.cc | 4 ---- 8 files changed, 2 insertions(+), 50 deletions(-) diff --git a/components/network_session_configurator/browser/network_session_configurator.cc b/components/network_session_configurator/browser/network_session_configurator.cc index 2c34461e6b1537..ccf09e2e1fa056 100644 --- a/components/network_session_configurator/browser/network_session_configurator.cc +++ b/components/network_session_configurator/browser/network_session_configurator.cc @@ -180,18 +180,6 @@ int GetQuicReducedPingTimeoutSeconds( return 0; } -int GetQuicPacketReaderYieldAfterDurationMilliseconds( - const VariationParameters& quic_trial_params) { - int value; - if (base::StringToInt( - GetVariationParam(quic_trial_params, - "packet_reader_yield_after_duration_milliseconds"), - &value)) { - return value; - } - return 0; -} - bool ShouldQuicRaceCertVerification( const VariationParameters& quic_trial_params) { return base::LowerCaseEqualsASCII( @@ -282,12 +270,6 @@ void ConfigureQuicParams(base::StringPiece quic_trial_group, reduced_ping_timeout_seconds < net::kPingTimeoutSecs) { params->quic_reduced_ping_timeout_seconds = reduced_ping_timeout_seconds; } - int packet_reader_yield_after_duration_milliseconds = - GetQuicPacketReaderYieldAfterDurationMilliseconds(quic_trial_params); - if (packet_reader_yield_after_duration_milliseconds != 0) { - params->quic_packet_reader_yield_after_duration_milliseconds = - packet_reader_yield_after_duration_milliseconds; - } params->quic_race_cert_verification = ShouldQuicRaceCertVerification(quic_trial_params); params->quic_estimate_initial_rtt = diff --git a/components/network_session_configurator/browser/network_session_configurator_unittest.cc b/components/network_session_configurator/browser/network_session_configurator_unittest.cc index b04421dd94756a..8fbc23e0dfabcd 100644 --- a/components/network_session_configurator/browser/network_session_configurator_unittest.cc +++ b/components/network_session_configurator/browser/network_session_configurator_unittest.cc @@ -87,8 +87,6 @@ TEST_F(NetworkSessionConfiguratorTest, EnableQuicFromFieldTrialGroup) { EXPECT_EQ(net::kIdleConnectionTimeoutSeconds, params_.quic_idle_connection_timeout_seconds); EXPECT_EQ(net::kPingTimeoutSecs, params_.quic_reduced_ping_timeout_seconds); - EXPECT_EQ(net::kQuicYieldAfterDurationMilliseconds, - params_.quic_packet_reader_yield_after_duration_milliseconds); EXPECT_FALSE(params_.quic_race_cert_verification); EXPECT_FALSE(params_.quic_estimate_initial_rtt); EXPECT_FALSE(params_.quic_migrate_sessions_on_network_change); @@ -199,18 +197,6 @@ TEST_F(NetworkSessionConfiguratorTest, EXPECT_EQ(10, params_.quic_reduced_ping_timeout_seconds); } -TEST_F(NetworkSessionConfiguratorTest, - QuicPacketReaderYieldAfterDurationMillisecondsFieldTrialParams) { - std::map field_trial_params; - field_trial_params["packet_reader_yield_after_duration_milliseconds"] = "10"; - variations::AssociateVariationParams("QUIC", "Enabled", field_trial_params); - base::FieldTrialList::CreateFieldTrial("QUIC", "Enabled"); - - ParseFieldTrials(); - - EXPECT_EQ(10, params_.quic_packet_reader_yield_after_duration_milliseconds); -} - TEST_F(NetworkSessionConfiguratorTest, QuicRaceCertVerification) { std::map field_trial_params; field_trial_params["race_cert_verification"] = "true"; diff --git a/net/http/http_network_session.cc b/net/http/http_network_session.cc index fe434493ef17ac..d18447d067c54b 100644 --- a/net/http/http_network_session.cc +++ b/net/http/http_network_session.cc @@ -121,8 +121,6 @@ HttpNetworkSession::Params::Params() quic_close_sessions_on_ip_change(false), quic_idle_connection_timeout_seconds(kIdleConnectionTimeoutSeconds), quic_reduced_ping_timeout_seconds(kPingTimeoutSecs), - quic_packet_reader_yield_after_duration_milliseconds( - kQuicYieldAfterDurationMilliseconds), quic_migrate_sessions_on_network_change(false), quic_migrate_sessions_early(false), quic_allow_server_migration(false), @@ -198,7 +196,6 @@ HttpNetworkSession::HttpNetworkSession(const Params& params, params.mark_quic_broken_when_network_blackholes, params.quic_idle_connection_timeout_seconds, params.quic_reduced_ping_timeout_seconds, - params.quic_packet_reader_yield_after_duration_milliseconds, params.quic_migrate_sessions_on_network_change, params.quic_migrate_sessions_early, params.quic_allow_server_migration, @@ -335,10 +332,6 @@ std::unique_ptr HttpNetworkSession::QuicInfoToValue() const { params_.quic_idle_connection_timeout_seconds); dict->SetInteger("reduced_ping_timeout_seconds", params_.quic_reduced_ping_timeout_seconds); - dict->SetInteger( - "packet_reader_yield_after_duration_milliseconds", - params_.quic_packet_reader_yield_after_duration_milliseconds); - dict->SetBoolean("mark_quic_broken_when_network_blackholes", params_.mark_quic_broken_when_network_blackholes); dict->SetBoolean("retry_without_alt_svc_on_quic_errors", diff --git a/net/http/http_network_session.h b/net/http/http_network_session.h index 013e0d1ecb0718..5cd08da91d0415 100644 --- a/net/http/http_network_session.h +++ b/net/http/http_network_session.h @@ -141,9 +141,6 @@ class NET_EXPORT HttpNetworkSession : public base::MemoryCoordinatorClient { // Specifies the reduced ping timeout subsequent connections should use when // a connection was timed out with open streams. int quic_reduced_ping_timeout_seconds; - // Specifies the maximum time duration that QUIC packet reader can perform - // consecutive packets reading. - int quic_packet_reader_yield_after_duration_milliseconds; // If true, active QUIC sessions may be migrated onto a new network when // the platform indicates that the default network is changing. bool quic_migrate_sessions_on_network_change; diff --git a/net/quic/chromium/quic_chromium_packet_reader.h b/net/quic/chromium/quic_chromium_packet_reader.h index 3918fc073d448c..113eeef3aed1e7 100644 --- a/net/quic/chromium/quic_chromium_packet_reader.h +++ b/net/quic/chromium/quic_chromium_packet_reader.h @@ -23,7 +23,7 @@ class QuicClock; // milliseconds have passed, QuicChromiumPacketReader::StartReading() yields by // doing a QuicChromiumPacketReader::PostTask(). const int kQuicYieldAfterPacketsRead = 32; -const int kQuicYieldAfterDurationMilliseconds = 20; +const int kQuicYieldAfterDurationMilliseconds = 2; class NET_EXPORT_PRIVATE QuicChromiumPacketReader { public: diff --git a/net/quic/chromium/quic_stream_factory.cc b/net/quic/chromium/quic_stream_factory.cc index e65830d50ec67c..b36c6e8677d99b 100644 --- a/net/quic/chromium/quic_stream_factory.cc +++ b/net/quic/chromium/quic_stream_factory.cc @@ -664,7 +664,6 @@ QuicStreamFactory::QuicStreamFactory( bool mark_quic_broken_when_network_blackholes, int idle_connection_timeout_seconds, int reduced_ping_timeout_seconds, - int packet_reader_yield_after_duration_milliseconds, bool migrate_sessions_on_network_change, bool migrate_sessions_early, bool allow_server_migration, @@ -702,7 +701,7 @@ QuicStreamFactory::QuicStreamFactory( QuicTime::Delta::FromSeconds(reduced_ping_timeout_seconds)), yield_after_packets_(kQuicYieldAfterPacketsRead), yield_after_duration_(QuicTime::Delta::FromMilliseconds( - packet_reader_yield_after_duration_milliseconds)), + kQuicYieldAfterDurationMilliseconds)), close_sessions_on_ip_change_(close_sessions_on_ip_change), migrate_sessions_on_network_change_( migrate_sessions_on_network_change && diff --git a/net/quic/chromium/quic_stream_factory.h b/net/quic/chromium/quic_stream_factory.h index cdd1f996f5121d..0d910623f2a24b 100644 --- a/net/quic/chromium/quic_stream_factory.h +++ b/net/quic/chromium/quic_stream_factory.h @@ -208,7 +208,6 @@ class NET_EXPORT_PRIVATE QuicStreamFactory bool mark_quic_broken_when_network_blackholes, int idle_connection_timeout_seconds, int reduced_ping_timeout_seconds, - int packet_reader_yield_after_duration_milliseconds, bool migrate_sessions_on_network_change, bool migrate_sessions_early, bool allow_server_migration, diff --git a/net/quic/chromium/quic_stream_factory_test.cc b/net/quic/chromium/quic_stream_factory_test.cc index 8a9c8c9f064eb3..459af54bfc465b 100644 --- a/net/quic/chromium/quic_stream_factory_test.cc +++ b/net/quic/chromium/quic_stream_factory_test.cc @@ -196,8 +196,6 @@ class QuicStreamFactoryTestBase { close_sessions_on_ip_change_(false), idle_connection_timeout_seconds_(kIdleConnectionTimeoutSeconds), reduced_ping_timeout_seconds_(kPingTimeoutSecs), - packet_reader_yield_after_duration_milliseconds_( - kQuicYieldAfterDurationMilliseconds), migrate_sessions_on_network_change_(false), migrate_sessions_early_(false), allow_server_migration_(false), @@ -220,7 +218,6 @@ class QuicStreamFactoryTestBase { close_sessions_on_ip_change_, /*mark_quic_broken_when_network_blackholes*/ false, idle_connection_timeout_seconds_, reduced_ping_timeout_seconds_, - packet_reader_yield_after_duration_milliseconds_, migrate_sessions_on_network_change_, migrate_sessions_early_, allow_server_migration_, force_hol_blocking_, race_cert_verification_, estimate_initial_rtt_, QuicTagVector(), @@ -729,7 +726,6 @@ class QuicStreamFactoryTestBase { bool close_sessions_on_ip_change_; int idle_connection_timeout_seconds_; int reduced_ping_timeout_seconds_; - int packet_reader_yield_after_duration_milliseconds_; bool migrate_sessions_on_network_change_; bool migrate_sessions_early_; bool allow_server_migration_;