On Sat, 3 May 2025 09:30:17 GMT, Markus KARG <d...@openjdk.org> wrote:
>> src/java.base/share/classes/java/io/Reader.java line 213:
>>
>>> 211: public String readAllChars() throws IOException {
>>> 212: ensureOpen();
>>> 213: return cs.toString().substring(next);
>>
>> Your change implies creating a full-length string *first* (including cyoping
>> and compression), just to strip it down to a smaller one *later*. It would
>> be more efficient to *first* strip it down, and compression into a `String`
>> *afterwards*. Imagine a huge `cs` with `next` being near to `length()`, and
>> you see the difference in efficiency! 🙂
>>
>> `cs.subSequence(next, cs.length()).toString()`
>>
>> (Edited my proposal to be more correct.)
>
> Oh, and we should set `next` afterwards, so the `Reader` knows that the end
> of the sequence is reached:
>
>
> public String readAllChars() throws IOException {
> ensureOpen();
> var len = cs.length();
> var remainder = cs.subSequence(next, len);
> next = len;
> return remainder.toString();
> }
>
>
> (Edited my proposal to be a bit more concurrency-friendly and correct.)
Probably best to ignore the implementation details as the discussions on what
methods to expose, and where, is still going on. It was probably a bit
premature to create the PR without getting agreement on the API first.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/24728#discussion_r2072521725
