RFR: 8358533: Improve performance of java.io.Reader.readAllLines

[Markus KARG]

On Wed, 18 Jun 2025 00:04:37 GMT, Brian Burkhalter <bpb at openjdk.org> wrote:

> Replaces the implementation `readAllCharsAsString().lines().toList()` with reading into a temporary `char` array which is then processed to detect line terminators and copy non-terminating characters into strings which are added to the list.

src/java.base/share/classes/java/io/Reader.java line 480:

> 478:                     pos++;
> 479:                 } else { // term > pos
> 480:                     if (eol && sb.length() == 0) {

Is there a reason for `sb.length() == 0` instead of `sb.isEmpty()`?

src/java.base/share/classes/java/io/Reader.java line 508:

> 506:         }
> 507: 
> 508:         return lines;

Do we really want to return a mutable `ArrayList` here? In earlier discussions about this very API I was told that it deliberately returns `String` instead of `CharSequence` due to *intended* immutability, even if that potentially implied slower performance. Following this logic, it would be just straightforward to `return Collections.unmodifiableList(lines);` here. 🤔

src/java.base/share/classes/java/io/Reader.java line 551:

> 549:     public String readAllAsString() throws IOException {
> 550:         StringBuilder result = new StringBuilder();
> 551:         char[] cbuf = new char[TRANSFER_BUFFER_SIZE];

As this PR explicitly targets performance and as the aim of this method is to keep **all** content in-memory anyways, I wonder if it would be acceptable and even faster to pre-allocate `new StringBuilder(TRANSFER_BUFFER_SIZE)`? In the end, this allocation is just temporary.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/25863#discussion_r2156727346
PR Review Comment: https://git.openjdk.org/jdk/pull/25863#discussion_r2156742905
PR Review Comment: https://git.openjdk.org/jdk/pull/25863#discussion_r2156750703