On Sun, 6 Oct 2024 16:39:02 GMT, Chen Liang <li...@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
