RFR: 8333103: Re-examine the console provider loading

[Naoto Sato]

On Thu, 30 May 2024 08:23:25 GMT, Pavel Rappo <prappo at openjdk.org> wrote:

>> It's only semantically the same if we assume a module can only provide a single `JdkConsoleProvider`, no? The `break;` disallows multiple providers (for disjoint sets of charsets) in a single module.
>
> Claes has described the issue well. Like I said, `break` short-circuits the search. If a module can provide more than one console provider, the suggested stream-less replacement is **not** equivalent.
> 
> If a module can provide more than one console provider, not only the code needs to be fixed, but a relevant test should be added too. The test should be in this PR, but tagged with the initial bug, [8295803], not this (performance) bug.
> 
> [8295803]: https://bugs.openjdk.org/browse/JDK-8295803

In fact, this started simply for incorporating JLine implementation into Console, and using ServiceLoader was a mere means to load its impl as it resides outside the java.base module. So yes, module and its console implementation is 1:1, which is reflected by the system property `jdk.console` that takes the module name. So that `break;` effectively shortcut the unnecessary looping (I don't think it would happen btw).
That said, I think it needs to be described in the comment above that piece of code. I will add it shortly.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/19467#discussion_r1621231072