RFR: 8341566: Adding factory for non-synchronized CharSequence Reader

[Chen Liang]

On Sat, 5 Oct 2024 16:48:56 GMT, Markus KARG <duke at openjdk.org> wrote:

>> This Pull Requests proposes an implementation for [JDK-8341566](https://bugs.openjdk.org/browse/JDK-8341566): Adding the new method `public static Reader Reader.of(CharSequence)` will return an anonymous, non-synchronized implementation of a `Reader` for each kind of `CharSequence` implementation. It is optimized for `String`, `StringBuilder`, `StringBuffer` and `CharBuffer`.
>> 
>> In addition, this Pull Request proposes to replace the implementation of `StringReader` to become a simple synchronized wrapper around `Reader.of(CharSequence)` for the case of `String` sources. To ensure correctness, this PR...
>> * ...simply moved the **original code** of `StringBuilder` to become the de-facto implementation of `Reader.of()`, then stripped synchronized from it on the left hand, but kept just a synchronized wrapper on the right hand. Then added a `switch` for optimizations within the original code, at the exact location where previously just an optimization for `String` lived in.
>> * ...added tests for all methods (`Of.java`), and applied that test upon the modified `StringBuilder`.
>> 
>> Wherever new JavaDocs were added, existing phrases from other code locations have been copied and adapted, to best match the same wording.
>
> src/java.base/share/classes/java/io/Reader.java line 341:
> 
>> 339:             public void close() {
>> 340:                 cs = null;
>> 341:             }
> 
> @AlanBateman I need to confess that I did not understand what you had in mind when you wrote this on the mailing list:
>> That doesn't excuse you completely from thinking about concurrent use as Readers have a close method so you'll need to think about how close is specified for when it is called while another thread is reading chars from a custom CS.
> 
> As this new implementation explicitly is for single-threaded use only, there is no such other thread that could call `close` concurrently.
> 
> Maybe I am missing something here, so I would kindly ask for an outline of a scenario where -despite the explicit single-thread note- a second thread *does* exist?

A scenario would be that this reader is shared by multiple threads in an argument to `Runnable`, like create 100 runnables, 50 are trying to close while 50 are reading to char arrays. There will be scenarios where some reading thread is reading while some closing thread invokes `close` and completes before the reading thread completes.

I think the key takeaway here is that `close` does not block until other threads finish reading, does not affect already-begun reading operations, and the closed state might not be immediately observed by other threads (as we do not release-write and acquire-read the cs clearing state). However, I don't know which of these attributes should be explicitly specified in our API specification.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/21371#discussion_r1789113952