Yes, David
You are correct these changes touch all part of VM and may affect Graal (which
also has EA) too.
Changes should be tested in all our modes: tiered, C1 only, Graal, Interpreter. And I realized that
I only ran tier3-graal testing so I submitted the rest of Graal's tiers now.
I had assumed that our current testing (I ran all from tier1 to tier8) should exercise all paths in
VM these changes touch. But I may be wrong and it is correct to ask author to add testing in all VM
modes to make sure new code in VM's runtime and JVMTI is tested.
I do like to keep what current test is doing with C2. May be add an other test for other modes or
modify current one to enable to run it in other modes.
Thanks,
Vladimir
On 12/12/19 3:32 PM, David Holmes wrote:
On 13/12/2019 9:02 am, Reingruber, Richard wrote:
Hello Vladimir,
thanks for having a look.
> Use vm.compMode == "Xmixed" instead of vm.compMode != "Xcomp" to skip
> test from running in Interpreter mode too.
Done.
> You don't need vm.opt.TieredCompilation != true in @requires because you
> specified -XX:-TieredCompilation in @run command.
Ok.
> The test is specifically written for C2 only (not for C1 or Graal) to
> verify its Escape Analysis optimization.
> I did not look in great details into test's code but its analysis may be
> affected if C1 compiler is also used.
>
> Richard may clarify this.
The test cases aim to get their testmethod 'dontinline_testMethod' compiled by
C2. If they get C1
compiled before doesn't matter all that much. I've got a slight preference to
disabled tiered
compilation for simplicity.
My concern - perhaps unfounded - is that this seems to be being tested only in a pure C2 environment
when the actual changes will have to operate correctly in a tiered environment (and JVMCI).
Thanks,
David
Thanks, Richard.
-----Original Message-----
From: Vladimir Kozlov <vladimir.koz...@oracle.com>
Sent: Donnerstag, 12. Dezember 2019 19:20
To: David Holmes <david.hol...@oracle.com>; hotspot-runtime-...@openjdk.java.net;
hotspot-compiler-...@openjdk.java.net; serviceability-dev@openjdk.java.net; Reingruber, Richard
<richard.reingru...@sap.com>
Subject: Re: RFR(L) 8227745: Enable Escape Analysis for Better Performance in the Presence of
JVMTI Agents
Hi David,
Tiered is disabled because we don't want to see compilations and outputs
from C1 compiler which does not have EA.
The test is specifically written for C2 only (not for C1 or Graal) to
verify its Escape Analysis optimization.
I did not look in great details into test's code but its analysis may be
affected if C1 compiler is also used.
Richard may clarify this.
thanks,
Vladimir
On 12/11/19 1:04 PM, David Holmes wrote:
On 12/12/2019 5:21 am, Vladimir Kozlov wrote:
I will do full review later. I want to comment about test command line.
You don't need vm.opt.TieredCompilation != true in @requires because
you specified -XX:-TieredCompilation in @run command.
And per my comment this should be being tested with tiered as well.
David
Use vm.compMode == "Xmixed" instead of vm.compMode != "Xcomp" to skip
test from running in Interpreter mode too.
Thanks,
Vladimir
On 12/11/19 7:07 AM, Reingruber, Richard wrote:
Hi David,
> Most of the details here are in areas I can comment on in
detail, but I
> did take an initial general look at things.
Thanks for taking the time!
> The only thing that jumped out at me is that I think the
> DeoptimizeObjectsALotThread should be a hidden thread.
>
> + bool is_hidden_from_external_view() const { return true; }
Yes, it should. Will add the method like above.
> Also I don't see any testing of the DeoptimizeObjectsALotThread.
Without
> active testing this will just bit-rot.
DeoptimizeObjectsALot is meant for stress testing with a larger
workload. I will add a minimal test
to keep it fresh.
> Also on the tests I don't understand your @requires clause:
>
> @requires ((vm.compMode != "Xcomp") & vm.compiler2.enabled &
> (vm.opt.TieredCompilation != true))
>
> This seems to require that TieredCompilation is disabled, but
tiered is
> our normal mode of operation. ??
>
I removed the clause. I guess I wanted to target the tests towards
the code they are supposed to
test, and it's easier to analyze failures w/o tiered compilation and
with just one compiler thread.
Additionally I will make use of
compiler.whitebox.CompilerWhiteBoxTest.THRESHOLD in the tests.
Thanks,
Richard.
-----Original Message-----
From: David Holmes <david.hol...@oracle.com>
Sent: Mittwoch, 11. Dezember 2019 08:03
To: Reingruber, Richard <richard.reingru...@sap.com>;
serviceability-dev@openjdk.java.net;
hotspot-compiler-...@openjdk.java.net;
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 11/12/2019 7:45 am, Reingruber, Richard wrote:
Hi,
I would like to get reviews please for
http://cr.openjdk.java.net/~rrich/webrevs/2019/8227745/webrev.3/
Corresponding RFE:
https://bugs.openjdk.java.net/browse/JDK-8227745
Fixes also https://bugs.openjdk.java.net/browse/JDK-8233915
And potentially https://bugs.openjdk.java.net/browse/JDK-8214584 [1]
Vladimir Kozlov kindly put webrev.3 through tier1-8 testing without
issues (thanks!). In addition the
change is being tested at SAP since I posted the first RFR some
months ago.
The intention of this enhancement is to benefit performance wise
from escape analysis even if JVMTI
agents request capabilities that allow them to access local variable
values. E.g. if you start-up
with -agentlib:jdwp=transport=dt_socket,server=y,suspend=n, then
escape analysis is disabled right
from the beginning, well before a debugger attaches -- if ever one
should do so. With the
enhancement, escape analysis will remain enabled until and after a
debugger attaches. EA based
optimizations are reverted just before an agent acquires the
reference to an object. In the JBS item
you'll find more details.
Most of the details here are in areas I can comment on in detail, but I
did take an initial general look at things.
The only thing that jumped out at me is that I think the
DeoptimizeObjectsALotThread should be a hidden thread.
+ bool is_hidden_from_external_view() const { return true; }
Also I don't see any testing of the DeoptimizeObjectsALotThread. Without
active testing this will just bit-rot.
Also on the tests I don't understand your @requires clause:
@requires ((vm.compMode != "Xcomp") & vm.compiler2.enabled &
(vm.opt.TieredCompilation != true))
This seems to require that TieredCompilation is disabled, but tiered is
our normal mode of operation. ??
Thanks,
David
Thanks,
Richard.
[1] Experimental fix for JDK-8214584 based on JDK-8227745
http://cr.openjdk.java.net/~rrich/webrevs/2019/8214584/experiment_v1.patch
