RFR(L) 8227745: Enable Escape Analysis for Better Performance in the Presence of JVMTI Agents
David Holmes
david.holmes at oracle.com
Fri Dec 13 01:52:48 UTC 2019
On 13/12/2019 10:56 am, Vladimir Kozlov wrote:
> Yes, David
>
> You are correct these changes touch all part of VM and may affect Graal
> (which also has EA) too.
> Changes should be tested in all our modes: tiered, C1 only, Graal,
> Interpreter. And I realized that I only ran tier3-graal testing so I
> submitted the rest of Graal's tiers now.
>
> I had assumed that our current testing (I ran all from tier1 to tier8)
> should exercise all paths in VM these changes touch. But I may be wrong
> and it is correct to ask author to add testing in all VM modes to make
> sure new code in VM's runtime and JVMTI is tested.
It may be that our existing JVM TI tests will exercise this adequately
and that the new tests are more "whitebox" testing than general
functional tests. But it is not obvious to me that we do have the
coverage we need.
Cheers,
David
> I do like to keep what current test is doing with C2. May be add an
> other test for other modes or modify current one to enable to run it in
> other modes.
>
> Thanks,
> Vladimir
>
> On 12/12/19 3:32 PM, David Holmes wrote:
>> On 13/12/2019 9:02 am, Reingruber, Richard wrote:
>>> Hello Vladimir,
>>>
>>> thanks for having a look.
>>>
>>> > Use vm.compMode == "Xmixed" instead of vm.compMode != "Xcomp" to
>>> skip
>>> > test from running in Interpreter mode too.
>>>
>>> Done.
>>>
>>> > You don't need vm.opt.TieredCompilation != true in @requires
>>> because you
>>> > specified -XX:-TieredCompilation in @run command.
>>>
>>> Ok.
>>>
>>> > The test is specifically written for C2 only (not for C1 or
>>> Graal) to
>>> > verify its Escape Analysis optimization.
>>> > I did not look in great details into test's code but its
>>> analysis may be
>>> > affected if C1 compiler is also used.
>>> >
>>> > Richard may clarify this.
>>>
>>> The test cases aim to get their testmethod 'dontinline_testMethod'
>>> compiled by C2. If they get C1
>>> compiled before doesn't matter all that much. I've got a slight
>>> preference to disabled tiered
>>> compilation for simplicity.
>>
>> My concern - perhaps unfounded - is that this seems to be being tested
>> only in a pure C2 environment when the actual changes will have to
>> operate correctly in a tiered environment (and JVMCI).
>>
>> Thanks,
>> David
>>
>>> Thanks, Richard.
>>>
>>> -----Original Message-----
>>> From: Vladimir Kozlov <vladimir.kozlov at oracle.com>
>>> Sent: Donnerstag, 12. Dezember 2019 19:20
>>> To: David Holmes <david.holmes at oracle.com>;
>>> hotspot-runtime-dev at openjdk.java.net;
>>> hotspot-compiler-dev at openjdk.java.net;
>>> serviceability-dev at openjdk.java.net; Reingruber, Richard
>>> <richard.reingruber at sap.com>
>>> Subject: Re: RFR(L) 8227745: Enable Escape Analysis for Better
>>> Performance in the Presence of JVMTI Agents
>>>
>>> Hi David,
>>>
>>> Tiered is disabled because we don't want to see compilations and outputs
>>> from C1 compiler which does not have EA.
>>>
>>> The test is specifically written for C2 only (not for C1 or Graal) to
>>> verify its Escape Analysis optimization.
>>> I did not look in great details into test's code but its analysis may be
>>> affected if C1 compiler is also used.
>>>
>>> Richard may clarify this.
>>>
>>> thanks,
>>> Vladimir
>>>
>>> On 12/11/19 1:04 PM, David Holmes wrote:
>>>> On 12/12/2019 5:21 am, Vladimir Kozlov wrote:
>>>>> I will do full review later. I want to comment about test command
>>>>> line.
>>>>>
>>>>> You don't need vm.opt.TieredCompilation != true in @requires because
>>>>> you specified -XX:-TieredCompilation in @run command.
>>>>
>>>> And per my comment this should be being tested with tiered as well.
>>>>
>>>> David
>>>>
>>>>> Use vm.compMode == "Xmixed" instead of vm.compMode != "Xcomp" to skip
>>>>> test from running in Interpreter mode too.
>>>>>
>>>>> Thanks,
>>>>> Vladimir
>>>>>
>>>>> On 12/11/19 7:07 AM, Reingruber, Richard wrote:
>>>>>> Hi David,
>>>>>>
>>>>>> > Most of the details here are in areas I can comment on in
>>>>>> detail, but I
>>>>>> > did take an initial general look at things.
>>>>>>
>>>>>> Thanks for taking the time!
>>>>>>
>>>>>> > The only thing that jumped out at me is that I think the
>>>>>> > DeoptimizeObjectsALotThread should be a hidden thread.
>>>>>> >
>>>>>> > + bool is_hidden_from_external_view() const { return true; }
>>>>>>
>>>>>> Yes, it should. Will add the method like above.
>>>>>>
>>>>>> > Also I don't see any testing of the
>>>>>> DeoptimizeObjectsALotThread.
>>>>>> Without
>>>>>> > active testing this will just bit-rot.
>>>>>>
>>>>>> DeoptimizeObjectsALot is meant for stress testing with a larger
>>>>>> workload. I will add a minimal test
>>>>>> to keep it fresh.
>>>>>>
>>>>>> > Also on the tests I don't understand your @requires clause:
>>>>>> >
>>>>>> > @requires ((vm.compMode != "Xcomp") & vm.compiler2.enabled &
>>>>>> > (vm.opt.TieredCompilation != true))
>>>>>> >
>>>>>> > This seems to require that TieredCompilation is disabled, but
>>>>>> tiered is
>>>>>> > our normal mode of operation. ??
>>>>>> >
>>>>>>
>>>>>> I removed the clause. I guess I wanted to target the tests towards
>>>>>> the code they are supposed to
>>>>>> test, and it's easier to analyze failures w/o tiered compilation and
>>>>>> with just one compiler thread.
>>>>>>
>>>>>> Additionally I will make use of
>>>>>> compiler.whitebox.CompilerWhiteBoxTest.THRESHOLD in the tests.
>>>>>>
>>>>>> Thanks,
>>>>>> Richard.
>>>>>>
>>>>>> -----Original Message-----
>>>>>> From: David Holmes <david.holmes at oracle.com>
>>>>>> Sent: Mittwoch, 11. Dezember 2019 08:03
>>>>>> To: Reingruber, Richard <richard.reingruber at sap.com>;
>>>>>> serviceability-dev at openjdk.java.net;
>>>>>> hotspot-compiler-dev at openjdk.java.net;
>>>>>> hotspot-runtime-dev at openjdk.java.net
>>>>>> Subject: Re: RFR(L) 8227745: Enable Escape Analysis for Better
>>>>>> Performance in the Presence of JVMTI Agents
>>>>>>
>>>>>> Hi Richard,
>>>>>>
>>>>>> On 11/12/2019 7:45 am, Reingruber, Richard wrote:
>>>>>>> Hi,
>>>>>>>
>>>>>>> I would like to get reviews please for
>>>>>>>
>>>>>>> http://cr.openjdk.java.net/~rrich/webrevs/2019/8227745/webrev.3/
>>>>>>>
>>>>>>> Corresponding RFE:
>>>>>>> https://bugs.openjdk.java.net/browse/JDK-8227745
>>>>>>>
>>>>>>> Fixes also https://bugs.openjdk.java.net/browse/JDK-8233915
>>>>>>> And potentially https://bugs.openjdk.java.net/browse/JDK-8214584 [1]
>>>>>>>
>>>>>>> Vladimir Kozlov kindly put webrev.3 through tier1-8 testing without
>>>>>>> issues (thanks!). In addition the
>>>>>>> change is being tested at SAP since I posted the first RFR some
>>>>>>> months ago.
>>>>>>>
>>>>>>> The intention of this enhancement is to benefit performance wise
>>>>>>> from escape analysis even if JVMTI
>>>>>>> agents request capabilities that allow them to access local variable
>>>>>>> values. E.g. if you start-up
>>>>>>> with -agentlib:jdwp=transport=dt_socket,server=y,suspend=n, then
>>>>>>> escape analysis is disabled right
>>>>>>> from the beginning, well before a debugger attaches -- if ever one
>>>>>>> should do so. With the
>>>>>>> enhancement, escape analysis will remain enabled until and after a
>>>>>>> debugger attaches. EA based
>>>>>>> optimizations are reverted just before an agent acquires the
>>>>>>> reference to an object. In the JBS item
>>>>>>> you'll find more details.
>>>>>>
>>>>>> Most of the details here are in areas I can comment on in detail,
>>>>>> but I
>>>>>> did take an initial general look at things.
>>>>>>
>>>>>> The only thing that jumped out at me is that I think the
>>>>>> DeoptimizeObjectsALotThread should be a hidden thread.
>>>>>>
>>>>>> + bool is_hidden_from_external_view() const { return true; }
>>>>>>
>>>>>> Also I don't see any testing of the DeoptimizeObjectsALotThread.
>>>>>> Without
>>>>>> active testing this will just bit-rot.
>>>>>>
>>>>>> Also on the tests I don't understand your @requires clause:
>>>>>>
>>>>>> @requires ((vm.compMode != "Xcomp") & vm.compiler2.enabled &
>>>>>> (vm.opt.TieredCompilation != true))
>>>>>>
>>>>>> This seems to require that TieredCompilation is disabled, but
>>>>>> tiered is
>>>>>> our normal mode of operation. ??
>>>>>>
>>>>>> Thanks,
>>>>>> David
>>>>>>
>>>>>>> Thanks,
>>>>>>> Richard.
>>>>>>>
>>>>>>> [1] Experimental fix for JDK-8214584 based on JDK-8227745
>>>>>>> http://cr.openjdk.java.net/~rrich/webrevs/2019/8214584/experiment_v1.patch
>>>>>>>
>>>>>>>
>>>>>>>
More information about the serviceability-dev
mailing list