Fix adaptive scheduler #463
Conversation
Also, fix some bad asserts that only cause problems in debug mode.
WalkthroughThe changes involve adjustments to the control flow and type handling within two core files: Changes
Sequence Diagram(s)sequenceDiagram
participant Reactor
participant Tracing as lf_tracing_global_shutdown()
note over Reactor: Old Flow
Reactor->>+Tracing: Initiate Shutdown
Reactor->>Reactor: Cleanup Reactors
Reactor-->>-Tracing: Complete
note over Reactor: New Flow
Reactor->>Reactor: Cleanup Reactors
Reactor->>+Tracing: Initiate Shutdown
Reactor-->>-Tracing: Complete
This diagram outlines the changes in the control flow for the Tip AI model upgrade
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (2)
- core/reactor_common.c (2 hunks)
- core/threaded/scheduler_adaptive.c (6 hunks)
Files not reviewed due to errors (1)
- core/reactor_common.c (no review received)
Additional comments not posted (5)
core/threaded/scheduler_adaptive.c (5)
137-137: LGTM!The type casting to
long longin the assertion ensures proper type handling.
227-227: LGTM!The adjustment in pointer arithmetic improves readability.
241-243: LGTM!The type casting to
long longin the assertions ensures proper type handling.
Line range hint
428-450: LGTM!The introduction of
total_num_reactionsand the logic modifications improve the handling of level advancement based on the number of reactions.
605-605: LGTM!The changes to handle jitter improve the flexibility and robustness of the function.
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- core/threaded/scheduler_adaptive.c (6 hunks)
Files skipped from review as they are similar to previous changes (1)
- core/threaded/scheduler_adaptive.c
As a reminder, the adaptive scheduler is a scheduler that, at least when it was created, either matched or outperformed our other schedulers in our benchmarks. Maybe we have too many schedulers, and maybe we should remove it, but this is the reason why we have it in our code base. See the graph here; the green line labeled "C heuristic" is the adaptive scheduler.
This fixes two serious bugs in the adaptive scheduler. In one bug, casting and pointer arithmetic are done in the wrong order. In the other bug, the level is advanced even if, in the process of waiting for the level to be ready to advance, more reactions were injected onto the reaction queue.
It seems like the adaptive scheduler may not have kept up with updates to federated execution in the past year.
Summary by CodeRabbit
Bug Fixes
Refactor