diff --git a/quic/congestion_control/Bbr.cpp b/quic/congestion_control/Bbr.cpp index 202651e9b..32cb89c60 100644 --- a/quic/congestion_control/Bbr.cpp +++ b/quic/congestion_control/Bbr.cpp @@ -73,7 +73,7 @@ void BbrCongestionController::onPacketLoss( if (!inRecovery()) { recoveryState_ = BbrCongestionController::RecoveryState::CONSERVATIVE; - recoveryWindow_ = inflightBytes_ + ackedBytes; + recoveryWindow_ = conn_.lossState.inflightBytes + ackedBytes; recoveryWindow_ = boundedCwnd( recoveryWindow_, conn_.udpSendPacketLen, @@ -96,7 +96,7 @@ void BbrCongestionController::onPacketLoss( recoveryWindow_ = conn_.udpSendPacketLen * kMinCwndInMssForBbr; if (conn_.qLogger) { conn_.qLogger->addCongestionMetricUpdate( - inflightBytes_, + conn_.lossState.inflightBytes, getCongestionWindow(), kPersistentCongestion, bbrStateToString(state_), @@ -108,15 +108,15 @@ void BbrCongestionController::onPacketLoss( bbrStateToString(state_), bbrRecoveryStateToString(recoveryState_), recoveryWindow_, - inflightBytes_); + conn_.lossState.inflightBytes); } } void BbrCongestionController::onPacketSent(const OutstandingPacket& packet) { - if (!inflightBytes_ && isAppLimited()) { + if (!conn_.lossState.inflightBytes && isAppLimited()) { exitingQuiescene_ = true; } - addAndCheckOverflow(inflightBytes_, packet.encodedSize); + addAndCheckOverflow(conn_.lossState.inflightBytes, packet.encodedSize); if (!ackAggregationStartTime_) { ackAggregationStartTime_ = packet.time; } @@ -151,12 +151,14 @@ uint64_t BbrCongestionController::updateAckAggregation(const AckEvent& ack) { void BbrCongestionController::onPacketAckOrLoss( folly::Optional ackEvent, folly::Optional lossEvent) { - auto prevInflightBytes = inflightBytes_; + auto prevInflightBytes = conn_.lossState.inflightBytes; if (ackEvent) { - subtractAndCheckUnderflow(inflightBytes_, ackEvent->ackedBytes); + subtractAndCheckUnderflow( + conn_.lossState.inflightBytes, ackEvent->ackedBytes); } if (lossEvent) { - subtractAndCheckUnderflow(inflightBytes_, lossEvent->lostBytes); + subtractAndCheckUnderflow( + conn_.lossState.inflightBytes, lossEvent->lostBytes); } if (lossEvent) { onPacketLoss(*lossEvent, ackEvent ? ackEvent->ackedBytes : 0); @@ -244,7 +246,7 @@ void BbrCongestionController::onPacketAcked( updatePacing(); if (conn_.qLogger) { conn_.qLogger->addCongestionMetricUpdate( - inflightBytes_, + conn_.lossState.inflightBytes, getCongestionWindow(), kCongestionPacketAck, bbrStateToString(state_), @@ -258,7 +260,7 @@ void BbrCongestionController::onPacketAcked( getCongestionWindow(), cwnd_, sendQuantum_, - inflightBytes_); + conn_.lossState.inflightBytes); } // TODO: We used to check if there is available bandwidth and rtt samples in @@ -295,7 +297,7 @@ void BbrCongestionController::handleAckInProbeBw( if (pacingGain_ > 1.0 && !hasLoss && prevInflightBytes < calculateTargetCwnd(pacingGain_)) { // pacingGain_ > 1.0 means BBR is probeing bandwidth. So we should let - // inflightBytes_ reach the target. + // inflight bytes reach the target. shouldAdvancePacingGainCycle = false; } @@ -303,9 +305,9 @@ void BbrCongestionController::handleAckInProbeBw( folly::Optional targetCwndCache; if (pacingGain_ < 1.0) { targetCwndCache = calculateTargetCwnd(1.0); - if (inflightBytes_ <= *targetCwndCache) { + if (conn_.lossState.inflightBytes <= *targetCwndCache) { // pacingGain_ < 1.0 means BBR is draining the network queue. If - // inflightBytes_ is below the target, then it's done. + // inflight bytes is below the target, then it's done. shouldAdvancePacingGainCycle = true; } } @@ -317,7 +319,7 @@ void BbrCongestionController::handleAckInProbeBw( kPacingGainCycles[pacingCycleIndex_] == 1.0) { auto drainTarget = targetCwndCache ? *targetCwndCache : calculateTargetCwnd(1.0); - if (inflightBytes_ > drainTarget) { + if (conn_.lossState.inflightBytes > drainTarget) { // Interestingly Chromium doesn't rollback pacingCycleIndex_ in this // case. // TODO: isn't this a bug? But we don't do drainToTarget today. @@ -334,7 +336,7 @@ bool BbrCongestionController::shouldExitStartup() noexcept { bool BbrCongestionController::shouldExitDrain() noexcept { return state_ == BbrState::Drain && - inflightBytes_ <= calculateTargetCwnd(1.0); + conn_.lossState.inflightBytes <= calculateTargetCwnd(1.0); } bool BbrCongestionController::shouldProbeRtt(TimePoint ackTime) noexcept { @@ -364,7 +366,8 @@ void BbrCongestionController::handleAckInProbeRtt( bandwidthSampler_->onAppLimited(); } if (!earliestTimeToExitProbeRtt_ && - inflightBytes_ < getCongestionWindow() + conn_.udpSendPacketLen) { + conn_.lossState.inflightBytes < + getCongestionWindow() + conn_.udpSendPacketLen) { earliestTimeToExitProbeRtt_ = ackTime + kProbeRttDuration; probeRttRound_ = folly::none; return; @@ -436,8 +439,8 @@ void BbrCongestionController::updateRecoveryWindowWithAck( conn_.transportSettings.bbrConfig.conservativeRecovery ? conn_.udpSendPacketLen : bytesAcked; - recoveryWindow_ = - std::max(recoveryWindow_, inflightBytes_ + recoveryIncrease); + recoveryWindow_ = std::max( + recoveryWindow_, conn_.lossState.inflightBytes + recoveryIncrease); recoveryWindow_ = boundedCwnd( recoveryWindow_, conn_.udpSendPacketLen, @@ -455,8 +458,8 @@ BbrCongestionController::BbrState BbrCongestionController::state() const } uint64_t BbrCongestionController::getWritableBytes() const noexcept { - return getCongestionWindow() > inflightBytes_ - ? getCongestionWindow() - inflightBytes_ + return getCongestionWindow() > conn_.lossState.inflightBytes + ? getCongestionWindow() - conn_.lossState.inflightBytes : 0; } @@ -532,7 +535,7 @@ void BbrCongestionController::setAppIdle( } void BbrCongestionController::setAppLimited() { - if (inflightBytes_ > getCongestionWindow()) { + if (conn_.lossState.inflightBytes > getCongestionWindow()) { return; } appLimitedSinceProbeRtt_ = true; @@ -594,7 +597,7 @@ void BbrCongestionController::detectBottleneckBandwidth(bool appLimitedSample) { void BbrCongestionController::onRemoveBytesFromInflight( uint64_t bytesToRemove) { - subtractAndCheckUnderflow(inflightBytes_, bytesToRemove); + subtractAndCheckUnderflow(conn_.lossState.inflightBytes, bytesToRemove); } std::string bbrStateToString(BbrCongestionController::BbrState state) { diff --git a/quic/congestion_control/Bbr.h b/quic/congestion_control/Bbr.h index 235640700..dce953a31 100644 --- a/quic/congestion_control/Bbr.h +++ b/quic/congestion_control/Bbr.h @@ -131,7 +131,7 @@ class BbrCongestionController : public CongestionController { BbrState state() const noexcept; private: - /* prevInflightBytes: the inflightBytes_ value before the current + /* prevInflightBytes: the inflightBytes value before the current * onPacketAckOrLoss invocation. * hasLoss: whether current onPacketAckOrLoss has loss. */ @@ -170,7 +170,7 @@ class BbrCongestionController : public CongestionController { /** * Special handling of AckEvent when connection is in ProbeBw state. * - * prevInflightBytes: the inflightBytes_ value before the current + * prevInflightBytes: the inflightBytes value before the current * onPacketAckOrLoss invocation. * hasLoss: whether the current onpacketAckOrLoss has loss. */ @@ -208,8 +208,6 @@ class BbrCongestionController : public CongestionController { uint64_t initialCwnd_; // Congestion window when the connection is in recovery uint64_t recoveryWindow_; - // inflight bytes - uint64_t inflightBytes_{0}; // Number of bytes we expect to send over one RTT when paced write. uint64_t pacingWindow_{0}; diff --git a/quic/congestion_control/Copa.cpp b/quic/congestion_control/Copa.cpp index 5301c4da9..447c24d67 100644 --- a/quic/congestion_control/Copa.cpp +++ b/quic/congestion_control/Copa.cpp @@ -26,8 +26,8 @@ Copa::Copa(QuicConnectionStateBase& conn) 0us, 0) { VLOG(10) << __func__ << " writable=" << getWritableBytes() - << " cwnd=" << cwndBytes_ << " inflight=" << bytesInFlight_ << " " - << conn_; + << " cwnd=" << cwndBytes_ + << " inflight=" << conn_.lossState.inflightBytes << " " << conn_; if (conn_.transportSettings.latencyFactor.has_value()) { latencyFactor_ = conn_.transportSettings.latencyFactor.value(); } @@ -35,27 +35,30 @@ Copa::Copa(QuicConnectionStateBase& conn) } void Copa::onRemoveBytesFromInflight(uint64_t bytes) { - subtractAndCheckUnderflow(bytesInFlight_, bytes); + subtractAndCheckUnderflow(conn_.lossState.inflightBytes, bytes); VLOG(10) << __func__ << " writable=" << getWritableBytes() - << " cwnd=" << cwndBytes_ << " inflight=" << bytesInFlight_ << " " - << conn_; + << " cwnd=" << cwndBytes_ + << " inflight=" << conn_.lossState.inflightBytes << " " << conn_; if (conn_.qLogger) { conn_.qLogger->addCongestionMetricUpdate( - bytesInFlight_, getCongestionWindow(), kRemoveInflight); + conn_.lossState.inflightBytes, getCongestionWindow(), kRemoveInflight); } } void Copa::onPacketSent(const OutstandingPacket& packet) { - addAndCheckOverflow(bytesInFlight_, packet.encodedSize); + addAndCheckOverflow(conn_.lossState.inflightBytes, packet.encodedSize); VLOG(10) << __func__ << " writable=" << getWritableBytes() - << " cwnd=" << cwndBytes_ << " inflight=" << bytesInFlight_ + << " cwnd=" << cwndBytes_ + << " inflight=" << conn_.lossState.inflightBytes << " bytesBufferred=" << conn_.flowControlState.sumCurStreamBufferLen << " packetNum=" << packet.packet.header.getPacketSequenceNum() << " " << conn_; if (conn_.qLogger) { conn_.qLogger->addCongestionMetricUpdate( - bytesInFlight_, getCongestionWindow(), kCongestionPacketSent); + conn_.lossState.inflightBytes, + getCongestionWindow(), + kCongestionPacketSent); } } @@ -131,17 +134,17 @@ void Copa::onPacketAckOrLoss( if (conn_.pacer) { conn_.pacer->onPacketsLoss(); } - QUIC_TRACE(copa_loss, conn_, cwndBytes_, bytesInFlight_); + QUIC_TRACE(copa_loss, conn_, cwndBytes_, conn_.lossState.inflightBytes); } if (ack && ack->largestAckedPacket.has_value()) { onPacketAcked(*ack); - QUIC_TRACE(copa_ack, conn_, cwndBytes_, bytesInFlight_); + QUIC_TRACE(copa_ack, conn_, cwndBytes_, conn_.lossState.inflightBytes); } } void Copa::onPacketAcked(const AckEvent& ack) { DCHECK(ack.largestAckedPacket.has_value()); - subtractAndCheckUnderflow(bytesInFlight_, ack.ackedBytes); + subtractAndCheckUnderflow(conn_.lossState.inflightBytes, ack.ackedBytes); minRTTFilter_.Update( conn_.lossState.lrtt, std::chrono::duration_cast(ack.ackTime.time_since_epoch()) @@ -157,7 +160,8 @@ void Copa::onPacketAcked(const AckEvent& ack) { VLOG(10) << __func__ << "ack size=" << ack.ackedBytes << " num packets acked=" << ack.ackedBytes / conn_.udpSendPacketLen << " writable=" << getWritableBytes() << " cwnd=" << cwndBytes_ - << " inflight=" << bytesInFlight_ << " rttMin=" << rttMin.count() + << " inflight=" << conn_.lossState.inflightBytes + << " rttMin=" << rttMin.count() << " sRTT=" << conn_.lossState.srtt.count() << " lRTT=" << conn_.lossState.lrtt.count() << " mRTT=" << conn_.lossState.mrtt.count() @@ -169,7 +173,9 @@ void Copa::onPacketAcked(const AckEvent& ack) { if (conn_.qLogger) { conn_.qLogger->addCongestionMetricUpdate( - bytesInFlight_, getCongestionWindow(), kCongestionPacketAck); + conn_.lossState.inflightBytes, + getCongestionWindow(), + kCongestionPacketAck); } auto delayInMicroSec = @@ -271,21 +277,25 @@ void Copa::onPacketAcked(const AckEvent& ack) { void Copa::onPacketLoss(const LossEvent& loss) { VLOG(10) << __func__ << " lostBytes=" << loss.lostBytes << " lostPackets=" << loss.lostPackets << " cwnd=" << cwndBytes_ - << " inflight=" << bytesInFlight_ << " " << conn_; + << " inflight=" << conn_.lossState.inflightBytes << " " << conn_; if (conn_.qLogger) { conn_.qLogger->addCongestionMetricUpdate( - bytesInFlight_, getCongestionWindow(), kCongestionPacketLoss); + conn_.lossState.inflightBytes, + getCongestionWindow(), + kCongestionPacketLoss); } DCHECK(loss.largestLostPacketNum.has_value()); - subtractAndCheckUnderflow(bytesInFlight_, loss.lostBytes); + subtractAndCheckUnderflow(conn_.lossState.inflightBytes, loss.lostBytes); if (loss.persistentCongestion) { // TODO See if we should go to slowStart here VLOG(10) << __func__ << " writable=" << getWritableBytes() - << " cwnd=" << cwndBytes_ << " inflight=" << bytesInFlight_ << " " - << conn_; + << " cwnd=" << cwndBytes_ + << " inflight=" << conn_.lossState.inflightBytes << " " << conn_; if (conn_.qLogger) { conn_.qLogger->addCongestionMetricUpdate( - bytesInFlight_, getCongestionWindow(), kPersistentCongestion); + conn_.lossState.inflightBytes, + getCongestionWindow(), + kPersistentCongestion); } cwndBytes_ = conn_.transportSettings.minCwndInMss * conn_.udpSendPacketLen; if (conn_.pacer) { @@ -295,10 +305,10 @@ void Copa::onPacketLoss(const LossEvent& loss) { } uint64_t Copa::getWritableBytes() const noexcept { - if (bytesInFlight_ > cwndBytes_) { + if (conn_.lossState.inflightBytes > cwndBytes_) { return 0; } else { - return cwndBytes_ - bytesInFlight_; + return cwndBytes_ - conn_.lossState.inflightBytes; } } @@ -315,7 +325,7 @@ CongestionControlType Copa::type() const noexcept { } uint64_t Copa::getBytesInFlight() const noexcept { - return bytesInFlight_; + return conn_.lossState.inflightBytes; } void Copa::setAppIdle(bool, TimePoint) noexcept { /* unsupported */ diff --git a/quic/congestion_control/Copa.h b/quic/congestion_control/Copa.h index aaeec8233..b351aed0c 100644 --- a/quic/congestion_control/Copa.h +++ b/quic/congestion_control/Copa.h @@ -69,7 +69,6 @@ class Copa : public CongestionController { VelocityState::Direction newDirection, const TimePoint ackTime); QuicConnectionStateBase& conn_; - uint64_t bytesInFlight_{0}; uint64_t cwndBytes_; bool isSlowStart_; diff --git a/quic/congestion_control/NewReno.cpp b/quic/congestion_control/NewReno.cpp index d981e1cf1..3d343b588 100644 --- a/quic/congestion_control/NewReno.cpp +++ b/quic/congestion_control/NewReno.cpp @@ -26,37 +26,42 @@ NewReno::NewReno(QuicConnectionStateBase& conn) } void NewReno::onRemoveBytesFromInflight(uint64_t bytes) { - subtractAndCheckUnderflow(bytesInFlight_, bytes); + subtractAndCheckUnderflow(conn_.lossState.inflightBytes, bytes); VLOG(10) << __func__ << " writable=" << getWritableBytes() - << " cwnd=" << cwndBytes_ << " inflight=" << bytesInFlight_ << " " - << conn_; + << " cwnd=" << cwndBytes_ + << " inflight=" << conn_.lossState.inflightBytes << " " << conn_; if (conn_.qLogger) { conn_.qLogger->addCongestionMetricUpdate( - bytesInFlight_, getCongestionWindow(), kRemoveInflight); + conn_.lossState.inflightBytes, getCongestionWindow(), kRemoveInflight); } } void NewReno::onPacketSent(const OutstandingPacket& packet) { - addAndCheckOverflow(bytesInFlight_, packet.encodedSize); + addAndCheckOverflow(conn_.lossState.inflightBytes, packet.encodedSize); VLOG(10) << __func__ << " writable=" << getWritableBytes() - << " cwnd=" << cwndBytes_ << " inflight=" << bytesInFlight_ + << " cwnd=" << cwndBytes_ + << " inflight=" << conn_.lossState.inflightBytes << " packetNum=" << packet.packet.header.getPacketSequenceNum() << " " << conn_; if (conn_.qLogger) { conn_.qLogger->addCongestionMetricUpdate( - bytesInFlight_, getCongestionWindow(), kCongestionPacketSent); + conn_.lossState.inflightBytes, + getCongestionWindow(), + kCongestionPacketSent); } } void NewReno::onAckEvent(const AckEvent& ack) { DCHECK(ack.largestAckedPacket.has_value() && !ack.ackedPackets.empty()); - subtractAndCheckUnderflow(bytesInFlight_, ack.ackedBytes); + subtractAndCheckUnderflow(conn_.lossState.inflightBytes, ack.ackedBytes); VLOG(10) << __func__ << " writable=" << getWritableBytes() - << " cwnd=" << cwndBytes_ << " inflight=" << bytesInFlight_ << " " - << conn_; + << " cwnd=" << cwndBytes_ + << " inflight=" << conn_.lossState.inflightBytes << " " << conn_; if (conn_.qLogger) { conn_.qLogger->addCongestionMetricUpdate( - bytesInFlight_, getCongestionWindow(), kCongestionPacketAck); + conn_.lossState.inflightBytes, + getCongestionWindow(), + kCongestionPacketAck); } for (const auto& packet : ack.ackedPackets) { onPacketAcked(packet); @@ -103,7 +108,7 @@ void NewReno::onPacketLoss(const LossEvent& loss) { DCHECK( loss.largestLostPacketNum.has_value() && loss.largestLostSentTime.has_value()); - subtractAndCheckUnderflow(bytesInFlight_, loss.lostBytes); + subtractAndCheckUnderflow(conn_.lossState.inflightBytes, loss.lostBytes); if (!endOfRecovery_ || *endOfRecovery_ < *loss.largestLostSentTime) { endOfRecovery_ = Clock::now(); cwndBytes_ = (cwndBytes_ >> kRenoLossReductionFactorShift); @@ -117,34 +122,38 @@ void NewReno::onPacketLoss(const LossEvent& loss) { VLOG(10) << __func__ << " exit slow start, ssthresh=" << ssthresh_ << " packetNum=" << *loss.largestLostPacketNum << " writable=" << getWritableBytes() << " cwnd=" << cwndBytes_ - << " inflight=" << bytesInFlight_ << " " << conn_; + << " inflight=" << conn_.lossState.inflightBytes << " " << conn_; } else { VLOG(10) << __func__ << " packetNum=" << *loss.largestLostPacketNum << " writable=" << getWritableBytes() << " cwnd=" << cwndBytes_ - << " inflight=" << bytesInFlight_ << " " << conn_; + << " inflight=" << conn_.lossState.inflightBytes << " " << conn_; } if (conn_.qLogger) { conn_.qLogger->addCongestionMetricUpdate( - bytesInFlight_, getCongestionWindow(), kCongestionPacketLoss); + conn_.lossState.inflightBytes, + getCongestionWindow(), + kCongestionPacketLoss); } if (loss.persistentCongestion) { VLOG(10) << __func__ << " writable=" << getWritableBytes() - << " cwnd=" << cwndBytes_ << " inflight=" << bytesInFlight_ << " " - << conn_; + << " cwnd=" << cwndBytes_ + << " inflight=" << conn_.lossState.inflightBytes << " " << conn_; if (conn_.qLogger) { conn_.qLogger->addCongestionMetricUpdate( - bytesInFlight_, getCongestionWindow(), kPersistentCongestion); + conn_.lossState.inflightBytes, + getCongestionWindow(), + kPersistentCongestion); } cwndBytes_ = conn_.transportSettings.minCwndInMss * conn_.udpSendPacketLen; } } uint64_t NewReno::getWritableBytes() const noexcept { - if (bytesInFlight_ > cwndBytes_) { + if (conn_.lossState.inflightBytes > cwndBytes_) { return 0; } else { - return cwndBytes_ - bytesInFlight_; + return cwndBytes_ - conn_.lossState.inflightBytes; } } @@ -161,7 +170,7 @@ CongestionControlType NewReno::type() const noexcept { } uint64_t NewReno::getBytesInFlight() const noexcept { - return bytesInFlight_; + return conn_.lossState.inflightBytes; } void NewReno::setAppIdle(bool, TimePoint) noexcept { /* unsupported */ diff --git a/quic/congestion_control/NewReno.h b/quic/congestion_control/NewReno.h index c7eb0e473..b82c8039b 100644 --- a/quic/congestion_control/NewReno.h +++ b/quic/congestion_control/NewReno.h @@ -43,7 +43,6 @@ class NewReno : public CongestionController { private: QuicConnectionStateBase& conn_; - uint64_t bytesInFlight_{0}; uint64_t ssthresh_; uint64_t cwndBytes_; folly::Optional endOfRecovery_; diff --git a/quic/congestion_control/QuicCubic.cpp b/quic/congestion_control/QuicCubic.cpp index 9dd620022..696231426 100644 --- a/quic/congestion_control/QuicCubic.cpp +++ b/quic/congestion_control/QuicCubic.cpp @@ -19,10 +19,7 @@ Cubic::Cubic( bool tcpFriendly, bool ackTrain, bool spreadAcrossRtt) - : conn_(conn), - inflightBytes_(0), - ssthresh_(initSsthresh), - spreadAcrossRtt_(spreadAcrossRtt) { + : conn_(conn), ssthresh_(initSsthresh), spreadAcrossRtt_(spreadAcrossRtt) { cwndBytes_ = std::min( conn.transportSettings.maxCwndInMss * conn.udpSendPacketLen, conn.transportSettings.initCwndInMss * conn.udpSendPacketLen); @@ -37,7 +34,9 @@ CubicStates Cubic::state() const noexcept { } uint64_t Cubic::getWritableBytes() const noexcept { - return cwndBytes_ > inflightBytes_ ? cwndBytes_ - inflightBytes_ : 0; + return cwndBytes_ > conn_.lossState.inflightBytes + ? cwndBytes_ - conn_.lossState.inflightBytes + : 0; } uint64_t Cubic::getCongestionWindow() const noexcept { @@ -71,11 +70,11 @@ void Cubic::onPersistentCongestion() { conn_, cubicStateToString(state_).data(), cwndBytes_, - inflightBytes_, + conn_.lossState.inflightBytes, steadyState_.lastMaxCwndBytes.value_or(0)); if (conn_.qLogger) { conn_.qLogger->addCongestionMetricUpdate( - inflightBytes_, + conn_.lossState.inflightBytes, getCongestionWindow(), kPersistentCongestion, cubicStateToString(state_).str()); @@ -83,13 +82,13 @@ void Cubic::onPersistentCongestion() { } void Cubic::onPacketSent(const OutstandingPacket& packet) { - if (std::numeric_limits::max() - inflightBytes_ < + if (std::numeric_limits::max() - conn_.lossState.inflightBytes < packet.encodedSize) { throw QuicInternalException( - "Cubic: inflightBytes_ overflow", + "Cubic: inflightBytes overflow", LocalErrorCode::INFLIGHT_BYTES_OVERFLOW); } - inflightBytes_ += packet.encodedSize; + conn_.lossState.inflightBytes += packet.encodedSize; } void Cubic::onPacketLoss(const LossEvent& loss) { @@ -118,11 +117,11 @@ void Cubic::onPacketLoss(const LossEvent& loss) { conn_, cubicStateToString(state_).str().data(), cwndBytes_, - inflightBytes_, + conn_.lossState.inflightBytes, steadyState_.lastMaxCwndBytes.value_or(0)); if (conn_.qLogger) { conn_.qLogger->addCongestionMetricUpdate( - inflightBytes_, + conn_.lossState.inflightBytes, getCongestionWindow(), kCubicLoss, cubicStateToString(state_).str()); @@ -132,7 +131,7 @@ void Cubic::onPacketLoss(const LossEvent& loss) { QUIC_TRACE(fst_trace, conn_, "cubic_skip_loss"); if (conn_.qLogger) { conn_.qLogger->addCongestionMetricUpdate( - inflightBytes_, + conn_.lossState.inflightBytes, getCongestionWindow(), kCubicSkipLoss, cubicStateToString(state_).str()); @@ -145,18 +144,18 @@ void Cubic::onPacketLoss(const LossEvent& loss) { } void Cubic::onRemoveBytesFromInflight(uint64_t bytes) { - DCHECK_LE(bytes, inflightBytes_); - inflightBytes_ -= bytes; + DCHECK_LE(bytes, conn_.lossState.inflightBytes); + conn_.lossState.inflightBytes -= bytes; QUIC_TRACE( cubic_remove_inflight, conn_, cubicStateToString(state_).str().data(), cwndBytes_, - inflightBytes_, + conn_.lossState.inflightBytes, steadyState_.lastMaxCwndBytes.value_or(0)); if (conn_.qLogger) { conn_.qLogger->addCongestionMetricUpdate( - inflightBytes_, + conn_.lossState.inflightBytes, getCongestionWindow(), kRemoveInflight, cubicStateToString(state_).str()); @@ -290,7 +289,7 @@ int64_t Cubic::calculateCubicCwndDelta(TimePoint ackTime) noexcept { static_cast(timeElapsedCount)); if (conn_.qLogger) { conn_.qLogger->addCongestionMetricUpdate( - inflightBytes_, + conn_.lossState.inflightBytes, getCongestionWindow(), kCubicSteadyCwnd, cubicStateToString(state_).str()); @@ -363,14 +362,14 @@ void Cubic::onPacketAckOrLoss( void Cubic::onPacketAcked(const AckEvent& ack) { auto currentCwnd = cwndBytes_; - DCHECK_LE(ack.ackedBytes, inflightBytes_); - inflightBytes_ -= ack.ackedBytes; + DCHECK_LE(ack.ackedBytes, conn_.lossState.inflightBytes); + conn_.lossState.inflightBytes -= ack.ackedBytes; if (recoveryState_.endOfRecovery.has_value() && *recoveryState_.endOfRecovery >= ack.largestAckedPacketSentTime) { QUIC_TRACE(fst_trace, conn_, "cubic_skip_ack"); if (conn_.qLogger) { conn_.qLogger->addCongestionMetricUpdate( - inflightBytes_, + conn_.lossState.inflightBytes, getCongestionWindow(), kCubicSkipAck, cubicStateToString(state_).str()); @@ -397,7 +396,7 @@ void Cubic::onPacketAcked(const AckEvent& ack) { fst_trace, conn_, "cwnd_no_change", quiescenceStart_.has_value()); if (conn_.qLogger) { conn_.qLogger->addCongestionMetricUpdate( - inflightBytes_, + conn_.lossState.inflightBytes, getCongestionWindow(), kCwndNoChange, cubicStateToString(state_).str()); @@ -408,11 +407,11 @@ void Cubic::onPacketAcked(const AckEvent& ack) { conn_, cubicStateToString(state_).str().data(), cwndBytes_, - inflightBytes_, + conn_.lossState.inflightBytes, steadyState_.lastMaxCwndBytes.value_or(0)); if (conn_.qLogger) { conn_.qLogger->addCongestionMetricUpdate( - inflightBytes_, + conn_.lossState.inflightBytes, getCongestionWindow(), kCongestionPacketAck, cubicStateToString(state_).str()); @@ -482,8 +481,8 @@ void Cubic::onPacketAckedInHystart(const AckEvent& ack) { } // TODO: Should we not increase cwnd if inflight is less than half of cwnd? - // Note that we take bytes out of inflightBytes_ before invoke the state - // machine. So the inflightBytes_ here is already reduced. + // Note that we take bytes out of inflightBytes before invoke the state + // machine. So the inflightBytes here is already reduced. if (std::numeric_limits::max() - cwndBytes_ < ack.ackedBytes) { throw QuicInternalException( @@ -620,7 +619,7 @@ void Cubic::onPacketAckedInSteady(const AckEvent& ack) { QUIC_TRACE(fst_trace, conn_, "ack_in_quiescence"); if (conn_.qLogger) { conn_.qLogger->addCongestionMetricUpdate( - inflightBytes_, + conn_.lossState.inflightBytes, getCongestionWindow(), kAckInQuiescence, cubicStateToString(state_).str()); @@ -641,7 +640,7 @@ void Cubic::onPacketAckedInSteady(const AckEvent& ack) { QUIC_TRACE(fst_trace, conn_, "reset_timetoorigin"); if (conn_.qLogger) { conn_.qLogger->addCongestionMetricUpdate( - inflightBytes_, + conn_.lossState.inflightBytes, getCongestionWindow(), kResetTimeToOrigin, cubicStateToString(state_).str()); @@ -662,7 +661,7 @@ void Cubic::onPacketAckedInSteady(const AckEvent& ack) { steadyState_.lastReductionTime = ack.ackTime; if (conn_.qLogger) { conn_.qLogger->addCongestionMetricUpdate( - inflightBytes_, + conn_.lossState.inflightBytes, getCongestionWindow(), kResetLastReductionTime, cubicStateToString(state_).str()); @@ -692,7 +691,7 @@ void Cubic::onPacketAckedInSteady(const AckEvent& ack) { cwndBytes_ = std::max(cwndBytes_, steadyState_.estRenoCwnd); if (conn_.qLogger) { conn_.qLogger->addCongestionMetricUpdate( - inflightBytes_, + conn_.lossState.inflightBytes, getCongestionWindow(), kRenoCwndEstimation, cubicStateToString(state_).str()); @@ -718,7 +717,7 @@ void Cubic::onPacketAckedInRecovery(const AckEvent& ack) { cwndBytes_ = calculateCubicCwnd(calculateCubicCwndDelta(ack.ackTime)); if (conn_.qLogger) { conn_.qLogger->addCongestionMetricUpdate( - inflightBytes_, + conn_.lossState.inflightBytes, getCongestionWindow(), kPacketAckedInRecovery, cubicStateToString(state_).str()); diff --git a/quic/congestion_control/QuicCubic.h b/quic/congestion_control/QuicCubic.h index d21519b57..d17361369 100644 --- a/quic/congestion_control/QuicCubic.h +++ b/quic/congestion_control/QuicCubic.h @@ -133,7 +133,6 @@ class Cubic : public CongestionController { folly::Optional lossCwndBytes_; // the value of ssthresh_ at the last loss event folly::Optional lossSsthresh_; - uint64_t inflightBytes_; uint64_t ssthresh_; struct HystartState { diff --git a/quic/state/StateData.h b/quic/state/StateData.h index a99eaa013..982708941 100644 --- a/quic/state/StateData.h +++ b/quic/state/StateData.h @@ -486,6 +486,8 @@ struct LossState { // The time when last retranmittable packet is sent for every packet number // space TimePoint lastRetransmittablePacketSentTime; + // Inflight bytes + uint64_t inflightBytes{0}; }; class Logger;