zhztheplayer commented on code in PR #11249:
URL:
https://github.com/apache/incubator-gluten/pull/11249#discussion_r2588561901
##########
cpp/velox/memory/VeloxMemoryManager.cc:
##########
@@ -445,27 +445,11 @@ bool VeloxMemoryManager::tryDestructSafe() {
}
VeloxMemoryManager::~VeloxMemoryManager() {
- static const uint32_t kWaitTimeoutMs =
FLAGS_gluten_velox_async_timeout_on_task_stopping; // 30s by default
- uint32_t accumulatedWaitMs = 0UL;
bool destructed = false;
- for (int32_t tryCount = 0; accumulatedWaitMs < kWaitTimeoutMs; tryCount++) {
Review Comment:
> This appears to be a different solution from the ones we previously
discussed.
Just to clarify a bit, `Task::requestCancel` is not a different solution but
something we already called and expected it could get reliable gradually in
Velox:
https://github.com/apache/incubator-gluten/blob/48efe26923f8e8148d9e3812b29343c6f6d398b2/cpp/velox/compute/WholeStageResultIterator.h#L51
We don't need this wait mechanism in memory manager if `Task::requestCancel`
works smoothly. Memory management is a sub-optimal place to determine whether
the task is completely ended or not.
--
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]
