RFR[L]: 8237767 Field layout computation overhaul

[coleen.phillimore at oracle.com]

On 1/29/20 8:33 AM, David Holmes wrote:
> 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.

We were bitten by this one:
   bool            _is_marked_dependent;  // used for marking during 
flushing and deoptimization

We think, but my memory is bad.  If we want to make some access to the 
misc flags thread safe, we can use the same mechanism as accessFlags.   
This seems unnecessary at this time.

Coleen
>
> 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
>>
>>
>>