On Tue, 9 Feb 2021 14:08:10 GMT, Philippe Marschall 
<github.com+471021+marsch...@openjdk.org> wrote:

>> src/java.base/share/classes/java/io/Reader.java line 198:
>> 
>>> 196:         } else {
>>> 197:             int remaining = target.remaining();
>>> 198:             char cbuf[] = new char[Math.min(remaining, 
>>> TRANSFER_BUFFER_SIZE)];
>> 
>> As `cbuf` for the off-heap case is used in a synchronized block, is there 
>> the opportunity for some sort of cached array here and would it help?
>
> That would be possible. It would help in cases where a large Reader is read 
> into one or several relatively small off-heap CharBuffers, requiring multiple 
> #read calls. This can only be done when the caller is able to work with only 
> a partial input. I don't know how common this case is.
> 
> We could re-purpose #skipBuffer, it has the same maximum size (8192) but 
> determined by a different constant (#maxSkipBufferSize instead of 
> #TRANSFER_BUFFER_SIZE). That would likely require it to be renamed and maybe 
> we should even remove #maxSkipBufferSize. We could also do the reallocation 
> and growing similar as is currently done in #skip.

Perhaps a static final `WORK_BUFFER_SIZE` could be added with value 8192 and 
`maxSkipBufferSize` and `TRANSFER_BUFFER_SIZE` replaced with that? Then 
`skipBuffer` could be renamed to `workBuffer` and used in both 
`read(CharBuffer)` and `skip(long)`. That shouldn't be a problem as both uses 
are in synchronized blocks. Also I suggest putting the declaration of 
`workBuffer` just below that of `lock` instead of lower down the file where 
`skipBuffer` is.

Lastly you mentioned C-style array declarations like `char buf[]`. As there are 
only four of these in the file it might be good to just go ahead and change 
them, I don't think that adds much noise or risk.

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

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

Reply via email to