RFR: 8341666: FileInputStream doesn't support readAllBytes() or readNBytes(int) on pseudo devices

[Jaikiran Pai]

On Thu, 24 Oct 2024 13:56:10 GMT, Alan Bateman <alanb at openjdk.org> wrote:

>> src/java.base/share/classes/java/io/FileInputStream.java line 334:
>> 
>>> 332:     @Override
>>> 333:     public byte[] readAllBytes() throws IOException {
>>> 334:         if (!canSeek())
>> 
>> Since `canSeek()` is implemented as a native call whose result depends on the file descriptor of this `FileInputStream`, do you think we should reduce these native calls in this change and call `canSeek()` just once (and store that state) in the constructors of this class (of course, after the call to open())?
>> If we do that, we might have to consider whether the `FileInputStream(FileDescriptor fdObj)` will end up throwing an exception (from the native canSeek() implementation) if the `FileDescriptor` passed to the constructor is not `valid()` https://docs.oracle.com/en/java/javase/23/docs/api/java.base/java/io/FileDescriptor.html#valid()
>
> readAllBytes reads to EOF so there isn't any benefit to caching. I think the main thing with this PR is whether canSeek is the right thing to use.

Hello Alan, I didn't just mean the readAllBytes. The rest of the changes in this PR calls canSeek() even in readNBytes() and skip() implementations which may not be reading till EOF and in theory could be invoked multiple times.
Having said that, the proposal to cache was merely a suggestion. I don't have any data to suggest if these methods are called frequent enough to have the additional native call show up prominently in that call path.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/21673#discussion_r1815051566