[9] RFR(S): 8005873: JRuby test_respond_to.rb asserts with: MT-unsafe modification of inline cache

[Vladimir Ivanov]

>>> Or perhaps use Unsafe.CAS directly within setCachedLambdaForm?
>> Yes, Unsafe is another option here. But since cache updates should be
>> rare, Unsafe is an overkill here IMO - locking should be fine.
>
> I don't get it, AtomicReferenceArray.CAS implementation uses Unsafe.CAS
By overkill I meant code complexity - it increases for no benefit.

If we use Unsafe directly, we need to duplicate offset calculation logic 
from AtomicReferenceArray.

AtomicReferenceArray doesn't fit, because to declare an array as @Stable 
we need to work with it directly, but AtomicReferenceArray manages it's 
private copy we don't have access to.

Best regards,
Vladimir Ivanov

>
>>
>>>
>>> Also, as a consequence of using AtomicReferenceArray the following
>>> change may result in a memory barrier on some architectures:
>>>
>>>       public LambdaForm cachedLambdaForm(int which) {
>>> -        return lambdaForms[which];
>>>
>>> +        return lambdaForms.get(which);
>>>       }
>>>
>>> since lambdaForms.get will call Unsafe.getObjectVolatile.
>> MTF.cachedLambaForm isn't on a fast path, so it shouldn't be a problem.
>>
>>> Separately, i think code that calls setCachedLambdaForm needs to be
>>> double checked to ensure that the return value is used. For example,
>>> in MethodHandleImpl.makeGuardWithCatchForm i see:
>>>
>>> basicType.form().setCachedLambdaForm(MethodTypeForm.LF_GWC, lform);
>>>          return lform;
>>>
>>> which i think needs to be:
>>>
>>>          return
>>> basicType.form().setCachedLambdaForm(MethodTypeForm.LF_GWC, lform);
>> Good catch! MTF.setCachedLambdaForm usages should be fixed as well.
>
> IMO, forms can be a classical Java array,
>
> *   final @Stable_LambdaForm[]_  lambdaForms;*
>
>
> setCachedLambdaForm can use Unsafe.compareAndSwapObject
>
> *   // Make sure cache entry is set only once*
> *   if (UNSAFE.compareAndSwap(***lambdaForms*, which, null, form)) {*
> *       return form;*
> *   }
>    **return cachedLambdaForm(which);*
>
>
> and cachedLambdaForm can try to use the stable value,
> if the stable value is not null, it's ok, otherwise we can use
> getVolatileObject
>
>    public LambdaForm cachedLambdaForm(int which) {
>      LambdaForm form = lambdaForms[which];
>      if (form != null) {
>        return form;
>      }
>      return UNSAFE.getObjectVolatile(lambdaForms, which);
>    }
>
>
> cheers,
> Rémi
>
>>
>> Best regards,
>> Vladimir Ivanov
>>
>>>
>>> Paul.
>>>
>>>
>>>
>>>
>