gharris1727 commented on PR #15469: URL: https://github.com/apache/kafka/pull/15469#issuecomment-2127953424
Here's the final performance changes: Benchmark | Before | Before Error | After | After Error | Speedup -- | -- | -- | -- | -- | -- ValuesBenchmark.testConvertToBoolean | 123.749 | 0.842 | 72.577 | 0.412 | 1.7 ValuesBenchmark.testConvertToByte | 117.883 | 1.397 | 62.387 | 0.148 | 1.9 ValuesBenchmark.testConvertToDate | 3700.318 | 160.043 | 3522.214 | 36.075 | 1.1 ValuesBenchmark.testConvertToDecimal | 1530.936 | 49.503 | 1485.654 | 11.766 | 1.0 ValuesBenchmark.testConvertToDouble | 163.937 | 55.591 | 60.378 | 0.577 | 2.7 ValuesBenchmark.testConvertToFloat | 204.591 | 109.567 | 57.611 | 0.49 | 3.6 ValuesBenchmark.testConvertToInteger | 140.586 | 3.809 | 66.496 | 0.371 | 2.1 ValuesBenchmark.testConvertToList | 1276.364 | 54.037 | 1601.568 | 28.972 | 0.8 ValuesBenchmark.testConvertToLong | 132.029 | 3.118 | 76.744 | 1.112 | 1.7 ValuesBenchmark.testConvertToMap | 1361.082 | 78.59 | 1244.339 | 11.019 | 1.1 ValuesBenchmark.testConvertToShort | 121.575 | 4.6 | 63.167 | 0.311 | 1.9 ValuesBenchmark.testConvertToString | 1667.243 | 51.186 | 1580.031 | 11.391 | 1.1 ValuesBenchmark.testConvertToStruct | 3.819 | 0.082 | 1.395 | 0.009 | 2.7 ValuesBenchmark.testConvertToTime | 2864.609 | 163.586 | 2701.721 | 60.677 | 1.1 ValuesBenchmark.testConvertToTimestamp | 2789.008 | 30.371 | 2738.573 | 19.6 | 1.0 ValuesBenchmark.testInferSchema | 123.196 | 3.292 | 99.336 | 0.867 | 1.2 ValuesBenchmark.testParseString | 43826.599 | 922.077 | 13429.742 | 133.089 | 3.3 There's a consistent performance degradation for testConvertToList that seems to come from the parseString implementation being slightly less efficient for array inputs. I'll follow up on that separately since this PR already has too much scope, and the degradation is only slight. One interesting final observation is that the variance/error for all of the tests is lower than the previous implementation. I suspect that this is because many of methods now avoid traversing the switch-case in the convertTo implementation, which lessens the number of branches and increases the branch predictor's success rate. And here's the final coverage changes: State | Class | Method | Line -- | -- | -- | -- Before | 100% (4/4) | 81% (40/49) | 78% (464/589) After | 100% (6/6) | 93% (77/82) | 84% (565/669) -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: jira-unsubscr...@kafka.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
