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

Markus KARG duke at openjdk.org
Fri Dec 22 14:29:54 UTC 2023


On Fri, 22 Dec 2023 13:53:42 GMT, Vladimir Sitnikov <vsitnikov at openjdk.org> wrote:

>> test/jdk/java/io/BufferedInputStream/TransferToTrusted.java line 68:
>> 
>>> 66: 
>>> 67:         var bis = new BufferedInputStream(new ByteArrayInputStream(dup));
>>> 68:         bis.mark(dup.length);
>> 
>> If you take a closer look into `BIS::transferTo()` then you will notice that in case you call `bis.mark()` then your optimization will effectively *not getting called at all*, due to this line: https://github.com/openjdk/jdk/pull/16879/files#diff-e19c508d1bb6ee78697ecca66947c395adda0d9c49a85bf696e677ecbd977af1L643. So Brian's comment still applies and you should fix your test before this PR can be accepted for a merge (hence: you MUST get rid of `mark()`).
>
> Can anybody give a hint how one can create assertions in OpenJDK test code that would check the amount of allocated heap for the tested method?
> 
> Since the change here is "removal of an allocation", the assert in the code should probably allow only a very small allocation in `.transferTo`.
> 
> Does `com.sun.management.ThreadMXBean#getCurrentThreadAllocatedBytes()` sound right for the test?

I think there is a misundertanding. This PR is not intendend to reduce the *amount* of allocated heap, it is about sparing time by not creating *temporary  copies*. The latter should be rather easy to check: Invoke `transferTo(out)` two times in a row and compare the *identity* of the two byte arrays passed to `out.write()`. If they stay the same, then apparently no *temporary copy* was created. Two achieve this, the BIS must be wrapper around an extendable input stream (like `FileInputStream`) so between calls the stream could get extended (e. g. by writing into the file).

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

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


More information about the core-libs-dev mailing list