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