-----Original Message-----
From: Reingruber, Richard <richard.reingru...@sap.com>
Sent: Tuesday, September 8, 2020 9:45 AM
To: Daniel Daugherty <daniel.daughe...@oracle.com>; serviceability-dev
<serviceability-dev@openjdk.java.net>; hotspot-compiler-
d...@openjdk.java.net; Hotspot dev runtime <hotspot-runtime-
d...@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.daughe...@oracle.com>
Sent: Dienstag, 8. September 2020 18:16
To: Reingruber, Richard <richard.reingru...@sap.com>; serviceability-dev
<serviceability-dev@openjdk.java.net>; hotspot-compiler-
d...@openjdk.java.net; Hotspot dev runtime <hotspot-runtime-
d...@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-r...@openjdk.java.net> On Behalf Of Reingruber,
Richard
Sent: Mittwoch, 2. September 2020 23:27
To: Robbin Ehn <robbin....@oracle.com>; serviceability-dev
<serviceability-dev@openjdk.java.net>;
hotspot-compiler-...@openjdk.java.net; Hotspot dev runtime
<hotspot-runtime-...@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....@oracle.com>
Sent: Mittwoch, 2. September 2020 16:54
To: Reingruber, Richard <richard.reingru...@sap.com>;
serviceability-dev <serviceability-dev@openjdk.java.net>;
hotspot-compiler-...@openjdk.java.net; Hotspot dev runtime
<hotspot-runtime-...@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....@oracle.com>
Sent: Mittwoch, 2. September 2020 13:56
To: Reingruber, Richard <richard.reingru...@sap.com>
Cc: Lindenmaier, Goetz <goetz.lindenma...@sap.com>; Vladimir Kozlov
<vladimir.koz...@oracle.com>; David Holmes
<david.hol...@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
