RFR: JDK-8222545: Safe klass asserts

[Roman Kennke]

Am 18.04.19 um 16:34 schrieb Stefan Karlsson:
> On 2019-04-18 15:13, Roman Kennke wrote:
>>>>>> To add a little more detail, I could move the change up into 
>>>>>> is_objArray(), but I don't want to expose it to any non-assert 
>>>>>> paths. Therefore I could do 2 different impls there, guarded by 
>>>>>> #ifdef ASSERT but I don't think it's a good idea to behave 
>>>>>> differently under ASSERT, that kindof defeats the point of assert, 
>>>>>> right?
>>>>>>
>>>>>> What do you think ?
>>>>>
>>>>> I don't follow your argument. Under asserts you need to access the 
>>>>> klass pointer "safely" but otherwise you do not. So there are two 
>>>>> behaviours related to accessing the klass pointer anyway. I'd 
>>>>> rather see that encapsulated in the accessor.
>>>>>
>>>>> I assume it's not just asserts but any debug only code that wants 
>>>>> to access the klass pointer.
>>>>
>>>> In general, for any runtime calls into oopDesc::klass() the access 
>>>> should be safe. The acrobatics is only necessary for *GC-internal* 
>>>
>>> This is the part I don't quite understand, and goes back to my 
>>> initial question. Why are you doing these operations on from-space 
>>> objects? I'm thinking you should be in a position in the GC to make 
>>> sure this can never happen. If you need to do that in the GC (which 
>>> is fine), then the GC could apply a "resolve" function to get the 
>>> to-space object, and call size() (or whatever) on that object. This 
>>> shouldn't have to leak out of the GC, right?
>>
>> It is a problem when we are about to evacuate an object. Then we need 
>> to know its size in order to allocate and copy an appropriate chunk. 
>> The problem is that this part is racy: two threads (e.g. two Java 
>> threads via barrier, or one Java thread vs one GC thread) might 
>> compete over this: both would create a copy of the object, but 
>> ultimately only one would succeed (by CASing the fwd pointer). 
>> Therefore, getting hold of the object size is racy, by design, and 
>> this requires to resolve the _klass. Now, we can do that ahead of 
>> time, and call oopDesc::size_given_klass() and all would be good, 
>> except that size_given_klass() asserts that the object is indeed of 
>> the given klass, and hence fetches _klass again, which, at this point, 
>> is racy. Solving this inside the GC would require to basically copy 
>> all the machinery to get hold of object size into the GC. Are you 
>> asking me to do that?
> 
> Other GCs store forwarding pointers in the mark word. See 
> oopDesc::forward_to and friends. Could you do the same and get rid of 
> this problem?
> 

No. Other GCs store the fwd pointer there, but only during a pause, and 
while possibly stashing the mark word somewhere else in the meantime.

We need to do it outside of GC pauses, plus we need a way (bit) to 
indicate what it actually is (fwd pointer or Klass*). The mark word is 
already badly overloaded and also accessed much more often and in 
critical paths (e.g. locking), while the Klass* is basically immutable, 
and has the lowest 3 bits free (when running with 
-UseCompressedClassPointers, which would be enforced by Shenandoah). 
Using the Klass* slot is therefore the simplest and most efficient place 
to keep the fwd pointer. Attempting to use the mark word would require 
much more barriers and cause more overhead to manage it.

Roman