RFR: 8214363: HeapWord should not be a fake class

Kim Barrett kim.barrett at oracle.com
Fri Feb 15 21:34:35 UTC 2019


> On Feb 15, 2019, at 4:13 PM, Mikael Vidstedt <mikael.vidstedt at oracle.com> wrote:
> 
> 
> 
>> On Feb 15, 2019, at 12:44 PM, Kim Barrett <kim.barrett at oracle.com> wrote:
>> 
>>> On Feb 15, 2019, at 3:27 PM, coleen.phillimore at oracle.com wrote:
>>> 
>>> 
>>> moving to hotspot-dev.
>>> Coleen
>>> 
>>> On 2/15/19 8:07 AM, coleen.phillimore at oracle.com wrote:
>>>> 
>>>> This is nice but what does this mean?
>>>> 
>>>> http://cr.openjdk.java.net/~kbarrett/8214363/open.02/src/hotspot/share/gc/shared/spaceDecorator.cpp.udiff.html 
>>>> 
>>>> + return badHeapWord == (max_juint & reinterpret_cast<uintptr_t>(*q));
>>>> 
>>>> 
>>>> I thought since it has no implementation class, you can't use *q ?
>> 
>> The type of q is HeapWord*, which is (with this change) HeapWordImpl**.
>> Dereferencing q => HeapWordImpl*, which is then cast to a uintptr_t.
>> 
>> A pretty similar effect could have been achieved by changing HeapWord to
>> struct HeapWord { HeapWordImpl* value; };
>> e.g. making the member public (to eliminate the new gcc warning) and changing
>> the pointee type of the member to an incomplete type (to prevent dereference).
> 
> I was also confused by that. Not enough to have a strong opinion though.
> 
> Looks good to me. For completeness I too tried your change with gcc 8.2 and the warnings are indeed gone.
> 
> Cheers,
> Mikael

I thought about adding “uintptr_t heap_word_value(HeapWord*)”, but it didn’t seem worth
the trouble for a single caller.  If that would have helped…



More information about the hotspot-dev mailing list