RFR(S) 8230677: Should disable Escape Analysis if JVMTI capability can_get_owned_monitor_info was taken

Thu Sep 19 16:42:42 UTC 2019

Hi David,

thanks for looking at this issue. And my appologies for the lengthy mail...

  > > The JVMTI functions GetOwnedMonitorInfo() and GetOwnedMonitorStackDepthInfo() can be used to
  > > retrieve objects locked by a thread. In terms of escape analysis those references escape and
  > > optimizations like scalar replacement become invalid.
  > 
  > What bothers me about this is that it seems like escape analysis is 
  > doing something wrong in this case.

Yes it is.

  > If the object is thread-local but is 
  > being synchronized upon then either:

The object is not local, because it can escape through JVMTI GetOwnedMonitorInfo(). Escape analysis
does not recognize this. That's what it is doing wrong. Consequently the state of the virtual
machine, as observed through JVMTI, is wrong. See below...

  > a) the synchronization is elided and so the object will not appear in 
  > the set of owned monitors; or
  > b) the fact synchronization occurs renders the object ineligible to be 
  > considered thread-local, and so there is no problem with it appearing in 
  > the set of owned monitors
  > 
  > I think there is a bigger design philosophy issue here about the 
  > interaction of escape analysis and debugging/management frameworks in 
  > general. I'd like to see a very clear description on exactly how they 
  > should interact.
  > 

I don't see too many design alternatives here. The JVMTI implementation has to present the correct
state of the virtual machine according to the spec. I think it fails to do so in this case.

Please look again at the test:

 172         public long dontinline_endlessLoop() {
 173             long cs = checkSum;
 174             while (doLoop && loopCount-- > 0) {
 175                 targetIsInLoop = true;
 176                 checkSum += checkSum % ++cs;
 177             }
 178             loopCount = 3;
 179             targetIsInLoop = false;
 180             return checkSum;
 181         }

 249         public void dontinline_testMethod() {
 250             LockCls l1 = new LockCls();        // to be scalar replaced
 251             synchronized (l1) {
 252                 inlinedTestMethodWithNestedLocking(l1);
 253             }
 254         }
 255 
 256         public void inlinedTestMethodWithNestedLocking(LockCls l1) {
 257             synchronized (l1) {              // nested
 258                 dontinline_endlessLoop();
 259             }
 260         }

This is the stack when the agent calls GetOwnedMonitorInfo()

dontinline_endlessLoop()                    at line 176
inlinedTestMethodWithNestedLocking()        at line 258  // inlined into caller frame
dontinline_testMethod()                     at line 252  // compiled frame

The state of the _virtual_ machine at that point is obvious:

- An instance of LockCls must exist. It was allocated by a new bytecode at line 250.
- That instance was locked by a monitorenter bytecode at line 251

This could be proven by interpreting the execution trace bytecode by bytecode using paper and
pencil (hope you won't make me do this, though ;))

JVMTI is used to examine the state of the virtual machine. The result of the JVMTI call
GetOwnedMonitorInfo() must include that locked instance of LockCls. It is obviously a bug if it does
not.

From a more philosophical point of view compiled code is free to change the state of the physical
machine in a way such that it cannot be mapped to a valid state of the virtual machine after each
and every machine instruction. But it must reach points in its execution trace, where it is actually
possible to present a valid state of the virtual machine to observers, e.g. JVMTI agents. These
points are called safepoints.

The test is a prove that compiled code fails to do so, as it reaches a safepoint where an invalid vm
state is presented. EA does not take into account that the lock state can be observed using
GetOwnedMonitorInfo(). As a fix EA is disabled if the corresponding capability
can_get_owned_monitor_info is taken. With the fix the test passes.

Note that for the very same reason EA is disabled if can_access_local_variables is taken, because
the JVMTI implementation cannot handout references to objects stored in local variables if they were
scalar replaced.

With the proposed enhancement JDK-8227745 it is not necessary to disable EA. It allows to revert EA
based optimizations just-in-time before local objects escape. Note that EA opts are already reverted
today if a compiled frame gets replaced by corresponding interpreted frames (see realloc_objects()
and relock_objects() in class Deoptimization)

Thanks and cheers, Richard.

[1] https://bugs.openjdk.java.net/browse/JDK-8227745

-----Original Message-----
From: David Holmes <david.holmes at oracle.com> 
Sent: Donnerstag, 19. September 2019 02:43
To: Reingruber, Richard <richard.reingruber at sap.com>; hotspot-compiler-dev at openjdk.java.net; serviceability-dev at openjdk.java.net
Subject: Re: RFR(S) 8230677: Should disable Escape Analysis if JVMTI capability can_get_owned_monitor_info was taken

Hi Richard,

On 7/09/2019 12:24 am, Reingruber, Richard wrote:
> Hi,
> 
> could I please get reviews for
> 
> Webrev: http://cr.openjdk.java.net/~rrich/webrevs/2019/8230677/webrev.0/
> Bug:    https://bugs.openjdk.java.net/browse/JDK-8230677
> 
> The JVMTI functions GetOwnedMonitorInfo() and GetOwnedMonitorStackDepthInfo() can be used to
> retrieve objects locked by a thread. In terms of escape analysis those references escape and
> optimizations like scalar replacement become invalid.

What bothers me about this is that it seems like escape analysis is 
doing something wrong in this case. If the object is thread-local but is 
being synchronized upon then either:
a) the synchronization is elided and so the object will not appear in 
the set of owned monitors; or
b) the fact synchronization occurs renders the object ineligible to be 
considered thread-local, and so there is no problem with it appearing in 
the set of owned monitors

I think there is a bigger design philosophy issue here about the 
interaction of escape analysis and debugging/management frameworks in 
general. I'd like to see a very clear description on exactly how they 
should interact.

Cheers,
David

> The runtime currently cannot cope with objects escaping through JVMTI (try included
> tests). Therefore escape analysis should be disabled if an agent requests the capabilities
> can_get_owned_monitor_info or can_get_owned_monitor_stack_depth_info.
> 
> This was taken out of JDK-8227745 [1] to make it smaller. With JDK-8227745 there's no need to
> disable escape analysis, instead optimizations based on escape analysis will be reverted just before
> objects escape through JVMTI.
> 
> I've run tier1 tests.
> 
> Thanks, Richard.
> 
> [1] https://bugs.openjdk.java.net/browse/JDK-8227745
> 