RFR: 8214363: HeapWord should not be a fake class

Mikael Vidstedt mikael.vidstedt at oracle.com
Fri Feb 15 21:36:29 UTC 2019


I wouldn’t be against that. :)

Cheers,
Mikael

> On Feb 15, 2019, at 1:34 PM, Kim Barrett <kim.barrett at oracle.com> wrote:
> 
>> 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