Fix, refactor, and optimize xnlock services, v2
cover Fix, refactor, and optimize xnlock services, v2
Hi,
here comes version 2 of my xnlock rework. Changes since the first release:
- Moved xnlock_spin out-of-line instead uninlining the irqsave/restore services
- Beautified xnlock_dbg interface (now only static-inlines)
- Split-up basic xnlock_dbg refactoring from whitespace fixes (interdiff is your friend)
- Added support for xnlock debugging on UP
- Dropped barrier patch (already merged)
The text size reduction of this series (for CONFIG_SMP, without debugging (CONFIG_DEBUG_INFO was on, though), and without enabling ticket locks):
Before:
text data bss dec hex filename 75371 2184 308 77863 13027 kernel/xenomai/skins/native/xeno_native.o 27407 2256 1112 30775 7837 kernel/xenomai/skins/rtdm/xeno_rtdm.o 96813 1472 160224 258509 3f1cd kernel/xenomai/skins/posix/xeno_posix.o 107476 5608 457140 570224 8b370 kernel/xenomai/nucleus/xeno_nucleus.o
After:
text data bss dec hex filename 71751 2184 308 74243 12203 kernel/xenomai/skins/native/xeno_native.o 26570 2192 1112 29874 74b2 kernel/xenomai/skins/rtdm/xeno_rtdm.o 91107 1392 160224 252723 3db33 kernel/xenomai/skins/posix/xeno_posix.o 102860 5496 454580 562936 896f8 kernel/xenomai/nucleus/xeno_nucleus.o
To estimate the latency reduction of this series (specifically due to patch 3 and 4), I ran latency -p100 over several hours on a widely isolated 1.6 GHz core (only Linux timer and rescheduling IRQs) under cache calibrator load. The results:
RTH|-----lat min|-----lat avg|-----lat max|-overrun|----lat best|---lat worst Before:
RTS| 0.457| 1.450| 28.156| 0| 04:00:52/04:00:52
After:
RTS| 0.450| 1.453| 23.283| 0| 06:37:19/06:37:19
I'm sure this gain is far to much for this changes, ie. we are missing some constellation in the second run that caused the 28 us peak in the first run. However, the tendency matches other experiments I did with more IRQ load on the target core. Any attempt to reproduce my results is welcome!
Note that the above numbers do not yet include the gain from my ipipe-head optimizations I posted a few days ago!
Jan
Quoted 7 lines · Jan Kiszka
Hi,
here comes version 2 of my xnlock rework. Changes since the first release:
- Moved
xnlock_spinout-of-line instead uninlining the irqsave/restore services
The size changes are not as dramatic as with the first version. Did you have any chance to test the latency with your first version?
Quoted 1 line
- Beautified
xnlock_dbginterface (now only static-inlines)
Well, I am still not convinced, I still find the debugging version harder to read. Is there a problem with implementing the two versions separately with a unique #ifdef?
Signature · 1 line
Gilles Chanteperdrix.
Quoted 11 lines · Gilles Chanteperdrix
Quoted 7 lines · Jan Kiszka
Hi,
here comes version 2 of my xnlock rework. Changes since the first release:
- Moved
xnlock_spinout-of-line instead uninlining the irqsave/restore servicesThe size changes are not as dramatic as with the first version. Did you have any chance to test the latency with your first version?
Not under the same conditions (isolated core). In essence, the gain was less significant (~1 us). I came to the conclusion that this might be due to some loss related to moving xnlock_put_irqrestore out-of-line (I think now that function was too small for this).
Instead of further digging into this I decided to address the root of the issue, ie. bringing down the text size of the pipeline-head services. So you now have to apply the ipipe patch to achieve both a text size reduction (almost as good as the original one) AND a code path length reduction (more important!). Actually, I haven't even done a full benchmark with this patch applied as well yet, but it is not that interesting as the runtime advantage is obvious (simply less ops).
Quoted 6 lines
Quoted 2 lines
- Beautified
xnlock_dbginterface (now only static-inlines)Well, I am still not convinced, I still find the debugging version harder to read. Is there a problem with implementing the two versions separately with a unique #ifdef?
Well, everything is possible. But we would start to duplicate LOC again, namely both the lock implementations and the debugging services. Just look at the final system.h and pod.h after applying all patches.
I could try this, but only if it is _really_ desired. I'm convinced it is the wrong way because I consider the readability and consistency of the lock implementation more important than its debug code.
Jan
Quoted 6 lines · Jan Kiszka
Quoted 4 lines · Gilles Chanteperdrix
Well, I am still not convinced, I still find the debugging version harder to read. Is there a problem with implementing the two versions separately with a unique #ifdef?
Quoted 7 lines
Well, everything is possible. But we would start to duplicate LOC again, namely both the lock implementations and the debugging services. Just look at the final
system.handpod.hafter applying all patches.I could try this, but only if it is _really_ desired. I'm convinced it is the wrong way because I consider the readability and consistency of the lock implementation more important than its debug code.
Well, it may happen that we have to debug the debug code. But Ok, I agree with your changes.
Signature · 1 line
Gilles Chanteperdrix
Philippe,
I noticed that you merged patch 1-4. What are the plans for (or concerns about) the ticket xnlock and the UP debugging changes?
Jan