Improve busy loop stability #90
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
douglas-raillard-arm
changed the title
Since the function is pure, the compiler is free to do anything it wants to that function, including removing all its call site. To avoid any such issues: * disable optimizations for that function * forbid inlining * add some no-op statement that is guaranteed to be treated as a side-effect by the compiler. Signed-off-by: Douglas RAILLARD <[email protected]>
Previous implementation based on ldexp() was not always giving consistent results from one run to another. Using more basic operations without extra branches makes the execution time of the body much more predictable, leading to more stable calibration values and precise reproduction of duty cycles. Signed-off-by: Douglas RAILLARD <[email protected]>
douglas-raillard-arm
force-pushed
the
fix_busy_loop
branch
from
ca88d46 to
884775a
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The current implementation of the busy loop seems to lead to somehow non-reproducible calibration values, and also potentially different duty cycle when the same workload is executed twice. Old versions of rt-app seem to behave well, but for some reason building a recent version with the same toolchain leads to these issues.
In order to solve that:
This PR will however change the behavior of rt-app on asymmetric systems with so called CPU PELT invariance. The invariance described by CPU capacities only holds for a given mix of instructions. Since the CPU capacities have typically been established using a benchmark X (supposedly Dhrystone), the duty cycle of any other periodic workload will scale differently when moved around on different CPUs. Changing the rt-app loop body will therefore change the utilization of the task when running on a little CPU. This can be accounted for when creating the JSON when the task will be pinned on a given class of CPUs, but there is no real solution when the task is free to move on any CPU.
Since there is no way of actually ensuring that rt-app calibration values will be inversely proportional to CPU capacities, it's a lost battle so IMO we should aim at getting reproducible results. People interested in reproducing very accurate util signals should update the in-kernel capacities of their CPUs based on rt-app calibration values before running their tests.
Fixes #89