RFR: 8229258: Make markOopDesc AllStatic

[Roman Kennke]

First glance.

Instead of making it AllStatic, I'd probably have made it a plain object:

class markWord {
}

and instead of changing e.g.:

obj->mark().set_unlocked();

 to

MarkWord::set_unlocked(obj->mark());

leave it like it is. Looks/feels better to me.

Have you tried that? Is this too tricky to do? Or what is the reason to
use static accessors there?

Roman

> Hi all,
> 
> Please review this patch to make markOopDesc AllStatic.
> 
> https://cr.openjdk.java.net/~stefank/8229258/webrev.01/
> https://bugs.openjdk.java.net/browse/JDK-8229258
> 
> From the RFE:
> 
> I want to change/rename markOop and markOopDesc
> 
> Today markOopDescs inherits from oopDesc even though they are not
> oopDescs (Java objects):
>  `class markOopDesc : public oopDesc {`
> 
> This is very confusing to new-comers (and probably to old-timers as well).
> 
> A simple fix would be to break the inheritance and do the following rename:
> markOopDesc -> MarkWord
> markOop -> markWord
> 
> However, there are more dubious code in this area. markOopDescs are
> created and used like this:
> 
> ```
> class oopDesc {
> ...
>   volatile markOop _mark;
> ...
> markOop  oopDesc::mark_raw()  const { return _mark; }
> ...
> // Usage
> obj->mark_raw()->is_marked()
> ...
> // Implementation
>   bool is_marked()   const {
>     return (mask_bits(value(), lock_mask_in_place) == marked_value);
>   }
> ...
> // Implementation
>   bool is_being_inflated() const { return (value() == 0); }
> ...
> // Implementation of markOopDesc::value()
>   uintptr_t value() const { return (uintptr_t) this; }
> ```
> 
> Remember, _mark is an arbitrary bit pattern describing the object. We
> treat it as if it were a pointer to a markOopDesc object, but it's not
> pointing to such an object. While using that (weird) pointer we call
> value() and extract it's bit pattern to be used for further bit pattern
> checking functions. AFAICT, this is also undefined behavior. At least
> is_being_inflated() const is effectively 'return (uintptr_t) this ==
> NULL'. This UB was recently discussed here:
>  https://mail.openjdk.java.net/pipermail/hotspot-dev/2019-July/038704.html
>  https://mail.openjdk.java.net/pipermail/hotspot-dev/2019-July/038712.html
> 
> I propose that we change MarkWord (markOopDesc) to be an AllStatic class
> instead and get rid of this (ab)use of the markWord (markOop) pointer.
> 
> The patch is large but mostly straight forward. However, there is some
> Java code that would need some extra scrutiny: SA and JVMCI.
> 
> Tested with tier1-3.
> 
> Coleen has asked for file rename from markOop.hpp to markWord.hpp. I'd
> like to do that as a separate RFE when/if this patch gets accepted.
> 
> Thanks,
> StefanK
>