RFR: 8229258: Rework markOop and markOopDesc into a simpler mark word value carrier

Kim Barrett kim.barrett at oracle.com
Fri Aug 16 18:33:54 UTC 2019


> On Aug 16, 2019, at 4:20 AM, Stefan Karlsson <stefan.karlsson at oracle.com> wrote:
> 
> On 2019-08-16 00:59, Kim Barrett wrote:
>> src/hotspot/share/oops/markOop.hpp
>>  109   template<typename T> operator T();
>> My mistake in the earlier review comment.  Function should be const
>> qualified, e.g. that should be
>>   template<typename T> operator T() const;
> 
> I added this after one of our earlier discussions. However, I don't think we need it (const or not). We already get sensible compiler errors without this function when we try to cast markWords to something else:
> 
>  void* p0 = m; 
>  void* p1 = (void*)m; 
>  int   i0 = m; 
>  int   i1 = (int)m;
> 
> [… various errors …]

You’re right.  It seems I need to refresh my recollection of what the valid conversions are.

> The poisoned constructor seems to be unnecessary as well, now that we have simplified markWord. Without it, I get appropriate error messages when I try to create a markWord from a pointer:
> 
> error: invalid conversion from ‘void*’ to ‘uintptr_t’ {aka ‘long unsigned int’} [-fpermissive] 
>   markWord p((void*)0x111); 
>              ^~~~~~~~~~~~ 
> note:   initializing argument 1 of ‘markWord::markWord(uintptr_t)’ 
>   explicit markWord(uintptr_t value) : _value(value) { }

I no longer recall why that one was even suggested.  But you are right that it isn’t needed.

> I've removed both of these.

Good.

> […]
> 
> I'll leave the rest of the comments below for follow-up RFEs.

OK.

> This is the last few cleanups:
> http://cr.openjdk.java.net/~stefank/8229258/webrev.valueMarkWord.04.delta/
> http://cr.openjdk.java.net/~stefank/8229258/webrev.valueMarkWord.04/

Looks good.

> I ran extended testing on .03 (tier1-7 on linux), checked the markWord functions were inlined, and checked that the generated code for G1ParScanThreadState::copy_to_survivor_space was the same before and after the patch. So I intend to run tier1 testing on .04 and then push this patch.
> 

Thanks for checking the generated code.  It’s what we expected, but compilers are sometimes surprising.



More information about the serviceability-dev mailing list