RFR: 8259956: jdk.jfr.internal.ChunkInputStream#available should return the sum of remaining available bytes
Erik Gahlin
egahlin at openjdk.java.net
Mon Feb 1 19:57:39 UTC 2021
On Mon, 1 Feb 2021 09:30:02 GMT, Denghui Dong <ddong at openjdk.org> wrote:
>> Not sure things should be changed. Javadoc for Inputstream::available() says:
>>
>> "Returns an estimate of the number of bytes that can be read (or skipped over) from this input stream without blocking by the next invocation of a method for this input stream."
>>
>> How often the stream will block depends on the underlying BufferInputStream, so it seems correct to just delegate. Purpose of the available() method is not to return the total.
>
>> Not sure things should be changed. Javadoc for Inputstream::available() says:
>>
>> "Returns an estimate of the number of bytes that can be read (or skipped over) from this input stream without blocking by the next invocation of a method for this input stream."
>>
>> How often the stream will block depends on the underlying BufferInputStream, so it seems correct to just delegate. Purpose of the available() method is not to return the total.
>
> In my humble opinion, Most users always expect this method to return the total readable size, and this is achievable for this method, although the java doc does not force the implementer to do so,
> it's meaningless if this method just returns the size of the current stream which is invisible to users, based on this return value, the user can hardly do anything.
> On the contrary, if the total size can be returned, the user can do further operations based on this data, such as selecting an appropriate compression algorithm or discarding, etc. (Of course this is just my idea)
>
> what do you think?
There is a method for finding the size of a recording, Recording::getSize(). It returns a long, which means it will work in cases where the recording is larger than 2 GB. I would recommend using it if you want to know the size in a reliable way.
That said, it will probably not hurt changing so that available() return the size of what is left unread on disk, potentially in memory in the future, or from an ongoing stream in a later release.
>From what I can see, Files.newInputStream(...).available() returns the full file size. This is also what java.io.FileInputStream does and BufferInputStream delegates to available() in the wrapped stream. java.io.SequenceStream on the other hand works like ChunkInputStream today.
One problem with the implementation is that you iterate over all chunks with every call to available(). It would be nice to avoid this.
I also wonder if you considered making total a long value and check for overflow once and near the return statement?
-------------
PR: https://git.openjdk.java.net/jdk/pull/2138
More information about the hotspot-jfr-dev
mailing list