From e7e3d5925ad2c96b840dedc2ba619edb3545a9c9 Mon Sep 17 00:00:00 2001 From: Ali Tofigh Date: Thu, 25 Aug 2022 10:09:48 +0000 Subject: [PATCH] Revert "Add TaskQueueStdlib experiment." This reverts commit 83db78e854ff35d57124f04aff9464c0862cd833. Reason for revert: Some tests in Chromium's blink no longer compile because of the change in the signature of the CreateDefaultTaskQueueFactory() function. Original change's description: > Add TaskQueueStdlib experiment. > > Bug: webrtc:14389 > Change-Id: I23c6e0ae675748ec35a99c334104dd2654995a33 > Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/265802 > Commit-Queue: Ali Tofigh > Reviewed-by: Harald Alvestrand > Reviewed-by: Tomas Gunnarsson > Reviewed-by: Jonas Oreland > Cr-Commit-Position: refs/heads/main@{#37888} Bug: webrtc:14389 Change-Id: If3e63d6b4ab9e838dc5020b88076a73fd29916e4 No-Presubmit: true No-Tree-Checks: true No-Try: true Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/272920 Bot-Commit: rubber-stamper@appspot.gserviceaccount.com Reviewed-by: Harald Alvestrand Commit-Queue: Harald Alvestrand Auto-Submit: Ali Tofigh Cr-Commit-Position: refs/heads/main@{#37902} --- api/create_peerconnection_factory.cc | 13 +++---- api/task_queue/BUILD.gn | 23 ++--------- api/task_queue/default_task_queue_factory.h | 4 +- .../default_task_queue_factory_gcd.cc | 4 +- .../default_task_queue_factory_libevent.cc | 4 +- .../default_task_queue_factory_stdlib.cc | 4 +- ...e_factory_stdlib_or_libevent_experiment.cc | 38 ------------------- .../default_task_queue_factory_win.cc | 4 +- api/task_queue/task_queue_test.cc | 23 ++++++----- api/task_queue/task_queue_test.h | 7 ++-- rtc_base/BUILD.gn | 1 - rtc_base/thread_unittest.cc | 8 +--- rtc_tools/video_replay.cc | 13 +++---- .../RTCPeerConnectionFactory.mm | 3 +- 14 files changed, 36 insertions(+), 113 deletions(-) delete mode 100644 api/task_queue/default_task_queue_factory_stdlib_or_libevent_experiment.cc diff --git a/api/create_peerconnection_factory.cc b/api/create_peerconnection_factory.cc index f9cc7ad3e2..4a01b2f3c9 100644 --- a/api/create_peerconnection_factory.cc +++ b/api/create_peerconnection_factory.cc @@ -40,20 +40,19 @@ rtc::scoped_refptr CreatePeerConnectionFactory( rtc::scoped_refptr audio_processing, AudioFrameProcessor* audio_frame_processor, std::unique_ptr field_trials) { - if (!field_trials) { - field_trials = std::make_unique(); - } - PeerConnectionFactoryDependencies dependencies; dependencies.network_thread = network_thread; dependencies.worker_thread = worker_thread; dependencies.signaling_thread = signaling_thread; - dependencies.task_queue_factory = - CreateDefaultTaskQueueFactory(field_trials.get()); + dependencies.task_queue_factory = CreateDefaultTaskQueueFactory(); dependencies.call_factory = CreateCallFactory(); dependencies.event_log_factory = std::make_unique( dependencies.task_queue_factory.get()); - dependencies.trials = std::move(field_trials); + if (field_trials) { + dependencies.trials = std::move(field_trials); + } else { + dependencies.trials = std::make_unique(); + } if (network_thread) { // TODO(bugs.webrtc.org/13145): Add an rtc::SocketFactory* argument. diff --git a/api/task_queue/BUILD.gn b/api/task_queue/BUILD.gn index 2406bc0c78..8daa01d26b 100644 --- a/api/task_queue/BUILD.gn +++ b/api/task_queue/BUILD.gn @@ -61,9 +61,7 @@ rtc_library("task_queue_test") { ] } else { deps = [ - ":default_task_queue_factory", ":task_queue", - "../../api:field_trials_view", "../../api/units:time_delta", "../../rtc_base:refcount", "../../rtc_base:rtc_event", @@ -83,26 +81,11 @@ rtc_library("default_task_queue_factory") { poisonous = [ "default_task_queue" ] } sources = [ "default_task_queue_factory.h" ] - deps = [ - ":task_queue", - "../../api:field_trials_view", - "../../rtc_base/memory:always_valid_pointer", - ] + deps = [ ":task_queue" ] if (rtc_enable_libevent) { - if (is_android) { - sources += - [ "default_task_queue_factory_stdlib_or_libevent_experiment.cc" ] - deps += [ - "../../api/transport:field_trial_based_config", - "../../rtc_base:logging", - "../../rtc_base:rtc_task_queue_libevent", - "../../rtc_base:rtc_task_queue_stdlib", - ] - } else { - sources += [ "default_task_queue_factory_libevent.cc" ] - deps += [ "../../rtc_base:rtc_task_queue_libevent" ] - } + sources += [ "default_task_queue_factory_libevent.cc" ] + deps += [ "../../rtc_base:rtc_task_queue_libevent" ] } else if (is_mac || is_ios) { sources += [ "default_task_queue_factory_gcd.cc" ] deps += [ "../../rtc_base:rtc_task_queue_gcd" ] diff --git a/api/task_queue/default_task_queue_factory.h b/api/task_queue/default_task_queue_factory.h index 1d2dbd7ec2..ccdd1ebec0 100644 --- a/api/task_queue/default_task_queue_factory.h +++ b/api/task_queue/default_task_queue_factory.h @@ -12,13 +12,11 @@ #include -#include "api/field_trials_view.h" #include "api/task_queue/task_queue_factory.h" namespace webrtc { -std::unique_ptr CreateDefaultTaskQueueFactory( - const FieldTrialsView* field_trials = nullptr); +std::unique_ptr CreateDefaultTaskQueueFactory(); } // namespace webrtc diff --git a/api/task_queue/default_task_queue_factory_gcd.cc b/api/task_queue/default_task_queue_factory_gcd.cc index 391f09b393..7e17b4846d 100644 --- a/api/task_queue/default_task_queue_factory_gcd.cc +++ b/api/task_queue/default_task_queue_factory_gcd.cc @@ -9,14 +9,12 @@ */ #include -#include "api/field_trials_view.h" #include "api/task_queue/task_queue_factory.h" #include "rtc_base/task_queue_gcd.h" namespace webrtc { -std::unique_ptr CreateDefaultTaskQueueFactory( - const FieldTrialsView* field_trials) { +std::unique_ptr CreateDefaultTaskQueueFactory() { return CreateTaskQueueGcdFactory(); } diff --git a/api/task_queue/default_task_queue_factory_libevent.cc b/api/task_queue/default_task_queue_factory_libevent.cc index 89079f51ca..f2fb418fd3 100644 --- a/api/task_queue/default_task_queue_factory_libevent.cc +++ b/api/task_queue/default_task_queue_factory_libevent.cc @@ -9,14 +9,12 @@ */ #include -#include "api/field_trials_view.h" #include "api/task_queue/task_queue_factory.h" #include "rtc_base/task_queue_libevent.h" namespace webrtc { -std::unique_ptr CreateDefaultTaskQueueFactory( - const FieldTrialsView* field_trials) { +std::unique_ptr CreateDefaultTaskQueueFactory() { return CreateTaskQueueLibeventFactory(); } diff --git a/api/task_queue/default_task_queue_factory_stdlib.cc b/api/task_queue/default_task_queue_factory_stdlib.cc index 10cda7c5ec..ca7d720cbe 100644 --- a/api/task_queue/default_task_queue_factory_stdlib.cc +++ b/api/task_queue/default_task_queue_factory_stdlib.cc @@ -9,14 +9,12 @@ */ #include -#include "api/field_trials_view.h" #include "api/task_queue/task_queue_factory.h" #include "rtc_base/task_queue_stdlib.h" namespace webrtc { -std::unique_ptr CreateDefaultTaskQueueFactory( - const FieldTrialsView* field_trials) { +std::unique_ptr CreateDefaultTaskQueueFactory() { return CreateTaskQueueStdlibFactory(); } diff --git a/api/task_queue/default_task_queue_factory_stdlib_or_libevent_experiment.cc b/api/task_queue/default_task_queue_factory_stdlib_or_libevent_experiment.cc deleted file mode 100644 index dc6e835907..0000000000 --- a/api/task_queue/default_task_queue_factory_stdlib_or_libevent_experiment.cc +++ /dev/null @@ -1,38 +0,0 @@ -/* - * Copyright 2022 The WebRTC project authors. All Rights Reserved. - * - * Use of this source code is governed by a BSD-style license - * that can be found in the LICENSE file in the root of the source - * tree. An additional intellectual property rights grant can be found - * in the file PATENTS. All contributing project authors may - * be found in the AUTHORS file in the root of the source tree. - */ - -#include - -#include "api/field_trials_view.h" -#include "api/task_queue/task_queue_factory.h" -#include "api/transport/field_trial_based_config.h" -#include "rtc_base/logging.h" -#include "rtc_base/memory/always_valid_pointer.h" -#include "rtc_base/task_queue_libevent.h" -#include "rtc_base/task_queue_stdlib.h" - -namespace webrtc { - -std::unique_ptr CreateDefaultTaskQueueFactory( - const FieldTrialsView* field_trials_view) { - AlwaysValidPointer field_trials( - field_trials_view); - if (field_trials->IsEnabled("WebRTC-TaskQueue-ReplaceLibeventWithStdlib")) { - RTC_LOG(LS_INFO) << "WebRTC-TaskQueue-ReplaceLibeventWithStdlib: " - << "using TaskQueueStdlibFactory."; - return CreateTaskQueueStdlibFactory(); - } - - RTC_LOG(LS_INFO) << "WebRTC-TaskQueue-ReplaceLibeventWithStdlib: " - << "using TaskQueueLibeventFactory."; - return CreateTaskQueueLibeventFactory(); -} - -} // namespace webrtc diff --git a/api/task_queue/default_task_queue_factory_win.cc b/api/task_queue/default_task_queue_factory_win.cc index e3adc07327..493ea66ea5 100644 --- a/api/task_queue/default_task_queue_factory_win.cc +++ b/api/task_queue/default_task_queue_factory_win.cc @@ -9,14 +9,12 @@ */ #include -#include "api/field_trials_view.h" #include "api/task_queue/task_queue_factory.h" #include "rtc_base/task_queue_win.h" namespace webrtc { -std::unique_ptr CreateDefaultTaskQueueFactory( - const FieldTrialsView* field_trials) { +std::unique_ptr CreateDefaultTaskQueueFactory() { return CreateTaskQueueWinFactory(); } diff --git a/api/task_queue/task_queue_test.cc b/api/task_queue/task_queue_test.cc index 0f6b1d013f..69532d61f4 100644 --- a/api/task_queue/task_queue_test.cc +++ b/api/task_queue/task_queue_test.cc @@ -13,7 +13,6 @@ #include "absl/cleanup/cleanup.h" #include "absl/strings/string_view.h" -#include "api/task_queue/default_task_queue_factory.h" #include "api/units/time_delta.h" #include "rtc_base/event.h" #include "rtc_base/ref_counter.h" @@ -30,13 +29,13 @@ std::unique_ptr CreateTaskQueue( } TEST_P(TaskQueueTest, Construct) { - std::unique_ptr factory = GetParam()(nullptr); + std::unique_ptr factory = GetParam()(); auto queue = CreateTaskQueue(factory, "Construct"); EXPECT_FALSE(queue->IsCurrent()); } TEST_P(TaskQueueTest, PostAndCheckCurrent) { - std::unique_ptr factory = GetParam()(nullptr); + std::unique_ptr factory = GetParam()(); rtc::Event event; auto queue = CreateTaskQueue(factory, "PostAndCheckCurrent"); @@ -54,7 +53,7 @@ TEST_P(TaskQueueTest, PostAndCheckCurrent) { } TEST_P(TaskQueueTest, PostCustomTask) { - std::unique_ptr factory = GetParam()(nullptr); + std::unique_ptr factory = GetParam()(); rtc::Event ran; auto queue = CreateTaskQueue(factory, "PostCustomImplementation"); @@ -73,7 +72,7 @@ TEST_P(TaskQueueTest, PostCustomTask) { } TEST_P(TaskQueueTest, PostDelayedZero) { - std::unique_ptr factory = GetParam()(nullptr); + std::unique_ptr factory = GetParam()(); rtc::Event event; auto queue = CreateTaskQueue(factory, "PostDelayedZero"); @@ -82,7 +81,7 @@ TEST_P(TaskQueueTest, PostDelayedZero) { } TEST_P(TaskQueueTest, PostFromQueue) { - std::unique_ptr factory = GetParam()(nullptr); + std::unique_ptr factory = GetParam()(); rtc::Event event; auto queue = CreateTaskQueue(factory, "PostFromQueue"); @@ -92,7 +91,7 @@ TEST_P(TaskQueueTest, PostFromQueue) { } TEST_P(TaskQueueTest, PostDelayed) { - std::unique_ptr factory = GetParam()(nullptr); + std::unique_ptr factory = GetParam()(); rtc::Event event; auto queue = CreateTaskQueue(factory, "PostDelayed", TaskQueueFactory::Priority::HIGH); @@ -114,7 +113,7 @@ TEST_P(TaskQueueTest, PostDelayed) { } TEST_P(TaskQueueTest, PostMultipleDelayed) { - std::unique_ptr factory = GetParam()(nullptr); + std::unique_ptr factory = GetParam()(); auto queue = CreateTaskQueue(factory, "PostMultipleDelayed"); std::vector events(100); @@ -133,7 +132,7 @@ TEST_P(TaskQueueTest, PostMultipleDelayed) { } TEST_P(TaskQueueTest, PostDelayedAfterDestruct) { - std::unique_ptr factory = GetParam()(nullptr); + std::unique_ptr factory = GetParam()(); rtc::Event run; rtc::Event deleted; auto queue = CreateTaskQueue(factory, "PostDelayedAfterDestruct"); @@ -148,7 +147,7 @@ TEST_P(TaskQueueTest, PostDelayedAfterDestruct) { } TEST_P(TaskQueueTest, PostAndReuse) { - std::unique_ptr factory = GetParam()(nullptr); + std::unique_ptr factory = GetParam()(); rtc::Event event; auto post_queue = CreateTaskQueue(factory, "PostQueue"); auto reply_queue = CreateTaskQueue(factory, "ReplyQueue"); @@ -204,7 +203,7 @@ TEST_P(TaskQueueTest, PostALot) { rtc::Event event_; }; - std::unique_ptr factory = GetParam()(nullptr); + std::unique_ptr factory = GetParam()(); static constexpr int kTaskCount = 0xffff; rtc::Event posting_done; BlockingCounter all_destroyed(kTaskCount); @@ -248,7 +247,7 @@ TEST_P(TaskQueueTest, PostALot) { // unit test, run it under TSan or some other tool that is able to // directly detect data races. TEST_P(TaskQueueTest, PostTwoWithSharedUnprotectedState) { - std::unique_ptr factory = GetParam()(nullptr); + std::unique_ptr factory = GetParam()(); struct SharedState { // First task will set this value to 1 and second will assert it. int state = 0; diff --git a/api/task_queue/task_queue_test.h b/api/task_queue/task_queue_test.h index 214f95008f..e2e473017f 100644 --- a/api/task_queue/task_queue_test.h +++ b/api/task_queue/task_queue_test.h @@ -13,7 +13,6 @@ #include #include -#include "api/field_trials_view.h" #include "api/task_queue/task_queue_factory.h" #include "test/gtest.h" @@ -32,9 +31,9 @@ namespace webrtc { // INSTANTIATE_TEST_SUITE_P(My, TaskQueueTest, Values(CreateMyFactory)); // // } // namespace -class TaskQueueTest - : public ::testing::TestWithParam(const FieldTrialsView*)>> {}; +class TaskQueueTest : public ::testing::TestWithParam< + std::function()>> { +}; } // namespace webrtc diff --git a/rtc_base/BUILD.gn b/rtc_base/BUILD.gn index 80839cf872..311b9e01e3 100644 --- a/rtc_base/BUILD.gn +++ b/rtc_base/BUILD.gn @@ -1753,7 +1753,6 @@ if (rtc_include_tests) { ":threading", ":timeutils", "../api:array_view", - "../api:field_trials_view", "../api:make_ref_counted", "../api/task_queue", "../api/task_queue:pending_task_safety_flag", diff --git a/rtc_base/thread_unittest.cc b/rtc_base/thread_unittest.cc index 32589b3216..c88233998b 100644 --- a/rtc_base/thread_unittest.cc +++ b/rtc_base/thread_unittest.cc @@ -12,7 +12,6 @@ #include -#include "api/field_trials_view.h" #include "api/task_queue/task_queue_factory.h" #include "api/task_queue/task_queue_test.h" #include "api/units/time_delta.h" @@ -1153,16 +1152,11 @@ class ThreadFactory : public webrtc::TaskQueueFactory { } }; -std::unique_ptr CreateDefaultThreadFactory( - const webrtc::FieldTrialsView*) { - return std::make_unique(); -} - using ::webrtc::TaskQueueTest; INSTANTIATE_TEST_SUITE_P(RtcThread, TaskQueueTest, - ::testing::Values(CreateDefaultThreadFactory)); + ::testing::Values(std::make_unique)); } // namespace } // namespace rtc diff --git a/rtc_tools/video_replay.cc b/rtc_tools/video_replay.cc index 1a7bfadbca..7d331ad366 100644 --- a/rtc_tools/video_replay.cc +++ b/rtc_tools/video_replay.cc @@ -340,22 +340,21 @@ class RtpReplayer final { // Replay a rtp dump with an optional json configuration. static void Replay(const std::string& replay_config_path, const std::string& rtp_dump_path) { - webrtc::RtcEventLogNull event_log; - Call::Config call_config(&event_log); - call_config.trials = new FieldTrialBasedConfig(); - std::unique_ptr task_queue_factory = - webrtc::CreateDefaultTaskQueueFactory(call_config.trials); + webrtc::CreateDefaultTaskQueueFactory(); auto worker_thread = task_queue_factory->CreateTaskQueue( "worker_thread", TaskQueueFactory::Priority::NORMAL); rtc::Event sync_event(/*manual_reset=*/false, /*initially_signalled=*/false); + webrtc::RtcEventLogNull event_log; + Call::Config call_config(&event_log); call_config.task_queue_factory = task_queue_factory.get(); + call_config.trials = new FieldTrialBasedConfig(); + std::unique_ptr call; + std::unique_ptr stream_state; // Creation of the streams must happen inside a task queue because it is // resued as a worker thread. - std::unique_ptr call; - std::unique_ptr stream_state; worker_thread->PostTask([&]() { call.reset(Call::Create(call_config)); diff --git a/sdk/objc/api/peerconnection/RTCPeerConnectionFactory.mm b/sdk/objc/api/peerconnection/RTCPeerConnectionFactory.mm index e2215c8b45..84c5f020b5 100644 --- a/sdk/objc/api/peerconnection/RTCPeerConnectionFactory.mm +++ b/sdk/objc/api/peerconnection/RTCPeerConnectionFactory.mm @@ -173,9 +173,8 @@ if (webrtc::field_trial::IsEnabled("WebRTC-Network-UseNWPathMonitor")) { dependencies.network_monitor_factory = webrtc::CreateNetworkMonitorFactory(); } + dependencies.task_queue_factory = webrtc::CreateDefaultTaskQueueFactory(); dependencies.trials = std::make_unique(); - dependencies.task_queue_factory = - webrtc::CreateDefaultTaskQueueFactory(dependencies.trials.get()); cricket::MediaEngineDependencies media_deps; media_deps.adm = std::move(audioDeviceModule); media_deps.task_queue_factory = dependencies.task_queue_factory.get();