RFR(L) 8227745: Enable Escape Analysis for Better Performance in the Presence of JVMTI Agents

Mon Sep 7 14:09:11 UTC 2020

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/028729.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/4c73e045ce815d52abcdc99499266ccf2e6e9b4c/src/hotspot/share/gc/parallel/parallelScavengeHeap.cpp*L258__;Iw!!GqivPVa7Brio!K0f5chjtePI6MKBSBOoBKya9YZTJlVhsExQYMDO96v3Af_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
>   
> 