On 2019-09-09 12:31, Peter Levart wrote:
Hi Claes,
Your changes look good.
Thanks! Already pushed, though.
But I spotted a pre-existing and unusual use of
@Stable annotation in java.lang.invoke.MethodTypeForm class:
// Cached adapter information:
@Stable final SoftReference<MethodHandle>[] methodHandles;
// Cached lambda form information, for basic types only:
final @Stable SoftReference<LambdaForm>[] lambdaForms;
This declarations are paired with the following caching logic that
returns pre-existing entry if it is already set and not yet cleared or
cache new entry and return it:
public synchronized MethodHandle setCachedMethodHandle(int which,
MethodHandle mh) {
// Simulate a CAS, to avoid racy duplication of results.
SoftReference<MethodHandle> entry = methodHandles[which];
if (entry != null) {
MethodHandle prev = entry.get();
if (prev != null) {
return prev;
}
}
methodHandles[which] = new SoftReference<>(mh);
return mh;
}
and:
public synchronized LambdaForm setCachedLambdaForm(int which,
LambdaForm form) {
// Simulate a CAS, to avoid racy duplication of results.
SoftReference<LambdaForm> entry = lambdaForms[which];
if (entry != null) {
LambdaForm prev = entry.get();
if (prev != null) {
return prev;
}
}
lambdaForms[which] = new SoftReference<>(form);
return form;
}
If these two @Stable annotations had any effect on JIT optimization,
then I think the caching logic would become ineffective for slots in
which SoftReference(s) got cleared by GC. In that case, such slots would
constantly be overwritten with new SoftReference(s) only to see old
constant-folded SoftReference(s) next time around.
So I think that these @Stable annotations do no good here. Either they
are ineffective in cases where MethodTypeForm instances are not
constant-folded, or they render caching ineffective when MethodTypeForm
instances are constant-folder and SoftReference(s) are cleared. In
either case it would be better without them.
What do you think?
You might be right, and this should be examined. It seems the @Stable
dates from an earlier version of MethodTypeForm that didn't use
SoftReferences:
https://bugs.openjdk.java.net/browse/JDK-8057020
Thanks!
/Claes
