RFR: 8026369: javac potentially ambiguous overload warning needs an improved scheme [v3]

[Vicente Romero]

On Fri, 24 Feb 2023 19:27:02 GMT, Archie L. Cobbs <duke at openjdk.org> wrote:

>> src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Check.java line 2719:
>> 
>>> 2717:      *  does not override any other methods (in which case site is responsible).
>>> 2718:      */
>>> 2719:     void checkPotentiallyAmbiguousOverloads(JCClassDecl tree, Type site) {
>> 
>> general comments: overall looks correct but I think the code should be split a bit using helper methods, that will help with readability, I think. Side: I'm a bit worried that overuse of streams in this code could imply some performance hit. Of course if the corresponding lint warning is not enabled we will skip it but a lot of projects compile with `-Xlint:all` nowadays.
>
>> I think the code should be split a bit using helper methods
> 
> OK - will do.
> 
>> I'm a bit worried that overuse of streams in this code could imply some performance hit
> 
> I asked basically the same question ([in a separate thread](https://mail.openjdk.org/pipermail/compiler-dev/2023-February/022259.html)) two days ago and nobody replied with a definitive answer (not surprising).
> 
> However, note also that in that same thread Christoph reported no timing difference between `Stream` vs. `for()` loop ([here](https://mail.openjdk.org/pipermail/compiler-dev/2023-February/022261.html)), although there were more allocations. FWIW.
> 
> Sorry to go off on a tangent here, but I'm sure this issue will come up again and it would be nice to have some kind of (informal) policy to go on...
> 
> I generally try to follow the "measure first, optimize second" rule to avoid preemptive "optimizations" that come at the expense of code clarity for unproven meaningful benefit.
> 
> So I can de-`Stream` the code but are we sure it's worth it? Are we going to have a no `Stream` policy in the compiler code? Why did we develop `Stream`'s if they can't be used in a mainstream tool like `javac`? Where does the madness end? :)
> 
> There is also the larger philosophical question as well, which is that if a `Stream` is appreciably slower than the semantically-equivalent `for()` loop, then isn't that a Hotspot problem, not a developer problem?

I'm not saying that we should de-stream the code, actually we can do that later on iff there is a performance related complain, but it is true that in the past, I have seen some performance issues and the final culprit have been streams. But you are right it could be that it is not worthy to affect the readability of the code.

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

PR: https://git.openjdk.org/jdk/pull/12645