diff --git a/quic/api/QuicTransportFunctions.cpp b/quic/api/QuicTransportFunctions.cpp index 905329557..8516c936c 100644 --- a/quic/api/QuicTransportFunctions.cpp +++ b/quic/api/QuicTransportFunctions.cpp @@ -700,6 +700,7 @@ void updateConnection( pkt.lastAckedPacketInfo.emplace( *conn.lossState.lastAckedPacketSentTime, *conn.lossState.lastAckedTime, + *conn.lossState.adjustedLastAckedTime, conn.lossState.totalBytesSentAtLastAck, conn.lossState.totalBytesAckedAtLastAck); } diff --git a/quic/api/test/QuicTransportFunctionsTest.cpp b/quic/api/test/QuicTransportFunctionsTest.cpp index c3b96b8c9..1ea90d5d1 100644 --- a/quic/api/test/QuicTransportFunctionsTest.cpp +++ b/quic/api/test/QuicTransportFunctionsTest.cpp @@ -937,6 +937,7 @@ TEST_F(QuicTransportFunctionsTest, TestUpdateConnectionWithBytesStats) { conn->lossState.totalBytesAcked = 8642; auto currentTime = Clock::now(); conn->lossState.lastAckedTime = currentTime - 123s; + conn->lossState.adjustedLastAckedTime = currentTime - 123s; conn->lossState.lastAckedPacketSentTime = currentTime - 234s; conn->lossState.totalBytesSentAtLastAck = 10000; conn->lossState.totalBytesAckedAtLastAck = 5000; diff --git a/quic/congestion_control/BbrBandwidthSampler.cpp b/quic/congestion_control/BbrBandwidthSampler.cpp index faafc6cb9..fca7a2e55 100644 --- a/quic/congestion_control/BbrBandwidthSampler.cpp +++ b/quic/congestion_control/BbrBandwidthSampler.cpp @@ -61,11 +61,15 @@ void BbrBandwidthSampler::onPacketAcked( DCHECK_GE( conn_.lossState.totalBytesAcked, ackedPacket.lastAckedPacketInfo->totalBytesAcked); + auto ackDuration = (ackEvent.adjustedAckTime > + ackedPacket.lastAckedPacketInfo->adjustedAckTime) + ? (ackEvent.adjustedAckTime - + ackedPacket.lastAckedPacketInfo->adjustedAckTime) + : (ackEvent.ackTime - ackedPacket.lastAckedPacketInfo->ackTime); ackRate = Bandwidth( conn_.lossState.totalBytesAcked - ackedPacket.lastAckedPacketInfo->totalBytesAcked, - std::chrono::duration_cast( - ackEvent.ackTime - ackedPacket.lastAckedPacketInfo->ackTime)); + std::chrono::duration_cast(ackDuration)); } else if (ackEvent.ackTime > ackedPacket.sentTime) { // No previous ack info from outstanding packet, fallback to units/lrtt. // This is a per packet delivery rate. Given there can be multiple packets diff --git a/quic/congestion_control/test/BbrBandwidthSamplerTest.cpp b/quic/congestion_control/test/BbrBandwidthSamplerTest.cpp index cd8fc1a1f..2b0bdea7c 100644 --- a/quic/congestion_control/test/BbrBandwidthSamplerTest.cpp +++ b/quic/congestion_control/test/BbrBandwidthSamplerTest.cpp @@ -65,7 +65,11 @@ TEST_F(BbrBandwidthSamplerTest, RateCalculation) { for (PacketNum pn = 0; pn < 5; pn++) { auto packet = makeTestingWritePacket(pn, 1000, 1000 + 1000 * pn); packet.lastAckedPacketInfo.emplace( - lastAckedPacketSentTime, lastAckedPacketAckTime, 0, 0); + lastAckedPacketSentTime, + lastAckedPacketAckTime, + lastAckedPacketAckTime, + 0, + 0); packet.time = ackTime - 50us; ackEvent.ackedPackets.push_back( makeAckPacketFromOutstandingPacket(std::move(packet))); @@ -76,6 +80,35 @@ TEST_F(BbrBandwidthSamplerTest, RateCalculation) { Bandwidth(5000, std::chrono::microseconds(100)), sampler.getBandwidth()); } +TEST_F(BbrBandwidthSamplerTest, RateCalculationWithAdjustedAckTime) { + BbrBandwidthSampler sampler(conn_); + CongestionController::AckEvent ackEvent; + ackEvent.ackedBytes = 5000; + conn_.lossState.totalBytesAcked = 5000; + auto ackTime = Clock::now(); + ackEvent.ackTime = ackTime; + ackEvent.adjustedAckTime = ackTime - 100us; + auto lastAckedPacketSentTime = ackTime - 500us; + auto lastAckedPacketAckTime = ackTime - 100us; + auto adjustedAckedPacketAckTime = lastAckedPacketAckTime - 200us; + for (PacketNum pn = 0; pn < 5; pn++) { + auto packet = makeTestingWritePacket(pn, 1000, 1000 + 1000 * pn); + packet.lastAckedPacketInfo.emplace( + lastAckedPacketSentTime, + lastAckedPacketAckTime, + adjustedAckedPacketAckTime, + 0, + 0); + packet.time = ackTime - 50us; + ackEvent.ackedPackets.push_back( + makeAckPacketFromOutstandingPacket(std::move(packet))); + } + + sampler.onPacketAcked(ackEvent, 0); + EXPECT_EQ( + Bandwidth(5000, std::chrono::microseconds(200)), sampler.getBandwidth()); +} + TEST_F(BbrBandwidthSamplerTest, SampleExpiration) { BbrBandwidthSampler sampler(conn_); CongestionController::AckEvent ackEvent; @@ -88,7 +121,11 @@ TEST_F(BbrBandwidthSamplerTest, SampleExpiration) { PacketNum pn = 1; auto packet = makeTestingWritePacket(pn, 1000, 2000); packet.lastAckedPacketInfo.emplace( - lastAckedPacketSentTime, lastAckedPacketAckTime, 1000, 1000); + lastAckedPacketSentTime, + lastAckedPacketAckTime, + lastAckedPacketAckTime, + 1000, + 1000); packet.time = ackTime - 50us; ackEvent.ackedPackets.push_back(makeAckPacketFromOutstandingPacket(packet)); sampler.onPacketAcked(ackEvent, 0); @@ -97,7 +134,8 @@ TEST_F(BbrBandwidthSamplerTest, SampleExpiration) { pn++; conn_.lossState.totalBytesAcked = 2000; auto packet2 = makeTestingWritePacket(pn, 500, 2500); - packet2.lastAckedPacketInfo.emplace(packet.time, ackTime, 2000, 1000); + packet2.lastAckedPacketInfo.emplace( + packet.time, ackTime, ackTime, 2000, 1000); auto ackTime2 = ackTime + 150us; CongestionController::AckEvent ackEvent2; ackEvent2.ackTime = ackTime2; @@ -110,7 +148,8 @@ TEST_F(BbrBandwidthSamplerTest, SampleExpiration) { pn++; conn_.lossState.totalBytesAcked = 2500; auto packet3 = makeTestingWritePacket(pn, 200, 2700); - packet3.lastAckedPacketInfo.emplace(packet2.time, ackTime2, 2500, 2000); + packet3.lastAckedPacketInfo.emplace( + packet2.time, ackTime2, ackTime2, 2500, 2000); auto ackTime3 = ackTime + 250us; CongestionController::AckEvent ackEvent3; ackEvent3.ackTime = ackTime3; @@ -122,7 +161,8 @@ TEST_F(BbrBandwidthSamplerTest, SampleExpiration) { pn++; conn_.lossState.totalBytesAcked = 2700; auto packet4 = makeTestingWritePacket(pn, 100, 2800); - packet4.lastAckedPacketInfo.emplace(packet3.time, ackTime3, 2700, 2500); + packet4.lastAckedPacketInfo.emplace( + packet3.time, ackTime3, ackTime3, 2700, 2500); auto ackTime4 = ackTime + 350us; CongestionController::AckEvent ackEvent4; ackEvent4.ackTime = ackTime4; @@ -179,7 +219,11 @@ TEST_F(BbrBandwidthSamplerTest, AppLimitedOutstandingPacket) { auto packet = makeTestingWritePacket(pn, 1000, 1000 + 1000 * pn); packet.isAppLimited = true; packet.lastAckedPacketInfo.emplace( - lastAckedPacketSentTime, lastAckedPacketAckTime, 0, 0); + lastAckedPacketSentTime, + lastAckedPacketAckTime, + lastAckedPacketAckTime, + 0, + 0); packet.time = ackTime - 50us; ackEvent.ackedPackets.push_back(makeAckPacketFromOutstandingPacket(packet)); // AppLimited packet, but sample is larger than current best @@ -190,7 +234,7 @@ TEST_F(BbrBandwidthSamplerTest, AppLimitedOutstandingPacket) { pn++; auto packet1 = makeTestingWritePacket(pn, 1000, 1000 + 1000 * pn); packet1.isAppLimited = true; - packet1.lastAckedPacketInfo.emplace(packet.time, ackTime, 1000, 0); + packet1.lastAckedPacketInfo.emplace(packet.time, ackTime, ackTime, 1000, 0); packet1.time = ackTime + 500us; CongestionController::AckEvent ackEvent1; ackEvent1.ackedBytes = 1000; diff --git a/quic/state/AckHandlers.cpp b/quic/state/AckHandlers.cpp index 153145b53..73279b94c 100644 --- a/quic/state/AckHandlers.cpp +++ b/quic/state/AckHandlers.cpp @@ -42,6 +42,7 @@ void processAckFrame( CongestionController::AckEvent ack; ack.ackTime = ackReceiveTime; ack.implicit = frame.implicit; + ack.adjustedAckTime = ackReceiveTime - frame.ackDelay; // Using kDefaultRxPacketsBeforeAckAfterInit to reseve for ackedPackets // container is a hueristic. Other quic implementations may have very // different acking policy. It's also possibly that all acked packets are pure @@ -178,6 +179,7 @@ void processAckFrame( lastAckedPacketSentTime = rPacketIt->time; } conn.lossState.lastAckedTime = ackReceiveTime; + conn.lossState.adjustedLastAckedTime = ackReceiveTime - frame.ackDelay; ack.ackedPackets.push_back( CongestionController::AckEvent::AckPacket::Builder() .setSentTime(rPacketIt->time) diff --git a/quic/state/OutstandingPacket.h b/quic/state/OutstandingPacket.h index a7e7a0e60..df196ec35 100644 --- a/quic/state/OutstandingPacket.h +++ b/quic/state/OutstandingPacket.h @@ -33,6 +33,7 @@ struct OutstandingPacket { struct LastAckedPacketInfo { TimePoint sentTime; TimePoint ackTime; + TimePoint adjustedAckTime; // Total sent bytes on this connection when the last acked packet is acked. uint64_t totalBytesSent; // Total acked bytes on this connection when last acked packet is acked, @@ -42,10 +43,12 @@ struct OutstandingPacket { LastAckedPacketInfo( TimePoint sentTimeIn, TimePoint ackTimeIn, + TimePoint adjustedAckTimeIn, uint64_t totalBytesSentIn, uint64_t totalBytesAckedIn) : sentTime(sentTimeIn), ackTime(ackTimeIn), + adjustedAckTime(adjustedAckTimeIn), totalBytesSent(totalBytesSentIn), totalBytesAcked(totalBytesAckedIn) {} }; diff --git a/quic/state/StateData.h b/quic/state/StateData.h index 569194139..890694739 100644 --- a/quic/state/StateData.h +++ b/quic/state/StateData.h @@ -252,6 +252,7 @@ struct CongestionController { bool largestAckedPacketAppLimited{false}; uint64_t ackedBytes{0}; TimePoint ackTime; + TimePoint adjustedAckTime; // The minimal RTT sample among packets acked by this AckEvent. This RTT // includes ack delay. folly::Optional mrttSample; @@ -451,6 +452,8 @@ struct LossState { folly::Optional lastAckedPacketSentTime; // The latest time a packet is acked folly::Optional lastAckedTime; + // The latest time a packet is acked, minus ack delay + folly::Optional adjustedLastAckedTime; // The time when last retranmittable packet is sent for every packet number // space TimePoint lastRetransmittablePacketSentTime;