kkrugler commented on PR #24717: URL: https://github.com/apache/flink/pull/24717#issuecomment-2100600574
I'm leaving in 2 days for a 4+ month backpack trip (hiking the PCT), otherwise what I'd do is try to hook up a test that uses JMH to accurately assess the impact of the change. To respond to the particular comment on why performance changed: > we call bytesRead < count twice The overhead of two comparisons, versus even one call to `inputStream.read`, should be miniscule. > I'm not sure whether [FLINK-34954](https://issues.apache.org/jira/browse/FLINK-34954) breaks any JIT optimization If a JIT optimization would result in skipping calls to `inputStream.read`, then yes that could have an impact. But that would be a change in logic. The one thing I see is in the change to `readBytes()`, where if it's often called with a count of 0, then your change skips the `try/catch` block, which is non-trivial. But I thought the fix was to avoid a failure for this exact case, so it shouldn't be happening in the current benchmark code. -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
