Delete WebRTC-SlackedTaskQueuePacer experiment
R=hbos@webrtc.org, sprang@webrtc.org
Bug: webrtc:14913
Change-Id: I1ea9d5bda798ea01fa9ec2a9b8d96cb50ccb9ec2
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/293920
Reviewed-by: Erik Språng <sprang@webrtc.org>
Reviewed-by: Henrik Boström <hbos@webrtc.org>
Commit-Queue: Evan Shrubsole <eshr@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#39356}
diff --git a/modules/pacing/task_queue_paced_sender.cc b/modules/pacing/task_queue_paced_sender.cc
index 4ba2495..bfe9792 100644
--- a/modules/pacing/task_queue_paced_sender.cc
+++ b/modules/pacing/task_queue_paced_sender.cc
@@ -13,13 +13,11 @@
#include <algorithm>
#include <utility>
-#include "absl/memory/memory.h"
#include "api/task_queue/pending_task_safety_flag.h"
#include "api/transport/network_types.h"
#include "rtc_base/checks.h"
#include "rtc_base/experiments/field_trial_parser.h"
#include "rtc_base/experiments/field_trial_units.h"
-#include "rtc_base/system/unused.h"
#include "rtc_base/trace_event.h"
namespace webrtc {
@@ -28,9 +26,6 @@
constexpr const char* kBurstyPacerFieldTrial = "WebRTC-BurstyPacer";
-constexpr const char* kSlackedTaskQueuePacedSenderFieldTrial =
- "WebRTC-SlackedTaskQueuePacedSender";
-
} // namespace
const int TaskQueuePacedSender::kNoPacketHoldback = -1;
@@ -41,16 +36,6 @@
ParseFieldTrial({&burst}, field_trials.Lookup(kBurstyPacerFieldTrial));
}
-TaskQueuePacedSender::SlackedPacerFlags::SlackedPacerFlags(
- const FieldTrialsView& field_trials)
- : allow_low_precision("Enabled"),
- max_low_precision_expected_queue_time("max_queue_time"),
- send_burst_interval("send_burst_interval") {
- ParseFieldTrial({&allow_low_precision, &max_low_precision_expected_queue_time,
- &send_burst_interval},
- field_trials.Lookup(kSlackedTaskQueuePacedSenderFieldTrial));
-}
-
TaskQueuePacedSender::TaskQueuePacedSender(
Clock* clock,
PacingController::PacketSender* packet_sender,
@@ -61,13 +46,8 @@
absl::optional<TimeDelta> burst_interval)
: clock_(clock),
bursty_pacer_flags_(field_trials),
- slacked_pacer_flags_(field_trials),
- max_hold_back_window_(slacked_pacer_flags_.allow_low_precision
- ? PacingController::kMinSleepTime
- : max_hold_back_window),
- max_hold_back_window_in_packets_(slacked_pacer_flags_.allow_low_precision
- ? 0
- : max_hold_back_window_in_packets),
+ max_hold_back_window_(max_hold_back_window),
+ max_hold_back_window_in_packets_(max_hold_back_window_in_packets),
pacing_controller_(clock, packet_sender, field_trials),
next_process_time_(Timestamp::MinusInfinity()),
is_started_(false),
@@ -79,13 +59,6 @@
// There are multiple field trials that can affect burst. If multiple bursts
// are specified we pick the largest of the values.
absl::optional<TimeDelta> burst = bursty_pacer_flags_.burst.GetOptional();
- if (slacked_pacer_flags_.allow_low_precision &&
- slacked_pacer_flags_.send_burst_interval) {
- TimeDelta slacked_burst = slacked_pacer_flags_.send_burst_interval.Value();
- if (!burst.has_value() || burst.value() < slacked_burst) {
- burst = slacked_burst;
- }
- }
// If not overriden by an experiment, the burst is specified by the
// `burst_interval` argument.
if (!burst.has_value()) {
@@ -320,32 +293,7 @@
if (next_process_time_.IsMinusInfinity() ||
next_process_time_ > next_send_time) {
// Prefer low precision if allowed and not probing.
- TaskQueueBase::DelayPrecision precision =
- slacked_pacer_flags_.allow_low_precision &&
- !pacing_controller_.IsProbing()
- ? TaskQueueBase::DelayPrecision::kLow
- : TaskQueueBase::DelayPrecision::kHigh;
- // Check for cases where we need high precision.
- if (precision == TaskQueueBase::DelayPrecision::kLow) {
- auto& packets_per_type =
- pacing_controller_.SizeInPacketsPerRtpPacketMediaType();
- bool audio_or_retransmission_packets_in_queue =
- packets_per_type[static_cast<size_t>(RtpPacketMediaType::kAudio)] >
- 0 ||
- packets_per_type[static_cast<size_t>(
- RtpPacketMediaType::kRetransmission)] > 0;
- bool queue_time_too_large =
- slacked_pacer_flags_.max_low_precision_expected_queue_time &&
- pacing_controller_.ExpectedQueueTime() >=
- slacked_pacer_flags_.max_low_precision_expected_queue_time
- .Value();
- if (audio_or_retransmission_packets_in_queue || queue_time_too_large) {
- precision = TaskQueueBase::DelayPrecision::kHigh;
- }
- }
-
- task_queue_.TaskQueueForDelayedTasks()->PostDelayedTaskWithPrecision(
- precision,
+ task_queue_.TaskQueueForDelayedTasks()->PostDelayedHighPrecisionTask(
task_queue_.MaybeSafeTask(
safety_.flag(),
[this, next_send_time]() { MaybeProcessPackets(next_send_time); }),
diff --git a/modules/pacing/task_queue_paced_sender.h b/modules/pacing/task_queue_paced_sender.h
index ea335fd..4d51695 100644
--- a/modules/pacing/task_queue_paced_sender.h
+++ b/modules/pacing/task_queue_paced_sender.h
@@ -150,26 +150,7 @@
FieldTrialOptional<TimeDelta> burst;
};
const BurstyPacerFlags bursty_pacer_flags_;
- struct SlackedPacerFlags {
- // Parses `kSlackedTaskQueuePacedSenderFieldTrial`. Example:
- // --force-fieldtrials=WebRTC-SlackedTaskQueuePacedSender/Enabled,max_queue_time:75ms/
- explicit SlackedPacerFlags(const FieldTrialsView& field_trials);
- // When "Enabled", delayed tasks invoking MaybeProcessPackets() are
- // scheduled using low precision instead of high precision, resulting in
- // less idle wake ups and packets being sent in bursts if the `task_queue_`
- // implementation supports slack. When probing, high precision is used
- // regardless to ensure good bandwidth estimation.
- FieldTrialFlag allow_low_precision;
- // Controlled via the "max_queue_time" experiment argument. If set, uses
- // high precision scheduling of MaybeProcessPackets() whenever the expected
- // queue time is greater than or equal to this value.
- FieldTrialOptional<TimeDelta> max_low_precision_expected_queue_time;
- // Controlled via "send_burst_interval" experiment argument. If set, the
- // pacer is allowed to build up a packet "debt" that correspond to
- // approximately the send rate during the specified interval.
- FieldTrialOptional<TimeDelta> send_burst_interval;
- };
- const SlackedPacerFlags slacked_pacer_flags_;
+
// The holdback window prevents too frequent delayed MaybeProcessPackets()
// calls. These are only applicable if `allow_low_precision` is false.
const TimeDelta max_hold_back_window_;
diff --git a/modules/pacing/task_queue_paced_sender_unittest.cc b/modules/pacing/task_queue_paced_sender_unittest.cc
index 69c7b9b..23a72ce 100644
--- a/modules/pacing/task_queue_paced_sender_unittest.cc
+++ b/modules/pacing/task_queue_paced_sender_unittest.cc
@@ -811,103 +811,5 @@
EXPECT_TRUE(pacer.ExpectedQueueTime().IsZero());
}
-// TODO(webrtc:14502): Rewrite these tests if the functionality is needed if
-// pacing is done on the worker thread.
-TEST(TaskQueuePacedSenderTest, HighPrecisionPacingWhenSlackIsDisabled) {
- ScopedKeyValueConfig trials("WebRTC-SlackedTaskQueuePacedSender/Disabled/");
-
- GlobalSimulatedTimeController time_controller(Timestamp::Millis(1234));
- TaskQueueWithFakePrecisionFactory task_queue_factory(
- time_controller.GetTaskQueueFactory());
-
- MockPacketRouter packet_router;
- TaskQueuePacedSender pacer(
- time_controller.GetClock(), &packet_router, trials, &task_queue_factory,
- PacingController::kMinSleepTime, TaskQueuePacedSender::kNoPacketHoldback);
-
- // Send enough packets (covering one second) that pacing is triggered, i.e.
- // delayed tasks being scheduled.
- static constexpr size_t kPacketsToSend = 42;
- static constexpr DataRate kPacingRate =
- DataRate::BitsPerSec(kDefaultPacketSize * 8 * kPacketsToSend);
- pacer.SetPacingRates(kPacingRate, DataRate::Zero());
- pacer.EnsureStarted();
- pacer.EnqueuePackets(
- GeneratePackets(RtpPacketMediaType::kVideo, kPacketsToSend));
- // Expect all of them to be sent.
- size_t packets_sent = 0;
- EXPECT_CALL(packet_router, SendPacket)
- .WillRepeatedly(
- [&](std::unique_ptr<RtpPacketToSend> packet,
- const PacedPacketInfo& cluster_info) { ++packets_sent; });
- time_controller.AdvanceTime(TimeDelta::Seconds(1));
- EXPECT_EQ(packets_sent, kPacketsToSend);
-
- // Expect pacing to make use of high precision.
- EXPECT_EQ(task_queue_factory.delayed_low_precision_count(), 0);
- EXPECT_GT(task_queue_factory.delayed_high_precision_count(), 0);
-
- // Create probe cluster which is also high precision.
- pacer.CreateProbeClusters(
- {{.at_time = time_controller.GetClock()->CurrentTime(),
- .target_data_rate = kPacingRate,
- .target_duration = TimeDelta::Millis(15),
- .target_probe_count = 4,
- .id = 123}});
- pacer.EnqueuePackets(GeneratePackets(RtpPacketMediaType::kVideo, 1));
- time_controller.AdvanceTime(TimeDelta::Seconds(1));
- EXPECT_EQ(task_queue_factory.delayed_low_precision_count(), 0);
- EXPECT_GT(task_queue_factory.delayed_high_precision_count(), 0);
-}
-
-// TODO(webrtc:14502): Rewrite these tests if the functionality is needed if
-// pacing is done on the worker thread.
-TEST(TaskQueuePacedSenderTest, LowPrecisionPacingWhenSlackIsEnabled) {
- ScopedKeyValueConfig trials("WebRTC-SlackedTaskQueuePacedSender/Enabled/");
-
- GlobalSimulatedTimeController time_controller(Timestamp::Millis(1234));
- TaskQueueWithFakePrecisionFactory task_queue_factory(
- time_controller.GetTaskQueueFactory());
-
- MockPacketRouter packet_router;
- TaskQueuePacedSender pacer(
- time_controller.GetClock(), &packet_router, trials, &task_queue_factory,
- PacingController::kMinSleepTime, TaskQueuePacedSender::kNoPacketHoldback);
-
- // Send enough packets (covering one second) that pacing is triggered, i.e.
- // delayed tasks being scheduled.
- static constexpr size_t kPacketsToSend = 42;
- static constexpr DataRate kPacingRate =
- DataRate::BitsPerSec(kDefaultPacketSize * 8 * kPacketsToSend);
- pacer.SetPacingRates(kPacingRate, DataRate::Zero());
- pacer.EnsureStarted();
- pacer.EnqueuePackets(
- GeneratePackets(RtpPacketMediaType::kVideo, kPacketsToSend));
- // Expect all of them to be sent.
- size_t packets_sent = 0;
- EXPECT_CALL(packet_router, SendPacket)
- .WillRepeatedly(
- [&](std::unique_ptr<RtpPacketToSend> packet,
- const PacedPacketInfo& cluster_info) { ++packets_sent; });
- time_controller.AdvanceTime(TimeDelta::Seconds(1));
- EXPECT_EQ(packets_sent, kPacketsToSend);
-
- // Expect pacing to make use of low precision.
- EXPECT_GT(task_queue_factory.delayed_low_precision_count(), 0);
- EXPECT_EQ(task_queue_factory.delayed_high_precision_count(), 0);
-
- // Create probe cluster, which uses high precision despite regular pacing
- // being low precision.
- pacer.CreateProbeClusters(
- {{.at_time = time_controller.GetClock()->CurrentTime(),
- .target_data_rate = kPacingRate,
- .target_duration = TimeDelta::Millis(15),
- .target_probe_count = 4,
- .id = 123}});
- pacer.EnqueuePackets(GeneratePackets(RtpPacketMediaType::kVideo, 1));
- time_controller.AdvanceTime(TimeDelta::Seconds(1));
- EXPECT_GT(task_queue_factory.delayed_high_precision_count(), 0);
-}
-
} // namespace test
} // namespace webrtc
