On Thu, 30 May 2024 18:10:14 GMT, Naoto Sato <na...@openjdk.org> wrote:

>> 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.

So, it's the previous (stream) version that was more permissive and 
inadvertently so, yes?

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

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

Reply via email to