RFR(m): 8221734: Deoptimize with handshakes

[Robbin Ehn]

Hi, please see this update.

I think I got all review comments fix.

Long story short, I was concerned about test coverage, so I added a stress test
using the WB, which sometimes crashed in rubbish code.

There are two bugs in the methods used by WB_DeoptimizeAll.
(Seems I'm the first user)

CodeCache::mark_all_nmethods_for_deoptimization();
When iterating the nmethods we could see the methods being create in:
void AdapterHandlerLibrary::create_native_wrapper(const methodHandle& method)
And deopt the method when it was in use or before.
Native wrappers are suppose to live as long as the class.
I filtered out not_installed and native methods.

Deoptimization::deoptimize_all_marked();
The issue is that a not_entrant method can go to zombie at anytime.
There are several ways to make a nmethod not go zombie: nmethodLocker, have it
on stack, avoid safepoint poll in some states, etc.., which is also depending on
what type of nmethod.
The iterator only_alive_and_not_unloading returns not_entrant nmethods, but we
don't know there state prior last poll.
in_use -> not_entrant -> #poll# -> not_entrant -> zombie
If the iterator returns the nmethod after we passed the poll it can still be
not_entrant but go zombie.
The problem happens when a second thread marks a method for deopt and makes it
not_entrant. Then after a poll we end-up in deoptimize_all_marked(), but the
method is not yet a zombie, so the iterator returns it, it becomes a zombie thus
pass the if check and later hit the assert.
So there is a race between the iterator check of state and if-statement check of
state. Fixed by also filtering out zombies.

If the stress test with correction of the bugs causes trouble in review, I can
do a follow-up with the stress test separately.

Good news, no issues found with deopt with handshakes.

This is v3:
http://cr.openjdk.java.net/~rehn/8221734/v3/webrev/

This full inc from v2 (review + stress test):
http://cr.openjdk.java.net/~rehn/8221734/v3/inc/

This inc is the review part from v2:
http://cr.openjdk.java.net/~rehn/8221734/v3/inc_review/

This inc is the additional stress test with bug fixes:
http://cr.openjdk.java.net/~rehn/8221734/v3/inc_test/

Additional biased locking change:
The original code use same copy of markOop in revoke_and_rebias.
The keep same behavior I now pass in that copy into fast_revoke.

The stress test passes hundreds of iterations in mach5.
Thousands stress tests locally, the issues above was reproduce-able.
Inc changes also passes t1-5.

As usual with this change-set, I'm continuously running more test.

Thanks, Robbin

On 2019-04-25 14:05, Robbin Ehn wrote:
> Hi all, please review.
> 
> Let's deopt with handshakes.
> Removed VM op Deoptimize, instead we handshake.
> Locks needs to be inflate since we are not in a safepoint.
> 
> Goes on top of:
> https://mail.openjdk.java.net/pipermail/hotspot-compiler-dev/2019-April/033491.html
> 
> Code:
> http://cr.openjdk.java.net/~rehn/8221734/v1/webrev/index.html
> Issue:
> https://bugs.openjdk.java.net/browse/JDK-8221734
> 
> Passes t1-7 and multiple t1-5 runs.
> 
> A few startup benchmark see a small speedup.
> 
> Thanks, Robbin