On Wed, 21 Jul 2021 04:09:23 GMT, Jaikiran Pai <j...@openjdk.org> wrote:
> > > For some context - the new `FileRolloverOutputStream` extends
> > > `ByteArrayOutputStream` and hence cannot have a `throws IOException` in
> > > its overridden `write` methods.
> >
> >
> > Have you tried wrapping a BAOS rather than extending, that might allow the
> > exception wrapping/unwapping to go away.
>
> Hello Alan,
>
> I did experiment with it earlier, before going with the current approach in
> this PR. The disadvantage, as I see it, with wrapping a
> `ByteArrayOutputStream` instead of extending it is that when trying to
> rollover the contents to a file, you don't have access to the (inner
> protected) byte array of the ByteArrayOutputStream.
>
> The rollover code would then look something like:
>
> ```
> private void transferToFile() throws IOException {
> // create a tempfile
> entry.file = getTempPathForEntry(null);
> // transfer the already written data from the byte array buffer into this
> tempfile
> try (OutputStream os = new
> BufferedOutputStream(Files.newOutputStream(entry.file))) {
> new ByteArrayInputStream(baos.toByteArray(), 0,
> baos.size()).transferTo(os);
> }
> // release the underlying byte array
> baos = null;
> // append any further data to the file with buffering enabled
> tmpFileOS = new BufferedOutputStream(Files.newOutputStream(entry.file,
> APPEND));
> }
> ```
>
> So although you can transfer the contents to the file without requiring the
> access to the byte array, you end up creating a new copy of that array
> (through the use of `baos.toByteArray()`), which can be at most 10MB in size.
> I thought avoiding a new copy of that (potentially 10MB) array during this
> transfer would be a good save and hence decided to settle on extending
> `ByteArrayOutputStream` instead of wrapping it.
>
> The use of `extends` of course now means dealing with the
> `UncheckedIOException` as done in this PR. But if you think that the array
> copy isn't a concern and wrapping the `ByteArrayOutputStream` is a better
> way, then I'll go ahead and update this PR accordingly.
Now that the mailing lists integration seems to be back to normal, just adding
this dummy comment to bring to attention the latest comments in this PR.
-------------
PR: https://git.openjdk.java.net/jdk/pull/4607
