RFR: 8320971: Use BufferedInputStream.buf directly when param of implTransferTo() is trusted [v15]
Brian Goetz
briangoetz at openjdk.org
Sat Dec 16 15:58:41 UTC 2023
On Sat, 16 Dec 2023 15:40:50 GMT, Vladimir Sitnikov <vsitnikov at openjdk.org> wrote:
>> @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
While frozen arrays are on our radar, this is a significant lift, so unlikely to be coming all that soon.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/16879#discussion_r1428846145
More information about the core-libs-dev
mailing list