ICE : add field trial for initial select dampening
The existing ICE stack will choose *the first* connection
that becomes writable.
It is possible that waiting a fixed time will choose a
better connection, avoiding a switch, and making the experience better
in total.
This patch is add two field trials to *explore*
that dimension. I.e the code will be rolled back once
experiments has been performed.
- initial_select_dampening, delays selection by X ms.
- initial_select_dampening_ping_received, delays selection for
candidate that has received ping by X ms.
BUG=webrtc:11054
Change-Id: Ifcdde5183f318815e0f5db5802fbf6b542a95f5b
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/158410
Reviewed-by: Honghai Zhang <honghaiz@webrtc.org>
Commit-Queue: Jonas Oreland <jonaso@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#29623}
diff --git a/p2p/base/p2p_transport_channel.cc b/p2p/base/p2p_transport_channel.cc
index f95e7ab..7f2b37a 100644
--- a/p2p/base/p2p_transport_channel.cc
+++ b/p2p/base/p2p_transport_channel.cc
@@ -278,10 +278,80 @@
return new_connection->rtt() <= selected_connection_->rtt() - kMinImprovement;
}
+bool P2PTransportChannel::HandleInitialSelectDampening(
+ Connection* new_connection,
+ const std::string& reason) {
+ RTC_DCHECK_RUN_ON(network_thread_);
+ if (!field_trials_.initial_select_dampening.has_value() &&
+ !field_trials_.initial_select_dampening_ping_received.has_value()) {
+ // experiment not enabled.
+ return true;
+ }
+
+ int64_t now = rtc::TimeMillis();
+ int64_t max_delay = 0;
+ if (new_connection->last_ping_received() > 0 &&
+ field_trials_.initial_select_dampening_ping_received.has_value()) {
+ max_delay = *field_trials_.initial_select_dampening_ping_received;
+ } else if (field_trials_.initial_select_dampening.has_value()) {
+ max_delay = *field_trials_.initial_select_dampening;
+ }
+
+ int64_t start_wait =
+ initial_select_timestamp_ms_ == 0 ? now : initial_select_timestamp_ms_;
+ int64_t max_wait_until = start_wait + max_delay;
+
+ if (now >= max_wait_until) {
+ RTC_LOG(LS_INFO) << "reset initial_select_timestamp_ = "
+ << initial_select_timestamp_ms_
+ << " selection delayed by: " << (now - start_wait) << "ms";
+ initial_select_timestamp_ms_ = 0;
+ return true;
+ }
+
+ // We are not yet ready to select first connection...
+ if (initial_select_timestamp_ms_ == 0) {
+ // Set timestamp on first time...
+ // but run the delayed invokation everytime to
+ // avoid possibility that we miss it.
+ initial_select_timestamp_ms_ = now;
+ RTC_LOG(LS_INFO) << "set initial_select_timestamp_ms_ = "
+ << initial_select_timestamp_ms_;
+ }
+
+ int min_delay = max_delay;
+ if (field_trials_.initial_select_dampening.has_value()) {
+ min_delay = std::min(min_delay, *field_trials_.initial_select_dampening);
+ }
+ if (field_trials_.initial_select_dampening_ping_received.has_value()) {
+ min_delay = std::min(min_delay,
+ *field_trials_.initial_select_dampening_ping_received);
+ }
+
+ const std::string reason_to_sort =
+ reason + " (after initial select dampening interval: " +
+ std::to_string(max_delay) + ")";
+ invoker_.AsyncInvokeDelayed<void>(
+ RTC_FROM_HERE, thread(),
+ rtc::Bind(&P2PTransportChannel::SortConnectionsAndUpdateState, this,
+ reason_to_sort),
+ min_delay);
+ RTC_LOG(LS_INFO) << "delay initial selection up to " << min_delay << "ms";
+ return false;
+}
+
bool P2PTransportChannel::MaybeSwitchSelectedConnection(
Connection* new_connection,
const std::string& reason) {
RTC_DCHECK_RUN_ON(network_thread_);
+
+ if (selected_connection_ == nullptr && ReadyToSend(new_connection)) {
+ if (!HandleInitialSelectDampening(new_connection, reason)) {
+ // Delay the initial selection a while waiting for a better connection.
+ return false;
+ }
+ }
+
bool missed_receiving_unchanged_threshold = false;
if (ShouldSwitchSelectedConnection(new_connection,
&missed_receiving_unchanged_threshold)) {
@@ -699,7 +769,10 @@
webrtc::StructParametersParser::Create(
"skip_relay_to_non_relay_connections",
&field_trials_.skip_relay_to_non_relay_connections,
- "max_outstanding_pings", &field_trials_.max_outstanding_pings)
+ "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)
->Parse(webrtc::field_trial::FindFullName("WebRTC-IceFieldTrials"));
if (field_trials_.skip_relay_to_non_relay_connections) {
@@ -711,6 +784,16 @@
<< *field_trials_.max_outstanding_pings;
}
+ if (field_trials_.initial_select_dampening.has_value()) {
+ RTC_LOG(LS_INFO) << "Set initial_select_dampening: "
+ << *field_trials_.initial_select_dampening;
+ }
+
+ if (field_trials_.initial_select_dampening_ping_received.has_value()) {
+ RTC_LOG(LS_INFO) << "Set initial_select_dampening_ping_received: "
+ << *field_trials_.initial_select_dampening_ping_received;
+ }
+
webrtc::BasicRegatheringController::Config regathering_config(
config_.regather_all_networks_interval_range,
config_.regather_on_failed_networks_interval_or_default());
diff --git a/p2p/base/p2p_transport_channel.h b/p2p/base/p2p_transport_channel.h
index a7a1fbe..7ce0651 100644
--- a/p2p/base/p2p_transport_channel.h
+++ b/p2p/base/p2p_transport_channel.h
@@ -78,6 +78,16 @@
struct IceFieldTrials {
bool skip_relay_to_non_relay_connections = false;
absl::optional<int> max_outstanding_pings;
+
+ // Wait X ms before selecting a connection when having none.
+ // This will make media slower, but will give us chance to find
+ // a better connection before starting.
+ absl::optional<int> initial_select_dampening;
+
+ // If the connection has recevied a ping-request, delay by
+ // maximum this delay. This will make media slower, but will
+ // give us chance to find a better connection before starting.
+ absl::optional<int> initial_select_dampening_ping_received;
};
// P2PTransportChannel manages the candidates and connection process to keep
@@ -418,6 +428,9 @@
// 2. Peer-reflexive remote candidates.
Candidate SanitizeRemoteCandidate(const Candidate& c) const;
+ bool HandleInitialSelectDampening(Connection* new_connection,
+ const std::string& reason);
+
std::string transport_name_ RTC_GUARDED_BY(network_thread_);
int component_ RTC_GUARDED_BY(network_thread_);
PortAllocator* allocator_ RTC_GUARDED_BY(network_thread_);
@@ -508,6 +521,9 @@
IceFieldTrials field_trials_;
+ // Timestamp for when we got the first selectable connection.
+ int64_t initial_select_timestamp_ms_ = 0;
+
RTC_DISALLOW_COPY_AND_ASSIGN(P2PTransportChannel);
};
diff --git a/p2p/base/p2p_transport_channel_unittest.cc b/p2p/base/p2p_transport_channel_unittest.cc
index 0c3474b..3919b3f 100644
--- a/p2p/base/p2p_transport_channel_unittest.cc
+++ b/p2p/base/p2p_transport_channel_unittest.cc
@@ -2142,7 +2142,7 @@
CreateUdpCandidate(RELAY_PORT_TYPE, "2.2.2.2", 2, 0));
// Expect that the TURN-TURN candidate pair will be prioritized since it's
// "probably writable".
- EXPECT_TRUE(ep1_ch1()->selected_connection() != nullptr);
+ EXPECT_TRUE_WAIT(ep1_ch1()->selected_connection() != nullptr, kShortTimeout);
EXPECT_EQ(RELAY_PORT_TYPE, LocalCandidate(ep1_ch1())->type());
EXPECT_EQ(RELAY_PORT_TYPE, RemoteCandidate(ep1_ch1())->type());
// Also expect that the channel instantly indicates that it's writable since
@@ -2265,7 +2265,7 @@
ep1_ch1()->AddRemoteCandidate(
CreateUdpCandidate(RELAY_PORT_TYPE, "1.1.1.1", 1, 0));
// Sanity checking the type of the connection.
- EXPECT_TRUE(ep1_ch1()->selected_connection() != nullptr);
+ EXPECT_TRUE_WAIT(ep1_ch1()->selected_connection() != nullptr, kShortTimeout);
EXPECT_EQ(RELAY_PORT_TYPE, LocalCandidate(ep1_ch1())->type());
EXPECT_EQ(RELAY_PORT_TYPE, RemoteCandidate(ep1_ch1())->type());
@@ -5575,4 +5575,105 @@
test_invariants();
}
+TEST_F(P2PTransportChannelPingTest, TestInitialSelectDampening0) {
+ webrtc::test::ScopedFieldTrials field_trials(
+ "WebRTC-IceFieldTrials/initial_select_dampening:0/");
+
+ constexpr int kMargin = 10;
+ rtc::ScopedFakeClock clock;
+ clock.AdvanceTime(webrtc::TimeDelta::seconds(1));
+
+ FakePortAllocator pa(rtc::Thread::Current(), nullptr);
+ P2PTransportChannel ch("test channel", 1, &pa);
+ PrepareChannel(&ch);
+ ch.SetIceConfig(ch.config());
+ ch.MaybeStartGathering();
+
+ ch.AddRemoteCandidate(CreateUdpCandidate(LOCAL_PORT_TYPE, "1.1.1.1", 1, 100));
+ Connection* conn1 = WaitForConnectionTo(&ch, "1.1.1.1", 1, &clock);
+ ASSERT_TRUE(conn1 != nullptr);
+ EXPECT_EQ(nullptr, ch.selected_connection());
+ conn1->ReceivedPingResponse(LOW_RTT, "id"); // Becomes writable and receiving
+ // It shall not be selected until 0ms has passed....i.e it should be connected
+ // directly.
+ EXPECT_EQ_SIMULATED_WAIT(conn1, ch.selected_connection(), kMargin, clock);
+}
+
+TEST_F(P2PTransportChannelPingTest, TestInitialSelectDampening) {
+ webrtc::test::ScopedFieldTrials field_trials(
+ "WebRTC-IceFieldTrials/initial_select_dampening:100/");
+
+ constexpr int kMargin = 10;
+ rtc::ScopedFakeClock clock;
+ clock.AdvanceTime(webrtc::TimeDelta::seconds(1));
+
+ FakePortAllocator pa(rtc::Thread::Current(), nullptr);
+ P2PTransportChannel ch("test channel", 1, &pa);
+ PrepareChannel(&ch);
+ ch.SetIceConfig(ch.config());
+ ch.MaybeStartGathering();
+
+ ch.AddRemoteCandidate(CreateUdpCandidate(LOCAL_PORT_TYPE, "1.1.1.1", 1, 100));
+ Connection* conn1 = WaitForConnectionTo(&ch, "1.1.1.1", 1, &clock);
+ ASSERT_TRUE(conn1 != nullptr);
+ EXPECT_EQ(nullptr, ch.selected_connection());
+ conn1->ReceivedPingResponse(LOW_RTT, "id"); // Becomes writable and receiving
+ // It shall not be selected until 100ms has passed.
+ SIMULATED_WAIT(conn1 == ch.selected_connection(), 100 - kMargin, clock);
+ EXPECT_EQ_SIMULATED_WAIT(conn1, ch.selected_connection(), 2 * kMargin, clock);
+}
+
+TEST_F(P2PTransportChannelPingTest, TestInitialSelectDampeningPingReceived) {
+ webrtc::test::ScopedFieldTrials field_trials(
+ "WebRTC-IceFieldTrials/initial_select_dampening_ping_received:100/");
+
+ constexpr int kMargin = 10;
+ rtc::ScopedFakeClock clock;
+ clock.AdvanceTime(webrtc::TimeDelta::seconds(1));
+
+ FakePortAllocator pa(rtc::Thread::Current(), nullptr);
+ P2PTransportChannel ch("test channel", 1, &pa);
+ PrepareChannel(&ch);
+ ch.SetIceConfig(ch.config());
+ ch.MaybeStartGathering();
+
+ ch.AddRemoteCandidate(CreateUdpCandidate(LOCAL_PORT_TYPE, "1.1.1.1", 1, 100));
+ Connection* conn1 = WaitForConnectionTo(&ch, "1.1.1.1", 1, &clock);
+ ASSERT_TRUE(conn1 != nullptr);
+ EXPECT_EQ(nullptr, ch.selected_connection());
+ conn1->ReceivedPingResponse(LOW_RTT, "id"); // Becomes writable and receiving
+ conn1->ReceivedPing("id1"); //
+ // It shall not be selected until 100ms has passed.
+ SIMULATED_WAIT(conn1 == ch.selected_connection(), 100 - kMargin, clock);
+ EXPECT_EQ_SIMULATED_WAIT(conn1, ch.selected_connection(), 2 * kMargin, clock);
+}
+
+TEST_F(P2PTransportChannelPingTest, TestInitialSelectDampeningBoth) {
+ webrtc::test::ScopedFieldTrials field_trials(
+ "WebRTC-IceFieldTrials/"
+ "initial_select_dampening:100,initial_select_dampening_ping_received:"
+ "50/");
+
+ constexpr int kMargin = 10;
+ rtc::ScopedFakeClock clock;
+ clock.AdvanceTime(webrtc::TimeDelta::seconds(1));
+
+ FakePortAllocator pa(rtc::Thread::Current(), nullptr);
+ P2PTransportChannel ch("test channel", 1, &pa);
+ PrepareChannel(&ch);
+ ch.SetIceConfig(ch.config());
+ ch.MaybeStartGathering();
+
+ ch.AddRemoteCandidate(CreateUdpCandidate(LOCAL_PORT_TYPE, "1.1.1.1", 1, 100));
+ Connection* conn1 = WaitForConnectionTo(&ch, "1.1.1.1", 1, &clock);
+ ASSERT_TRUE(conn1 != nullptr);
+ EXPECT_EQ(nullptr, ch.selected_connection());
+ conn1->ReceivedPingResponse(LOW_RTT, "id"); // Becomes writable and receiving
+ // It shall not be selected until 100ms has passed....but only wait ~50 now.
+ SIMULATED_WAIT(conn1 == ch.selected_connection(), 50 - kMargin, clock);
+ // Now receiving ping and new timeout should kick in.
+ conn1->ReceivedPing("id1"); //
+ EXPECT_EQ_SIMULATED_WAIT(conn1, ch.selected_connection(), 2 * kMargin, clock);
+}
+
} // namespace cricket
