Skip to content

Commit 750cba7

Browse files
authored
Revert "Add EXECUTORCH_THREADPOOL_SIZE options, default to u… (#14307)
…sing only performance cores (#14090)" This reverts commit 72d50b2. ### Summary Seeing crashes in macos unittest job.
1 parent 0f066e0 commit 750cba7

File tree

6 files changed

+2
-101
lines changed

6 files changed

+2
-101
lines changed

extension/threadpool/CMakeLists.txt

Lines changed: 1 addition & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -20,16 +20,6 @@ if(NOT CMAKE_CXX_STANDARD)
2020
set(CMAKE_CXX_STANDARD 17)
2121
endif()
2222

23-
# Threadpool size specifiers. Mutual exclusion is checking in default.cmake.
24-
# Default to using performance cores if
25-
# EXECUTORCH_THREADPOOL_USE_ALL_LOGICAL_CORES isn't set.
26-
set(_threadpool_size_flag)
27-
if(EXECUTORCH_THREADPOOL_USE_ALL_LOGICAL_CORES)
28-
set(_threadpool_size_flag "EXECUTORCH_THREADPOOL_USE_ALL_LOGICAL_CORES")
29-
else()
30-
set(_threadpool_size_flag "EXECUTORCH_THREADPOOL_USE_PERFORMANCE_CORES")
31-
endif()
32-
3323
add_library(
3424
extension_threadpool threadpool.cpp threadpool_guard.cpp thread_parallel.cpp
3525
cpuinfo_utils.cpp
@@ -46,9 +36,7 @@ target_include_directories(
4636
$<BUILD_INTERFACE:${EXECUTORCH_ROOT}/backends/xnnpack/third-party/cpuinfo/include>
4737
$<BUILD_INTERFACE:${EXECUTORCH_ROOT}/backends/xnnpack/third-party/pthreadpool/include>
4838
)
49-
target_compile_definitions(
50-
extension_threadpool PUBLIC ET_USE_THREADPOOL ${_threadpool_size_flag}
51-
)
39+
target_compile_definitions(extension_threadpool PUBLIC ET_USE_THREADPOOL)
5240
target_compile_options(extension_threadpool PUBLIC ${_common_compile_options})
5341

5442
# Install libraries

extension/threadpool/targets.bzl

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,6 @@ def define_common_targets():
2222
name = "threadpool_lib",
2323
srcs = _THREADPOOL_SRCS,
2424
deps = [
25-
":cpuinfo_utils",
2625
"//executorch/runtime/core:core",
2726
"//executorch/runtime/core/portable_type/c10/c10:c10",
2827
],

extension/threadpool/test/threadpool_test.cpp

Lines changed: 0 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@
77
*/
88

99
#include <executorch/extension/threadpool/threadpool.h>
10-
#include <executorch/runtime/platform/runtime.h>
1110

1211
#include <mutex>
1312
#include <numeric>
@@ -72,8 +71,6 @@ void run_lambda_with_size(
7271
} // namespace
7372

7473
TEST(ThreadPoolTest, ParallelAdd) {
75-
executorch::runtime::runtime_init();
76-
7774
std::vector<int32_t> a, b, c, c_ref;
7875
size_t vector_size = 100;
7976
size_t grain_size = 10;
@@ -114,8 +111,6 @@ TEST(ThreadPoolTest, ParallelAdd) {
114111

115112
// Test parallel reduction where we acquire lock within lambda
116113
TEST(ThreadPoolTest, ParallelReduce) {
117-
executorch::runtime::runtime_init();
118-
119114
std::vector<int32_t> a;
120115
int32_t c = 0, c_ref = 0;
121116
size_t vector_size = 100;
@@ -149,8 +144,6 @@ TEST(ThreadPoolTest, ParallelReduce) {
149144
// Copied from
150145
// caffe2/aten/src/ATen/test/test_thread_pool_guard.cp
151146
TEST(TestNoThreadPoolGuard, TestThreadPoolGuard) {
152-
executorch::runtime::runtime_init();
153-
154147
auto threadpool_ptr = ::executorch::extension::threadpool::get_pthreadpool();
155148

156149
ASSERT_NE(threadpool_ptr, nullptr);
@@ -180,8 +173,6 @@ TEST(TestNoThreadPoolGuard, TestThreadPoolGuard) {
180173
}
181174

182175
TEST(TestNoThreadPoolGuard, TestRunWithGuard) {
183-
executorch::runtime::runtime_init();
184-
185176
const std::vector<int64_t> array = {1, 2, 3};
186177

187178
auto pool = ::executorch::extension::threadpool::get_threadpool();

extension/threadpool/threadpool.cpp

Lines changed: 1 addition & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,6 @@
66
* LICENSE file in the root directory of this source tree.
77
*/
88

9-
#include <executorch/extension/threadpool/cpuinfo_utils.h>
109
#include <executorch/extension/threadpool/threadpool.h>
1110

1211
#include <algorithm>
@@ -15,26 +14,9 @@
1514

1615
#include <executorch/extension/threadpool/threadpool_guard.h>
1716
#include <executorch/runtime/platform/assert.h>
18-
#include <executorch/runtime/platform/runtime.h>
1917

2018
#include <cpuinfo.h>
2119

22-
// At most one mode should be set.
23-
#if ( \
24-
defined(EXECUTORCH_THREADPOOL_USE_ALL_LOGICAL_CORES) && \
25-
defined(EXECUTORCH_THREADPOOL_USE_PERFORMANCE_CORES))
26-
#error Multiple \
27-
threadpool size specifiers are set.At most one of \
28-
EXECUTORCH_THREADPOOL_USE_ALL_LOGICAL_CORES, \
29-
and EXECUTORCH_THREADPOOL_USE_PERFORMANCE_CORES may be defined.
30-
#endif
31-
32-
// Default to EXECUTORCH_THREADPOOL_USE_ALL_LOGICAL_CORES if no mode is set.
33-
#if !defined(EXECUTORCH_THREADPOOL_USE_ALL_LOGICAL_CORES) && \
34-
!defined(EXECUTORCH_THREADPOOL_USE_PERFORMANCE_CORES)
35-
#define EXECUTORCH_THREADPOOL_USE_ALL_LOGICAL_CORES 1
36-
#endif
37-
3820
namespace executorch::extension::threadpool {
3921

4022
#if !(defined(WIN32))
@@ -114,25 +96,12 @@ void ThreadPool::run(
11496
// get_threadpool is not thread safe due to leak_corrupted_threadpool
11597
// Make this part threadsafe: TODO(kimishpatel)
11698
ThreadPool* get_threadpool() {
117-
executorch::runtime::runtime_init();
118-
11999
if (!cpuinfo_initialize()) {
120100
ET_LOG(Error, "cpuinfo initialization failed");
121101
return nullptr; // NOLINT(facebook-hte-NullableReturn)
122102
}
123103

124-
// Choose the number of threads according to the EXECUTORCH_THREADPOOL_
125-
// options. See the description in threadpool.h.
126-
127-
#if defined(EXECUTORCH_THREADPOOL_USE_ALL_LOGICAL_CORES)
128-
// Use threads=cores.
129-
static int num_threads = cpuinfo_get_processors_count();
130-
#else
131-
// Set threads equal to the number of performance cores.
132-
static int num_threads =
133-
::executorch::extension::cpuinfo::get_num_performant_cores();
134-
#endif
135-
104+
int num_threads = cpuinfo_get_processors_count();
136105
/*
137106
* For llvm-tsan, holding limit for the number of locks for a single thread
138107
* is 63 (because of comparison < 64 instead of <=). pthreadpool's worst

extension/threadpool/threadpool.h

Lines changed: 0 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -14,22 +14,6 @@
1414

1515
#include <pthreadpool.h>
1616

17-
/*
18-
* Threadpool Options:
19-
*
20-
* Threadpool size has a sizble affect on performance. By default, the
21-
* threadpool will be sized according to the number of performance cores. This
22-
* behavior can be overriden with the following build-time options. Note that
23-
* these options are mutually exclusive.
24-
*
25-
* - EXECUTORCH_THREADPOOL_USE_PERFORMANCE_CORES (flag) - Sizes the threadpool
26-
* equal to the number of performance cores on the system. This is the default
27-
* behavior.
28-
* - EXECUTORCH_THREADPOOL_USE_ALL_LOGICAL_CORES (flag) - Sizes the threadpool
29-
* equal to the number of logical cores on system. This is the historical
30-
* behavior.
31-
*/
32-
3317
namespace executorch::extension::threadpool {
3418

3519
class ThreadPool final {

tools/cmake/preset/default.cmake

Lines changed: 0 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -176,36 +176,6 @@ define_overridable_option(
176176
${_default_executorch_build_cpuinfo}
177177
)
178178

179-
# Threadpool size options. At most one can be specified. Note that the default
180-
# is managed in threadpool.cpp to allow the user to specify an alternate mode
181-
# without needing to explicitly set the default to off.
182-
define_overridable_option(
183-
EXECUTORCH_THREADPOOL_USE_PERFORMANCE_CORES
184-
"Set the number of threads used for CPU parallel computation equal to the number of performant CPU cores."
185-
BOOL
186-
OFF
187-
)
188-
define_overridable_option(
189-
EXECUTORCH_THREADPOOL_USE_ALL_LOGICAL_CORES
190-
"Set the number of threads used for CPU parallel computation equal to the number of logical CPU cores."
191-
BOOL
192-
OFF
193-
)
194-
195-
check_required_options_on(
196-
IF_ON EXECUTORCH_THREADPOOL_USE_ALL_LOGICAL_CORES REQUIRES
197-
EXECUTORCH_BUILD_PTHREADPOOL EXECUTORCH_BUILD_CPUINFO
198-
)
199-
check_required_options_on(
200-
IF_ON EXECUTORCH_THREADPOOL_USE_PERFORMANCE_CORES REQUIRES
201-
EXECUTORCH_BUILD_PTHREADPOOL EXECUTORCH_BUILD_CPUINFO
202-
)
203-
204-
check_conflicting_options_on(
205-
IF_ON EXECUTORCH_THREADPOOL_USE_PERFORMANCE_CORES CONFLICTS_WITH
206-
EXECUTORCH_THREADPOOL_USE_ALL_LOGICAL_CORES
207-
)
208-
209179
# TODO(jathu): move this to platform specific presets when created
210180
set(_default_executorch_build_executor_runner ON)
211181
if(APPLE AND "${SDK_NAME}" STREQUAL "iphoneos")

0 commit comments

Comments
 (0)