On Thu, 24 Oct 2024 15:11:54 GMT, Brian Burkhalter <b...@openjdk.org> wrote:
>> 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. > >> [...] do you think we should reduce these native calls in this change and >> call `canSeek()` just once [...] > > I think that is a good idea but I would think it best to do i in a similar > way to the recent change to `ChannelInputStream` that added `isOther`. > I think the main thing with this PR is whether canSeek is the right thing to > use. `canSeek` is really the same as `isOther` in the recent `Files.newInputStream` change, but given that on Windows the result of the function is always `true`, the naming `isOther` (which would always return `false`) did not seem apropos. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/21673#discussion_r1815205981
