zqz/platform-external-webrtc
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Revert "Experimentally reduce TaskQueuePacedSender's delayed task precision."

Browse Source 
This reverts commit 43a69b3f46059b103c35079744dc09a0e2bb2948.

Reason for revert: Breaks downstream project.

Original change's description:
> Experimentally reduce TaskQueuePacedSender's delayed task precision.
>
> This CL reduces the delayed task precision of non-probes in accordance
> with DD go/slacked-task-queue-paced-sender. The precision is only
> deduced if field trial "WebRTC-SlackedTaskQueuePacedSender" is enabled
> though.
>
> Bug: webrtc:13824
> Change-Id: I37e53b24e343f4f08059be08a3cda74f5484cc05
> Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/255341
> Reviewed-by: Erik Språng <sprang@webrtc.org>
> Commit-Queue: Henrik Boström <hbos@webrtc.org>
> Cr-Commit-Position: refs/heads/main@{#36214}

Bug: webrtc:13824
Change-Id: Iccdadcfaa0c490a1b9e5636cd695c5673c3c09a0
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/255825
Auto-Submit: Mirko Bonadei <mbonadei@webrtc.org>
Bot-Commit: rubber-stamper@appspot.gserviceaccount.com <rubber-stamper@appspot.gserviceaccount.com>
Owners-Override: Mirko Bonadei <mbonadei@webrtc.org>
Commit-Queue: Mirko Bonadei <mbonadei@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#36217}
This commit is contained in:
Mirko Bonadei
2022-03-16 14:12:36 +00:00
committed by WebRTC LUCI CQ
parent 6160ca53d1
commit 4a08d5013b
4 changed files with 3 additions and 211 deletions
Download Patch File Download Diff File Expand all files Collapse all files

3
modules/pacing/BUILD.gn
View File

@ -35,7 +35,6 @@ rtc_library("pacing") {
"..:module_api",
"../../api:function_view",
"../../api:sequence_checker",
"../../api:webrtc_key_value_config",
"../../api/rtc_event_log",
"../../api/task_queue:task_queue",
"../../api/transport:field_trial_based_config",
@ -94,7 +93,6 @@ if (rtc_include_tests) {
deps = [
":interval_budget",
":pacing",
"../../api/task_queue:task_queue",
"../../api/transport:network_control",
"../../api/units:data_rate",
"../../api/units:time_delta",
@ -103,7 +101,6 @@ if (rtc_include_tests) {
"../../rtc_base:rtc_base_approved",
"../../rtc_base:rtc_base_tests_utils",
"../../rtc_base/experiments:alr_experiment",
"../../rtc_base/task_utils:to_queued_task",
"../../system_wrappers",
"../../test:explicit_key_value_config",
"../../test:scoped_key_value_config",

26
modules/pacing/task_queue_paced_sender.cc
View File

@ -12,7 +12,6 @@
#include <algorithm>
#include <utility>
#include "absl/memory/memory.h"
#include "rtc_base/checks.h"
#include "rtc_base/event.h"
@ -22,13 +21,6 @@
namespace webrtc {
namespace {
constexpr const char* kSlackedTaskQueuePacedSenderFieldTrial =
"WebRTC-SlackedTaskQueuePacedSender";
} // namespace
TaskQueuePacedSender::TaskQueuePacedSender(
Clock* clock,
PacingController::PacketSender* packet_sender,
@ -54,13 +46,8 @@ TaskQueuePacedSender::TaskQueuePacedSender(
TimeDelta max_hold_back_window,
int max_hold_back_window_in_packets)
: clock_(clock),
allow_low_precision_(
field_trials.IsEnabled(kSlackedTaskQueuePacedSenderFieldTrial)),
max_hold_back_window_(allow_low_precision_
? PacingController::kMinSleepTime
: max_hold_back_window),
max_hold_back_window_in_packets_(
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,
event_log,
@ -300,14 +287,7 @@ void TaskQueuePacedSender::MaybeProcessPackets(
// Set a new scheduled process time and post a delayed task.
next_process_time_ = next_process_time;
// Prefer low precision if allowed and not probing.
TaskQueueBase::DelayPrecision precision =
allow_low_precision_ && !pacing_controller_.IsProbing()
? TaskQueueBase::DelayPrecision::kLow
: TaskQueueBase::DelayPrecision::kHigh;
task_queue_.PostDelayedTaskWithPrecision(
precision,
task_queue_.PostDelayedHighPrecisionTask(
[this, next_process_time]() { MaybeProcessPackets(next_process_time); },
time_to_next_process->ms<uint32_t>());
}

11
modules/pacing/task_queue_paced_sender.h
View File

@ -26,7 +26,6 @@
#include "api/units/data_size.h"
#include "api/units/time_delta.h"
#include "api/units/timestamp.h"
#include "api/webrtc_key_value_config.h"
#include "modules/pacing/pacing_controller.h"
#include "modules/pacing/rtp_packet_pacer.h"
#include "modules/rtp_rtcp/source/rtp_packet_to_send.h"
@ -145,16 +144,6 @@ class TaskQueuePacedSender : public RtpPacketPacer, public RtpPacketSender {
Stats GetStats() const;
Clock* const clock_;
// If `kSlackedTaskQueuePacedSenderFieldTrial` is 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 of `allow_low_precision_`
// to ensure good bandwidth estimation.
const bool allow_low_precision_;
// 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_;
const int max_hold_back_window_in_packets_;

174
modules/pacing/task_queue_paced_sender_unittest.cc
View File

@ -11,19 +11,15 @@
#include "modules/pacing/task_queue_paced_sender.h"
#include <algorithm>
#include <atomic>
#include <list>
#include <memory>
#include <string>
#include <utility>
#include <vector>
#include "api/task_queue/task_queue_base.h"
#include "api/transport/network_types.h"
#include "api/units/data_rate.h"
#include "modules/pacing/packet_router.h"
#include "modules/utility/include/mock/mock_process_thread.h"
#include "rtc_base/task_utils/to_queued_task.h"
#include "test/gmock.h"
#include "test/gtest.h"
#include "test/scoped_key_value_config.h"
@ -80,88 +76,6 @@ std::vector<std::unique_ptr<RtpPacketToSend>> GeneratePadding(
return padding_packets;
}
class TaskQueueWithFakePrecisionFactory : public TaskQueueFactory {
public:
explicit TaskQueueWithFakePrecisionFactory(
TaskQueueFactory* task_queue_factory)
: task_queue_factory_(task_queue_factory) {}
std::unique_ptr<TaskQueueBase, TaskQueueDeleter> CreateTaskQueue(
absl::string_view name,
Priority priority) const override {
return std::unique_ptr<TaskQueueBase, TaskQueueDeleter>(
new TaskQueueWithFakePrecision(
const_cast<TaskQueueWithFakePrecisionFactory*>(this),
task_queue_factory_));
}
int delayed_low_precision_count() const {
return delayed_low_precision_count_;
}
int delayed_high_precision_count() const {
return delayed_high_precision_count_;
}
private:
friend class TaskQueueWithFakePrecision;
class TaskQueueWithFakePrecision : public TaskQueueBase {
public:
TaskQueueWithFakePrecision(
TaskQueueWithFakePrecisionFactory* parent_factory,
TaskQueueFactory* task_queue_factory)
: parent_factory_(parent_factory),
task_queue_(task_queue_factory->CreateTaskQueue(
"TaskQueueWithFakePrecision",
TaskQueueFactory::Priority::NORMAL)) {}
~TaskQueueWithFakePrecision() override {}
void Delete() override {
// `task_queue_->Delete()` is implicitly called in the destructor due to
// TaskQueueDeleter.
delete this;
}
void PostTask(std::unique_ptr<QueuedTask> task) override {
task_queue_->PostTask(
ToQueuedTask([this, task = std::move(task)]() mutable {
RunTask(std::move(task));
}));
}
void PostDelayedTask(std::unique_ptr<QueuedTask> task,
uint32_t milliseconds) override {
++parent_factory_->delayed_low_precision_count_;
task_queue_->PostDelayedTask(
ToQueuedTask([this, task = std::move(task)]() mutable {
RunTask(std::move(task));
}),
milliseconds);
}
void PostDelayedHighPrecisionTask(std::unique_ptr<QueuedTask> task,
uint32_t milliseconds) override {
++parent_factory_->delayed_high_precision_count_;
task_queue_->PostDelayedHighPrecisionTask(
ToQueuedTask([this, task = std::move(task)]() mutable {
RunTask(std::move(task));
}),
milliseconds);
}
private:
void RunTask(std::unique_ptr<QueuedTask> task) {
CurrentTaskQueueSetter set_current(this);
if (!task->Run())
task.release();
}
TaskQueueWithFakePrecisionFactory* parent_factory_;
std::unique_ptr<TaskQueueBase, TaskQueueDeleter> task_queue_;
};
TaskQueueFactory* task_queue_factory_;
std::atomic<int> delayed_low_precision_count_ = 0u;
std::atomic<int> delayed_high_precision_count_ = 0u;
};
} // namespace
namespace test {
@ -683,93 +597,5 @@ TEST(TaskQueuePacedSenderTest, Stats) {
EXPECT_TRUE(pacer.ExpectedQueueTime().IsZero());
}
TEST(TaskQueuePacedSenderTest, HighPrecisionPacingWhenSlackIsDisabled) {
test::ScopedKeyValueConfig experiments(
"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, /*event_log=*/nullptr,
experiments, &task_queue_factory, PacingController::kMinSleepTime,
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.CreateProbeCluster(kPacingRate, 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);
}
TEST(TaskQueuePacedSenderTest, LowPrecisionPacingWhenSlackIsEnabled) {
test::ScopedKeyValueConfig experiments(
"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, /*event_log=*/nullptr,
experiments, &task_queue_factory, PacingController::kMinSleepTime,
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.CreateProbeCluster(kPacingRate, 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