amazon-ion / ion-java-benchmark-cli

Apache License 2.0
7 stars 9 forks source link

Account for a margin of error when comparing benchmark results #53

Open popematt opened 2 years ago

popematt commented 2 years ago

As stable as the performance tests might be, they are not perfect. See for example, this PR that does not modify any of the library code, but failed the performance regression workflow because of a detected regression. We should determine some margin of error for the tests, and only flag a regression if it exceeds the margin of error.

I'm not sure the best way to make this change, but it is possible that change would need to be made here: https://github.com/amzn/ion-java-benchmark-cli/blob/10807b6b3e06181752052da6f0febedf1b05f8ec/src/com/amazon/ion/benchmark/ParseAndCompareBenchmarkResults.java#L262

Or here: https://github.com/amzn/ion-java-benchmark-cli/blob/10807b6b3e06181752052da6f0febedf1b05f8ec/src/com/amazon/ion/benchmark/ParseAndCompareBenchmarkResults.java#L179-L188

linlin-s commented 2 years ago

Thanks for the suggestions. The second suggested solution would be a great start to make some changes. We should update the strategy of setting up the threshold value to avoid false positive of regression detection.

 /** 
      * Calculate the relative difference between scores from benchmark results of different in-java commits. 
      * @param previousScore is score from the benchmark result of the existing ion-java commit. 
      * @param newScore is score from the benchmark result of the new ion-java commit. 
      * @return relative changes of two scores from different benchmark results in BigDecimal format. 
      */ 
     private static BigDecimal calculateDifference(BigDecimal previousScore, BigDecimal newScore) { 
         BigDecimal scoreDifference = newScore.subtract(previousScore); 
         return scoreDifference.divide(previousScore, RoundingMode.HALF_UP); 
     }