JDK-8255522: was Re: [lworld] RFR: 8255396: [lworld] locking breaks oopDesc is_flatArray/is_nullfreeArray type checks

[David Simms]

Filed a new issue for further testing and cleanup JDK-8255522, ...

On 2020-10-26 17:53, Sergey Kuksenko wrote:
> I have a question about the following methods from markWord.hpp:
>
> bool is_locked()const {
>   return (mask_bits(value(),lock_mask_in_place) !=unlocked_value);
> }
> bool is_unlocked()const {
>   return (mask_bits(value(),biased_lock_mask_in_place) ==unlocked_value);
> }
>
You are correct, remains from the original code, due to a lack of 
testing quite frankly. Filed 8255522  to:

* add more gtest mark word tests

* assert any use of "biased locking" (there are some existing 
"has_biased_pattern()" asserts not guarded "UseBiasedLocking")

* remove biased_lock_mask_in_place from is_unlocked


> First of all these methods are not symmetrical. 
> "lock_mask_in_place"==0x011 when "biased_lock_mask_in_place"==0x111.
>
> The second. If we invoke is_unlocked() on inline/primitive object we 
> get "false". is_locked() on inline also gives "false".
> I am not sure if may cause an error, maybe all such checks are 
> preguarded with is_inline_type().

Not much code uses the runtime check, and those that do would have 
fallen back to klass->layout_helper()

That said, I will spend some time looking into further testing and 
asserts that could have found that.

>
> The third. Valhalla can't work with BiasedLocking. I think it would be 
> better to remove all usage of BiasedLocking constant in Valhalla to 
> avoid errors. As far as I understand, BiasedLocking has became 
> "obsolete" in jdk16.
> That means:
> // When the JDK version reaches 'obsolete_in' limit, the JVM will 
> continue accepting this flag on // the command-line, while issuing a 
> warning and ignoring the flag value.
>
> So, now BasedLocking can't be turn on in mainline jdk. To simplify 
> merging, maybe make sense to remove biased locking constants and masks 
> in mainline jdk.


So "markWord.hpp" changes very little, and when it does, we want to know 
about it. Therefore I was more than happy to make potentially 
conflicting code changes. The rest of the code base, I was not so 
interested in having unnecessary merge conflicts with a future 
removal...so the rest of the code is left in situ. But yes 
UseBiasedLocking will exit VM init in Valhalla. The addition of 
ShouldNotReadHere() in markWord.hpp biased locking code should do the 
trick for now ?

Cheers

/Simms