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

refactor(threads): avoid unnecessary scheduler invocations #459

Merged

Conversation

elenaf9
Copy link
Collaborator

@elenaf9 elenaf9 commented Oct 9, 2024

Description

This PR implements some changes and checks to avoid unnecessary scheduler invocations:

  • the scheduler is now triggered in set_state as a central point
  • if a thread changes into ThreadState::Running, only trigger the scheduler if the thread has a higher priority than the currently running one (new)
  • always trigger the scheduler when a thread changes from ThreadState::Running into another state because this only ever happens if a currently running thread gets blocked
  • only trigger the scheduler in yield_now if the runqueue contains any other threads

Issues/PRs references

Extracted from #397.

Benchmarks

(ticks per iteration)
bench_sched_flags is PR'd in #456

nRF52840dk:

benchmark main this PR
bench_sched_yield with two threads 560 580
bench_sched_yield with one thread 235 98
bench_sched_flags 1848 1461

RPI-Pico

benchmark main this PR
bench_sched_yield with two threads 631 647
bench_sched_yield with one thread 243 78
bench_sched_flags 1739 1495

The additional checks come a some cost when the scheduler invocation is really needed, as visible in the bench_sched_yield benchmark.
But I think this is tolerable, because the actual context switching logic (i.e. sched) is not affected, and in many cases we save an unnecessary scheduler call that would be much more expensive.

Open Questions

See self review.

Change checklist

  • I have cleaned up my commit history and squashed fixup commits.
  • I have followed the Coding Conventions.
  • I have performed a self-review of my own code.
  • I have made corresponding changes to the documentation.

src/riot-rs-threads/src/lib.rs Outdated Show resolved Hide resolved
src/riot-rs-threads/src/lib.rs Show resolved Hide resolved
src/riot-rs-threads/src/lib.rs Outdated Show resolved Hide resolved
src/riot-rs-threads/src/lib.rs Outdated Show resolved Hide resolved
src/riot-rs-threads/src/lib.rs Outdated Show resolved Hide resolved
@elenaf9 elenaf9 force-pushed the threads/avoid-unneeded-sched branch from b52846c to 7f3bf34 Compare October 10, 2024 10:02
@elenaf9
Copy link
Collaborator Author

elenaf9 commented Oct 10, 2024

Rebased so that recently merged bench_sched_flags can be run on this branch.

@kaspar030
Copy link
Collaborator

LGTM!

@kaspar030 kaspar030 added this pull request to the merge queue Oct 10, 2024
Merged via the queue into future-proof-iot:main with commit fe3e23c Oct 10, 2024
26 checks passed
@elenaf9 elenaf9 deleted the threads/avoid-unneeded-sched branch October 10, 2024 21:44
@elenaf9 elenaf9 mentioned this pull request Oct 12, 2024
6 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants