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/028729.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/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....@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
