Revert "Only process cross traffic simulation if added."
This reverts commit 491d29448e4a0f501f1ea115b218ed5250fbd3dc.
Reason for revert: Breaks internal iOS waterfall
Original change's description:
> Only process cross traffic simulation if added.
>
> This avoids extra processing overhead when there's no cross traffic
> simulation active. To allow the use of SendTask, GlobalTimeController
> is adjusted so it always overrides yield behavior.
>
> Also adding sequence checkers to protect against races on
> read access.
>
> Bug: webrtc:10365
> Change-Id: I55c6ceb22f36ec19a4fca48cff500905192d1a16
> Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/133167
> Reviewed-by: Niels Moller <nisse@webrtc.org>
> Reviewed-by: Artem Titov <titovartem@webrtc.org>
> Commit-Queue: Sebastian Jansson <srte@webrtc.org>
> Cr-Commit-Position: refs/heads/master@{#27658}
TBR=nisse@webrtc.org,srte@webrtc.org,titovartem@webrtc.org
Change-Id: I3b176b0ec202a047c4436e3c2136e8170faea41e
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: webrtc:10365
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/133482
Reviewed-by: Artem Titarenko <artit@google.com>
Commit-Queue: Artem Titarenko <artit@google.com>
Cr-Commit-Position: refs/heads/master@{#27669}diff --git a/test/scenario/network/BUILD.gn b/test/scenario/network/BUILD.gn
index 1f690c5..0bcd65d 100644
--- a/test/scenario/network/BUILD.gn
+++ b/test/scenario/network/BUILD.gn
@@ -46,7 +46,6 @@
"../../../rtc_base:rtc_task_queue",
"../../../rtc_base:safe_minmax",
"../../../rtc_base:task_queue_for_test",
- "../../../rtc_base/synchronization:sequence_checker",
"../../../rtc_base/task_utils:repeating_task",
"../../../rtc_base/third_party/sigslot",
"../../../system_wrappers",
diff --git a/test/scenario/network/cross_traffic.cc b/test/scenario/network/cross_traffic.cc
index 1d4efef..bd63a08 100644
--- a/test/scenario/network/cross_traffic.cc
+++ b/test/scenario/network/cross_traffic.cc
@@ -25,13 +25,10 @@
TrafficRoute* traffic_route)
: config_(config),
traffic_route_(traffic_route),
- random_(config_.random_seed) {
- sequence_checker_.Detach();
-}
+ random_(config_.random_seed) {}
RandomWalkCrossTraffic::~RandomWalkCrossTraffic() = default;
void RandomWalkCrossTraffic::Process(Timestamp at_time) {
- RTC_DCHECK_RUN_ON(&sequence_checker_);
if (last_process_time_.IsMinusInfinity()) {
last_process_time_ = at_time;
}
@@ -55,7 +52,6 @@
}
DataRate RandomWalkCrossTraffic::TrafficRate() const {
- RTC_DCHECK_RUN_ON(&sequence_checker_);
return config_.peak_rate * intensity_;
}
@@ -70,13 +66,10 @@
PulsedPeaksCrossTraffic::PulsedPeaksCrossTraffic(PulsedPeaksConfig config,
TrafficRoute* traffic_route)
- : config_(config), traffic_route_(traffic_route) {
- sequence_checker_.Detach();
-}
+ : config_(config), traffic_route_(traffic_route) {}
PulsedPeaksCrossTraffic::~PulsedPeaksCrossTraffic() = default;
void PulsedPeaksCrossTraffic::Process(Timestamp at_time) {
- RTC_DCHECK_RUN_ON(&sequence_checker_);
TimeDelta time_since_toggle = at_time - last_update_time_;
if (time_since_toggle.IsInfinite() ||
(sending_ && time_since_toggle >= config_.send_duration)) {
@@ -101,7 +94,6 @@
}
DataRate PulsedPeaksCrossTraffic::TrafficRate() const {
- RTC_DCHECK_RUN_ON(&sequence_checker_);
return sending_ ? config_.peak_rate : DataRate::Zero();
}
diff --git a/test/scenario/network/cross_traffic.h b/test/scenario/network/cross_traffic.h
index e888274..b5a65fb 100644
--- a/test/scenario/network/cross_traffic.h
+++ b/test/scenario/network/cross_traffic.h
@@ -18,7 +18,6 @@
#include "api/units/time_delta.h"
#include "api/units/timestamp.h"
#include "rtc_base/random.h"
-#include "rtc_base/synchronization/sequence_checker.h"
#include "test/scenario/column_printer.h"
#include "test/scenario/network/traffic_route.h"
@@ -45,19 +44,15 @@
ColumnPrinter StatsPrinter();
private:
- SequenceChecker sequence_checker_;
- const RandomWalkConfig config_;
- TrafficRoute* const traffic_route_ RTC_PT_GUARDED_BY(sequence_checker_);
- webrtc::Random random_ RTC_GUARDED_BY(sequence_checker_);
+ RandomWalkConfig config_;
+ TrafficRoute* const traffic_route_;
+ webrtc::Random random_;
- Timestamp last_process_time_ RTC_GUARDED_BY(sequence_checker_) =
- Timestamp::MinusInfinity();
- Timestamp last_update_time_ RTC_GUARDED_BY(sequence_checker_) =
- Timestamp::MinusInfinity();
- Timestamp last_send_time_ RTC_GUARDED_BY(sequence_checker_) =
- Timestamp::MinusInfinity();
- double intensity_ RTC_GUARDED_BY(sequence_checker_) = 0;
- DataSize pending_size_ RTC_GUARDED_BY(sequence_checker_) = DataSize::Zero();
+ Timestamp last_process_time_ = Timestamp::MinusInfinity();
+ Timestamp last_update_time_ = Timestamp::MinusInfinity();
+ Timestamp last_send_time_ = Timestamp::MinusInfinity();
+ double intensity_ = 0;
+ DataSize pending_size_ = DataSize::Zero();
};
struct PulsedPeaksConfig {
@@ -79,15 +74,12 @@
ColumnPrinter StatsPrinter();
private:
- SequenceChecker sequence_checker_;
- const PulsedPeaksConfig config_;
- TrafficRoute* const traffic_route_ RTC_PT_GUARDED_BY(sequence_checker_);
+ PulsedPeaksConfig config_;
+ TrafficRoute* const traffic_route_;
- Timestamp last_update_time_ RTC_GUARDED_BY(sequence_checker_) =
- Timestamp::MinusInfinity();
- Timestamp last_send_time_ RTC_GUARDED_BY(sequence_checker_) =
- Timestamp::MinusInfinity();
- bool sending_ RTC_GUARDED_BY(sequence_checker_) = false;
+ Timestamp last_update_time_ = Timestamp::MinusInfinity();
+ Timestamp last_send_time_ = Timestamp::MinusInfinity();
+ bool sending_ = false;
};
} // namespace test
diff --git a/test/scenario/network/network_emulation_manager.cc b/test/scenario/network/network_emulation_manager.cc
index 8e1dc76..483dba3 100644
--- a/test/scenario/network/network_emulation_manager.cc
+++ b/test/scenario/network/network_emulation_manager.cc
@@ -23,33 +23,12 @@
namespace test {
namespace {
+constexpr int64_t kPacketProcessingIntervalMs = 1;
// uint32_t representation of 192.168.0.0 address
constexpr uint32_t kMinIPv4Address = 0xC0A80000;
// uint32_t representation of 192.168.255.255 address
constexpr uint32_t kMaxIPv4Address = 0xC0A8FFFF;
-template <typename T, typename Closure>
-class ResourceOwningTask final : public QueuedTask {
- public:
- ResourceOwningTask(T&& resource, Closure&& handler)
- : resource_(std::move(resource)),
- handler_(std::forward<Closure>(handler)) {}
-
- bool Run() override {
- handler_(std::move(resource_));
- return true;
- }
-
- private:
- T resource_;
- Closure handler_;
-};
-template <typename T, typename Closure>
-std::unique_ptr<QueuedTask> CreateResourceOwningTask(T resource,
- Closure&& closure) {
- return absl::make_unique<ResourceOwningTask<T, Closure>>(
- std::forward<T>(resource), std::forward<Closure>(closure));
-}
} // namespace
NetworkEmulationManagerImpl::NetworkEmulationManagerImpl()
@@ -63,6 +42,10 @@
task_queue_(time_controller->GetTaskQueueFactory()->CreateTaskQueue(
"NetworkEmulation",
TaskQueueFactory::Priority::NORMAL)) {
+ process_task_handle_ = RepeatingTaskHandle::Start(task_queue_.Get(), [this] {
+ ProcessNetworkPackets();
+ return TimeDelta::ms(kPacketProcessingIntervalMs);
+ });
}
// TODO(srte): Ensure that any pending task that must be run for consistency
@@ -184,21 +167,17 @@
NetworkEmulationManagerImpl::CreateRandomWalkCrossTraffic(
TrafficRoute* traffic_route,
RandomWalkConfig config) {
- auto traffic =
- absl::make_unique<RandomWalkCrossTraffic>(config, traffic_route);
+ auto traffic = absl::make_unique<RandomWalkCrossTraffic>(std::move(config),
+ traffic_route);
RandomWalkCrossTraffic* out = traffic.get();
-
- task_queue_.PostTask(CreateResourceOwningTask(
- std::move(traffic),
- [this, config](std::unique_ptr<RandomWalkCrossTraffic> traffic) {
- auto* traffic_ptr = traffic.get();
- random_cross_traffics_.push_back(std::move(traffic));
- RepeatingTaskHandle::Start(task_queue_.Get(),
- [this, config, traffic_ptr] {
- traffic_ptr->Process(Now());
- return config.min_packet_interval;
- });
- }));
+ struct Closure {
+ void operator()() {
+ manager->random_cross_traffics_.push_back(std::move(traffic));
+ }
+ NetworkEmulationManagerImpl* manager;
+ std::unique_ptr<RandomWalkCrossTraffic> traffic;
+ };
+ task_queue_.PostTask(Closure{this, std::move(traffic)});
return out;
}
@@ -206,20 +185,17 @@
NetworkEmulationManagerImpl::CreatePulsedPeaksCrossTraffic(
TrafficRoute* traffic_route,
PulsedPeaksConfig config) {
- auto traffic =
- absl::make_unique<PulsedPeaksCrossTraffic>(config, traffic_route);
+ auto traffic = absl::make_unique<PulsedPeaksCrossTraffic>(std::move(config),
+ traffic_route);
PulsedPeaksCrossTraffic* out = traffic.get();
- task_queue_.PostTask(CreateResourceOwningTask(
- std::move(traffic),
- [this, config](std::unique_ptr<PulsedPeaksCrossTraffic> traffic) {
- auto* traffic_ptr = traffic.get();
- pulsed_cross_traffics_.push_back(std::move(traffic));
- RepeatingTaskHandle::Start(task_queue_.Get(),
- [this, config, traffic_ptr] {
- traffic_ptr->Process(Now());
- return config.min_packet_interval;
- });
- }));
+ struct Closure {
+ void operator()() {
+ manager->pulsed_cross_traffics_.push_back(std::move(traffic));
+ }
+ NetworkEmulationManagerImpl* manager;
+ std::unique_ptr<PulsedPeaksCrossTraffic> traffic;
+ };
+ task_queue_.PostTask(Closure{this, std::move(traffic)});
return out;
}
@@ -263,6 +239,16 @@
return absl::nullopt;
}
+void NetworkEmulationManagerImpl::ProcessNetworkPackets() {
+ Timestamp current_time = Now();
+ for (auto& traffic : random_cross_traffics_) {
+ traffic->Process(current_time);
+ }
+ for (auto& traffic : pulsed_cross_traffics_) {
+ traffic->Process(current_time);
+ }
+}
+
Timestamp NetworkEmulationManagerImpl::Now() const {
return Timestamp::us(clock_->TimeInMicroseconds());
}
diff --git a/test/scenario/network/network_emulation_manager.h b/test/scenario/network/network_emulation_manager.h
index cb1ace7..c164514 100644
--- a/test/scenario/network/network_emulation_manager.h
+++ b/test/scenario/network/network_emulation_manager.h
@@ -67,6 +67,7 @@
private:
absl::optional<rtc::IPAddress> GetNextIPv4Address();
+ void ProcessNetworkPackets();
Timestamp Now() const;
Clock* const clock_;
