Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Redesign of GEDF scheduler #433

Merged
merged 42 commits into from
May 28, 2024
Merged

Redesign of GEDF scheduler #433

merged 42 commits into from
May 28, 2024

Conversation

edwardalee
Copy link
Contributor

@edwardalee edwardalee commented May 21, 2024

This PR has a companion PR in lingua-franca.

This a PR addresses issue #426, which points out that the GEDF scheduler was not prioritizing reactions with tighter deadlines unless they also had the same or lower levels compared to reactions with looser deadlines. It also fixes a bug (exposed by the change to GEDF) where deadlines were not propagated upstream to reactions that lexically precede a reaction with a deadline.

This scheduler also achieves slightly better performance than the previous scheduler (390 ns/reaction vs. 450 ns/reaction on a performance test).

This PR also cleans up scheduler_instance.h, removing several items that are relevant only to specific schedulers and are not part of the scheduling API. There are also various other code cleanups.

This PR also removes the chain_ID field of reactions, which never worked.

@edwardalee edwardalee added enhancement Enhancement of existing feature schedulers cleanup labels May 22, 2024
@edwardalee edwardalee marked this pull request as ready for review May 22, 2024 08:32
@lhstrh
Copy link
Member

lhstrh commented May 22, 2024

Seems to work well on Linux, but not on macOS 🤔

@edwardalee
Copy link
Contributor Author

Seems to work well on Linux, but not on macOS 🤔

It didn't run on Linux. Looks like a problem with propagation of deadlines across federates! We were masking this problem by having a flawed scheduler...

@edwardalee
Copy link
Contributor Author

Seems to work well on Linux, but not on macOS 🤔

It didn't run on Linux. Looks like a problem with propagation of deadlines across federates! We were masking this problem by having a flawed scheduler...

Actually, the situation looks worse. Deadlines are not propagated anywhere! ReactorInstance.assignDeadlines is never called anywhere in the code generation tree.

@lhstrh
Copy link
Member

lhstrh commented May 23, 2024

Oh you're right. I must have looked at the wrong CI output 🫣

@lhstrh lhstrh requested a review from erlingrj May 24, 2024 00:53
Copy link
Contributor

@petervdonovan petervdonovan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I left minor comments. Overall this looks like an improvement to the comprehensibility and scheduling behavior of the code.

core/threaded/scheduler_GEDF_NP.c Show resolved Hide resolved
core/threaded/scheduler_GEDF_NP.c Outdated Show resolved Hide resolved
core/threaded/scheduler_GEDF_NP.c Outdated Show resolved Hide resolved
core/threaded/scheduler_GEDF_NP.c Outdated Show resolved Hide resolved
@lhstrh lhstrh removed the request for review from erlingrj May 28, 2024 05:46
Copy link
Contributor

@petervdonovan petervdonovan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am happy about the latest changes -- thanks!

@lhstrh lhstrh enabled auto-merge May 28, 2024 16:42
@lhstrh lhstrh added this pull request to the merge queue May 28, 2024
Merged via the queue into main with commit d54dbd6 May 28, 2024
29 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bugfix cleanup enhancement Enhancement of existing feature schedulers
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants