sys/xtimer: make xtimer_ztimer_compat default backend #17721

fjmolinas · 2022-03-01T11:08:47Z

Contribution description

This PR make xtimer_ztimer_compat the default xtimer backend. Old xtimer code is kept and can be selected by asking for xtimer_no_ztimer_default, or in Kconfig by select MODULE_XTIMER_NO_ZTIMER_BACKEND. So as suggested in #17721 (comment) this PR change now limits to a oneliner for Kconfig and make.

With this commit xtimer code is not used anymore except for xtimer specific tests: xtimer_now32_overflow and xtimer_now64_continuity.

So to summarize anyone wanting to revert back can do:

USEMODULE += xtimer_no_ztimer_default

Or from the command line

# make
USEMODULE=xtimer_no_ztimer_default make
# kconfig
RIOT_CONFIG_MODULE_XTIMER_NO_ZTIMER_DEFAULT=y make

Testing procedure

Green Murdock
Run many tests on as many BOARDs as possible: I'm starting a batch of tests on many BOARDs

Issues/PRs references

#17111

sys/include/ztimer/xtimer_compat.h

With this commit xtimer will always be implemented with ztimer. The default compat layer will be ztimer_xtimer_compat, so xtimer calls will be inlined to ZTIMER_USEC, or ZTIMER64_USEC when the 64bit xtimer API is required. This can be reverted by adding 'xtimer_no_ztimer_default' module to a build.

fjmolinas · 2022-03-21T08:38:15Z

I ran all test on a series of BOARDs besides usually failing tests or tests known to fail I see just as in the murdock run that tests/thread_float is failing...

fjmolinas · 2022-03-22T14:16:25Z

I ran tests on multiple BOARD, the only failure that seems relevant is tests/evtimer_msg failing on z1. With this PR evtimer uses ZTIMER_MSEC and with running bench_ztimer it seems ZTIMER_MSEC is 4 times slower than ZTIMER_USEC, with ztimer_set, most likely due to frequency conversion since no rtt is used. @kaspar030 could you maybe take a look?

fjmolinas · 2022-03-22T16:08:53Z

I ran tests on multiple BOARD, the only failure that seems relevant is tests/evtimer_msg failing on z1. With this PR evtimer uses ZTIMER_MSEC and with running bench_ztimer it seems ZTIMER_MSEC is 4 times slower than ZTIMER_USEC, with ztimer_set, most likely due to frequency conversion since no rtt is used. @kaspar030 could you maybe take a look?

For the record here is a comparison, this kind of difference is not observed on other platforms.

usec

                     set() one    75134 / 1000 = 75
                  remove() one     9822 / 1000 = 9
          set() + remove() one   128896 / 1000 = 128
  set() many increasing target    22602 / 100 = 226
               re-set()  first   492829 / 1000 = 492
               re-set() middle   494256 / 1000 = 494
               re-set()   last   487628 / 1000 = 487
       remove() + set()  first   543417 / 1000 = 543
       remove() + set() middle   542216 / 1000 = 542
       remove() + set()   last   539568 / 1000 = 539
      remove() many decreasing    11782 / 100 = 117
                  ztimer_now()    17517 / 1000 = 17
              sizeof(ztimer_t)     1000 / 100 = 10

msec

                     set() one   469110 / 1000 = 469
                  remove() one     8842 / 1000 = 8
          set() + remove() one   912485 / 1000 = 912
  set() many increasing target    43961 / 100 = 439
               re-set()  first   471125 / 1000 = 471
               re-set() middle   471182 / 1000 = 471
               re-set()   last   471238 / 1000 = 471
       remove() + set()  first   919610 / 1000 = 919
       remove() + set() middle   919735 / 1000 = 919
       remove() + set()   last   919611 / 1000 = 919
      remove() many decreasing    51409 / 100 = 514
                  ztimer_now()   208456 / 1000 = 208
              sizeof(ztimer_t)     1000 / 100 = 10

usec64

                     set() one   174819 / 1000 = 174
                  remove() one     7399 / 1000 = 7
          set() + remove() one   181448 / 1000 = 181
  set() many increasing target    30257 / 100 = 302
               re-set()  first   596820 / 1000 = 596
               re-set() middle   597118 / 1000 = 597
               re-set()   last   594431 / 1000 = 594
       remove() + set()  first   718652 / 1000 = 718
       remove() + set() middle   719723 / 1000 = 719
       remove() + set()   last   719585 / 1000 = 719
      remove() many decreasing    18094 / 100 = 180
                  ztimer_now()    24393 / 1000 = 24
              sizeof(ztimer_t)     1400 / 100 = 14
done.

fjmolinas · 2022-03-22T16:52:33Z

I wanted to see if there was another arch where we would observe this increase because of what seems to be the use of conver_fract, so I benched ztimer_usec, ztimer_msec and ztimer64_usec for avr, msp430, m0, m3, m4, and msp430 (z1) is the only one showing such a performance loss. (if someone else has a riscv, esp or arm7 BOARD to complement that would be awesome).

Since its only one platform that is not very used, I still think we should be moving on with this PR, in parallel we could start working into the init logic how to use different ztimer_periph_timer for different backends to avoid converting altogether.

All benchmarks where ran with USEMODULE+=ztimer_no_periph_rtt

nrf52dk-tests/bench_ztimer_usec

READY
s
START
main(): This is RIOT! (Version: buildtest)
ztimer benchmark application.

                     set() one     6814 / 1000 = 6
                  remove() one      991 / 1000 = 0
          set() + remove() one    11877 / 1000 = 11
  set() many increasing target   136051 / 1000 = 136
               re-set()  first   207097 / 1000 = 207
               re-set() middle   271677 / 1000 = 271
               re-set()   last   292163 / 1000 = 292
       remove() + set()  first   234916 / 1000 = 234
       remove() + set() middle   281417 / 1000 = 281
       remove() + set()   last   300203 / 1000 = 300
      remove() many decreasing    60542 / 1000 = 60
                  ztimer_now()     1127 / 1000 = 1
              sizeof(ztimer_t)    16000 / 1000 = 16
done.

nrf52dk-tests/bench_ztimer_msec

Welcome to pyterm!
Type '/exit' to exit.
READY
s
START
main(): This is RIOT! (Version: buildtest)
ztimer benchmark application.

                     set() one    12126 / 1000 = 12
                  remove() one      919 / 1000 = 0
          set() + remove() one    22503 / 1000 = 22
  set() many increasing target   113759 / 1000 = 113
               re-set()  first    12315 / 1000 = 12
               re-set() middle    12306 / 1000 = 12
               re-set()   last    12424 / 1000 = 12
       remove() + set()  first    23003 / 1000 = 23
       remove() + set() middle    23003 / 1000 = 23
       remove() + set()   last    23002 / 1000 = 23
      remove() many decreasing    66119 / 1000 = 66
                  ztimer_now()     2439 / 1000 = 2
              sizeof(ztimer_t)    16000 / 1000 = 16
done.

nrf52dk-tests/bench_ztimer64_usec

READY
s
START
main(): This is RIOT! (Version: buildtest)
ztimer benchmark application.

                     set() one    12502 / 1000 = 12
                  remove() one      829 / 1000 = 0
          set() + remove() one    13315 / 1000 = 13
  set() many increasing target    89074 / 1000 = 89
               re-set()  first   160439 / 1000 = 160
               re-set() middle   216325 / 1000 = 216
               re-set()   last   232783 / 1000 = 232
       remove() + set()  first   198210 / 1000 = 198
       remove() + set() middle   231782 / 1000 = 231
       remove() + set()   last   245902 / 1000 = 245
      remove() many decreasing    64383 / 1000 = 64
                  ztimer_now()     1688 / 1000 = 1
              sizeof(ztimer_t)    24000 / 1000 = 24
done.

nucleo-l073rz-tests/bench_ztimer_usec

main(): This is RIOT! (Version: buildtest)
ztimer benchmark application.

                     set() one    16751 / 1000 = 16
                  remove() one     2796 / 1000 = 2
          set() + remove() one    28910 / 1000 = 28
  set() many increasing target     2870 / 100 = 28
               re-set()  first    77092 / 1000 = 77
               re-set() middle    78034 / 1000 = 78
               re-set()   last    78225 / 1000 = 78
       remove() + set()  first    89227 / 1000 = 89
       remove() + set() middle    90219 / 1000 = 90
       remove() + set()   last    90565 / 1000 = 90
      remove() many decreasing     2983 / 100 = 29
                  ztimer_now()     3691 / 1000 = 3
              sizeof(ztimer_t)     1600 / 100 = 16
done.

nucleo-l073rz-tests/bench_ztimer_msec

START
main(): This is RIOT! (Version: buildtest)
ztimer benchmark application.

                     set() one    37656 / 1000 = 37
                  remove() one     2568 / 1000 = 2
          set() + remove() one    70004 / 1000 = 70
  set() many increasing target     3389 / 100 = 33
               re-set()  first    37472 / 1000 = 37
               re-set() middle    37426 / 1000 = 37
               re-set()   last    37504 / 1000 = 37
       remove() + set()  first    70254 / 1000 = 70
       remove() + set() middle    70380 / 1000 = 70
       remove() + set()   last    70286 / 1000 = 70
      remove() many decreasing     5066 / 100 = 50
                  ztimer_now()     9160 / 1000 = 9
              sizeof(ztimer_t)     1600 / 100 = 16
done.

nucleo-l073rz-tests/bench_ztimer64_usec

main(): This is RIOT! (Version: buildtest)
ztimer benchmark application.

                     set() one    32471 / 1000 = 32
                  remove() one     2255 / 1000 = 2
          set() + remove() one    34660 / 1000 = 34
  set() many increasing target     3255 / 100 = 32
               re-set()  first    80764 / 1000 = 80
               re-set() middle    81917 / 1000 = 81
               re-set()   last    82151 / 1000 = 82
       remove() + set()  first   105739 / 1000 = 105
       remove() + set() middle   106519 / 1000 = 106
       remove() + set()   last   106720 / 1000 = 106
      remove() many decreasing     3880 / 100 = 38
                  ztimer_now()     5067 / 1000 = 5
              sizeof(ztimer_t)     2400 / 100 = 24
done.

nucleo-l152re-tests/bench_ztimer_usec

main(): This is RIOT! (Version: buildtest)
ztimer benchmark application.

                     set() one    11905 / 1000 = 11
                  remove() one     2197 / 1000 = 2
          set() + remove() one    20095 / 1000 = 20
  set() many increasing target   224911 / 1000 = 224
               re-set()  first   585262 / 1000 = 585
               re-set() middle   666133 / 1000 = 666
               re-set()   last   693341 / 1000 = 693
       remove() + set()  first   629185 / 1000 = 629
       remove() + set() middle   681542 / 1000 = 681
       remove() + set()   last   705869 / 1000 = 705
      remove() many decreasing   212753 / 1000 = 212
                  ztimer_now()     1532 / 1000 = 1
              sizeof(ztimer_t)    16000 / 1000 = 16
done.

nucleo-l152re-tests/bench_ztimer_msec

main(): This is RIOT! (Version: buildtest)
ztimer benchmark application.

                     set() one    23904 / 1000 = 23
                  remove() one     1990 / 1000 = 1
          set() + remove() one    43376 / 1000 = 43
  set() many increasing target   227513 / 1000 = 227
               re-set()  first    23720 / 1000 = 23
               re-set() middle    23939 / 1000 = 23
               re-set()   last    24157 / 1000 = 24
       remove() + set()  first    43596 / 1000 = 43
       remove() + set() middle    43627 / 1000 = 43
       remove() + set()   last    43533 / 1000 = 43
      remove() many decreasing   240157 / 1000 = 240
                  ztimer_now()     4408 / 1000 = 4
              sizeof(ztimer_t)    16000 / 1000 = 16
done.

nucleo-l152re-tests/bench_ztimer64_usec

main(): This is RIOT! (Version: buildtest)
ztimer benchmark application.

                     set() one    21626 / 1000 = 21
                  remove() one     1534 / 1000 = 1
          set() + remove() one    23158 / 1000 = 23
  set() many increasing target   208595 / 1000 = 208
               re-set()  first   446188 / 1000 = 446
               re-set() middle   523048 / 1000 = 523
               re-set()   last   549042 / 1000 = 549
       remove() + set()  first   506753 / 1000 = 506
       remove() + set() middle   551862 / 1000 = 551
       remove() + set()   last   573707 / 1000 = 573
      remove() many decreasing   157441 / 1000 = 157
                  ztimer_now()     2783 / 1000 = 2
              sizeof(ztimer_t)    24000 / 1000 = 24
done.

z1-tests/bench_ztimer_usec

main(): This is RIOT! (Version: buildtest)
ztimer benchmark application.

                     set() one    75134 / 1000 = 75
                  remove() one     9822 / 1000 = 9
          set() + remove() one   128897 / 1000 = 128
  set() many increasing target    22601 / 100 = 226
               re-set()  first   492833 / 1000 = 492
               re-set() middle   494254 / 1000 = 494
               re-set()   last   487628 / 1000 = 487
       remove() + set()  first   543417 / 1000 = 543
       remove() + set() middle   542215 / 1000 = 542
       remove() + set()   last   539568 / 1000 = 539
      remove() many decreasing    11782 / 100 = 117
                  ztimer_now()    17518 / 1000 = 17
              sizeof(ztimer_t)     1000 / 100 = 10
done.

z1-tests/bench_ztimer_msec

main(): This is RIOT! (Version: buildtest)
ztimer benchmark application.

                     set() one   479504 / 1000 = 479
                  remove() one     8853 / 1000 = 8
          set() + remove() one   933272 / 1000 = 933
  set() many increasing target    43972 / 100 = 439
               re-set()  first   481520 / 1000 = 481
               re-set() middle   481576 / 1000 = 481
               re-set()   last   481631 / 1000 = 481
       remove() + set()  first   940397 / 1000 = 940
       remove() + set() middle   940522 / 1000 = 940
       remove() + set()   last   940397 / 1000 = 940
      remove() many decreasing    52417 / 100 = 524
                  ztimer_now()   208456 / 1000 = 208
              sizeof(ztimer_t)     1000 / 100 = 10
done.

z1-tests/bench_ztimer64_usec

READY
s
START
main(): This is RIOT! (Version: buildtest)
ztimer benchmark application.

                     set() one   174813 / 1000 = 174
                  remove() one     7400 / 1000 = 7
          set() + remove() one   181447 / 1000 = 181
  set() many increasing target    30815 / 100 = 308
               re-set()  first   598572 / 1000 = 598
               re-set() middle   598762 / 1000 = 598
               re-set()   last   596157 / 1000 = 596
       remove() + set()  first   720213 / 1000 = 720
       remove() + set() middle   721151 / 1000 = 721
       remove() + set()   last   721053 / 1000 = 721
      remove() many decreasing    18093 / 100 = 180
                  ztimer_now()    24393 / 1000 = 24
              sizeof(ztimer_t)     1400 / 100 = 14
done.

arduino-mega2560-tests/bench_ztimer_usec

READY
s
START
main(): This is RIOT! (Version: buildtest)
ztimer benchmark application.

                     set() one   117256 / 1000 = 117
                  remove() one     5552 / 1000 = 5
          set() + remove() one   194152 / 1000 = 194
  set() many increasing target    13776 / 100 = 137
               re-set()  first   313504 / 1000 = 313
               re-set() middle   315628 / 1000 = 315
               re-set()   last   314624 / 1000 = 314
       remove() + set()  first   415824 / 1000 = 415
       remove() + set() middle   416664 / 1000 = 416
       remove() + set()   last   416656 / 1000 = 416
      remove() many decreasing    15060 / 100 = 150
                  ztimer_now()    18464 / 1000 = 18
              sizeof(ztimer_t)     1000 / 100 = 10
done.

arduino-mega2560-tests/bench_ztimer_msec

READY
s
START
main(): This is RIOT! (Version: buildtest)
ztimer benchmark application.

                     set() one   162828 / 1000 = 162
                  remove() one     5608 / 1000 = 5
          set() + remove() one   303716 / 1000 = 303
  set() many increasing target    16868 / 100 = 168
               re-set()  first   165972 / 1000 = 165
               re-set() middle   166008 / 1000 = 166
               re-set()   last   166064 / 1000 = 166
       remove() + set()  first   310840 / 1000 = 310
       remove() + set() middle   310796 / 1000 = 310
       remove() + set()   last   310740 / 1000 = 310
      remove() many decreasing    19556 / 100 = 195
                  ztimer_now()    49088 / 1000 = 49
              sizeof(ztimer_t)     1000 / 100 = 10
done.

arduino-mega2560-tests/bench_ztimer64_usec

main(): This is RIOT! (Version: buildtest)
ztimer benchmark application.

                     set() one   239392 / 1000 = 239
                  remove() one     5908 / 1000 = 5
          set() + remove() one   244836 / 1000 = 244
  set() many increasing target    32864 / 100 = 328
               re-set()  first   643608 / 1000 = 643
               re-set() middle   644128 / 1000 = 644
               re-set()   last   641264 / 1000 = 641
       remove() + set()  first   820916 / 1000 = 820
       remove() + set() middle   821688 / 1000 = 821
       remove() + set()   last   821312 / 1000 = 821
      remove() many decreasing    21176 / 100 = 211
                  ztimer_now()    28464 / 1000 = 28
              sizeof(ztimer_t)     1400 / 100 = 14
done.

kaspar030 · 2022-03-22T16:56:25Z

So, probably msp430 with 8 mhz really doesn't like what ztimer is doing (maybe ztimer_convert_frac?).
I think that's ok, we should open an issue. it's not like z1 is our most popular platform.

kaspar030

ACK.

This is the culmination of tons of work, I'm happy to see it done!

(there's some CI blacklisting that's not directly caused by the switch to ztimer, they could be split out but our CI would turn that another day of waiting...)

fjmolinas · 2022-03-22T17:05:50Z

Opened the issue for z1 #17845.

fjmolinas added Impact: major The PR changes a significant part of the code base. It should be reviewed carefully State: waiting for other PR State: The PR requires another PR to be merged first labels Mar 1, 2022

fjmolinas requested review from leandrolanzieri, jia200x, cgundogan, smlng, aabadie, MichelRottleuthner, kaspar030 and bergzand as code owners March 1, 2022 11:08

This was referenced Mar 2, 2022

sys: sort out ztimer_xtimer_compat and ztimer64_xtimer_compat depes #17732

Merged

sys/syscalls: make gettimeofday() implementation optional #17733

Merged

sys: some simple xtimer->ztimer conversions #17693

Closed

kfessel reviewed Mar 3, 2022

View reviewed changes

sys/include/ztimer/xtimer_compat.h Outdated Show resolved Hide resolved

fjmolinas force-pushed the pr_xtimer_ztimer_default_backend branch from 42a294e to b99946a Compare March 4, 2022 07:18

github-actions bot removed Platform: native Platform: This PR/issue effects the native platform Area: cpu Area: CPU/MCU ports labels Mar 4, 2022

fjmolinas added CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR and removed State: waiting for other PR State: The PR requires another PR to be merged first labels Mar 4, 2022

github-actions bot added the Area: CI Area: Continuous Integration of RIOT components label Mar 4, 2022

fjmolinas force-pushed the pr_xtimer_ztimer_default_backend branch from 1dc1be4 to a0e5010 Compare March 4, 2022 07:34

fjmolinas added CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR and removed CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR labels Mar 4, 2022

github-actions bot added the Platform: ARM Platform: This PR/issue effects ARM-based platforms label Mar 17, 2022

fjmolinas added State: waiting for other PR State: The PR requires another PR to be merged first labels Mar 17, 2022

fjmolinas added 2 commits March 18, 2022 08:23

tests/*: update Makefile.ci

2681b6c

fjmolinas force-pushed the pr_xtimer_ztimer_default_backend branch from 3c93df9 to 20e68fb Compare March 18, 2022 07:23

fjmolinas removed the State: waiting for other PR State: The PR requires another PR to be merged first label Mar 18, 2022

github-actions bot removed Area: build system Area: Build system Area: cpu Area: CPU/MCU ports Area: drivers Area: Device drivers Area: pkg Area: External package ports Platform: ARM Platform: This PR/issue effects ARM-based platforms labels Mar 18, 2022

fjmolinas added the CI: run tests If set, CI server will run tests on hardware for the labeled PR label Mar 18, 2022

This was referenced Mar 22, 2022

tests/thread_float: use ztimer_usec #17835

Merged

drivers/mtd_spi_nor: add ztimer_usec alternative #17836

Merged

fjmolinas added CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR and removed CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR labels Mar 22, 2022

fjmolinas mentioned this pull request Mar 22, 2022

tests: multiple fixes #17840

Merged

kaspar030 approved these changes Mar 22, 2022

View reviewed changes

kaspar030 merged commit feda38c into RIOT-OS:master Mar 22, 2022

fjmolinas deleted the pr_xtimer_ztimer_default_backend branch March 22, 2022 17:03

OlegHahm added this to the Release 2022.04 milestone Apr 25, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

sys/xtimer: make xtimer_ztimer_compat default backend #17721

sys/xtimer: make xtimer_ztimer_compat default backend #17721

fjmolinas commented Mar 1, 2022 •

edited

fjmolinas commented Mar 21, 2022

fjmolinas commented Mar 22, 2022

fjmolinas commented Mar 22, 2022

fjmolinas commented Mar 22, 2022 •

edited

kaspar030 commented Mar 22, 2022

kaspar030 left a comment

fjmolinas commented Mar 22, 2022

sys/xtimer: make xtimer_ztimer_compat default backend #17721

sys/xtimer: make xtimer_ztimer_compat default backend #17721

Conversation

fjmolinas commented Mar 1, 2022 • edited

Contribution description

Testing procedure

Issues/PRs references

fjmolinas commented Mar 21, 2022

fjmolinas commented Mar 22, 2022

fjmolinas commented Mar 22, 2022

fjmolinas commented Mar 22, 2022 • edited

kaspar030 commented Mar 22, 2022

kaspar030 left a comment

Choose a reason for hiding this comment

fjmolinas commented Mar 22, 2022

fjmolinas commented Mar 1, 2022 •

edited

fjmolinas commented Mar 22, 2022 •

edited