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 <zhongyi@chromium.org> Reviewed-by: Ryan Hamilton <rch@chromium.org> Cr-Commit-Position: refs/heads/master@{#489413}
marcosholgado · Jul 25, 2017 · d1853b9 · d1853b9
1 parent fb3feae
commit d1853b9
Show file tree

Hide file tree

Showing 8 changed files with 2 additions and 50 deletions.
diff --git 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
@@ -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<std::string, std::string> 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<std::string, std::string> 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
@@ -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<base::Value> 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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_;