WIP: refactored event handling #290
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #290 +/- ##
===========================================
- Coverage 57.67% 37.10% -20.57%
===========================================
Files 70 71 +1
Lines 10149 10484 +335
Branches 1335 1341 +6
===========================================
- Hits 5853 3890 -1963
- Misses 3462 5955 +2493
+ Partials 834 639 -195 ☔ View full report in Codecov by Sentry. |
- this is a performance optimization; it should greatly reduce calls to cond_signal without any change in functionality
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
- something isn't quite working right in some unit tests
src/margo-prio-pool.c
Outdated
| */ | ||
| ABT_pool_config_get(config, MARGO_PRIO_POOL_CONFIG_KEY_EFD, NULL, | ||
| &p_pool->efd); | ||
| // fprintf(stderr, "DBG: prio_pool got efd %d\n", p_pool->efd); |
Check notice
Code scanning / CodeQL
Commented-out code
- for now same as old loop, but with features stripped out - temporarily disabled all tests that rely on timers
- also fewer fn calls to clear state
- disabled for now, need to make some other adjustments
- this was only necessary as a workaround to a previous bug; its a bad idea - re-enable event progress path
- only use event method if abt pool and hg context both support it - use eventfd to break progress loop on finalize - use infinite timeout on epoll_wait
- revert prio_pool to use condition variable at all times for its own internal purposes - add user-space tracking of eventfd state to avoid writing events when it is already activated - clear eventfd only when it has produced epoll events
|
Closing PR but keeping branch for historical reference if we find a need to revisit it. Adding the ability for an Argobots pool to signal when it needs attention (so that it could be intergrated more naturally into an event loop) proved to cause too much overhead vs. our existing approach. |
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
work in progress for #288