From bfcb6c3f13f776c72db2f405d823e1e582695543 Mon Sep 17 00:00:00 2001 From: Jonas Oreland Date: Thu, 28 Nov 2019 09:40:01 +0100 Subject: [PATCH] Add rtt estimate EventBasedExponentialMovingAverage to Connection This patch estimates the connection RTT using EventBasedExponentialMovingAverage. The half time is set to 500 but can be modified using field trials. This new metric is currently unused, but will be used for exploration of of whether it can be used instead of the existing metric. Bug: webrtc:11140 Change-Id: I9db93e9b9eb932e3cd18935cd4ce0d90fc1cb293 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/161000 Commit-Queue: Jonas Oreland Reviewed-by: Harald Alvestrand Cr-Commit-Position: refs/heads/master@{#29944} --- p2p/BUILD.gn | 1 + p2p/base/connection.cc | 18 +++++- p2p/base/connection.h | 10 +++ p2p/base/p2p_transport_channel.cc | 4 +- .../p2p_transport_channel_ice_field_trials.h | 4 ++ .../event_based_exponential_moving_average.cc | 22 ++++++- .../event_based_exponential_moving_average.h | 9 ++- ...sed_exponential_moving_average_unittest.cc | 61 ++++++++++++++++++- 8 files changed, 121 insertions(+), 8 deletions(-) diff --git a/p2p/BUILD.gn b/p2p/BUILD.gn index 301b86c066..747609e1ca 100644 --- a/p2p/BUILD.gn +++ b/p2p/BUILD.gn @@ -98,6 +98,7 @@ rtc_library("rtc_p2p") { "../logging:ice_log", "../rtc_base", "../rtc_base:checks", + "../rtc_base:rtc_numerics", "../rtc_base/experiments:field_trial_parser", "//third_party/abseil-cpp/absl/memory", diff --git a/p2p/base/connection.cc b/p2p/base/connection.cc index cb8c7c7e3d..bea98cf029 100644 --- a/p2p/base/connection.cc +++ b/p2p/base/connection.cc @@ -128,6 +128,8 @@ const int DEFAULT_RTT = 3000; // 3 seconds const int MINIMUM_RTT = 100; // 0.1 seconds const int MAXIMUM_RTT = 60000; // 60 seconds +const int DEFAULT_RTT_ESTIMATE_HALF_TIME_MS = 500; + // Computes our estimate of the RTT given the current estimate. inline int ConservativeRTTEstimate(int rtt) { return rtc::SafeClamp(2 * rtt, MINIMUM_RTT, MAXIMUM_RTT); @@ -138,6 +140,9 @@ const int RTT_RATIO = 3; // 3 : 1 constexpr int64_t kMinExtraPingDelayMs = 100; +// Default field trials. +const cricket::IceFieldTrials kDefaultFieldTrials; + } // namespace namespace cricket { @@ -267,7 +272,9 @@ Connection::Connection(Port* port, last_ping_response_received_(0), reported_(false), state_(IceCandidatePairState::WAITING), - time_created_ms_(rtc::TimeMillis()) { + time_created_ms_(rtc::TimeMillis()), + field_trials_(&kDefaultFieldTrials), + rtt_estimate_(DEFAULT_RTT_ESTIMATE_HALF_TIME_MS) { // All of our connections start in WAITING state. // TODO(mallinath) - Start connections from STATE_FROZEN. // Wire up to send stun packets @@ -391,6 +398,11 @@ int Connection::receiving_timeout() const { return receiving_timeout_.value_or(WEAK_CONNECTION_RECEIVE_TIMEOUT); } +void Connection::SetIceFieldTrials(const IceFieldTrials* field_trials) { + field_trials_ = field_trials; + rtt_estimate_.SetHalfTime(field_trials->rtt_estimate_halftime_ms); +} + void Connection::OnSendStunPacket(const void* data, size_t size, StunRequest* req) { @@ -741,11 +753,13 @@ void Connection::ReceivedPingResponse( acked_nomination_ = nomination.value(); } + int64_t now = rtc::TimeMillis(); total_round_trip_time_ms_ += rtt; current_round_trip_time_ms_ = static_cast(rtt); + rtt_estimate_.AddSample(now, rtt); pings_since_last_response_.clear(); - last_ping_response_received_ = rtc::TimeMillis(); + last_ping_response_received_ = now; UpdateReceiving(last_ping_response_received_); set_write_state(STATE_WRITABLE); set_state(IceCandidatePairState::SUCCEEDED); diff --git a/p2p/base/connection.h b/p2p/base/connection.h index fa9a519307..bc37429cda 100644 --- a/p2p/base/connection.h +++ b/p2p/base/connection.h @@ -20,11 +20,13 @@ #include "logging/rtc_event_log/ice_logger.h" #include "p2p/base/candidate_pair_interface.h" #include "p2p/base/connection_info.h" +#include "p2p/base/p2p_transport_channel_ice_field_trials.h" #include "p2p/base/stun_request.h" #include "p2p/base/transport_description.h" #include "rtc_base/async_packet_socket.h" #include "rtc_base/message_handler.h" #include "rtc_base/network.h" +#include "rtc_base/numerics/event_based_exponential_moving_average.h" #include "rtc_base/rate_tracker.h" namespace cricket { @@ -302,6 +304,11 @@ class Connection : public CandidatePairInterface, Port* PortForTest() { return port_; } const Port* PortForTest() const { return port_; } + void SetIceFieldTrials(const IceFieldTrials* field_trials); + const rtc::EventBasedExponentialMovingAverage& GetRttEstimate() const { + return rtt_estimate_; + } + protected: enum { MSG_DELETE = 0, MSG_FIRST_AVAILABLE }; @@ -414,6 +421,9 @@ class Connection : public CandidatePairInterface, absl::optional log_description_; webrtc::IceEventLog* ice_event_log_ = nullptr; + const IceFieldTrials* field_trials_; + rtc::EventBasedExponentialMovingAverage rtt_estimate_; + friend class Port; friend class ConnectionRequest; friend class P2PTransportChannel; diff --git a/p2p/base/p2p_transport_channel.cc b/p2p/base/p2p_transport_channel.cc index e26b06549b..8d67690f80 100644 --- a/p2p/base/p2p_transport_channel.cc +++ b/p2p/base/p2p_transport_channel.cc @@ -207,6 +207,7 @@ void P2PTransportChannel::AddConnection(Connection* connection) { had_connection_ = true; connection->set_ice_event_log(&ice_event_log_); + connection->SetIceFieldTrials(&field_trials_); LogCandidatePairConfig(connection, webrtc::IceCandidatePairConfigType::kAdded); @@ -646,7 +647,8 @@ void P2PTransportChannel::SetIceConfig(const IceConfig& config) { "max_outstanding_pings", &field_trials_.max_outstanding_pings, "initial_select_dampening", &field_trials_.initial_select_dampening, "initial_select_dampening_ping_received", - &field_trials_.initial_select_dampening_ping_received) + &field_trials_.initial_select_dampening_ping_received, + "rtt_estimate_halftime_ms", &field_trials_.rtt_estimate_halftime_ms) ->Parse(webrtc::field_trial::FindFullName("WebRTC-IceFieldTrials")); if (field_trials_.skip_relay_to_non_relay_connections) { diff --git a/p2p/base/p2p_transport_channel_ice_field_trials.h b/p2p/base/p2p_transport_channel_ice_field_trials.h index 60a3777482..e0854a15c7 100644 --- a/p2p/base/p2p_transport_channel_ice_field_trials.h +++ b/p2p/base/p2p_transport_channel_ice_field_trials.h @@ -31,6 +31,10 @@ struct IceFieldTrials { // maximum this delay. This will make media slower, but will // give us chance to find a better connection before starting. absl::optional initial_select_dampening_ping_received; + + // Decay rate for RTT estimate using EventBasedExponentialMovingAverage + // expressed as halving time. + int rtt_estimate_halftime_ms = 500; }; } // namespace cricket diff --git a/rtc_base/numerics/event_based_exponential_moving_average.cc b/rtc_base/numerics/event_based_exponential_moving_average.cc index 18242bd5f9..36c5b8994c 100644 --- a/rtc_base/numerics/event_based_exponential_moving_average.cc +++ b/rtc_base/numerics/event_based_exponential_moving_average.cc @@ -28,14 +28,30 @@ namespace rtc { // a sample gets exponentially less weight so that it's 50% // after |half_time| time units has passed. EventBasedExponentialMovingAverage::EventBasedExponentialMovingAverage( - int half_time) - : tau_(static_cast(half_time) / log(2)) {} + int half_time) { + SetHalfTime(half_time); +} + +void EventBasedExponentialMovingAverage::SetHalfTime(int half_time) { + tau_ = static_cast(half_time) / log(2); + Reset(); +} + +void EventBasedExponentialMovingAverage::Reset() { + value_ = std::nan("uninit"); + sample_variance_ = std::numeric_limits::infinity(); + estimator_variance_ = 1; + last_observation_timestamp_.reset(); +} void EventBasedExponentialMovingAverage::AddSample(int64_t now, int sample) { if (!last_observation_timestamp_.has_value()) { value_ = sample; } else { - RTC_DCHECK(now > *last_observation_timestamp_); + // TODO(webrtc:11140): This should really be > (e.g not >=) + // but some pesky tests run with simulated clock and let + // samples arrive simultaneously! + RTC_DCHECK(now >= *last_observation_timestamp_); // Variance gets computed after second sample. int64_t age = now - *last_observation_timestamp_; double e = exp(-age / tau_); diff --git a/rtc_base/numerics/event_based_exponential_moving_average.h b/rtc_base/numerics/event_based_exponential_moving_average.h index a72aa271ef..352b55f5ae 100644 --- a/rtc_base/numerics/event_based_exponential_moving_average.h +++ b/rtc_base/numerics/event_based_exponential_moving_average.h @@ -49,8 +49,15 @@ class EventBasedExponentialMovingAverage { // [ X +/- m ]. double GetConfidenceInterval() const; + // Reset + void Reset(); + + // Update the half_time. + // NOTE: resets estimate too. + void SetHalfTime(int half_time); + private: - const double tau_; + double tau_; double value_ = std::nan("uninit"); double sample_variance_ = std::numeric_limits::infinity(); // This is the ratio between variance of the estimate and variance of samples. diff --git a/rtc_base/numerics/event_based_exponential_moving_average_unittest.cc b/rtc_base/numerics/event_based_exponential_moving_average_unittest.cc index 53b094e10e..967be41213 100644 --- a/rtc_base/numerics/event_based_exponential_moving_average_unittest.cc +++ b/rtc_base/numerics/event_based_exponential_moving_average_unittest.cc @@ -92,7 +92,7 @@ TEST(EventBasedExponentialMovingAverageTest, Almost100) { // Test that getting a value at X and another at X+1 // is almost the same as getting another at X and a value at X+1. -TEST(EventBasedExponentialMovingAverageTest, SameTime) { +TEST(EventBasedExponentialMovingAverageTest, AlmostSameTime) { int64_t time = 23; constexpr int value = 100; @@ -165,4 +165,63 @@ TEST(EventBasedExponentialMovingAverageTest, NonUniformSamplesHalftime100) { } } +TEST(EventBasedExponentialMovingAverageTest, Reset) { + constexpr int64_t time = 23; + constexpr int value = 100; + + EventBasedExponentialMovingAverage average(100); + EXPECT_TRUE(std::isnan(average.GetAverage())); + EXPECT_EQ(std::numeric_limits::infinity(), average.GetVariance()); + EXPECT_EQ(std::numeric_limits::infinity(), + average.GetConfidenceInterval()); + + average.AddSample(time + 0, value); + average.AddSample(time + 100, value); + average.AddSample(time + 101, 0); + EXPECT_FALSE(std::isnan(average.GetAverage())); + + average.Reset(); + EXPECT_TRUE(std::isnan(average.GetAverage())); + EXPECT_EQ(std::numeric_limits::infinity(), average.GetVariance()); + EXPECT_EQ(std::numeric_limits::infinity(), + average.GetConfidenceInterval()); +} + +// Test that SetHalfTime modifies behavior and resets average. +TEST(EventBasedExponentialMovingAverageTest, SetHalfTime) { + constexpr int64_t time = 23; + constexpr int value = 100; + + EventBasedExponentialMovingAverage average(100); + + average.AddSample(time + 0, value); + average.AddSample(time + 100, 0); + EXPECT_NEAR(66.7, average.GetAverage(), kError); + + average.SetHalfTime(1000); + EXPECT_TRUE(std::isnan(average.GetAverage())); + EXPECT_EQ(std::numeric_limits::infinity(), average.GetVariance()); + EXPECT_EQ(std::numeric_limits::infinity(), + average.GetConfidenceInterval()); + + average.AddSample(time + 0, value); + average.AddSample(time + 100, 0); + EXPECT_NEAR(51.7, average.GetAverage(), kError); +} + +TEST(EventBasedExponentialMovingAverageTest, SimultaneousSamples) { + constexpr int64_t time = 23; + constexpr int value = 100; + + EventBasedExponentialMovingAverage average(100); + + average.AddSample(time, value); + // This should really NOT be supported, + // i.e 2 samples with same timestamp. + // But there are tests running with simulated clock + // that produce this. + // TODO(webrtc:11140) : Fix those tests and remove this! + average.AddSample(time, value); +} + } // namespace rtc