Repository navigation
Index counter_data_ by the pool-local thread number in thread_func's exit log - #7838
Conversation
…exit log counter_data_ has one entry per worker of the pool and is indexed by the pool-local thread number everywhere else. The log message at the end of thread_func used the global thread number, which for every pool but the first one reads past the end of the vector or another worker's count. Signed-off-by: Hartmut Kaiser <[email protected]>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
📝 Summary
Merge Risk: ⚪ Minimal · up to
|
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Docstring Coverage |  |
Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 1 files. | Write docstrings for the functions missing them to satisfy the coverage threshold. |
-
Check name Status Explanation Title check The title names the actual change: the exit log in thread_func now indexes counter_data_ by the pool-local thread_num instead of global_thread_num. It matches the raw summary and the PR objectives, an… Description check The description states the bug, the cause, and the fix. The read used global_thread_num, but counter_data_ is indexed by the pool-local thread_num. For pools after the first, the global number is offs… Linked Issues check Check skipped because no linked issues were found for this pull request. Out of Scope Changes check Check skipped because no linked issues were found for this pull request. Full details: Title check
-
Explanation
The title names the actual change: the exit log in thread_func now indexes counter_data_ by the pool-local thread_num instead of global_thread_num. It matches the raw summary and the PR objectives, and it is specific enough for a reviewer scanning history.
Full details: Description check
-
Explanation
The description states the bug, the cause, and the fix. The read used global_thread_num, but counter_data_ is indexed by the pool-local thread_num. For pools after the first, the global number is offset, which can read past the vector or report another worker's count. The fix changes the index to thread_num. The description also explains when the bug shows up (info-level logging) and how it relates to related PRs. This matches the one-line code change in the raw summary and the PR objectives.
-
✨ Finishing Touches: 💡 1
-
📝 Generate docstrings: 💡
-
- Commit to this branch
- Create a new PR
🧪 Generate unit tests (beta)
-
- Commit to this branch
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.
Comment @coderabbitai help to get the list of available commands.
Up to standards ✅🟢 Issues
|
|
Commit 59c460962c74574edef1e8021e239deba7c395d5, Jenkins build 6138 Performance test report
Explanation of SymbolsThe benchmarks ran on medusa00, the results are compared with the references of that node. Percentage Diff. is the change of the median time relative to the reference, positive values mean faster.
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Describe the PRs merged since release-2.0.X was last rebuilt on master (#7769, #7784, #7787, #7829, #7832, #7833, #7835, #7836, #7838, #7839, #7845, #7855, #7858, #7860, #7864, #7865, #7867), and state in the C++20 modules section and under Breaking changes that applications must include only the public headers and the generated hpx/modules/<module>.hpp headers, not the headers inside an HPX module. Regenerate the closed issue and pull request lists for release-2.0.X at b059b41 against the v1.11.0 tag (795 PRs, 178 issues). Signed-off-by: Hartmut Kaiser <[email protected]>
Proposed Changes
scheduled_thread_pool<Scheduler>::thread_funcreadcounter_data_[global_thread_num], butcounter_data_has one entry per worker of the pool and is indexed by the pool-localthread_numeverywhere else (the scheduling loop of the same function usescounter_data_[thread_num]). For every pool except the first one,global_thread_numis offset by the pool's first thread, so the read went past the end of the vector or reported another worker's count. It now usesthread_num.Any background context you want to provide?
The read only happens when thread manager logging is enabled at level
infoor above, otherwise the arguments ofLTM_(info)are not evaluated, which is probably why it went unnoticed.This is the same kind of mix-up between pool-local and global worker numbers that #7812 fixes for the scheduler mode setters, but in a different file, so the two PRs don't conflict.
Found while reviewing #7836.
Checklist
Not all points below apply to all pull requests.