Skip to content

Add embassy executor cpu stats #3728

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

Draft
wants to merge 11 commits into
base: main
Choose a base branch
from

Conversation

Szybet
Copy link
Contributor

@Szybet Szybet commented Jul 1, 2025

Thank you for your contribution!

We appreciate the time and effort you've put into this pull request.
To help us review it efficiently, please ensure you've gone through the following checklist:

Submission Checklist 📝

  • I have updated existing examples or added new ones (if applicable).
  • I have used cargo xtask fmt-packages command to ensure that all changed code is formatted correctly.
  • My changes were added to the CHANGELOG.md in the proper section.
  • I have added necessary changes to user code to the Migration Guide.
  • My changes are in accordance to the esp-rs developer guidelines

Extra:

Pull Request Details 📖

Added CPU usage measurements via Embassy executor using the low_power_wait feature

Description

Based on this PR: embassy-rs/embassy#3920, added the same functionality internally (not as an example). Also added it to the new YAML-based config, but couldn't find a way for the config to check if the required low_power_wait feature is enabled, as none of the existing validators (negative_integer, non_negative_integer, positive_integer, integer_in_range, enumeration) are suitable for this.

Testing

Used it to debug an issue with bad UART communication, because CPU was fully being utilised by another part of the program.

@@ -21,7 +21,7 @@ test = false
[dependencies]
cfg-if = "1.0.0"
critical-section = "1.2.0"
esp-hal = { version = "1.0.0-beta.1", path = "../esp-hal" }
esp-hal = { version = "1.0.0-beta.1", path = "../esp-hal", features = ["unstable"] }
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
esp-hal = { version = "1.0.0-beta.1", path = "../esp-hal", features = ["unstable"] }
esp-hal = { version = "1.0.0-beta.1", path = "../esp-hal" }

We intentionally don't enable the feature.

Copy link
Contributor Author

@Szybet Szybet Jul 1, 2025

Choose a reason for hiding this comment

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

Without it it doesn't compile standalone. If that is intentional I will disable it

Copy link
Contributor

Choose a reason for hiding this comment

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

You need to enable unstable in your own project, not like this.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

@bugadani bugadani added the status:blocked Unable to progress - dependent on another task label Jul 1, 2025
@bugadani bugadani removed the status:blocked Unable to progress - dependent on another task label Jul 1, 2025
@Szybet
Copy link
Contributor Author

Szybet commented Jul 3, 2025

As #3737 is merged do you wish to redo this PR as an example using hooking into executor? I would still be beneficial to have something like that visable for new devs

@bugadani
Copy link
Contributor

bugadani commented Jul 3, 2025

Sure, why not? rtos-trace is a bit better tool to gather usage stats in general, but this would still be a fine example.

A few notes:

  • Use esp_hal::time::Instant so that the stats aren't dependent on the time driver's tick rate
  • Make sure you collect stats into different counters for different executors.
  • We should ideally extend this to interrupt executors, too. You don't have to, but if you have a good idea, you can play with it as a stretch goal. Most likely the callbacks should be part of the constructor & the type of the executor. This also means that the timings you measure can overlap (thread executor starts polling, interrupt happens & interrupt executor start polling, interrupt executor finishes, thread executor finishes). We probably don't want to end up measuring >100% CPU use.

@Szybet
Copy link
Contributor Author

Szybet commented Jul 18, 2025

I would like to write the example with #3813 as it's simple, perfect for an example, so I will wait for it to be merged (in one form or another)

Make sure you collect stats into different counters for different executors.

Is that a thing you can do now with #3737?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants