RFR[L]: 8237767 Field layout computation overhaul

[David Holmes]

Hi Coleen,

On 29/01/2020 11:37 pm, coleen.phillimore at oracle.com wrote:
> 
> Snippet reply.
> 
> On 1/29/20 2:06 AM, David Holmes wrote:
>>
>>>>
>>>> src/hotspot/share/oops/instanceKlass.hpp
>>>>
>>>> You need to be careful with _extra_flags usage if there can be 
>>>> concurrently updated bits. At the moment it looks like redefinition 
>>>> is a mutable dynamic property, whilst "contended annotations" should 
>>>> be a static immutable property - is that right?
>>>
>>> Correct, _has_contended_annotations is a static immutable property, 
>>> while _is_being_redefined is a mutable one.
>>
>> Good to know. My concern is that if someone adds a new mutable flag 
>> bit the need for atomic updates may not be noticed. We got bitten by 
>> this in the past with a flag field and I think we eventually migrated 
>> all of the mutable bits out into their own field. (Coleen should 
>> recall that :) ).
> 
> A lot of the _misc_flags are mutable.  I think we should notice if 
> someone adds a new flag that requires concurrent access, and not waste 
> space in InstanceKlass for this flag.   The other mutable bool doesn't 
> seem to have any memory synchronization, iirc, which seems wrong.

I have a distinct recollection that we were bitten by this in the past.

Cheers,
David

>>
>>>>   61 FLATTENED,     // flattened field
>>>>
>>>> Does this have any meaning before inline types come in?
>>>
>>> Yes, I wanted to reserved the entry in the enum.
>>
>> Hmmm a tenuous "okay". Seems odd to require this now to support code 
>> that is still some way from joining mainline.
> 
> I didn't think it's a big deal if some small pieces of future code are 
> in this change.
> 
> Coleen
> 
> 
>