From f6e6435ab8fdba8e9921ec4096b5fb3ab643b239 Mon Sep 17 00:00:00 2001 From: Sebastian Jansson Date: Wed, 17 Apr 2019 18:02:34 +0200 Subject: [PATCH] Cleanup in NetworkEmulationManager This prepares for an upcoming CL removing cross traffic processing when it's not used. Bug: webrtc:10365 Change-Id: I7f1f3998f7f38c2a627b888c3db6b0c459d8271d Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/133485 Reviewed-by: Artem Titov Commit-Queue: Sebastian Jansson Cr-Commit-Position: refs/heads/master@{#27682} --- test/scenario/network/BUILD.gn | 1 + test/scenario/network/cross_traffic.cc | 12 +++- test/scenario/network/cross_traffic.h | 34 ++++++---- .../network/network_emulation_manager.cc | 68 +++++++++++-------- .../network/network_emulation_manager.h | 2 - 5 files changed, 73 insertions(+), 44 deletions(-) diff --git a/test/scenario/network/BUILD.gn b/test/scenario/network/BUILD.gn index 0bcd65d999..1f690c5994 100644 --- a/test/scenario/network/BUILD.gn +++ b/test/scenario/network/BUILD.gn @@ -46,6 +46,7 @@ rtc_source_set("emulated_network") { "../../../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 bd63a08c1a..1d4efef866 100644 --- a/test/scenario/network/cross_traffic.cc +++ b/test/scenario/network/cross_traffic.cc @@ -25,10 +25,13 @@ RandomWalkCrossTraffic::RandomWalkCrossTraffic(RandomWalkConfig config, TrafficRoute* traffic_route) : config_(config), traffic_route_(traffic_route), - random_(config_.random_seed) {} + random_(config_.random_seed) { + sequence_checker_.Detach(); +} 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; } @@ -52,6 +55,7 @@ void RandomWalkCrossTraffic::Process(Timestamp at_time) { } DataRate RandomWalkCrossTraffic::TrafficRate() const { + RTC_DCHECK_RUN_ON(&sequence_checker_); return config_.peak_rate * intensity_; } @@ -66,10 +70,13 @@ ColumnPrinter RandomWalkCrossTraffic::StatsPrinter() { PulsedPeaksCrossTraffic::PulsedPeaksCrossTraffic(PulsedPeaksConfig config, TrafficRoute* traffic_route) - : config_(config), traffic_route_(traffic_route) {} + : config_(config), traffic_route_(traffic_route) { + sequence_checker_.Detach(); +} 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)) { @@ -94,6 +101,7 @@ void PulsedPeaksCrossTraffic::Process(Timestamp at_time) { } 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 b5a65fb23e..e88827466c 100644 --- a/test/scenario/network/cross_traffic.h +++ b/test/scenario/network/cross_traffic.h @@ -18,6 +18,7 @@ #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" @@ -44,15 +45,19 @@ class RandomWalkCrossTraffic { ColumnPrinter StatsPrinter(); private: - RandomWalkConfig config_; - TrafficRoute* const traffic_route_; - webrtc::Random random_; + SequenceChecker sequence_checker_; + const RandomWalkConfig config_; + TrafficRoute* const traffic_route_ RTC_PT_GUARDED_BY(sequence_checker_); + webrtc::Random random_ RTC_GUARDED_BY(sequence_checker_); - 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(); + 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(); }; struct PulsedPeaksConfig { @@ -74,12 +79,15 @@ class PulsedPeaksCrossTraffic { ColumnPrinter StatsPrinter(); private: - PulsedPeaksConfig config_; - TrafficRoute* const traffic_route_; + SequenceChecker sequence_checker_; + const PulsedPeaksConfig config_; + TrafficRoute* const traffic_route_ RTC_PT_GUARDED_BY(sequence_checker_); - Timestamp last_update_time_ = Timestamp::MinusInfinity(); - Timestamp last_send_time_ = Timestamp::MinusInfinity(); - bool sending_ = false; + 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; }; } // namespace test diff --git a/test/scenario/network/network_emulation_manager.cc b/test/scenario/network/network_emulation_manager.cc index ae67f6fb5e..8451c97487 100644 --- a/test/scenario/network/network_emulation_manager.cc +++ b/test/scenario/network/network_emulation_manager.cc @@ -29,6 +29,28 @@ constexpr uint32_t kMinIPv4Address = 0xC0A80000; // uint32_t representation of 192.168.255.255 address constexpr uint32_t kMaxIPv4Address = 0xC0A8FFFF; +template +class ResourceOwningTask final : public QueuedTask { + public: + ResourceOwningTask(T&& resource, Closure&& handler) + : resource_(std::move(resource)), + handler_(std::forward(handler)) {} + + bool Run() override { + handler_(std::move(resource_)); + return true; + } + + private: + T resource_; + Closure handler_; +}; +template +std::unique_ptr CreateResourceOwningTask(T resource, + Closure&& closure) { + return absl::make_unique>( + std::forward(resource), std::forward(closure)); +} } // namespace NetworkEmulationManagerImpl::NetworkEmulationManagerImpl() @@ -58,13 +80,10 @@ EmulatedNetworkNode* NetworkEmulationManagerImpl::CreateEmulatedNode( auto node = absl::make_unique( clock_, &task_queue_, std::move(network_behavior)); EmulatedNetworkNode* out = node.get(); - - struct Closure { - void operator()() { manager->network_nodes_.push_back(std::move(node)); } - NetworkEmulationManagerImpl* manager; - std::unique_ptr node; - }; - task_queue_.PostTask(Closure{this, std::move(node)}); + task_queue_.PostTask(CreateResourceOwningTask( + std::move(node), [this](std::unique_ptr node) { + network_nodes_.push_back(std::move(node)); + })); return out; } @@ -170,17 +189,15 @@ RandomWalkCrossTraffic* NetworkEmulationManagerImpl::CreateRandomWalkCrossTraffic( TrafficRoute* traffic_route, RandomWalkConfig config) { - auto traffic = absl::make_unique(std::move(config), - traffic_route); + auto traffic = + absl::make_unique(config, traffic_route); RandomWalkCrossTraffic* out = traffic.get(); - struct Closure { - void operator()() { - manager->random_cross_traffics_.push_back(std::move(traffic)); - } - NetworkEmulationManagerImpl* manager; - std::unique_ptr traffic; - }; - task_queue_.PostTask(Closure{this, std::move(traffic)}); + + task_queue_.PostTask(CreateResourceOwningTask( + std::move(traffic), + [this](std::unique_ptr traffic) { + random_cross_traffics_.push_back(std::move(traffic)); + })); return out; } @@ -188,17 +205,14 @@ PulsedPeaksCrossTraffic* NetworkEmulationManagerImpl::CreatePulsedPeaksCrossTraffic( TrafficRoute* traffic_route, PulsedPeaksConfig config) { - auto traffic = absl::make_unique(std::move(config), - traffic_route); + auto traffic = + absl::make_unique(config, traffic_route); PulsedPeaksCrossTraffic* out = traffic.get(); - struct Closure { - void operator()() { - manager->pulsed_cross_traffics_.push_back(std::move(traffic)); - } - NetworkEmulationManagerImpl* manager; - std::unique_ptr traffic; - }; - task_queue_.PostTask(Closure{this, std::move(traffic)}); + task_queue_.PostTask(CreateResourceOwningTask( + std::move(traffic), + [this](std::unique_ptr traffic) { + pulsed_cross_traffics_.push_back(std::move(traffic)); + })); return out; } diff --git a/test/scenario/network/network_emulation_manager.h b/test/scenario/network/network_emulation_manager.h index b3fd2a5c3e..c1645141dc 100644 --- a/test/scenario/network/network_emulation_manager.h +++ b/test/scenario/network/network_emulation_manager.h @@ -66,8 +66,6 @@ class NetworkEmulationManagerImpl : public NetworkEmulationManager { const std::vector& endpoints) override; private: - FakeNetworkSocketServer* CreateSocketServer( - const std::vector& endpoints); absl::optional GetNextIPv4Address(); void ProcessNetworkPackets(); Timestamp Now() const;