Copilot commented on code in PR #13115:
URL: https://github.com/apache/gluten/pull/13115#discussion_r4091508318
##########
cpp/velox/compute/VeloxBackend.cc:
##########
@@ -328,6 +328,20 @@ ReaderThreadPool* VeloxBackend::getReaderThreadPool() {
return readerThreadPool_.get();
}
+folly::Executor* VeloxBackend::hashTableBuildExecutor() {
+ static std::once_flag hashTableBuildExecutorInit;
+ std::call_once(hashTableBuildExecutorInit, [this] {
Review Comment:
This flag is function-static, so it outlives every `VeloxBackend` instance.
`tearDown()` resets `hashTableBuildExecutor_`, and `create()` can replace
`instance_`; after a subsequent initialization, `call_once` is skipped and this
returns a null executor, so the next parallel build fails when `folly::via`
submits to it. Make the initialization guard instance-owned (or reinitialize it
together with the member) instead of function-static.
##########
cpp/velox/compute/VeloxBackend.cc:
##########
@@ -328,6 +328,20 @@ ReaderThreadPool* VeloxBackend::getReaderThreadPool() {
return readerThreadPool_.get();
}
+folly::Executor* VeloxBackend::hashTableBuildExecutor() {
+ static std::once_flag hashTableBuildExecutorInit;
+ std::call_once(hashTableBuildExecutorInit, [this] {
+ auto numThreads = backendConf_->get<int32_t>(kHashTableBuildThreads,
kHashTableBuildThreadsDefault);
+ if (numThreads <= 0) {
+ // Fall back to the executor's task-slot count, matching the sizing this
build path
+ // previously inherited from the io executor.
+ numThreads = backendConf_->get<int32_t>(kNumTaskSlotsPerExecutor, 1);
+ }
+ hashTableBuildExecutor_ =
std::make_unique<folly::CPUThreadPoolExecutor>(numThreads);
Review Comment:
The fallback can legally produce zero here: `init()` accepts
`kNumTaskSlotsPerExecutor == 0`, and the existing IO-pool path avoids
constructing a `CPUThreadPoolExecutor` when its thread count is zero.
Constructing this pool with zero threads is invalid (and leaves the parallel
build without an executor), so clamp the fallback to at least one thread or
otherwise handle zero before constructing the pool.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
