Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(415)

Unified Diff: net/quic/core/congestion_control/bbr_sender_test.cc

Issue 2856243003: Revert of Landing Recent QUIC changes until Sat Apr 29 00:22:04 2017 +0000 (Closed)
Patch Set: Created 3 years, 7 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « net/quic/core/congestion_control/bbr_sender.cc ('k') | net/quic/core/frames/quic_window_update_frame.h » ('j') | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: net/quic/core/congestion_control/bbr_sender_test.cc
diff --git a/net/quic/core/congestion_control/bbr_sender_test.cc b/net/quic/core/congestion_control/bbr_sender_test.cc
index 400b3d4eb07c7e7d23fc7dcb94052fdfffea2d5e..a0d4b505d41791a6d6c971217732a80d463971d8 100644
--- a/net/quic/core/congestion_control/bbr_sender_test.cc
+++ b/net/quic/core/congestion_control/bbr_sender_test.cc
@@ -91,12 +91,13 @@
receiver_multiplexer_("Receiver multiplexer",
{&receiver_, &competing_receiver_}) {
// These will be changed by the appropriate tests as necessary.
+ FLAGS_quic_reloadable_flag_quic_bbr_keep_sending_at_recent_rate = false;
FLAGS_quic_reloadable_flag_quic_bbr_slow_recent_delivery = false;
FLAGS_quic_reloadable_flag_quic_bbr_add_tso_cwnd = false;
// TODO(ianswett): Determine why tests become flaky with CWND based on SRTT.
FLAGS_quic_reloadable_flag_quic_bbr_base_cwnd_on_srtt = false;
FLAGS_quic_reloadable_flag_quic_bbr_extra_conservation = true;
- FLAGS_quic_reloadable_flag_quic_bbr_fix_conservation2 = true;
+ FLAGS_quic_reloadable_flag_quic_bbr_fix_conservation = true;
rtt_stats_ = bbr_sender_.connection()->sent_packet_manager().GetRttStats();
sender_ = SetupBbrSender(&bbr_sender_);
@@ -319,6 +320,34 @@
// smoothed rtt.
EXPECT_GE(kTestRtt * 4, rtt_stats_->smoothed_rtt());
ExpectApproxEq(kTestRtt, rtt_stats_->min_rtt(), 0.12f);
+}
+
+TEST_F(BbrSenderTest, SimpleTransfer2RTTAggregationKeepSending) {
+ FLAGS_quic_reloadable_flag_quic_bbr_ack_aggregation_bytes = false;
+ FLAGS_quic_reloadable_flag_quic_bbr_add_tso_cwnd = false;
+ FLAGS_quic_reloadable_flag_quic_bbr_keep_sending_at_recent_rate = true;
+ CreateDefaultSetup();
+ // 2 RTTs of aggregation, with a max of 10kb.
+ EnableAggregation(10 * 1024, 2 * kTestRtt);
+
+ // Transfer 12MB.
+ DoSimpleTransfer(12 * 1024 * 1024, QuicTime::Delta::FromSeconds(35));
+ EXPECT_EQ(BbrSender::PROBE_BW, sender_->ExportDebugState().mode);
+ // It's possible to read a bandwidth as much as 50% too high with aggregation.
+ EXPECT_LE(kTestLinkBandwidth * 0.95f,
+ sender_->ExportDebugState().max_bandwidth);
+ // TODO(ianswett): Tighten this bound once we understand why BBR is
+ // overestimating bandwidth with aggregation. b/36022633
+ EXPECT_GE(kTestLinkBandwidth * 1.5f,
+ sender_->ExportDebugState().max_bandwidth);
+ // TODO(ianswett): Expect 0 packets are lost once BBR no longer measures
+ // bandwidth higher than the link rate.
+ // TODO(vasilvv): figure out why the line below is occasionally flaky.
+ // EXPECT_FALSE(sender_->ExportDebugState().last_sample_is_app_limited);
+ // The margin here is high, because the aggregation greatly increases
+ // smoothed rtt.
+ EXPECT_GE(kTestRtt * 4.5, rtt_stats_->smoothed_rtt());
+ ExpectApproxEq(kTestRtt, rtt_stats_->min_rtt(), 0.1f);
}
// Test a simple long data transfer with 2 rtts of aggregation.
@@ -357,11 +386,49 @@
ExpectApproxEq(kTestRtt, rtt_stats_->min_rtt(), 0.1f);
}
+TEST_F(BbrSenderTest, SimpleTransferAckDecimationKeepSending) {
+ FLAGS_quic_reloadable_flag_quic_bbr_ack_aggregation_bytes = false;
+ FLAGS_quic_reloadable_flag_quic_bbr_add_tso_cwnd = true;
+ FLAGS_quic_reloadable_flag_quic_bbr_keep_sending_at_recent_rate = true;
+ // Decrease the CWND gain so extra CWND is required with stretch acks.
+ FLAGS_quic_bbr_cwnd_gain = 1.0;
+ sender_ = new BbrSender(
+ rtt_stats_,
+ QuicSentPacketManagerPeer::GetUnackedPacketMap(
+ QuicConnectionPeer::GetSentPacketManager(bbr_sender_.connection())),
+ kInitialCongestionWindowPackets, kDefaultMaxCongestionWindowPackets,
+ &random_);
+ QuicConnectionPeer::SetSendAlgorithm(bbr_sender_.connection(), sender_);
+ // Enable Ack Decimation on the receiver.
+ QuicConnectionPeer::SetAckMode(receiver_.connection(),
+ QuicConnection::AckMode::ACK_DECIMATION);
+ CreateDefaultSetup();
+
+ // Transfer 12MB.
+ DoSimpleTransfer(12 * 1024 * 1024, QuicTime::Delta::FromSeconds(35));
+ EXPECT_EQ(BbrSender::PROBE_BW, sender_->ExportDebugState().mode);
+ // It's possible to read a bandwidth as much as 50% too high with aggregation.
+ EXPECT_LE(kTestLinkBandwidth * 0.95f,
+ sender_->ExportDebugState().max_bandwidth);
+ // TODO(ianswett): Tighten this bound once we understand why BBR is
+ // overestimating bandwidth with aggregation. b/36022633
+ EXPECT_GE(kTestLinkBandwidth * 1.5f,
+ sender_->ExportDebugState().max_bandwidth);
+ // TODO(ianswett): Expect 0 packets are lost once BBR no longer measures
+ // bandwidth higher than the link rate.
+ EXPECT_FALSE(sender_->ExportDebugState().last_sample_is_app_limited);
+ // The margin here is high, because the aggregation greatly increases
+ // smoothed rtt.
+ EXPECT_GE(kTestRtt * 2, rtt_stats_->smoothed_rtt());
+ ExpectApproxEq(kTestRtt, rtt_stats_->min_rtt(), 0.1f);
+}
+
// Test a simple long data transfer with 2 rtts of aggregation.
TEST_F(BbrSenderTest,
SimpleTransfer2RTTAggregationBytesWithIncreasedInflightLimit) {
FLAGS_quic_reloadable_flag_quic_bbr_ack_aggregation_bytes = false;
FLAGS_quic_reloadable_flag_quic_bbr_add_tso_cwnd = false;
+ FLAGS_quic_reloadable_flag_quic_bbr_keep_sending_at_recent_rate = false;
FLAGS_quic_reloadable_flag_quic_bbr_slow_recent_delivery = true;
FLAGS_quic_bbr_slow_delivery_threshold_multiplier = 0.5;
FLAGS_quic_bbr_slow_delivery_cwnd_gain = 4.0;
@@ -427,12 +494,18 @@
ASSERT_EQ(BbrSender::CONSERVATION,
sender_->ExportDebugState().recovery_state);
- simulator_result = simulator_.RunUntilOrTimeout(
- [this]() {
+ const QuicByteCount cwnd_at_recovery_start = sender_->GetCongestionWindow();
+ simulator_result = simulator_.RunUntilOrTimeout(
+ [this, cwnd_at_recovery_start]() {
+ // Ensure that the CWND never drops due to conservation.
+ if (sender_->GetCongestionWindow() < cwnd_at_recovery_start) {
+ return true;
+ }
return sender_->ExportDebugState().recovery_state !=
BbrSender::CONSERVATION;
},
timeout);
+ ASSERT_GE(sender_->GetCongestionWindow(), cwnd_at_recovery_start);
ASSERT_TRUE(simulator_result);
ASSERT_EQ(BbrSender::GROWTH, sender_->ExportDebugState().recovery_state);
« no previous file with comments | « net/quic/core/congestion_control/bbr_sender.cc ('k') | net/quic/core/frames/quic_window_update_frame.h » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698