RFR: JDK-8222545: Safe klass asserts
Per Liden
per.liden at oracle.com
Thu Apr 18 16:47:30 UTC 2019
On 04/18/2019 06:21 PM, Roman Kennke wrote:
>
>
> Am 18.04.19 um 17:34 schrieb Stefan Karlsson:
>> On 2019-04-18 17:29, Roman Kennke wrote:
>>>
>>>
>>> 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.
>>
>> Are you sure? Remember, the object is in from-space and no thread is
>> allowed to change it, except the threads that are copying out of the
>> from-space.
>
> Right. But we still need one bit in it to differentiate between fwd ptr
> and regular mark word. And installing the fwd pointer concurrently with
We already have such bits, and oopDesc::forward_to_atomic() will set
them for you. Aren't they enough?
> locking seems more of a horror story than dealing with an otherwise
> immutable Klass*.
With a to-space invariant, I can't see how this can happen concurrently
with locking.
cheers,
Per
>
> In any case, I guess we could do without any GC interface changes by
> dropping the asserts in size_given_klass() if you think that is
> reasonable, also avoid the extra asserts in klass() and friends (even
> though I would prefer to have some way to assert sanity there...), plus
> the changes for JDK-8222537 which are arguably an improvement in any case.
>
> Roman
More information about the hotspot-runtime-dev
mailing list