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

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

Issue 2959373002: Create a new QUIC BBR ack aggregation patch which allows extra CWND only when bytes in flight has g… (Closed)
Patch Set: Rebase Created 3 years, 6 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/quic_flags_list.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 770169f37d3ec57e7cdd65d224508f3c5b1a094c..68b37d91b0594df68010093231cd0cf38c4d44d2 100644
--- a/net/quic/core/congestion_control/bbr_sender_test.cc
+++ b/net/quic/core/congestion_control/bbr_sender_test.cc
@@ -296,6 +296,7 @@ TEST_F(BbrSenderTest, SimpleTransferSmallBuffer) {
// Test a simple long data transfer with 2 rtts of aggregation.
TEST_F(BbrSenderTest, SimpleTransfer2RTTAggregationBytes) {
FLAGS_quic_reloadable_flag_quic_bbr_ack_aggregation_bytes = true;
+ FLAGS_quic_reloadable_flag_quic_bbr_ack_aggregation_bytes2 = false;
FLAGS_quic_reloadable_flag_quic_bbr_add_tso_cwnd = false;
CreateDefaultSetup();
// 2 RTTs of aggregation, with a max of 10kb.
@@ -323,6 +324,70 @@ TEST_F(BbrSenderTest, SimpleTransfer2RTTAggregationBytes) {
// Test a simple long data transfer with 2 rtts of aggregation.
TEST_F(BbrSenderTest, SimpleTransferAckDecimation) {
FLAGS_quic_reloadable_flag_quic_bbr_ack_aggregation_bytes = true;
+ FLAGS_quic_reloadable_flag_quic_bbr_ack_aggregation_bytes2 = false;
+ // 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.99f,
+ 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, SimpleTransfer2RTTAggregationBytes2) {
+ FLAGS_quic_reloadable_flag_quic_bbr_ack_aggregation_bytes2 = true;
+ FLAGS_quic_reloadable_flag_quic_bbr_add_tso_cwnd = false;
+ 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.99f,
+ 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 * 4, rtt_stats_->smoothed_rtt());
+ ExpectApproxEq(kTestRtt, rtt_stats_->min_rtt(), 0.12f);
+}
+
+// Test a simple long data transfer with 2 rtts of aggregation.
+TEST_F(BbrSenderTest, SimpleTransferAckDecimation2) {
+ FLAGS_quic_reloadable_flag_quic_bbr_ack_aggregation_bytes2 = true;
// Decrease the CWND gain so extra CWND is required with stretch acks.
FLAGS_quic_bbr_cwnd_gain = 1.0;
sender_ = new BbrSender(
« no previous file with comments | « net/quic/core/congestion_control/bbr_sender.cc ('k') | net/quic/core/quic_flags_list.h » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698