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

Vladimir Sitnikov vsitnikov at openjdk.org
Sat Dec 16 15:43:40 UTC 2023


On Sat, 16 Dec 2023 14:52:42 GMT, Markus KARG <duke at openjdk.org> wrote:

>> I've created a draft change for `OutputStream#write(ByteBuffer)`, and I have benchmarked several cases including `ByteArrayInputStream.transferTo(BufferedOutputStream(ByteArrayOutputStream))`, `ByteArrayInputStream.transferTo(DataOutputStream(FileOutputStream))`.
>> 
>> Benchmarks show there's improvement in both allocation rate and latency.
>> 
>> Read-only `ByteBuffers` can address poisoning concerns as non-jdk code can't peek into read-only arrays.
>> 
>> The `write(ByteBuffer)` approach supports cases like `CheckedOutputStream`, `DataOutputStream` which would be hard or impossible to optimize when passing naked byte arrays.
>> 
>> Here are the results: https://gist.github.com/vlsi/7f4411515a4f2dbb0925fffde92ccb1d
>> Here is the diff: https://github.com/vlsi/jdk/compare/ce108446ca1fe604ecc24bbefb0bf1c6318271c7...write_bytebuffer
>> 
>> @mkarg , @bplb , @AlanBateman , could you please review `OutputStream.write(ByteBuffer)` approach?
>
> @vlsi This is a very interesting solution 🤩, but it opens a number of questions! 🤔 As a start, here are mine:
> * You propose a new public method (`OutputStream.write(ByteBuffer)`) as part of your solution. We need to discuss whether this is worth the effort to change all implementations (and to perceive acceptable performance, all custom authors need to optimize their custom classes, too). We also need to discuss whether we like the design choice that de facto the public API (not just the implementation) of the legacy IO domain (`OutputStream`) is now linked to the NIO domain (`ByteBuffer`) (which, IMHO, feels some kind of scary beyond `ChannelOutputStream`).
> * Just thinking loudly: I wonder if we could simplify the solution, or if we could turn parts of it into some generic `byte[] readOnly(byte[])` utility.
> * I would like to know how much faster the solution with `ByteBuffer` is compared to `Arrays.copyOfRange()`: Is it really so huge that it is worth the additional trouble?
> * I would like to know how much slower the solution with `ByteBuffer` is compared to *skipping* `Arrays.copyOfRange()` for trusted cases (as you removed the skipping).
> * I would like to know the performance of custom streams, because your default implementation is filling a temporary byte array with a Java-implemented byte-by-byte loop, which IMHO would be rather painful compared to hardware-supported `copyOrRange()`, and it does that even in case of *trusted* targets.
> * @briangoetz Wouldn't we rather teach the Java language some day to provide native read-only arrays? That would be much faster, much better to read and implies less complex code for the end user.

@mkarg , thank you for the review, I replied in JIRA to avoid mixing comments across different issues: https://bugs.openjdk.org/browse/JDK-8321271?focusedId=14634794&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-14634794

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

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


More information about the core-libs-dev mailing list