RFR(L) 8227745: Enable Escape Analysis for Better Performance in the Presence of JVMTI Agents
serguei.spitsyn at oracle.com
serguei.spitsyn at oracle.com
Tue Sep 15 20:28:50 UTC 2020
Hi Richard,
This is on my review list. I'll try to get it reviewed by the end of
this week.
Thanks,
Serguei
On 9/8/20 10:02, 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