On Sat, 16 Dec 2023 07:24:59 GMT, Vladimir Sitnikov <vsitni...@openjdk.org> wrote:
>>>The backing array is not frozen so a read-only view doesn't address the >>>second part of the concern where the target keeps a reference. >> >> The users outside of OpenJDK would not have a reference to `byte[]`. >> Do you mean a reference to `ByteBuffer`? >> What if there was a `ByteBuffer#forbidReads()` method that would permanently >> forbid subsequent reads from the `ByteBuffer`? > > 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. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/16879#discussion_r1428830013
