Skip to content

sys/benchmark: Convert to ztimer #17126

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 2 commits into from
Nov 15, 2021
Merged

sys/benchmark: Convert to ztimer #17126

merged 2 commits into from
Nov 15, 2021

Conversation

bergzand
Copy link
Member

@bergzand bergzand commented Nov 3, 2021

Contribution description

Converts the benchmark utility to use ztimer

Testing procedure

tests/bench_runtime_coreapis/ makes use of this

On master (nrf52840dk) 
/home/koen/dev/RIOT-review/dist/tools/pyterm/pyterm -p "/dev/ttyACM0" -b "115200"  
python-exec: Invalid impl in /etc/python-exec/python-exec.conf: python3.7
Twisted not available, please install it if you want to use pyterm's JSON capabilities
2021-11-03 13:07:58,196 # Connect to serial port /dev/ttyACM0
Welcome to pyterm!
Type '/exit' to exit.
s
2021-11-03 13:08:00,174 # START
2021-11-03 13:08:00,179 # main(): This is RIOT! (Version: 2022.01-devel-322-g49cec-HEAD)
2021-11-03 13:08:00,183 # Runtime of Selected Core API functions
2021-11-03 13:08:00,184 # 
2021-11-03 13:08:00,253 #                  nop loop:     62501us  ---   0.062us per call  ---   15999744 calls per sec
2021-11-03 13:08:00,253 # 
2021-11-03 13:08:00,261 #              mutex_init():         2us  ---   0.000us per call  ---  1783793664 calls per sec
2021-11-03 13:08:01,274 #         mutex lock/unlock:   1015627us  ---   1.015us per call  ---     984613 calls per sec
2021-11-03 13:08:01,274 # 
2021-11-03 13:08:01,839 #        thread_flags_set():    562501us  ---   0.562us per call  ---    1777774 calls per sec
2021-11-03 13:08:02,388 #      thread_flags_clear():    546877us  ---   0.546us per call  ---    1828564 calls per sec
2021-11-03 13:08:04,066 # thread flags set/wait any:   1687502us  ---   1.687us per call  ---     592591 calls per sec
2021-11-03 13:08:05,434 # thread flags set/wait all:   1375002us  ---   1.375us per call  ---     727271 calls per sec
2021-11-03 13:08:07,081 # thread flags set/wait one:   1656252us  ---   1.656us per call  ---     603772 calls per sec
2021-11-03 13:08:07,082 # 
2021-11-03 13:08:07,924 #         msg_try_receive():    843752us  ---   0.843us per call  ---    1185182 calls per sec
2021-11-03 13:08:08,242 #               msg_avail():    312501us  ---   0.312us per call  ---    3199989 calls per sec
2021-11-03 13:08:08,243 # 
2021-11-03 13:08:08,243 # [SUCCESS]
This PR (nrf52840dk) 
/home/koen/dev/RIOT-review/dist/tools/pyterm/pyterm -p "/dev/ttyACM0" -b "115200"  
python-exec: Invalid impl in /etc/python-exec/python-exec.conf: python3.7
Twisted not available, please install it if you want to use pyterm's JSON capabilities
2021-11-03 13:07:00,126 # Connect to serial port /dev/ttyACM0
s
Welcome to pyterm!
Type '/exit' to exit.
s
2021-11-03 13:07:01,130 # START
2021-11-03 13:07:01,136 # main(): This is RIOT! (Version: 2022.01-devel-323-g5e286-pr/benchmark/ztimer)
2021-11-03 13:07:01,140 # Runtime of Selected Core API functions
2021-11-03 13:07:01,140 # 
2021-11-03 13:07:01,210 #                  nop loop:     62501us  ---   0.062us per call  ---   15999744 calls per sec
2021-11-03 13:07:01,210 # 
2021-11-03 13:07:01,218 #              mutex_init():         1us  ---   0.000us per call  ---  3567587328 calls per sec
2021-11-03 13:07:02,185 #         mutex lock/unlock:    968751us  ---   0.968us per call  ---    1032256 calls per sec
2021-11-03 13:07:02,186 # 
2021-11-03 13:07:02,704 #        thread_flags_set():    515626us  ---   0.515us per call  ---    1939390 calls per sec
2021-11-03 13:07:03,207 #      thread_flags_clear():    500001us  ---   0.500us per call  ---    1999996 calls per sec
2021-11-03 13:07:04,932 # thread flags set/wait any:   1734377us  ---   1.734us per call  ---     576575 calls per sec
2021-11-03 13:07:06,301 # thread flags set/wait all:   1375001us  ---   1.375us per call  ---     727272 calls per sec
2021-11-03 13:07:07,995 # thread flags set/wait one:   1703126us  ---   1.703us per call  ---     587155 calls per sec
2021-11-03 13:07:07,996 # 
2021-11-03 13:07:08,793 #         msg_try_receive():    796876us  ---   0.796us per call  ---    1254900 calls per sec
2021-11-03 13:07:09,064 #               msg_avail():    265626us  ---   0.265us per call  ---    3764691 calls per sec
2021-11-03 13:07:09,064 # 
2021-11-03 13:07:09,065 # [SUCCESS]

Issues/PRs references

Another ztimer conversion

@bergzand bergzand requested a review from fjmolinas November 3, 2021 12:08
@bergzand bergzand added the Type: cleanup The issue proposes a clean-up / The PR cleans-up parts of the codebase / documentation label Nov 3, 2021
@github-actions github-actions bot added the Area: sys Area: System label Nov 3, 2021
@bergzand bergzand added CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Impact: minor The PR is small in size and might only require a quick look of a knowledgeable reviewer labels Nov 3, 2021
@kaspar030 kaspar030 changed the title benchmark: Convert to ztimer sys/benchmark: Convert to ztimer Nov 3, 2021
@bergzand bergzand force-pushed the pr/benchmark/ztimer branch from 5e28680 to d137e8c Compare November 3, 2021 12:41
kaspar030
kaspar030 previously approved these changes Nov 3, 2021
View reviewed changes
Copy link
Contributor

@kaspar030 kaspar030 left a comment

Choose a reason for hiding this comment

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

ACK.

@github-actions github-actions bot added the Area: tests Area: tests and testing framework label Nov 10, 2021
@bergzand bergzand requested a review from kaspar030 November 10, 2021 09:31
@bergzand bergzand dismissed kaspar030’s stale review November 10, 2021 09:31

Added tests/periph_gpio

kaspar030
kaspar030 approved these changes Nov 10, 2021
View reviewed changes
Copy link
Contributor

@kaspar030 kaspar030 left a comment

Choose a reason for hiding this comment

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

re-ACK, pls squash!

@bergzand bergzand force-pushed the pr/benchmark/ztimer branch 2 times, most recently from 19e56ab to 9fd09cf Compare November 15, 2021 09:19
@bergzand
Copy link
Member Author

Force pushed the missing kconfig bits

@github-actions github-actions bot added the Area: Kconfig Area: Kconfig integration label Nov 15, 2021
@bergzand bergzand force-pushed the pr/benchmark/ztimer branch from 9fd09cf to 9df794e Compare November 15, 2021 13:17
@bergzand bergzand force-pushed the pr/benchmark/ztimer branch from 9df794e to f1feaa2 Compare November 15, 2021 14:24
@bergzand
Copy link
Member Author

All green!

@kaspar030 kaspar030 merged commit 3871948 into RIOT-OS:master Nov 15, 2021
@bergzand bergzand deleted the pr/benchmark/ztimer branch November 16, 2021 16:21
@fjmolinas fjmolinas added this to the Release 2022.01 milestone Nov 18, 2021
gschorcht
gschorcht reviewed Nov 29, 2021
View reviewed changes
sys/benchmark/Kconfig
@@ -7,5 +7,5 @@

config MODULE_BENCHMARK
bool "Simple benchmarks support"
depends on MODULE_XTIMER
depends on MODULE_ZTIMER_USEC
Copy link
Contributor

@gschorcht gschorcht Nov 29, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

Shouldn't this be select MODULE_ZTIMER_USEC?
sys/Makefile.dep pulls in ztimer_usec automatically if benchmark is used

RIOT/sys/Makefile.dep

Lines 574 to 576 in c84a40a

ifneq (,$(filter benchmark,$(USEMODULE)))
USEMODULE += ztimer_usec
endif
Kconfig does not.

Copy link
Contributor

Choose a reason for hiding this comment

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

@leandrolanzieri @bergzand ping.

I have an application here that uses benchmark. With this PR it is now necessary also to use ztimer_usec in application's makefile to be compatible with Kconfig even though ztimer_usec is pulled in automatically by the benchmark module. Was there any reason not to select ztimer_usec in Kconfig with benchmark?

@gschorcht gschorcht mentioned this pull request Dec 2, 2021
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gschorcht gschorcht gschorcht left review comments

@kaspar030 kaspar030 kaspar030 approved these changes

@fjmolinas fjmolinas Awaiting requested review from fjmolinas

@aabadie aabadie Awaiting requested review from aabadie aabadie is a code owner

@leandrolanzieri leandrolanzieri Awaiting requested review from leandrolanzieri leandrolanzieri is a code owner

@MichelRottleuthner MichelRottleuthner Awaiting requested review from MichelRottleuthner MichelRottleuthner is a code owner

@smlng smlng Awaiting requested review from smlng

@cgundogan cgundogan Awaiting requested review from cgundogan

@jia200x jia200x Awaiting requested review from jia200x jia200x is a code owner

Assignees
No one assigned
Labels
Area: Kconfig Area: Kconfig integration Area: sys Area: System Area: tests Area: tests and testing framework CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Impact: minor The PR is small in size and might only require a quick look of a knowledgeable reviewer Type: cleanup The issue proposes a clean-up / The PR cleans-up parts of the codebase / documentation
Projects
None yet
Milestone
Release 2022.01
Development

Successfully merging this pull request may close these issues.
4 participants
@bergzand @kaspar030 @gschorcht @fjmolinas