search

zephyrproject-rtos / zephyr

Primary Git Repository for the Zephyr Project. Zephyr is a new generation, scalable, optimized, secure RTOS for multiple hardware architectures.
https://docs.zephyrproject.org
Apache License 2.0
10.52k stars 6.44k forks source link

Address latency/performance in nRF51 timer ISR #14577

Closed andyross closed 1 year ago

andyross commented 5 years ago

Following discussion in #13610, we need an answer for latency control in the nRF5x timer ISR. It uses a software division, which is slow. Changes there (somewhat inexplicably) caused a latency regression, and regardless the ISR is slower than it needs to be.

Options:

  1. The divisor (CYC_PER_TICK) is a compile time constant, and can be configured to be a power of two and optimized into a shift with no changes but the tick rate. This is a little complicated as many existing tests like to set CONFIG_SYS_CLOCK_TICKS_PER_SEC on their own -- it hasn't historically been a SoC or board tunable. We might consider making non-power-of-two divisors into a compile time error and force such code to set a CONFIG_YEAH_I_REALLY_MEAN_IT kind of override.

  2. The locking done in the ISR is for protection against other Zephyr code that might preempt the timer. In almost all cases, the timer ISR is at the highest maskable priority and can't be preempted by zephyr interrupts (it can be preempted by non-OS code like zero latency IRQs for hardware events, so some care needs to be taken about timing). This can be detected at compile time and the locking elided.

  3. When CONFIG_TICKLESS_KERNEL is not set, the ISR is poorly optimized. In theory it could queue up the next event with a single addition and a register read/write, but in fact it shares the same code as the tickless model where it needlessly computes the next tick based on the current counter. Using a traditional ticked kernel for latency control on nRF51 seems like a reasonable choice for some applications.

pabigot commented 5 years ago

@andyross @carlescufi @nordic-krch Are there remaining concerns to be addressed here? Since originally reported Nordic boards use 32768 ticks per sec, essentially selecting option 1. The driver has also been significantly reworked.

carlescufi commented 5 years ago

@pabigot we are still observing increased latency when running the new controller on nRF51. The right person to answer the question you asked is @cvinayak, who has been benchmarking it.

pabigot commented 5 years ago

@cvinayak If you are able to reproduce latency issues please see https://github.com/zephyrproject-rtos/zephyr/issues/17965#issuecomment-522096114 and try changing the referenced line 136 of kernel/timeout.c to consider next <= 2 as the condition for an imminent timeout. If that improves things it may be worth making the threshold target-specific.

carlescufi commented 2 years ago

@nordic-krch is this still applicable?

carlescufi commented 1 year ago

Closing as not applicable after discussing with @nordic-krch