Re: RFR: 8230662: Remove dead code from MethodTypeForm
On 2019-09-09 13:56, Claes Redestad wrote: On 2019-09-09 12:31, Peter Levart wrote: Hi Claes,>> 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. I agree the @Stable should be removed here, and filed a bug to that effect: https://bugs.openjdk.java.net/browse/JDK-8230768 /Claes
Re: RFR: 8230662: Remove dead code from MethodTypeForm
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[] methodHandles;
// Cached lambda form information, for basic types only:
final @Stable SoftReference[] 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 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 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
Re: RFR: 8230662: Remove dead code from MethodTypeForm
Hi Claes,
Your changes look good. But I spotted a pre-existing and unusual use of
@Stable annotation in java.lang.invoke.MethodTypeForm class:
// Cached adapter information:
@Stable final SoftReference[] methodHandles;
// Cached lambda form information, for basic types only:
final @Stable SoftReference[] 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 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 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?
Regards, Peter
On 9/5/19 4:41 PM, Claes Redestad wrote:
Hi,
I noticed some unused methods in java.lang.invoke.MethodTypeForm and
ended up with a rather substantial cleanup after pulling that particular
thread for a bit:
http://cr.openjdk.java.net/~redestad/8230662/jdk.00/
https://bugs.openjdk.java.net/browse/JDK-8230662
Testing: tier1-3
Thanks!
/Claes
Re: RFR: 8230662: Remove dead code from MethodTypeForm
Looks good. This is a good simplification. thanks Mandy On 9/5/19 7:41 AM, Claes Redestad wrote: Hi, I noticed some unused methods in java.lang.invoke.MethodTypeForm and ended up with a rather substantial cleanup after pulling that particular thread for a bit: http://cr.openjdk.java.net/~redestad/8230662/jdk.00/ https://bugs.openjdk.java.net/browse/JDK-8230662 Testing: tier1-3 Thanks! /Claes
Re: RFR: 8230662: Remove dead code from MethodTypeForm
Vladimir, thanks for reviewing! /Claes On 2019-09-06 15:48, Vladimir Ivanov wrote: Looks good. Best regards, Vladimir Ivanov On 05/09/2019 17:41, Claes Redestad wrote: Hi, I noticed some unused methods in java.lang.invoke.MethodTypeForm and ended up with a rather substantial cleanup after pulling that particular thread for a bit: http://cr.openjdk.java.net/~redestad/8230662/jdk.00/ https://bugs.openjdk.java.net/browse/JDK-8230662 Testing: tier1-3 Thanks! /Claes
Re: RFR: 8230662: Remove dead code from MethodTypeForm
Looks good. Best regards, Vladimir Ivanov On 05/09/2019 17:41, Claes Redestad wrote: Hi, I noticed some unused methods in java.lang.invoke.MethodTypeForm and ended up with a rather substantial cleanup after pulling that particular thread for a bit: http://cr.openjdk.java.net/~redestad/8230662/jdk.00/ https://bugs.openjdk.java.net/browse/JDK-8230662 Testing: tier1-3 Thanks! /Claes
RFR: 8230662: Remove dead code from MethodTypeForm
Hi, I noticed some unused methods in java.lang.invoke.MethodTypeForm and ended up with a rather substantial cleanup after pulling that particular thread for a bit: http://cr.openjdk.java.net/~redestad/8230662/jdk.00/ https://bugs.openjdk.java.net/browse/JDK-8230662 Testing: tier1-3 Thanks! /Claes
