RFR(S): 8216556: Unnecessary liveness computation with JVMTI

[dean.long at oracle.com]

+1

dl

On 1/15/19 3:05 AM, Doerr, Martin wrote:
> Hi Vladimir, Dean and Claes,
>
> thank you for reviewing.
> I assume the version which moves the implementation of should_retain_local_variables() to the hpp file (as suggested by Claes) is fine.
> I'll push this version if there are no objections.
>
> Best regards,
> Martin
>
>
> -----Original Message-----
> From: hotspot-compiler-dev <hotspot-compiler-dev-bounces at openjdk.java.net> On Behalf Of Doerr, Martin
> Sent: Montag, 14. Januar 2019 09:31
> To: Claes Redestad <claes.redestad at oracle.com>; 'hotspot-compiler-dev at openjdk.java.net' <hotspot-compiler-dev at openjdk.java.net>
> Subject: [CAUTION] RE: RFR(S): 8216556: Unnecessary liveness computation with JVMTI
>
> Hi Claes,
>
> excellent proposal. Thanks. I had not noticed that it currently is in a cpp file.
>
> New webrev:
> http://cr.openjdk.java.net/~mdoerr/8216556_JVMTI_liveness/webrev.01/
>
> What I still don't really like is that we're passing MethodLivenessResult objects on stack via 3 compilation units.
> But I don't know if it's worth refactoring the code.
>
> Best regards,
> Martin
>
>
> -----Original Message-----
> From: Claes Redestad <claes.redestad at oracle.com>
> Sent: Freitag, 11. Januar 2019 16:45
> To: Doerr, Martin <martin.doerr at sap.com>
> Subject: Re: RFR(S): 8216556: Unnecessary liveness computation with JVMTI
>
> Hi,
>
>    just a random thought, but if you're optimizing this and got some
> measure where it matters(?), maybe you should also try inlining
> ciEnv::should_retain_local_variables(), i.e., move definition to
> ciEnv.hpp. If it doesn't bloat static binary size it seems like it won't
> hurt, at least.
>
> /Claes
>
> On 2019-01-11 13:55, Doerr, Martin wrote:
>> Hi,
>>
>> I'd like to contribute a small JIT improvement for JVMTI to avoid
>> calling raw_liveness_at_bci when its result is not needed.
>>
>> Bug with description:
>>
>> https://bugs.openjdk.java.net/browse/JDK-8216556
>>
>> Webrev:
>>
>> http://cr.openjdk.java.net/~mdoerr/8216556_JVMTI_liveness/webrev.00/
>>
>> Please review.
>>
>> Best regards,
>>
>> Martin
>>