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

[Robbin Ehn]

Hi Richard,

On 2019-12-16 14:41, Reingruber, Richard wrote:
> Hi Robbin,
> 
> first of all: thanks a lot for providing feedback. I do appreciate it.
> 
> I am absolutely willing to move this to handshakes. Only I still can't see how to achieve it.
> 
> Could you explain the drafted class EscapeBarrierSuspendHandshake a little bit? [1]
> 
> I'd like to look at it by example of JvmtiEnv::GetOwnedMonitorStackDepthInfo() where calling_thread
> T1 would apply it on another thread T2.

Sorry I don't immediately see what issue there is in doing a handshake 
instead of:
VM_GetOwnedMonitorInfo op(this, calling_thread, java_thread, 
owned_monitors_list);

> 
> 1. L13: is wait_until_eb_stopped to be called by T1 to wait until T2 cannot move anymore?
> 
> 2. Handshakes between two threads are synchronous, correct? If so, then T1 will block handshaking
>     T2, because either T2 or the VMThread will block in L10.

Yes, sorry, I forgot/confused myself about asynch handshake.
(I have a test prototype for that, which removes suspend flag)

> 
> I cannot figure out, how you mean this. Only if a helper thread H would handshake T2 then T1 could
> continue and call wait_until_eb_stopped(). But returning from there T1 would block if reallocating
> objects triggers GC or attempting to execute the vm operation in
> JvmtiEnv::GetOwnedMonitorStackDepthInfo().
> 
> It might be impossible to replace my suspend flag with handshakes that are available today, because
> if it was you could replace all the suspend flags right away, couldn't you?

So adding asynch handshakes and a per thread handshake queue, we can. 
(which this test prototype does)
The issue I'm thinking of is if we need selective polling first.
Suspend flags are not checked in every transition, e.g. vm->native.
A JVM TI agent don't expect to suspend it's own thread when suspending
all threads.
(that thread would be suspended when trying to get back to agent code
when it does vm->native transition)

> 
> Or I'm simply missing something... quite possible... :)

No I think you got it right.

Thanks, Robbin

> 
> Thanks, Richard.
> 
> [1] Drafted by Robbin (thanks!)
> 
>       1	class EscapeBarrierSuspendHandshake : public HandshakeClosure {
>       2	  Semaphore _is_waiting;
>       3	  Semaphore _wait;
>       4	  bool _started;
>       5	public:
>       6	  EscapeBarrierSuspendHandshake() : HandshakeClosure("EscapeBarrierSuspend"),
>       7	  _wait(0), _started(false) { }
>       8	  void do_thread(Thread* th) {
>       9	    _is_waiting.signal();
>      10	    _wait.wait();
>      11	    Atomic::store(&_started, true);
>      12	  }
>      13	  void wait_until_eb_stopped() { _is_waiting.wait(); }
>      14	  void start_thread() {
>      15	    _wait.signal();
>      16	    while(!Atomic::load(&_started)) {
>      17	      os::naked_yield();
>      18	    }
>      19	  }
>      20	};
> 
> -----Original Message-----
> From: Robbin Ehn <robbin.ehn at oracle.com>
> Sent: Montag, 16. Dezember 2019 11:21
> 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, as mentioned it would be better if you could do this with
> handshakes, instead of using _suspend_flag (since they are going away).
> But I can't think of a way doing it without blocking safepoints, so we need to
> add some more features in handshakes first.
> When possible I hope you are willing to move this code to handshakes instead.
> 
> You could stop one thread with, e.g.:
> class EscapeBarrierSuspendHandshake : public HandshakeClosure {
>     Semaphore _is_waiting;
>     Semaphore _wait;
>     bool _started;
>    public:
>     EscapeBarrierSuspendHandshake() : HandshakeClosure("EscapeBarrierSuspend"),
> _wait(0), _started(false) { }
>     void do_thread(Thread* th) {
>       _is_waiting.signal();
>       _wait.wait();
>       Atomic::store(&_started, true);
>     }
>     void wait_until_eb_stopped() { _is_waiting.wait(); }
>     void start_thread() {
>       _wait.signal();
>       while(!Atomic::load(&_started)) {
>         os::naked_yield();
>       }
>     }
> };
> 
> But it would block safepoints.
> 
> Thanks, Robbin
> 
> On 12/10/19 10:45 PM, 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.
>>
>> Thanks,
>> Richard.
>>
>> [1] Experimental fix for JDK-8214584 based on JDK-8227745
>>       http://cr.openjdk.java.net/~rrich/webrevs/2019/8214584/experiment_v1.patch
>>