Investigate benchmark changes between 1.16.7 and 1.17 #2116
Labels
evaluation needed
proposal needs to be validated or tested before fully implementing it in k6
help wanted
performance
Milestone
Comments
mstoykov
added
help wanted
performance
evaluation needed
|
For additional context, here is benchstat at arm64 which supposedly does not have new regabi. ARM64 result |
|
I spent sometime on friday and over the weekend to bisect golang and: for RampingArrivalRate
in the real example this seem to not matter at all or at least I can't find it mattering when I run like 10 runs which do have some variation, but in general no trend in 1.17 being slower(or faster for that matter) for ExecutionSegmentScale
I will try to make a simpler example reproducing both and making go issues so we might get better results either way :) From looking at the ARM benchmarks (thanks @vearutop 🙇 ) which doesn't have the ABI changes, it looks like they also play a role in combination with the above changes. |
|
After some more internal tests All in all this all seems like a red herring and just a case of the compiler will do some optimizations in benchmark/test code that it won't do in real one or/and that + other optimizations nullifying any regressions we actually see here. So we can update to 1.17 hopefully without any problems 🤞 I will close this and will reopen it if we need to investigate after some 1.17 usage |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
As usual when the final go version is release I run all the benchmarks and in this case (as expected due to compiler changes), unfortunately in some cases not in the direction we would want
Gist with the results and a benchstat comparison.
Notes:
ExecutionSegmentScaleis strange - some prefilled are a lot better some are worse (the worse case (usually and in this benchmarks) by 13x+) - this is worrying as this is used a lot, but probably not a deal breaker as it's mostly used during initialization and most of the cases are still pretty fastCal(the calculation for ramping arrival rate) has improved on the other ... theRampingArrivalRateRun's iterations are down by quite a lot - which is likely the bigger problem so far.MetricMarshalGzipAllis the thing that is mostly close to what happens inside the actual code, so the others are mostly for reference if they become a lot better someday, although maybe we should remove the benchmarks as I doubt that will change before we move away from json.Given the above I think we need to first confirm the current results and see whether we can improve the performance of the problematic benchmarks before releasing with go1.17, so I am against this being done for 0.34.0
The text was updated successfully, but these errors were encountered: