On Sat, 16 Dec 2023 14:52:42 GMT, Markus KARG <d...@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
