RFR(L) 8227745: Enable Escape Analysis for Better Performance in the Presence of JVMTI Agents
Reingruber, Richard
richard.reingruber at sap.com
Wed Sep 9 07:14:15 UTC 2020
> Hi Richard,
> I suspect this one fell off the radar due to the extended review period.
> The actual review started last December (there was prior discussion
> IIRC) and only seemed to get partial reviews. I only looked at some
> parts. Robbin may have given things a deeper look, but seemed focused on
> the handshake aspects. Vladimir said he would do a full review but I
> can't find it. Eventually Martin and Goetz took over reviewing and
> everyone else dropped off. :(
That's how it went I reckon. I repeatedly asked for feedback and reviews, and
also tried to keep Vladimir, Robbin, and you in the loop addressing you directly
(e.g. [1])
> As this covers a number of areas it really does need "approval" from
> each area (and yes the hotspot wiki should reflect this).
I agree. The wiki should define that in a clear manner. And the community should
be involved in that definition.
> I will try to take another look while we await Serguei's return (and I
> never did follow up on the problem I had with the nested lock
> elimination handling. :( ).
Thanks for doing it.
> Meanwhile this will need to be converted to a PR in any case.
I hope to get the PR out later but we've got a team outing today... we haven't seen
each other since months... :)
Cheers, Richard.
[1] http://mail.openjdk.java.net/pipermail/serviceability-dev/2020-April/030911.html
-----Original Message-----
From: David Holmes <david.holmes at oracle.com>
Sent: Mittwoch, 9. September 2020 00:29
To: Reingruber, Richard <richard.reingruber at sap.com>; Marty Thompson <martin.thompson at oracle.com>; Daniel Daugherty <daniel.daugherty at oracle.com>; serviceability-dev <serviceability-dev at openjdk.java.net>; hotspot-compiler-dev at openjdk.java.net; Hotspot dev runtime <hotspot-runtime-dev at openjdk.java.net>; Robbin Ehn <robbin.ehn at oracle.com>; Vladimir Kozlov <vladimir.kozlov at oracle.com>
Subject: Re: RFR(L) 8227745: Enable Escape Analysis for Better Performance in the Presence of JVMTI Agents
Hi Richard,
I suspect this one fell off the radar due to the extended review period.
The actual review started last December (there was prior discussion
IIRC) and only seemed to get partial reviews. I only looked at some
parts. Robbin may have given things a deeper look, but seemed focused on
the handshake aspects. Vladimir said he would do a full review but I
can't find it. Eventually Martin and Goetz took over reviewing and
everyone else dropped off. :(
As this covers a number of areas it really does need "approval" from
each area (and yes the hotspot wiki should reflect this).
I will try to take another look while we await Serguei's return (and I
never did follow up on the problem I had with the nested lock
elimination handling. :( ).
Meanwhile this will need to be converted to a PR in any case.
Thanks,
David
On 9/09/2020 3:02 am, Reingruber, Richard wrote:
> Hello Marty,
>
> Sure. I'd be happy if Serguei could review the change.
>
> Thanks, Richard.
>
> -----Original Message-----
> From: Marty Thompson <martin.thompson at oracle.com>
> Sent: Dienstag, 8. September 2020 18:55
> To: Reingruber, Richard <richard.reingruber at sap.com>; Daniel Daugherty <daniel.daugherty at oracle.com>; serviceability-dev <serviceability-dev at openjdk.java.net>; hotspot-compiler-dev at openjdk.java.net; Hotspot dev runtime <hotspot-runtime-dev at openjdk.java.net>
> Subject: RE: RFR(L) 8227745: Enable Escape Analysis for Better Performance in the Presence of JVMTI Agents
>
> Hello Richard,
>
> It would be good if Serguei Spitsyn could review before this is pushed. Serguei is out this week. Can you wait until Serguei is back in the office the week of Sept 14?
>
> Regards,
>
> Marty
>
>> -----Original Message-----
>> From: Reingruber, Richard <richard.reingruber at sap.com>
>> Sent: Tuesday, September 8, 2020 9:45 AM
>> To: Daniel Daugherty <daniel.daugherty at oracle.com>; serviceability-dev
>> <serviceability-dev at openjdk.java.net>; hotspot-compiler-
>> dev at openjdk.java.net; Hotspot dev runtime <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 Dan,
>>
>> I'd be very happy about a review from somebody on the Serviceability team.
>> I have asked for reviews many times (kindly I hope). And the change is for
>> review for more than a year now.
>>
>> According to [1] I'd think all requirements to push are met already. But
>> maybe I missed something?
>>
>> After renaming of methods in SafepointMechanism the change needs to be
>> rebased (already done). I'll publish a pull request as soon as possible.
>>
>> Thanks, Richard.
>>
>> [1]
>> https://wiki.openjdk.java.net/display/HotSpot/Pushing+a+HotSpot+change
>>
>> -----Original Message-----
>> From: Daniel D. Daugherty <daniel.daugherty at oracle.com>
>> Sent: Dienstag, 8. September 2020 18:16
>> To: Reingruber, Richard <richard.reingruber at sap.com>; serviceability-dev
>> <serviceability-dev at openjdk.java.net>; hotspot-compiler-
>> dev at openjdk.java.net; Hotspot dev runtime <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,
>>
>> I haven't seen a review from anyone on the Serviceability team and I think
>> you should get a review from them since JVM/TI is involved.
>> Perhaps I missed it...
>>
>> Dan
>>
>>
>> On 9/7/20 10:09 AM, Reingruber, Richard wrote:
>>> Hi,
>>>
>>> I would like to close the review of this change.
>>>
>>> It has received a lot of helpful feedback during the process and 2
>>> full Reviews. Thanks everybody!
>>>
>>> I'm planning to push it this week on Thursday as solution for JBS items:
>>>
>>> https://bugs.openjdk.java.net/browse/JDK-8227745
>>> https://bugs.openjdk.java.net/browse/JDK-8233915
>>>
>>> Version to be pushed:
>>>
>>> http://cr.openjdk.java.net/~rrich/webrevs/8227745/webrev.8/
>>>
>>> Hope to get my GIT/Skara setup going until then... :)
>>>
>>> Thanks, Richard.
>>>
>>> -----Original Message-----
>>> From: hotspot-compiler-dev
>>> <hotspot-compiler-dev-retn at openjdk.java.net> On Behalf Of Reingruber,
>>> Richard
>>> Sent: Mittwoch, 2. September 2020 23:27
>>> To: Robbin Ehn <robbin.ehn at oracle.com>; serviceability-dev
>>> <serviceability-dev at openjdk.java.net>;
>>> hotspot-compiler-dev at openjdk.java.net; Hotspot dev runtime
>>> <hotspot-runtime-dev at openjdk.java.net>
>>> Subject: [CAUTION] RE: RFR(L) 8227745: Enable Escape Analysis for
>>> Better Performance in the Presence of JVMTI Agents
>>>
>>> Hi Robin,
>>>
>>>> On 2020-09-02 15:48, Reingruber, Richard wrote:
>>>>> Hi Robbin,
>>>>>
>>>>> // taking the discussion back to the mailing lists
>>>>>
>>>>> > I still don't understand why you don't deoptimize the objects inside
>> the
>>>>> > handshake/safepoint instead?
>>>> So for handshakes using asynch handshake and allowing blocking inside
>>>> would fix that. (future fix, I'm working on that now)
>>> Just to make it clear: I'm not fond of the extra suspension mechanism
>>> currently used for JDK-8227745 either. I want to get rid of it and I
>>> will work on it. Asynch handshakes (JDK-8238761) could be a
>>> replacement for it. At least I think they can be used to suspend the target
>> thread.
>>>
>>>> For safepoint, since we have suspended all threads, ~'safepointed them'
>>>> with a JavaThread, you _could_ just execute the action directly (e.g.
>>>> skipping VM_HeapWalkOperation safepoint) since they are suppose to be
>>>> safely suspended until the destructor of EB, no?
>>> Yes, this should be possible. This would be an advanced change though.
>>> I would like EscapeBarriers to be a no-op and fall back to current
>>> implementation, if C2-EscapeAnalysis/Graal are disabled.
>>>
>>>> So I suggest future work to instead just execute the safepoint with
>>>> the requesting JT instead of having a this special safepoiting mechanism.
>>>> Since you are missing above functionality I see why you went this way.
>>>> If you need to push it, it's fine by me.
>>> We will work on further improvements. Top of the list would be
>>> eliminating the extra suspend mechanism.
>>>
>>> The implementation has matured for more than 12 months now [1]. It's
>>> been tested extensively at SAP over that time and passed also extended
>>> testing at Oracle kindly conducted by Vladimir Kozlov. We've got two
>>> full Reviews and incorporated extensive feedback from a number of
>>> OpenJDK Reviewers (including you, thanks!). Based on that I reckon
>>> we're good to push the change as enhancement
>>> (JDK-8227745) and bug fix (JDK-8233915).
>>>
>>>> Thanks for explaining once again :)
>>> Pleasure :)
>>>
>>> Thanks, Richard.
>>>
>>> [1]
>>> http://mail.openjdk.java.net/pipermail/serviceability-dev/2019-July/02
>>> 8729.html
>>>
>>> -----Original Message-----
>>> From: Robbin Ehn <robbin.ehn at oracle.com>
>>> Sent: Mittwoch, 2. September 2020 16:54
>>> To: Reingruber, Richard <richard.reingruber at sap.com>;
>>> serviceability-dev <serviceability-dev at openjdk.java.net>;
>>> hotspot-compiler-dev at openjdk.java.net; Hotspot dev runtime
>>> <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 2020-09-02 15:48, Reingruber, Richard wrote:
>>>> Hi Robbin,
>>>>
>>>> // taking the discussion back to the mailing lists
>>>>
>>>> > I still don't understand why you don't deoptimize the objects inside
>> the
>>>> > handshake/safepoint instead?
>>> So for handshakes using asynch handshake and allowing blocking inside
>>> would fix that. (future fix, I'm working on that now)
>>>
>>> For safepoint, since we have suspended all threads, ~'safepointed them'
>>> with a JavaThread, you _could_ just execute the action directly (e.g.
>>> skipping VM_HeapWalkOperation safepoint) since they are suppose to be
>>> safely suspended until the destructor of EB, no?
>>>
>>> So I suggest future work to instead just execute the safepoint with
>>> the requesting JT instead of having a this special safepoiting mechanism.
>>>
>>> Since you are missing above functionality I see why you went this way.
>>> If you need to push it, it's fine by me.
>>>
>>> Thanks for explaining once again :)
>>>
>>> /Robbin
>>>
>>>> This is unfortunately not possible. Deoptimizing objects includes
>>>> reallocating scalar replaced objects, i.e. calling
>>>> Deoptimization::realloc_objects(). This cannot be done at a safepoint or
>> handshake.
>>>>
>>>> 1. The vm thread is not allowed to allocate on the java heap
>>>> See for instance assertions in ParallelScavengeHeap::mem_allocate()
>>>>
>>>>
>> https://urldefense.com/v3/__https://github.com/openjdk/jdk/blob/4c73e
>>>>
>> 045ce815d52abcdc99499266ccf2e6e9b4c/src/hotspot/share/gc/parallel/par
>>>>
>> allelScavengeHeap.cpp*L258__;Iw!!GqivPVa7Brio!K0f5chjtePI6MKBSBOoBKy
>> a
>>>> 9YZTJlVhsExQYMDO96v3Af_Klc_E4R26_dSyowotF$
>>>>
>>>> This is not easy to change, I suppose, because it will be difficult to gc if
>>>> necessary.
>>>>
>>>> 2. Using a direct handshake would not work either. The problem there is
>> again
>>>> gc. Let J be the JavaThread that is executing the direct handshake. The
>> vm
>>>> would deadlock if the vm thread waits for J to execute the closure of a
>>>> handshake-all and J waits for the vm thread to execute a gc vm
>> operation.
>>>> Patricio Chilano made me aware of this:
>>>> https://bugs.openjdk.java.net/browse/JDK-8230594
>>>>
>>>> Cheers, Richard.
>>>>
>>>> -----Original Message-----
>>>> From: Robbin Ehn <robbin.ehn at oracle.com>
>>>> Sent: Mittwoch, 2. September 2020 13:56
>>>> To: Reingruber, Richard <richard.reingruber at sap.com>
>>>> Cc: Lindenmaier, Goetz <goetz.lindenmaier at sap.com>; Vladimir Kozlov
>>>> <vladimir.kozlov at oracle.com>; David Holmes
>> <david.holmes at oracle.com>
>>>> Subject: Re: RFR(L) 8227745: Enable Escape Analysis for Better
>>>> Performance in the Presence of JVMTI Agents
>>>>
>>>> Hi,
>>>>
>>>> I still don't understand why you don't deoptimize the objects inside
>>>> the handshake/safepoint instead?
>>>>
>>>> E.g.
>>>>
>>>> JvmtiEnv::GetOwnedMonitorInfo you only should need the execute the
>>>> code
>>>> from:
>>>> eb.deoptimize_objects(MaxJavaStackTraceDepth)) before looping over
>>>> the stack, so:
>>>>
>>>> void
>>>> GetOwnedMonitorInfoClosure::do_thread(Thread *target) {
>>>> assert(target->is_Java_thread(), "just checking");
>>>> JavaThread *jt = (JavaThread *)target;
>>>>
>>>> if (!jt->is_exiting() && (jt->threadObj() != NULL)) {
>>>> + if (EscapeBarrier::deoptimize_objects(jt,
>>>> + MaxJavaStackTraceDepth)) {
>>>> _result =
>>>> ((JvmtiEnvBase*)_env)->get_owned_monitors(_calling_thread, jt,
>>>> _owned_monitors_list);
>>>> } else {
>>>> _result = JVMTI_ERROR_OUT_OF_MEMORY;
>>>> }
>>>> }
>>>> }
>>>>
>>>> Why try 'suspend' the thread first?
>>>>
>>>>
>>>> When we de-optimize all threads why not just in the following safepoint?
>>>> E.g.
>>>> VM_HeapWalkOperation::doit() {
>>>> + EscapeBarrier::deoptimize_objects_all_threads();
>>>> ...
>>>> }
>>>>
>>>> Thanks, Robbin
>>>>
>>>>
>>
More information about the hotspot-runtime-dev
mailing list