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

Extend platform API with functions for setting thread scheduling policy, core affinity and priority + remove C11 as platform #355

Merged
merged 36 commits into from
Apr 19, 2024

Conversation

erlingrj
Copy link
Collaborator

@erlingrj erlingrj commented Feb 13, 2024

This PR adds the following functions to the platform API:

  • lf_thread_self
  • lf_thread_set_priority
  • lf_thread_set_cpu
  • lf_thread_set_scheduling_policy

It provides implementations for POSIX and Zephyr. It also removes the C11 as a platform because it provides a strict subset of the functionality of the POSIX and Windows platforms which we already support. I have not heard about any platform not supporting POSIX (or windows) that does support C11.

I view this thread scheduling API as experimental and that it is likely to be changed in the future when we start using it with things like enclaved scheduling. The reason to add it now, before we use it in the runtime, is to enable the user to write real-time programs by setting a high priority and a real-time scheduling policy for their workers.

TODOs:

  • Can we merge lf_thread_self() and lf_thread_id()
  • Zephyr should return error if fair scheduler is requested (it only does priority)

Closes #290

@erlingrj erlingrj marked this pull request as draft February 13, 2024 12:36
@erlingrj
Copy link
Collaborator Author

erlingrj commented Feb 13, 2024

@soyerefsane, this should make your benchmarking easier. Here is an example program using this API: https://github.com/lf-lang/lingua-franca/blob/thread-scheduler-api/test/C/src/concurrent/ThreadScheduling.lf

Note that you can not change thread scheduling from user space, a simple fix is to run the program with sudo.

@erlingrj erlingrj added the feature New feature label Mar 8, 2024
@edwardalee
Copy link
Contributor

What is the status of this PR? The changes look very useful to me.

@erlingrj
Copy link
Collaborator Author

erlingrj commented Apr 9, 2024

I havnt had the time to revisit this draft again. I think it is quite complete. I will need to look through it again. There is a couple of unanswered questions still. I have updated the PR with a checklist of missing items.

@edwardalee
Copy link
Contributor

This branch has relatively simple conflicts with main. Is it OK to just overwrite main so we can get the tests to run?

@erlingrj
Copy link
Collaborator Author

erlingrj commented Apr 9, 2024

I can solve the conflicts. Two secs

Copy link
Collaborator Author

@erlingrj erlingrj left a comment

Choose a reason for hiding this comment

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

Did a self-review and I believe it looks OK now

@erlingrj erlingrj marked this pull request as ready for review April 17, 2024 13:31
@erlingrj erlingrj requested a review from edwardalee April 17, 2024 13:32
Copy link
Member

@lhstrh lhstrh left a comment

Choose a reason for hiding this comment

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

Only nitpicks regarding comments from my side.

low_level_platform/api/low_level_platform.h Outdated Show resolved Hide resolved
low_level_platform/api/low_level_platform.h Outdated Show resolved Hide resolved
low_level_platform/api/low_level_platform.h Outdated Show resolved Hide resolved
low_level_platform/api/low_level_platform.h Outdated Show resolved Hide resolved
Copy link
Contributor

@edwardalee edwardalee left a comment

Choose a reason for hiding this comment

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

Looks great to me! I added some small suggestions, mostly about documentation. Let's merge this!

low_level_platform/api/low_level_platform.h Outdated Show resolved Hide resolved
low_level_platform/api/low_level_platform.h Outdated Show resolved Hide resolved
low_level_platform/impl/src/lf_zephyr_support.c Outdated Show resolved Hide resolved
Copy link
Collaborator Author

@erlingrj erlingrj left a comment

Choose a reason for hiding this comment

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

Thanks for the feedback now I have improved the implementation to map a priority in the range [LF_SCHED_MIN_PRI, LF_SCHED_MAX_PRI] to the range of the target platform.

low_level_platform/api/low_level_platform.h Outdated Show resolved Hide resolved
low_level_platform/impl/src/lf_zephyr_support.c Outdated Show resolved Hide resolved
low_level_platform/api/low_level_platform.h Outdated Show resolved Hide resolved
@lhstrh
Copy link
Member

lhstrh commented Apr 18, 2024

Looks like Arduino is now failing...

@erlingrj
Copy link
Collaborator Author

Our Arduino-support is really frustrating as adding source files and header files in reactor-c must be accompanied by changes to our compiler to move/delete files because the arduino-cli is so limited...

@edwardalee edwardalee added this pull request to the merge queue Apr 19, 2024
Merged via the queue into main with commit 4f39b72 Apr 19, 2024
30 checks passed
@erlingrj erlingrj deleted the thread-scheduler-api branch May 6, 2024 09:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature New feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Extensions to the threaded API, do we really need the C11 threads platform?
3 participants