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

Maurizio Cimadamore maurizio.cimadamore at oracle.com
Fri Sep 6 11:07:06 UTC 2019


On 06/09/2019 11:44, Maurizio Cimadamore wrote:
>
> 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.
>
More generally speaking, for this - and others - performance related 
issues, we're happy to look into them, but I think it is generally good 
to submit a reproducible test case that we can use as a playground; this 
will help us understanding what the problem is, understanding what are 
the connections to other problems in the code base, as well as to assess 
its priority. Such a test case would also form the backbone of any test 
that would be pushed upstream (as it is generally preferrable to always 
include tests in patches). I know that sometimes distilling such tests 
takes time, but without them we're basically throwing darts in the dark.

Maurizio


> 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/98b221d0/attachment-0001.html>


More information about the compiler-dev mailing list