gfe-relnote: Refactor how QuicConnectionStats.slowstart_duration is calculated in QUIC BBR1, and start populate it in QUIC BBR2. Not protected. (Worst case we get some incorrect values in transport connection stats) Similar to slowstart_duration, I will add drain_duration, probe_rtt_duration, probe_bw_(down|up|cruise)_duration, in a follow up CL. PiperOrigin-RevId: 282224334 Change-Id: I873c39dcd47eb03d22d245cdfbf13aabc1a814e6

diff --git a/quic/core/congestion_control/bbr2_drain.h b/quic/core/congestion_control/bbr2_drain.h
index ccc0320..e083aeb 100644
--- a/quic/core/congestion_control/bbr2_drain.h
+++ b/quic/core/congestion_control/bbr2_drain.h

@@ -18,6 +18,7 @@
   using Bbr2ModeBase::Bbr2ModeBase;
 
   void Enter(const Bbr2CongestionEvent& congestion_event) override;
+  void Leave(const Bbr2CongestionEvent& /*congestion_event*/) override {}
 
   Bbr2Mode OnCongestionEvent(
       QuicByteCount prior_in_flight,

diff --git a/quic/core/congestion_control/bbr2_misc.h b/quic/core/congestion_control/bbr2_misc.h
index 3e8a518..139b120 100644
--- a/quic/core/congestion_control/bbr2_misc.h
+++ b/quic/core/congestion_control/bbr2_misc.h

@@ -473,7 +473,9 @@
 
   virtual ~Bbr2ModeBase() = default;
 
+  // Called when entering/leaving this mode.
   virtual void Enter(const Bbr2CongestionEvent& congestion_event) = 0;
+  virtual void Leave(const Bbr2CongestionEvent& congestion_event) = 0;
 
   virtual Bbr2Mode OnCongestionEvent(
       QuicByteCount prior_in_flight,

diff --git a/quic/core/congestion_control/bbr2_probe_bw.h b/quic/core/congestion_control/bbr2_probe_bw.h
index 429b6fe..d64bd65 100644
--- a/quic/core/congestion_control/bbr2_probe_bw.h
+++ b/quic/core/congestion_control/bbr2_probe_bw.h

@@ -20,6 +20,7 @@
   using Bbr2ModeBase::Bbr2ModeBase;
 
   void Enter(const Bbr2CongestionEvent& congestion_event) override;
+  void Leave(const Bbr2CongestionEvent& /*congestion_event*/) override {}
 
   Bbr2Mode OnCongestionEvent(
       QuicByteCount prior_in_flight,

diff --git a/quic/core/congestion_control/bbr2_probe_rtt.h b/quic/core/congestion_control/bbr2_probe_rtt.h
index c112a33..80a9d93 100644
--- a/quic/core/congestion_control/bbr2_probe_rtt.h
+++ b/quic/core/congestion_control/bbr2_probe_rtt.h

@@ -18,6 +18,7 @@
   using Bbr2ModeBase::Bbr2ModeBase;
 
   void Enter(const Bbr2CongestionEvent& congestion_event) override;
+  void Leave(const Bbr2CongestionEvent& /*congestion_event*/) override {}
 
   Bbr2Mode OnCongestionEvent(
       QuicByteCount prior_in_flight,

diff --git a/quic/core/congestion_control/bbr2_sender.cc b/quic/core/congestion_control/bbr2_sender.cc
index c90decd..73787a2 100644
--- a/quic/core/congestion_control/bbr2_sender.cc
+++ b/quic/core/congestion_control/bbr2_sender.cc

@@ -57,11 +57,12 @@
                        QuicPacketCount initial_cwnd_in_packets,
                        QuicPacketCount max_cwnd_in_packets,
                        QuicRandom* random,
-                       QuicConnectionStats* /*stats*/)
+                       QuicConnectionStats* stats)
     : mode_(Bbr2Mode::STARTUP),
       rtt_stats_(rtt_stats),
       unacked_packets_(unacked_packets),
       random_(random),
+      connection_stats_(stats),
       params_(kDefaultMinimumCongestionWindow,
               max_cwnd_in_packets * kDefaultTCPMSS),
       model_(&params_,
@@ -75,7 +76,7 @@
       pacing_rate_(kInitialPacingGain * QuicBandwidth::FromBytesAndTimeDelta(
                                             cwnd_,
                                             rtt_stats->SmoothedOrInitialRtt())),
-      startup_(this, &model_),
+      startup_(this, &model_, now),
       drain_(this, &model_),
       probe_bw_(this, &model_),
       probe_rtt_(this, &model_),
@@ -168,6 +169,7 @@
 
     QUIC_DVLOG(2) << this << " Mode change:  " << mode_ << " ==> " << next_mode
                   << "  @ " << event_time;
+    BBR2_MODE_DISPATCH(Leave(congestion_event));
     mode_ = next_mode;
     BBR2_MODE_DISPATCH(Enter(congestion_event));
     --mode_changes_allowed;

diff --git a/quic/core/congestion_control/bbr2_sender.h b/quic/core/congestion_control/bbr2_sender.h
index bff38f6..f0a3b5f 100644
--- a/quic/core/congestion_control/bbr2_sender.h
+++ b/quic/core/congestion_control/bbr2_sender.h

@@ -30,7 +30,7 @@
              QuicPacketCount initial_cwnd_in_packets,
              QuicPacketCount max_cwnd_in_packets,
              QuicRandom* random,
-             QuicConnectionStats* /*stats*/);
+             QuicConnectionStats* stats);
 
   ~Bbr2Sender() override = default;
 
@@ -157,6 +157,7 @@
   const RttStats* const rtt_stats_;
   const QuicUnackedPacketMap* const unacked_packets_;
   QuicRandom* random_;
+  QuicConnectionStats* connection_stats_;
 
   const Bbr2Params params_;
 

diff --git a/quic/core/congestion_control/bbr2_simulator_test.cc b/quic/core/congestion_control/bbr2_simulator_test.cc
index cab9cb8..005e11d 100644
--- a/quic/core/congestion_control/bbr2_simulator_test.cc
+++ b/quic/core/congestion_control/bbr2_simulator_test.cc

@@ -24,6 +24,10 @@
 #include "net/third_party/quiche/src/quic/test_tools/simulator/switch.h"
 #include "net/third_party/quiche/src/quic/test_tools/simulator/traffic_policer.h"
 
+using testing::AllOf;
+using testing::Ge;
+using testing::Le;
+
 namespace quic {
 
 using CyclePhase = Bbr2ProbeBwMode::CyclePhase;
@@ -676,6 +680,25 @@
   EXPECT_LE(sender_loss_rate_in_packets(), 0.30);
 }
 
+// TODO(wub): Add other slowstart stats to BBRv2.
+TEST_F(Bbr2DefaultTopologyTest, StartupStats) {
+  DefaultTopologyParams params;
+  CreateNetwork(params);
+
+  DriveOutOfStartup(params);
+  ASSERT_FALSE(sender_->InSlowStart());
+
+  const QuicConnectionStats& stats = sender_connection_stats();
+  EXPECT_EQ(1u, stats.slowstart_count);
+  EXPECT_FALSE(stats.slowstart_duration.IsRunning());
+  EXPECT_THAT(stats.slowstart_duration.GetTotalElapsedTime(),
+              AllOf(Ge(QuicTime::Delta::FromMilliseconds(500)),
+                    Le(QuicTime::Delta::FromMilliseconds(1500))));
+  EXPECT_EQ(stats.slowstart_duration.GetTotalElapsedTime(),
+            QuicConnectionPeer::GetSentPacketManager(sender_connection())
+                ->GetSlowStartDuration());
+}
+
 // All Bbr2MultiSenderTests uses the following network topology:
 //
 //   Sender 0  (A Bbr2Sender)

diff --git a/quic/core/congestion_control/bbr2_startup.cc b/quic/core/congestion_control/bbr2_startup.cc
index 5ff1050..187b349 100644
--- a/quic/core/congestion_control/bbr2_startup.cc
+++ b/quic/core/congestion_control/bbr2_startup.cc

@@ -12,17 +12,30 @@
 namespace quic {
 
 Bbr2StartupMode::Bbr2StartupMode(const Bbr2Sender* sender,
-                                 Bbr2NetworkModel* model)
+                                 Bbr2NetworkModel* model,
+                                 QuicTime now)
     : Bbr2ModeBase(sender, model),
       full_bandwidth_reached_(false),
       full_bandwidth_baseline_(QuicBandwidth::Zero()),
       rounds_without_bandwidth_growth_(0),
-      loss_events_in_round_(0) {}
+      loss_events_in_round_(0) {
+  // Clear some startup stats if |sender_->connection_stats_| has been used by
+  // another sender, which happens e.g. when QuicConnection switch send
+  // algorithms.
+  sender_->connection_stats_->slowstart_count = 1;
+  sender_->connection_stats_->slowstart_duration = QuicTimeAccumulator();
+  sender_->connection_stats_->slowstart_duration.Start(now);
+}
 
 void Bbr2StartupMode::Enter(const Bbr2CongestionEvent& /*congestion_event*/) {
   QUIC_BUG << "Bbr2StartupMode::Enter should not be called";
 }
 
+void Bbr2StartupMode::Leave(const Bbr2CongestionEvent& congestion_event) {
+  sender_->connection_stats_->slowstart_duration.Stop(
+      congestion_event.event_time);
+}
+
 Bbr2Mode Bbr2StartupMode::OnCongestionEvent(
     QuicByteCount /*prior_in_flight*/,
     QuicTime /*event_time*/,

diff --git a/quic/core/congestion_control/bbr2_startup.h b/quic/core/congestion_control/bbr2_startup.h
index 489bd55..80539d9 100644
--- a/quic/core/congestion_control/bbr2_startup.h
+++ b/quic/core/congestion_control/bbr2_startup.h

@@ -16,9 +16,12 @@
 class Bbr2Sender;
 class QUIC_EXPORT_PRIVATE Bbr2StartupMode final : public Bbr2ModeBase {
  public:
-  Bbr2StartupMode(const Bbr2Sender* sender, Bbr2NetworkModel* model);
+  Bbr2StartupMode(const Bbr2Sender* sender,
+                  Bbr2NetworkModel* model,
+                  QuicTime now);
 
   void Enter(const Bbr2CongestionEvent& congestion_event) override;
+  void Leave(const Bbr2CongestionEvent& congestion_event) override;
 
   Bbr2Mode OnCongestionEvent(
       QuicByteCount prior_in_flight,

diff --git a/quic/core/congestion_control/bbr_sender.cc b/quic/core/congestion_control/bbr_sender.cc
index 93ee5cb..4217048 100644
--- a/quic/core/congestion_control/bbr_sender.cc
+++ b/quic/core/congestion_control/bbr_sender.cc

@@ -11,6 +11,7 @@
 #include "net/third_party/quiche/src/quic/core/congestion_control/rtt_stats.h"
 #include "net/third_party/quiche/src/quic/core/crypto/crypto_protocol.h"
 #include "net/third_party/quiche/src/quic/core/quic_time.h"
+#include "net/third_party/quiche/src/quic/core/quic_time_accumulator.h"
 #include "net/third_party/quiche/src/quic/platform/api/quic_bug_tracker.h"
 #include "net/third_party/quiche/src/quic/platform/api/quic_fallthrough.h"
 #include "net/third_party/quiche/src/quic/platform/api/quic_flag_utils.h"
@@ -136,8 +137,10 @@
       app_limited_since_last_probe_rtt_(false),
       min_rtt_since_last_probe_rtt_(QuicTime::Delta::Infinite()) {
   if (stats_) {
+    // Clear some startup stats if |stats_| has been used by another sender,
+    // which happens e.g. when QuicConnection switch send algorithms.
     stats_->slowstart_count = 0;
-    stats_->slowstart_start_time = QuicTime::Zero();
+    stats_->slowstart_duration = QuicTimeAccumulator();
   }
   EnterStartupMode(now);
 }
@@ -465,8 +468,7 @@
 void BbrSender::EnterStartupMode(QuicTime now) {
   if (stats_) {
     ++stats_->slowstart_count;
-    DCHECK_EQ(stats_->slowstart_start_time, QuicTime::Zero()) << mode_;
-    stats_->slowstart_start_time = now;
+    stats_->slowstart_duration.Start(now);
   }
   mode_ = STARTUP;
   pacing_gain_ = high_gain_;
@@ -671,12 +673,7 @@
 void BbrSender::OnExitStartup(QuicTime now) {
   DCHECK_EQ(mode_, STARTUP);
   if (stats_) {
-    DCHECK_NE(stats_->slowstart_start_time, QuicTime::Zero());
-    if (now > stats_->slowstart_start_time) {
-      stats_->slowstart_duration =
-          now - stats_->slowstart_start_time + stats_->slowstart_duration;
-    }
-    stats_->slowstart_start_time = QuicTime::Zero();
+    stats_->slowstart_duration.Stop(now);
   }
 }
 

diff --git a/quic/core/congestion_control/bbr_sender_test.cc b/quic/core/congestion_control/bbr_sender_test.cc
index 60b303e..6acfe80 100644
--- a/quic/core/congestion_control/bbr_sender_test.cc
+++ b/quic/core/congestion_control/bbr_sender_test.cc

@@ -1309,11 +1309,11 @@
   EXPECT_THAT(stats.slowstart_bytes_sent, AllOf(Ge(100000u), Le(1000000u)));
   EXPECT_LE(stats.slowstart_packets_lost, 10u);
   EXPECT_LE(stats.slowstart_bytes_lost, 10000u);
-  EXPECT_THAT(stats.slowstart_duration,
+  EXPECT_FALSE(stats.slowstart_duration.IsRunning());
+  EXPECT_THAT(stats.slowstart_duration.GetTotalElapsedTime(),
               AllOf(Ge(QuicTime::Delta::FromMilliseconds(500)),
                     Le(QuicTime::Delta::FromMilliseconds(1500))));
-  EXPECT_EQ(QuicTime::Zero(), stats.slowstart_start_time);
-  EXPECT_EQ(stats.slowstart_duration,
+  EXPECT_EQ(stats.slowstart_duration.GetTotalElapsedTime(),
             QuicConnectionPeer::GetSentPacketManager(bbr_sender_.connection())
                 ->GetSlowStartDuration());
 }

diff --git a/quic/core/quic_connection_stats.cc b/quic/core/quic_connection_stats.cc
index e3652b0..ff067bd 100644
--- a/quic/core/quic_connection_stats.cc
+++ b/quic/core/quic_connection_stats.cc

@@ -26,8 +26,6 @@
       slowstart_bytes_sent(0),
       slowstart_packets_lost(0),
       slowstart_bytes_lost(0),
-      slowstart_duration(QuicTime::Delta::Zero()),
-      slowstart_start_time(QuicTime::Zero()),
       packets_dropped(0),
       undecryptable_packets_received_before_handshake_complete(0),
       crypto_retransmit_count(0),

diff --git a/quic/core/quic_connection_stats.h b/quic/core/quic_connection_stats.h
index a67d867..552e8a6 100644
--- a/quic/core/quic_connection_stats.h
+++ b/quic/core/quic_connection_stats.h

@@ -11,6 +11,7 @@
 #include "net/third_party/quiche/src/quic/core/quic_bandwidth.h"
 #include "net/third_party/quiche/src/quic/core/quic_packets.h"
 #include "net/third_party/quiche/src/quic/core/quic_time.h"
+#include "net/third_party/quiche/src/quic/core/quic_time_accumulator.h"
 #include "net/third_party/quiche/src/quic/platform/api/quic_export.h"
 
 namespace quic {
@@ -60,10 +61,8 @@
   QuicPacketCount slowstart_packets_lost;
   // Number of bytes lost exiting slow start.
   QuicByteCount slowstart_bytes_lost;
-  // Time spent in COMPLETED slow start phases.
-  QuicTime::Delta slowstart_duration;
-  // Start time of the last slow start phase.
-  QuicTime slowstart_start_time;
+  // Time spent in slow start. Populated for BBRv1 and BBRv2.
+  QuicTimeAccumulator slowstart_duration;
 
   QuicPacketCount packets_dropped;  // Duplicate or less than least unacked.
 

diff --git a/quic/core/quic_sent_packet_manager.cc b/quic/core/quic_sent_packet_manager.cc
index ba3c081..bd0343b 100644
--- a/quic/core/quic_sent_packet_manager.cc
+++ b/quic/core/quic_sent_packet_manager.cc

@@ -1077,16 +1077,12 @@
 }
 
 QuicTime::Delta QuicSentPacketManager::GetSlowStartDuration() const {
-  if (send_algorithm_->GetCongestionControlType() != kBBR) {
-    return QuicTime::Delta::Infinite();
+  if (send_algorithm_->GetCongestionControlType() == kBBR ||
+      send_algorithm_->GetCongestionControlType() == kBBRv2) {
+    return stats_->slowstart_duration.GetTotalElapsedTime(
+        clock_->ApproximateNow());
   }
-
-  if (!send_algorithm_->InSlowStart()) {
-    return stats_->slowstart_duration;
-  }
-
-  return clock_->ApproximateNow() - stats_->slowstart_start_time +
-         stats_->slowstart_duration;
+  return QuicTime::Delta::Infinite();
 }
 
 std::string QuicSentPacketManager::GetDebugState() const {

diff --git a/quic/core/quic_time_accumulator.h b/quic/core/quic_time_accumulator.h
new file mode 100644
index 0000000..772e5ff
--- /dev/null
+++ b/quic/core/quic_time_accumulator.h

@@ -0,0 +1,66 @@
+// Copyright (c) 2019 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef QUICHE_QUIC_CORE_QUIC_TIME_ACCUMULATOR_H_
+#define QUICHE_QUIC_CORE_QUIC_TIME_ACCUMULATOR_H_
+
+#include "net/third_party/quiche/src/quic/core/quic_time.h"
+#include "net/third_party/quiche/src/quic/platform/api/quic_export.h"
+#include "net/third_party/quiche/src/quic/platform/api/quic_logging.h"
+
+namespace quic {
+
+// QuicTimeAccumulator accumulates elapsed times between Start(s) and Stop(s).
+class QUIC_EXPORT_PRIVATE QuicTimeAccumulator {
+  static constexpr QuicTime kNotRunningSentinel = QuicTime::Infinite();
+
+ public:
+  // True if Started and not Stopped.
+  bool IsRunning() const { return last_start_time_ != kNotRunningSentinel; }
+
+  void Start(QuicTime now) {
+    DCHECK(!IsRunning());
+    last_start_time_ = now;
+    DCHECK(IsRunning());
+  }
+
+  void Stop(QuicTime now) {
+    DCHECK(IsRunning());
+    if (now > last_start_time_) {
+      total_elapsed_ = total_elapsed_ + (now - last_start_time_);
+    }
+    last_start_time_ = kNotRunningSentinel;
+    DCHECK(!IsRunning());
+  }
+
+  // Get total elapsed time between COMPLETED Start/Stop pairs.
+  QuicTime::Delta GetTotalElapsedTime() const { return total_elapsed_; }
+
+  // Get total elapsed time between COMPLETED Start/Stop pairs, plus, if it is
+  // running, the elapsed time between |last_start_time_| and |now|.
+  QuicTime::Delta GetTotalElapsedTime(QuicTime now) const {
+    if (!IsRunning()) {
+      return total_elapsed_;
+    }
+    if (now <= last_start_time_) {
+      return total_elapsed_;
+    }
+    return total_elapsed_ + (now - last_start_time_);
+  }
+
+ private:
+  //
+  //                                       |last_start_time_|
+  //                                         |
+  //                                         V
+  // Start => Stop  =>  Start => Stop  =>  Start
+  // |           |      |           |
+  // |___________|  +   |___________|  =   |total_elapsed_|
+  QuicTime::Delta total_elapsed_ = QuicTime::Delta::Zero();
+  QuicTime last_start_time_ = kNotRunningSentinel;
+};
+
+}  // namespace quic
+
+#endif  // QUICHE_QUIC_CORE_QUIC_TIME_ACCUMULATOR_H_

diff --git a/quic/core/quic_time_accumulator_test.cc b/quic/core/quic_time_accumulator_test.cc
new file mode 100644
index 0000000..e7bc43a
--- /dev/null
+++ b/quic/core/quic_time_accumulator_test.cc

@@ -0,0 +1,82 @@
+// Copyright (c) 2019 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "net/third_party/quiche/src/quic/core/quic_time_accumulator.h"
+#include "net/third_party/quiche/src/quic/platform/api/quic_test.h"
+#include "net/third_party/quiche/src/quic/test_tools/mock_clock.h"
+
+namespace quic {
+namespace test {
+
+TEST(QuicTimeAccumulator, DefaultConstruct) {
+  MockClock clock;
+  clock.AdvanceTime(QuicTime::Delta::FromMilliseconds(1));
+
+  QuicTimeAccumulator acc;
+  EXPECT_FALSE(acc.IsRunning());
+
+  clock.AdvanceTime(QuicTime::Delta::FromMilliseconds(1));
+  EXPECT_EQ(QuicTime::Delta::Zero(), acc.GetTotalElapsedTime());
+  EXPECT_EQ(QuicTime::Delta::Zero(), acc.GetTotalElapsedTime(clock.Now()));
+}
+
+TEST(QuicTimeAccumulator, StartStop) {
+  MockClock clock;
+  clock.AdvanceTime(QuicTime::Delta::FromMilliseconds(1));
+
+  QuicTimeAccumulator acc;
+  acc.Start(clock.Now());
+  EXPECT_TRUE(acc.IsRunning());
+
+  clock.AdvanceTime(QuicTime::Delta::FromMilliseconds(10));
+  acc.Stop(clock.Now());
+  EXPECT_FALSE(acc.IsRunning());
+
+  clock.AdvanceTime(QuicTime::Delta::FromMilliseconds(5));
+  EXPECT_EQ(QuicTime::Delta::FromMilliseconds(10), acc.GetTotalElapsedTime());
+  EXPECT_EQ(QuicTime::Delta::FromMilliseconds(10),
+            acc.GetTotalElapsedTime(clock.Now()));
+
+  acc.Start(clock.Now());
+  clock.AdvanceTime(QuicTime::Delta::FromMilliseconds(5));
+  EXPECT_EQ(QuicTime::Delta::FromMilliseconds(10), acc.GetTotalElapsedTime());
+  EXPECT_EQ(QuicTime::Delta::FromMilliseconds(15),
+            acc.GetTotalElapsedTime(clock.Now()));
+
+  clock.AdvanceTime(QuicTime::Delta::FromMilliseconds(5));
+  EXPECT_EQ(QuicTime::Delta::FromMilliseconds(10), acc.GetTotalElapsedTime());
+  EXPECT_EQ(QuicTime::Delta::FromMilliseconds(20),
+            acc.GetTotalElapsedTime(clock.Now()));
+
+  acc.Stop(clock.Now());
+  EXPECT_EQ(QuicTime::Delta::FromMilliseconds(20), acc.GetTotalElapsedTime());
+  EXPECT_EQ(QuicTime::Delta::FromMilliseconds(20),
+            acc.GetTotalElapsedTime(clock.Now()));
+}
+
+TEST(QuicTimeAccumulator, ClockStepBackwards) {
+  MockClock clock;
+  clock.AdvanceTime(QuicTime::Delta::FromMilliseconds(100));
+
+  QuicTimeAccumulator acc;
+  acc.Start(clock.Now());
+
+  clock.AdvanceTime(QuicTime::Delta::FromMilliseconds(-10));
+  acc.Stop(clock.Now());
+  EXPECT_EQ(QuicTime::Delta::Zero(), acc.GetTotalElapsedTime());
+  EXPECT_EQ(QuicTime::Delta::Zero(), acc.GetTotalElapsedTime(clock.Now()));
+
+  acc.Start(clock.Now());
+  clock.AdvanceTime(QuicTime::Delta::FromMilliseconds(50));
+  acc.Stop(clock.Now());
+
+  acc.Start(clock.Now());
+  clock.AdvanceTime(QuicTime::Delta::FromMilliseconds(-80));
+  EXPECT_EQ(QuicTime::Delta::FromMilliseconds(50), acc.GetTotalElapsedTime());
+  EXPECT_EQ(QuicTime::Delta::FromMilliseconds(50),
+            acc.GetTotalElapsedTime(clock.Now()));
+}
+
+}  // namespace test
+}  // namespace quic