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

[Markus KARG]

On Sat, 5 Oct 2024 16:32:39 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?

src/java.base/share/classes/java/io/StringReader.java line 183:

> 181:                 r.close();
> 182:             } catch (IOException e) {
> 183:                 throw new UncheckedIOException(e);

In fact, the implementation of `close` in the original class will never throw `IOException`, but unfortunately `Reader.close()` declares `throws IOException`.

test/jdk/java/io/Reader/Of.java line 51:

> 49:     public static Reader[] readers() {
> 50:         return new Reader[] {
> 51:             new StringReader(CONTENT),

Explicitly including that original class here (even if it has nothing to do with the `of` method) to be sure that we did not modify it in an incompatible way. Unfortunately there is no full test coverage for `StringReader`, and it does not make much sense to duplicate the tests.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/21371#discussion_r1788644851
PR Review Comment: https://git.openjdk.org/jdk/pull/21371#discussion_r1788644490
PR Review Comment: https://git.openjdk.org/jdk/pull/21371#discussion_r1788644282