Revert "[TaskScheduler]: Create no detach below initial capacity feature"
This reverts commit 3f40244c. Reason for revert: Caused a data race running components_unittests with ScopedFeatureList. Sample failure: https://ci.chromium.org/p/chromium/builders/luci.chromium.ci/Linux%20TSan%20Tests/30398 WARNING: ThreadSanitizer: data race (pid=9805) Write of size 8 at 0x55b63bfba2b0 by main thread: #0 base::FeatureList::ClearInstanceForTesting() base/feature_list.cc:284:27 (components_unittests+0x85bec63) #1 base::test::ScopedFeatureList::~ScopedFeatureList() base/test/scoped_feature_list.cc:98:3 (components_unittests+0x9a65d52) #2 content::UnitTestTestSuite::~UnitTestTestSuite() content/public/test/unittest_test_suite.cc:61:1 (components_unittests+0xb03cf21) #3 operator() buildtools/third_party/libc++/trunk/include/memory:2325:5 (components_unittests+0x8256779) #4 reset buildtools/third_party/libc++/trunk/include/memory:2638 (components_unittests+0x8256779) #5 ~unique_ptr buildtools/third_party/libc++/trunk/include/memory:2592 (components_unittests+0x8256779) #6 ~__tuple_leaf buildtools/third_party/libc++/trunk/include/tuple:171 (components_unittests+0x8256779) #7 ~tuple buildtools/third_party/libc++/trunk/include/tuple:470 (components_unittests+0x8256779) #8 ~BindState base/bind_internal.h:871 (components_unittests+0x8256779) #9 base::internal::BindState<int (content::UnitTestTestSuite::*)(), std::__1::unique_ptr<content::UnitTestTestSuite, std::__1::default_delete<content::UnitTestTestSuite> > >::Destroy(base::internal::BindStateBase const*) base/bind_internal.h:874 (components_unittests+0x8256779) #10 Destruct base/callback_internal.cc:29:3 (components_unittests+0x85b80c7) #11 Release base/memory/ref_counted.h:403 (components_unittests+0x85b80c7) #12 Release base/memory/scoped_refptr.h:284 (components_unittests+0x85b80c7) #13 ~scoped_refptr base/memory/scoped_refptr.h:208 (components_unittests+0x85b80c7) #14 base::internal::CallbackBase::~CallbackBase() base/callback_internal.cc:84 (components_unittests+0x85b80c7) #15 base::LaunchUnitTests(int, char**, base::OnceCallback<int ()>) base/test/launcher/unit_test_launcher.cc:575:3 (components_unittests+0x9a71272) #16 main components/test/run_all_unittests.cc:8:10 (components_unittests+0x4e49ad5) Previous read of size 8 at 0x55b63bfba2b0 by thread T12 (mutexes: write M19294): #0 base::FeatureList::IsEnabled(base::Feature const&) base/feature_list.cc:200:8 (components_unittests+0x85be82d) #1 CanCleanupLockRequired base/task/task_scheduler/scheduler_worker_pool_impl.cc:672:12 (components_unittests+0x866a5e8) #2 base::internal::SchedulerWorkerPoolImpl::SchedulerWorkerDelegateImpl::GetWork(base::internal::SchedulerWorker*) base/task/task_scheduler/scheduler_worker_pool_impl.cc:538 (components_unittests+0x866a5e8) #3 base::internal::SchedulerWorker::RunWorker() base/task/task_scheduler/scheduler_worker.cc:324:51 (components_unittests+0x866f81d) #4 base::internal::SchedulerWorker::RunPooledWorker() base/task/task_scheduler/scheduler_worker.cc:229:3 (components_unittests+0x866f481) #5 base::internal::SchedulerWorker::ThreadMain() base/task/task_scheduler/scheduler_worker.cc:208:7 (components_unittests+0x866f2f1) #6 base::(anonymous namespace)::ThreadFunc(void*) base/threading/platform_thread_posix.cc:81:13 (components_unittests+0x86daf04) Location is global 'base::(anonymous namespace)::g_feature_list_instance' of size 8 at 0x55b63bfba2b0 (components_unittests+0x00000ebe52b0) Original change's description: > [TaskScheduler]: Create no detach below initial capacity feature > > Under this experiment, scheduler workers are only detached if the pool is > above its initial capacity (threads that are created to replace blocked threads). > > 2 options were considered: > Option A: Detach only when over initial capacity. > > Option B: Detach only when over current capacity (includes currently blocked threads in capacity). > This might better handle the following case: At any given time, there is at least 1 blocked thread. > On top of that, some periodic work uses all worker every 30s or so. The current capacity will > encompass for the blocked thread and avoid detaching it periodically. > > Option A was picked because it is more conservative. Initial capacity is smaller or > equal to current capacity, so detaching is closer to current behavior. We want to avoid having > too many threads that aren't used. > > Bug: 847501 > Change-Id: I0b116db54095767768b158d92f5f146249720b45 > Reviewed-on: https://chromium-review.googlesource.com/c/1348863 > Commit-Queue: Etienne Pierre-Doray <etiennep@chromium.org> > Reviewed-by: François Doray <fdoray@chromium.org> > Cr-Commit-Position: refs/heads/master@{#612698} TBR=fdoray@chromium.org,etiennep@chromium.org # Not skipping CQ checks because original CL landed > 1 day ago. Bug: 847501 Change-Id: I951f5c5701e2d296b2c4edef37648105c4911cf9 Reviewed-on: https://chromium-review.googlesource.com/c/1359127Reviewed-by:Robert Flack <flackr@chromium.org> Commit-Queue: Robert Flack <flackr@chromium.org> Cr-Commit-Position: refs/heads/master@{#613229}
Showing
Please register or sign in to comment