RFR: 8278065: Refactor subclassAudits to use ClassValue [v3]

[Roman Kennke]

On Fri, 10 Dec 2021 21:07:37 GMT, Roman Kennke <rkennke at openjdk.org> wrote:

>> As a follow-up to #6375, this change refactors java.io.ObjectInputStream.Caches#subclassAudits and java.io.ObjectOutputStream.Caches#subclassAudits to use ClassValue instead of SoftReference, similar to what we did in #6375 for java.io.ObjectStreamClass.Caches#localDescs. Then we can now also remove the common machinery java.io.ObjectStreamClass#processQueue and java.io.ObjectStreamClass.WeakClassKey.
>> 
>> Testing:
>>  - [x] tier1
>>  - [x] tier2
>>  - [ ] tier3
>
> Roman Kennke has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains ten commits:
> 
>  - Merge branch 'master' into JDK-8278065
>  - 8278065: Refactor subclassAudits to use ClassValue
>  - Remove unused EntryFuture inner class from ObjectSteamClass
>  - Merge remote-tracking branch 'jdk-plevart/JDK-8277072-peter' into JDK-8277072
>  - Use ClassValue to solve JDK-8277072
>  - Use ForceGC instead of System.gc()
>  - Convert test to testng
>  - Fix indentation of new testcase
>  - 8277072: ObjectStreamClass caches keep ClassLoaders alive

I think we shoul

> ...well I tried to do that. And it is not so simple. Here's what I came up with:
> 
> ```java
> public final class ClassFlags {
> 
>     private static final class AtomicByte {
>         private static final VarHandle VALUE;
> 
>         static {
>             try {
>                 VALUE = MethodHandles.lookup().findVarHandle(AtomicByte.class, "value", byte.class);
>             } catch (NoSuchFieldException | IllegalAccessException e) {
>                 throw new InternalError(e);
>             }
>         }
> 
>         private volatile byte value;
> 
>         byte get() {
>             return value;
>         }
> 
>         byte compareAndExchange(byte expectedValue, byte newValue) {
>             return (byte) VALUE.compareAndExchange(this, (byte) expectedValue, (byte) newValue);
>         }
>     }
> 
>     private final Predicate<? super Class<?>>[] typePredicates;
>     private final ClassValue<AtomicByte> flagsCV = new ClassValue<>() {
>         @Override
>         protected AtomicByte computeValue(Class<?> type) {
>             return new AtomicByte();
>         }
>     };
> 
>     @SafeVarargs
>     public ClassFlags(Predicate<? super Class<?>>... typePredicates) {
>         if (typePredicates.length > 4) {
>             throw new IllegalArgumentException("Up to 4 flags are supported in a single ClassFlags instance");
>         }
>         this.typePredicates = typePredicates;
>     }
> 
>     public boolean get(Class<?> type, int index) {
>         Objects.checkIndex(index, typePredicates.length);
> 
>         AtomicByte flags = flagsCV.get(type);
>         int f = flags.get() & 0xFF;
>         int falseMask = 0x1 << (index + index);
>         int trueMask = falseMask << 1;
>         int mask = falseMask | trueMask;
>         int value = 0;
>         while ((f & mask) == 0) {
>             if (value == 0) {
>                 value = typePredicates[index].test(type) ? trueMask : falseMask;
>             }
>             int fn = (f & ~mask) | value;
>             int fv = flags.compareAndExchange((byte) f, (byte) fn) & 0xFF;
>             if (fv == f) {
>                 f = fn;
>                 break;
>             } else {
>                 f = fv;
>             }
>         }
>         return (f & trueMask) != 0;
>     }
> }
> ```
> 
> So I don't know if it's worth it... Simplicity of your implementation probably out-weights the footprint savings of above code.
> 
> Regards, Peter

I think I'd rather keep it simple. Can you please give it a review, too? Thanks, Roman

-------------

PR: https://git.openjdk.java.net/jdk/pull/6637