Copilot commented on code in PR #62190:
URL: https://github.com/apache/doris/pull/62190#discussion_r3048817292
##########
be/src/exec/pipeline/dependency.cpp:
##########
@@ -77,9 +77,39 @@ void Dependency::set_ready() {
local_block_task.swap(_blocked_task);
}
for (auto task : local_block_task) {
Review Comment:
In the loop `for (auto task : local_block_task)`, `task` is copied (atomic
ref-count adjustments on `weak_ptr`). Iterating by const reference (`for (const
auto& task : local_block_task)`) avoids that overhead and is sufficient here
since the loop body doesn’t modify the `weak_ptr` itself.
```suggestion
for (const auto& task : local_block_task) {
```
##########
be/src/exec/pipeline/dependency.cpp:
##########
@@ -77,9 +77,39 @@ void Dependency::set_ready() {
local_block_task.swap(_blocked_task);
}
Review Comment:
`set_ready()` is now `noexcept`, but the initial critical section (the
`std::unique_lock<std::mutex> lc(_task_lock);` block and the code inside it) is
not inside any try/catch. If `std::unique_lock` throws `std::system_error` (or
any other unexpected exception escapes from this section), the process will
`std::terminate` due to `noexcept`. Consider wrapping the entire function body
in a top-level try/catch (similar to the per-task wake-up handling) or
otherwise ensuring all potentially-throwing operations are contained so
`noexcept` is actually honored.
--
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]
