RFR: 6445283: ProgressMonitorInputStream not large file aware (>2GB) [v2]

[Phil Race]

On Fri, 22 Jul 2022 09:15:47 GMT, Prasanta Sadhukhan <psadhukhan at openjdk.org> wrote:

>> If ProgressMonitor has to show progress for reading file > 2GB, it goes to 100% and then again start from 0. 
>> This is because
>> it uses "int" to store bytes read (`nread`) and when it reads data from file, it adds 
>> "number-bytes-to-read "nr" to number-bytes-already-read "nread" variable [`nread += nr`] which can cause it to overflow and so "nread" becomes -ve.
>> 
>> Fix is to check if adding bytes-to-read to number-bytes-already-read will exceed MAX_INT, then set Progress to max so that ProgressMonitor can close the tracker 
>> https://github.com/openjdk/jdk/blob/master/src/java.desktop/share/classes/javax/swing/ProgressMonitor.java#L265
>> 
>> No regression test is added as it involves using filesize of >2GB.
>
> Prasanta Sadhukhan has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Review comment

It isn't clear to me that this is "better".
Before although it would jump to zero it would continue to increment and so the user would know something is happening. Now it just gets to max and looks "stuck".
So I don't think it should be fixed this way.

If we could find a way to use longs here .. but then there are other problems .. 
The ProgressMonitor usage seems a bit less than ideal to me.
This class bases its estimate of max to pass to ProgressMonitor on InputStream.available().
If it is a real file, then this may be fine .. but available() returns an int .. so I suppose can't tell you the real size of a large file.
If it is any other kind of stream, then the size estimate is perhaps worse than useless.
It must be continually re-evaluating max in such a case ? Right ?

Perhaps that tells us what we should do 

Rather than sitting at max, or jumping back to zero, we can go back to something like
90% .. or 50% .. and so we are signalling we've had to re-evaluate where we are but ar still reading.

BTW I don't see why you need a large file for a test. You can just have an InputStream subclass which keeps generating data ..

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

PR: https://git.openjdk.org/jdk/pull/9588