On Sat, 15 Aug 2026 18:06:35 GMT, Alan Bateman <[email protected]> wrote:
>> I don't think this is quite right. A CharsetDecoder can be configured with a >> "replacement value" so it may be different than the default. If someone >> creates an InputStreamReader with a configured CharsetDecoder then the >> proposed fast path will use the String constructor and the default >> replacement value, oops! >> >> The slow path looks like it has an issue too. The decoder needs to be reset >> at EOF, otherwise a subsequent read will throw rather than return -1. Look >> at the existing implRead implementation to see what I mean. >> >> It might be simpler to reduce the focus of the proposal to only the cases >> where an InputStreamReader is created with a Charset and no characters are >> read from the reader before readAsString is invoked to consume to EOF. Leave >> all other cases to the existing code. > > Given the land mines, maybe it would be better to park this PR and focus > first on adding tests for corner cases. These tests will be needed anyway to > be confident with any re-implementation or changes to this code. Fast path is now enabled only for charset-created InputStreamReaders. Slow path is now handling "read() after readAllAsString()" (returns `-1`, does not throw). There is a test for it. I am not sure I understood your proposal correct, so kindly asking for advice: - Do you want me to *remove* the slow path? That would be ok for me, as my original proposal only had the *fast* path in mind, actually. OTOH I think it should be working now, and is covered with tests. - Do you want me to cover the slow path *with more tests*? In fact I do not see *which ones*. - Do you want me to provide a full suite of tests covering *all* corner cases, including externally provided CharsetDecoder? This would be a real lot of tests. So I added *just some* for now. You're the lead. What direction do you want me to go? ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/32264#discussion_r3853071497
