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

[Reingruber, Richard]

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.holmes at oracle.com> 
Sent: Mittwoch, 11. Dezember 2019 08:03
To: Reingruber, Richard <richard.reingruber at sap.com>; serviceability-dev at openjdk.java.net; hotspot-compiler-dev at openjdk.java.net; 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 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
>