On Fri, 24 Feb 2023 19:27:02 GMT, Archie L. Cobbs <d...@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
