RFR 8228675: Resolve.findMethod doesn't cache interface type calculation

[Maurizio Cimadamore]

For the records, I did some internal performance tests, and this patch 
doesn't seem to make the performance situation worse in the general case.

I'm now curious to see if it helps with your specific use case.

Maurizio

On 04/09/2019 20:58, Maurizio Cimadamore wrote:
>
> Hi Ron,
> I put together something, albeit not the approach I described in this 
> thread.
>
> Since, from what I'm reading, the main issue you are seeing is that 
> the use of Types.closure is basically killing performances, I 
> addressed that problem instead, and rewrote the code not to use that 
> method. The new code is probably executing more checks than the old 
> (since interfaces could be implemented multiple times at different 
> levels of the hierarchy), but I think it should be enough to resolve 
> the issue you are seeing.
>
> I did not spend a lot of time testing the patch, other than doing some 
> basic validation - so handle with care :-)
>
> http://cr.openjdk.java.net/~mcimadamore/8228675/
>
> Maurizio
>
> On 04/09/2019 15:18, Maurizio Cimadamore wrote:
>>
>> I didn't realize you were waiting on me to produce a patch; I'm 
>> afraid at the moment I don't have many cycles to look into this.
>>
>> Maurizio
>>
>> On 04/09/2019 13:05, Ron Shapiro wrote:
>>> Friendly ping. This is really hurting some of our builds. Maurizio, 
>>> are you able to prepare a patch for us to look at?
>>>
>>> On Tue, Aug 13, 2019 at 8:12 AM Ron Shapiro <ronshapiro at google.com 
>>> <mailto:ronshapiro at google.com>> wrote:
>>>
>>>     Aren't types recreated across rounds if there are any transitive
>>>     error types? That was at least my assumption.
>>>
>>>     On Mon, Aug 12, 2019 at 5:50 PM Jan Lahoda
>>>     <jan.lahoda at oracle.com <mailto:jan.lahoda at oracle.com>> wrote:
>>>
>>>         I wonder if this will work with annotation processors? (As
>>>         these may
>>>         create new interfaces into the hierarchy.)
>>>
>>>         Jan
>>>
>>>         On 07. 08. 19 12:25, Ron Shapiro wrote:
>>>         > Made the change for a WeakHashMap:
>>>         > http://cr.openjdk.java.net/~ronsh/8228675/webrev.01/.
>>>         >
>>>         > Perhaps it would make better sense for this to be a field
>>>         on ClassType?
>>>         > That would avoid the need for managing a cache with weak keys.
>>>         >
>>>         > Do you have any guidance on how you approach tradeoffs in
>>>         memory vs. speed?
>>>         >
>>>         > On Mon, Aug 5, 2019 at 7:25 PM Vicente Romero
>>>         <vicente.romero at oracle.com <mailto:vicente.romero at oracle.com>
>>>         > <mailto:vicente.romero at oracle.com
>>>         <mailto:vicente.romero at oracle.com>>> wrote:
>>>         >
>>>         >     not sure if we should add yet another cache to javac
>>>         but in any case
>>>         >     it should be implemented with a: WeakHashMap,
>>>         >
>>>         >     Thanks,
>>>         >     Vicente
>>>         >
>>>         >     On 8/5/19 10:32 AM, Ron Shapiro wrote:
>>>         >>     Fair question.
>>>         >>
>>>         >> github.com/google/dagger
>>>         <http://github.com/google/dagger>
>>>         <http://github.com/google/dagger>
>>>         >>     generates implementations of interfaces that implement a
>>>         >>     dependency injection graph. Sometimes, though not
>>>         always, these
>>>         >>     interfaces have dozens if not hundreds of
>>>         (super)interfaces which
>>>         >>     allow for builds to be sharded: each subsection of
>>>         the build
>>>         >>     refers to the superinterface and then at the root of
>>>         the build
>>>         >>     there is a union of them all. Naturally, this means
>>>         this must be
>>>         >>     rebuilt on most/all changes to any subproject of the
>>>         build.
>>>         >>
>>>         >>     בתאריך יום ב׳, 5 באוג׳ 2019, 17:05, מאת Vicente Romero
>>>         >>     ‏<vicente.romero at oracle.com
>>>         <mailto:vicente.romero at oracle.com>
>>>         <mailto:vicente.romero at oracle.com
>>>         <mailto:vicente.romero at oracle.com>>>:
>>>         >>
>>>         >>         Hi Ron,
>>>         >>
>>>         >>         Just out of curiosity, what is the context in
>>>         which this issue
>>>         >>         arises? Naturally consuming more memory we can
>>>         speed up javac
>>>         >>         but this can lead to other problems too.
>>>         >>
>>>         >>         Thanks,
>>>         >>         Vicente
>>>         >>
>>>         >>         On 8/5/19 8:24 AM, Ron Shapiro wrote:
>>>         >>>         Friendly ping
>>>         >>>
>>>         >>>         בתאריך שבת, 27 ביולי 2019, 0:05, מאת Ron Shapiro
>>>         >>>         ‏<ronshapiro at google.com
>>>         <mailto:ronshapiro at google.com> <mailto:ronshapiro at google.com
>>>         <mailto:ronshapiro at google.com>>>:
>>>         >>>
>>>         >>>             Hi,
>>>         >>>
>>>         >>>             Please review this change to speed up
>>>         Resolve.findMethod
>>>         >>>             for large classes that have large numbers of
>>>         >>>             (super)interfaces
>>>         >>>
>>>         >>>             webrev:
>>>         http://cr.openjdk.java.net/~ronsh/8228675/webrev.00/
>>>         >>>             bug:
>>>         https://bugs.openjdk.java.net/browse/JDK-8228675
>>>         >>>
>>>         >>>             Thanks,
>>>         >>>             Ron
>>>         >>>
>>>         >>
>>>         >
>>>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.java.net/pipermail/compiler-dev/attachments/20190906/4bf18b3a/attachment.html>