On Wed, 18 Jun 2025 00:04:37 GMT, Brian Burkhalter <b...@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
