RFR 8229844: Remove attempt_rebias parameter from revoke_and_rebias()

[David Holmes]

Hi Patricio,

On 23/08/2019 5:27 am, Patricio Chilano wrote:
> Hi all,
> 
> Please have a look at the following patch.
> 
> JBS: https://bugs.openjdk.java.net/browse/JDK-8229844
> Webrev: http://cr.openjdk.java.net/~pchilanomate/8229844/v01/webrev/
> 
> The attempt_rebias parameter is only used by 
> ObjectSynchronizer::fast_enter() after we failed to acquire the lock in 
> interpreter/compiler code. But even in that case the rebiasing will only 
> work for the bulk rebiasing case, i.e. after a safepoint occurs, so not 
> only this is not the common case but also there is nothing really fast 
> about it. We can remove it without any real performance penalty and 
> simplify the code. Also this allows to merge the fast_enter() and 
> slow_enter() into a common enter() and remove biased locking knowledge 
> in other parts of the code. Tested with tiers1-6 on Linux, Windows, OSX 
> and Solaris.

I really like the simplification and removing the biased locking 
knowledge from external sites!

I have one concern. We have this comment:

// The interpreter and compiler use  assembly copies of these routines.
// Please keep them synchronized.

and you've made changes to these routines but not to anything in the 
interpreter or compiler. So were they already out of sync or ??

src/hotspot/share/c1/c1_Runtime1.cpp

708   assert(obj == lock->obj(), "must match");

It isn't at all obvious to me that this assert, which was previously 
only applied to !UseBiasedLocking&&UseFastLocking is now always valid. 
In particular I'd find it suspect is UseFastLocking** is disabled.

** UseFastLocking must surely be a candidate for removal! :)

---

test/hotspot/gtest/oops/test_markOop.cpp

Not sure the change here really makes sense. Previously the test was 
testing the actions of fast_enter but now its just checking its own 
previous setup. ??

Thanks,
David
-----

> 
> Thanks!
> Patricio