RFR: 8320971: Use BufferedInputStream.buf directly when param of implTransferTo() is trusted [v17]

Markus KARG duke at openjdk.org
Thu Dec 28 15:09:09 UTC 2023


On Fri, 22 Dec 2023 16:25:29 GMT, Vladimir Sitnikov <vsitnikov at openjdk.org> wrote:

>> IMHO the trigger for this PR was sparing *time*, not necessarily sparing *bytes* (the default buffer size is just 8K); the latter certainly is a nice and beneficial side effect. But I may be wrong here, then the original contributor should chime in now and clarify.
>
>> IMHO the trigger for this PR was sparing time
> 
> That is fair. Do you know how one could create a reliable test for performance that would fail without the fix and succeed with the fix?
> 
> I think the allocation is a reasonable proxy for the duration in this case, and the allocation is more-or-less easy to measure and test if the JVM supports `getCurrentThreadAllocatedBytes`.

Why do you want to put *that* in a test case at all? So far I did not come across performance-based *tests* (only *benchmarks*), i. e. nobody ever requested that from me in any of my NIO optimizations. Typically it was sufficient to document the JMH results before and after a PR *just once* (not dynamically in the form of a test).

For *this* PR it is sufficient to proof what the PR's title says: Proof the *direct* call (i. e. the *absence of a copy*) and we're good to merge. I already explained how to proof *that*.

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/16879#discussion_r1435596696


More information about the core-libs-dev mailing list