RFR: 4926314: Optimize Reader.read(CharBuffer)

[Brian Burkhalter]

On Thu, 31 Dec 2020 10:11:58 GMT, Philippe Marschall <github.com+471021+marschall at openjdk.org> wrote:

>> Implement three optimiztations for Reader.read(CharBuffer)
>> 
>> * Add a code path for heap buffers in Reader#read to use the backing array instead of allocating a new one.
>> * Change the code path for direct buffers in Reader#read to limit the intermediate allocation to `TRANSFER_BUFFER_SIZE`.
>> * Implement `InputStreamReader#read(CharBuffer)` and delegate to `StreamDecoder`.
>> * Implement `StreamDecoder#read(CharBuffer)` and avoid buffer allocation.
>
> A couple of implementation notes:
> 
> Reader#read(CharBuffer)
> 
> on-heap case
> 
> I introduced a dedicated path for the on-heap case and directly read into the backing array. This completely avoids the intermediate allocation and copy. Compared to the initial proposal the buffer position is updated. This has to be done because we bypass the buffer and directly read into the array. This also handles the case that #read returns -1.
> 
> I am using a c-style array declaration because the rest of the class uses it.
> 
> off-heap case
> 
> I limit the intermadiate allocation to TRANSFER_BUFFER_SIZE. In order to reduce the total intermediate allocation we now call #read multiple times until the buffer is full or EOF is reached. This changes the behavior slightly as now possibly more data is read. However this should contribute to reduce the overall intermediate allocations.
> 
> A lock is acquired to keep the the read atomic. This is consistent with #skip which also holds a lock over multiple #read calls. This is inconsistent with #transferTo which does not acquire a lock over multiple #read calls. 
> 
> The implementation took inspiration from java.io.InputStream#readNBytes(int).
> 
> InputStreamReader#read(CharBuffer)
> 
> Since StreamDecoder is a Reader as well we can simply delegate.
> 
> StreamDecoder#read(CharBuffer)
> 
> Interestingly this was not implemented even though StreamDecoder internally works on NIO buffers.
> 
> on-heap case
> 
> We see a performance improvement and the elimination of all intermediate allocation.
> 
> StreamDecoder#read(char[], int, int) and InputStreamReader#read(char[], int, int) may get a small improvement as we no longer #slice the buffer.
> 
> off-heap case
> 
> We see the elimination of all intermediate allocation but a performance penalty because we hit the slow path in #decodeLoop. There is a trade-off here, we could improve the performance to previous levels by introducing intermediate allocation again. I don't know how much the users of off-heap buffers care about introducing intermediate allocation vs maximum throughput.
> 
> I was struggling to come up with microbenchmarks because the built in InputStream and Reader implementations are finite but JMH calls the benchmark methods repeatably. I ended up implementing custom infinite InputStream and Reader implementations. The code can be found there:
> 
> https://github.com/marschall/reader-benchmarks/tree/master/src/main/java/com/github/marschall/readerbenchmarks
> 
> An Excel with charts can be found here:
> 
> https://github.com/marschall/reader-benchmarks/raw/master/src/main/resources/4926314.xlsx
> 
> I looked for java.io.Reader#read(CharBuffer) users in the JDK and only found java.util.Scanner#readInput(). I wrote a microbenchmark for this but only found minuscule improvements due to regex dominating the runtime.
> 
> There seem to be no direct Reader tests in the tier1 suite, the initial code was wrong because I forgot to update the buffer code position but I only found out because some Scanner tests failed.

I changed the JBS issue summary to match the title of this PR so that integration blocker is removed.

How does the performance of `InputStreamReader.read(CharBuffer)` compare for the case where only the change to `Reader` is made versus if all your proposed changes are applied?

Some kind of specific test should likely be included.

Note that the more recent copyright year is now 2021.

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

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