On 5/14/14 4:12 PM, Paul Sandoz wrote:
On May 14, 2014, at 12:47 PM, Vladimir Ivanov <vladimir.x.iva...@oracle.com>
wrote:
Tobias, I agree with your evaluation.
V. tricky one to track down!
From @Stable:
* It is (currently) undefined what happens if a field annotated as stable
* is given a third value. In practice, if the JVM relies on this annotation
* to promote a field reference to a constant, it may be that the Java memory
* model would appear to be broken, if such a constant (the second value of
the field)
* is used as the value of the field even after the field value has changed.
I dunno if that was a contributing factor in this case.
No, @Stable doesn't contribute to the problem.
My only concern is that @Stable doesn't work for AtomicReferenceArray, so JIT
doesn't see what is stored in the array.
Yes, stability needs to be associated with the array elements.
Maybe use a lock instead?
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.
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.
Best regards,
Vladimir Ivanov
Paul.
