Skip to content
New issue

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

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

Already on GitHub? # to your account

ztimer: add ztimer_stopwatch convenience functions #19343

Merged
merged 2 commits into from
Mar 8, 2023

Conversation

benpicco
Copy link
Contributor

@benpicco benpicco commented Mar 3, 2023

Contribution description

Ztimer is often used to measure time, but often without the needed ztimer_aquire().

Add some convenience functions that automatically do the right thing and that are easier to use than manually doing the time measuring code, so they will hopefully see more adoption over the error-prone open-coded solution.

Testing procedure

The benchmark function has been converted to use the new API. tests/bench_runtime_coreapis still works as before.

Issues/PRs references

@benpicco benpicco requested review from kfessel and jue89 March 3, 2023 14:27
@github-actions github-actions bot added Area: sys Area: System Area: timers Area: timer subsystems labels Mar 3, 2023
@benpicco benpicco force-pushed the ztimer_stop_watch.h branch from 4b5c786 to 7b4498e Compare March 3, 2023 15:08
@kfessel
Copy link
Contributor

kfessel commented Mar 3, 2023

i like how this interface is flexible enough to be written like

typedef struct {
    ztimer_clock_t *clock;  /**< the clock to use     */
    ztimer zt;              /** some timer to keep this working */
    uint32_t start_time;    /**< start of measurement */
} ztimer_stopwatch_t;

static void _keep_active_cb(void *timer_ptr){
    if(!timer_ptr) return;
    ztimer_stopwatch_t timer = timer_ptr;
    ztimer_set(timer->clock, &timer->zt, 0xfff);
}

static inline void ztimer_stopwatch_init(ztimer_clock_t *clock, ztimer_stopwatch_t *timer)
{
    timer->clock = clock;
    timer->zt = { .callback=_keep_active_cb, .arg=timer };
}

static inline void ztimer_stopwatch_start(ztimer_stopwatch_t *timer)
{
    timer->zt.arg=timer;
    ztimer_set(timer->clock, &timer->zt, 0xfff);
    timer->start_time = ztimer_now(timer->clock);
}

static inline void ztimer_stopwatch_start_timout(ztimer_stopwatch_t *timer, uint32_t timeout)
{
    timer->zt.arg = (void *) 0;
    ztimer_set(timer->clock, &timer->zt, timeout);
    timer->start_time = ztimer_now(timer->clock);
}

static inline uint32_t ztimer_stopwatch_stop(ztimer_stopwatch_t *timer)
{
    uint32_t now = ztimer_now(timer->clock);
    ztimer_remove(timer->clock, &timer->zt)
    return now - timer->start_time;
}

@benpicco
Copy link
Contributor Author

benpicco commented Mar 3, 2023

Now I'm confused - isn't the purpose of ztimer_ondemand that you don't need to keep a dummy timer running? Or did I misunderstand the purpose of that module?

@kfessel
Copy link
Contributor

kfessel commented Mar 3, 2023

You are right this is the purpose of ondemand and this PR is good.
I am just thinking that acquire an release will lead to timer leakage or double release. And like how this interface can work without them. Missing something this was also a huge argument for acquire and release. We can even extend its functionality without acquire and release like counting more than 32Bit without requiring ztimer64, just by adding a small rollover-counter.

@benpicco benpicco force-pushed the ztimer_stop_watch.h branch from 7b4498e to d91c0e8 Compare March 6, 2023 13:31
@benpicco benpicco added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Mar 6, 2023
@riot-ci
Copy link

riot-ci commented Mar 6, 2023

Murdock results

✔️ PASSED

d91c0e8 sys/benchmark: make use of ztimer stopwatch

Success Failures Total Runtime
6919 0 6919 12m:40s

Artifacts

Copy link
Contributor

@kfessel kfessel left a comment

Choose a reason for hiding this comment

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

nice API; implementation working great; test provided 👍

@benpicco
Copy link
Contributor Author

benpicco commented Mar 8, 2023

bors merge

@bors bors bot merged commit fae992e into RIOT-OS:master Mar 8, 2023
@bors
Copy link
Contributor

bors bot commented Mar 8, 2023

Build succeeded:

@benpicco benpicco deleted the ztimer_stop_watch.h branch March 8, 2023 09:51
@MrKevinWeiss MrKevinWeiss added this to the Release 2023.04 milestone Apr 25, 2023
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
Area: sys Area: System Area: timers Area: timer subsystems CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants