Use alloca to address memory layout effects#744
Use alloca to address memory layout effects#744PSeitz wants to merge 4 commits intobheisler:masterfrom
Conversation
the iter collect pollutes the call stack (10 levels off calls)
samueltardieu
left a comment
There was a problem hiding this comment.
I like this changes. They should give better stability by increasing testing time variability.
|
|
||
| b.iters = b.iters.wrapping_mul(2); | ||
| b.iters = b.iters.min(64); // To make sure we offset the test at least with 0-64 bytes | ||
| // wit alloca |
There was a problem hiding this comment.
Nit: typo ("wit" -> "with"). Also, the comment is typically placed above the line, not on the side.
| stack_alloc, /* how much bytes we want to allocate */ | ||
| |_memory: &mut [core::mem::MaybeUninit<u8>] /* dynamically stack allocated slice itself */| { |
There was a problem hiding this comment.
Maybe stack_alloc should be inlined here, to prevent getting a warning on platforms not selected by the cfg attribute. Using a name such as _shifting_stack_space or similar instead of _memory would be self-documenting.
|
Seems quite straightforward and if this helps in reducing random changes between runs I'm all for this. |
|
@PSeitz Thanks for your contribution! Are you interested in updating this PR for the review comments or should we take it on? |
|
Merged, can be closed. |
the iter collect pollutes the call stack (10 levels of calls)
As mentioned in Eliminate memory layout bias when measuring (with LLVM stabilizer) #334, randomness in memory layout has a huge impact on the bench results. When I benchmark with criterion it's not uncommon to see effects of +-30%. Tests seem much more stable with alloca, there's still random 6% jumps in one test I've been running.
Adresses #334