On Wed, 13 Apr 2022 14:56:12 GMT, XenoAmess <d...@openjdk.java.net> wrote:

>> @jmehrens what about this then?
>> I think it safe now(actually this mechanism is learned from Reader)
>
> XenoAmess has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   moving nr declaration from the beginning of the method to where it's 
> actually used

On a side note for unifying the skip buffer implementation of reader vs input 
stream: For the input stream subclasses in the JDK that have their own skip 
with buffering logic (as described in 
https://github.com/openjdk/jdk/pull/5872#discussion_r848950065), they almost 
always have only local-variable skip buffers (not kept as fields for reuse), 
and their buffers' max sizes are smaller that provided by the InputStream class.

Imo we should check the usage of `skip` in other projects; in JDK it's like 
skipping 2 bytes in certain image formats, and I would expect usages like 
reading class file attribute name and size then skip by the read size.

> This change may be problematic for servers with a large number connections 
> and an input stream for each connection. It could add up to 2k to the 
> footprint of each connection when skip is used.

If per-object allocation is a problem, would it be feasible to allocate a 
static soft reference to a max-sized skip buffer? It can be potentially shared 
with the `Reader` class, too.

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

PR: https://git.openjdk.java.net/jdk/pull/5872

Reply via email to