Rewrite time driver - Githubissues

esp-rs / esp-hal

no_std Hardware Abstraction Layers for ESP32 microcontrollers

https://docs.esp-rs.org/esp-hal/

Apache License 2.0

764 stars 214 forks source link

Rewrite time driver #2559

Open bugadani opened 4 days ago

bugadani commented 4 days ago

This PR does a few things:

Reduces the use of critical sections
Moves the interrupt handler and/or the associated timer into Alarm, so that we don't have to maintain an array of handlers, and we don't have to index to access initialized timers.
Uses embassy's (un)safety requirement to avoid a null check when calling timer callbacks.
Checks if there are enough timers to initialize the pre-allocated alarms
Creates the alarms array with a macro, so that we don't have to manually define interrupt handlers.
Bonus change, though it's not needed with the last commit: #[handler] functions can now be references in const context.
The PR also adds a basic embassy benchmark and slightly optimizes the thread-mode executor, though I can submit that separately if needed.

bugadani commented 1 day ago

Ran the added benchmark on 0.22 and this PR:

0.22: INFO - Test OK, count=26628, cycles=45065/100 PR: INFO - Test OK, count=26616, cycles=45085/100 PR with Relaxed memory ordering: INFO - Test OK, count=27535, cycles=43580/100

This isn't surprising as the benchmark doesn't rely on the time driver, but at least it also means that set_alarm(u64::MAX) didn't get worse.

bjoernQ commented 4 hours ago

Is the benchmark supposed to stay or is it a temporary thing?

bugadani commented 4 hours ago

I'd like to keep it around

bjoernQ commented 3 hours ago

I'd like to keep it around

Just asking because it looks a bit out of place here. I'm also wondering if we should have more benchmarks (for other things) in general and if we should have such a thing in parallel to qa-test and hil-test (maybe not part of this PR)

bugadani commented 3 hours ago

Just asking because it looks a bit out of place here. I'm also wondering if we should have more benchmarks (for other things) in general and if we should have such a thing in parallel to qa-test and hil-test (maybe not part of this PR)

I guess it fits well enough into qa-test, I'll move it.