RFR: 8341566: Adding factory for non-synchronized CharSequence Reader [v2]

[Markus KARG]

On Sun, 6 Oct 2024 16:39:02 GMT, Chen Liang <liach at openjdk.org> wrote:

>> Markus KARG has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   fixup! Reader.of(String)
>>   
>>   Updated JavaDocs according to Alan Bateman's review comments:
>>   * Dropped "API compatible with StringReader" from description
>>   * @apiNote in StringReader refers to static factory method
>>   * Dropped "lock" field from API docs
>>   * Added "The resulting Reader is not safe for use by multiple concurrent threads. If the Reader is to be used by more than one thread it should be controlled by appropriate synchronization"
>>   * Deviating from your proposal, I renamed parameter "c" to "source" for clarity as the name "cs" already exists as an internal variable
>>   * Method specifies NPE for `of(null)` case
>>   
>>   I addition, JavaDocs now says "reader", not "stream" anymore.
>
> src/java.base/share/classes/java/io/Reader.java line 174:
> 
>> 172:         return new Reader() {
>> 173:             private final int length = source.length();
>> 174:             private CharSequence cs = source;
> 
> Can we make this `cs` final and track the closed state either in a new boolean field or with a negative value in `next`? In fact, if we make `cs` final, we can just replace the `cs` field referenced with the captured `source`.

This is correct, but then we would keep the reference to the char sequence "forever", without any good reason. This content could be *huge* (and often it is, as `CharSequence` most often is a `StringBuilder` just because the application *wants* to create huge content, and `String`-concatenation would be too costly with *huge* content). In fact, I see this scenario as the top use case for this new API.

Having said that, I would really like to have this field non-final to unlink the reference to the source as soon as possible, implicitly allowing GC to free this unused memory.

> src/java.base/share/classes/java/io/Reader.java line 187:
> 
>> 185:              * Reads a single character.
>> 186:              *
>> 187:              * @return     The character read, or -1 if the end of the stream has been
> 
> Don't need these javadocs; this implementation class is not publicly visible.

Correct. If everybody is fine with that, I will drop it.

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

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