From: Robert Varga Date: Mon, 11 Apr 2022 13:57:28 +0000 (+0200) Subject: Split STATIC_CODECS into per-type caches X-Git-Tag: v9.0.2~11 X-Git-Url: https://git.opendaylight.org/gerrit/gitweb?p=mdsal.git;a=commitdiff_plain;h=1a2d7e719b78cd564fe37578cab270372628d096 Split STATIC_CODECS into per-type caches We have a rather sorry tangle of call sites due to the fact we one instance of a cache with multiple loaders. Split the cache up separately for each type, co-locating the cache and the loader. This makes it a tad clearer as to what the type hierarchy looks like: ValueTypeCodec really is just a useless holder. JIRA: MDSAL-704 Change-Id: Ia334d373090a119297e7b9c92bccb166e80bcac6 Signed-off-by: Robert Varga --- diff --git a/binding/mdsal-binding-dom-codec/src/main/java/org/opendaylight/mdsal/binding/dom/codec/impl/BitsCodec.java b/binding/mdsal-binding-dom-codec/src/main/java/org/opendaylight/mdsal/binding/dom/codec/impl/BitsCodec.java index 340ad77f37..f5b0908961 100644 --- a/binding/mdsal-binding-dom-codec/src/main/java/org/opendaylight/mdsal/binding/dom/codec/impl/BitsCodec.java +++ b/binding/mdsal-binding-dom-codec/src/main/java/org/opendaylight/mdsal/binding/dom/codec/impl/BitsCodec.java @@ -10,6 +10,8 @@ package org.opendaylight.mdsal.binding.dom.codec.impl; import static com.google.common.base.Preconditions.checkArgument; import static java.util.Objects.requireNonNull; +import com.google.common.cache.Cache; +import com.google.common.cache.CacheBuilder; import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; import java.lang.invoke.MethodHandle; @@ -25,13 +27,26 @@ import java.util.Map; import java.util.Map.Entry; import java.util.Set; import java.util.TreeSet; -import java.util.concurrent.Callable; +import java.util.concurrent.ExecutionException; +import org.eclipse.jdt.annotation.NonNull; import org.opendaylight.mdsal.binding.dom.codec.impl.ValueTypeCodec.SchemaUnawareCodec; import org.opendaylight.mdsal.binding.spec.naming.BindingMapping; import org.opendaylight.yangtools.yang.model.api.type.BitsTypeDefinition; import org.opendaylight.yangtools.yang.model.api.type.BitsTypeDefinition.Bit; final class BitsCodec extends ReflectionBasedCodec implements SchemaUnawareCodec { + /* + * Use identity comparison for keys and allow classes to be GCd themselves. + * + * Since codecs can (and typically do) hold a direct or indirect strong reference to the class, they need to be also + * accessed via reference. Using a weak reference could be problematic, because the codec would quite often be only + * weakly reachable. We therefore use a soft reference, whose implementation guidance is suitable to our use case: + * + * "Virtual machine implementations are, however, encouraged to bias against clearing recently-created or + * recently-used soft references." + */ + private static final Cache, @NonNull BitsCodec> CACHE = CacheBuilder.newBuilder().weakKeys().softValues() + .build(); private static final MethodType CONSTRUCTOR_INVOKE_TYPE = MethodType.methodType(Object.class, Boolean[].class); // Ordered by position @@ -48,8 +63,9 @@ final class BitsCodec extends ReflectionBasedCodec implements SchemaUnawareCodec this.getters = ImmutableMap.copyOf(getters); } - static Callable loader(final Class returnType, final BitsTypeDefinition rootType) { - return () -> { + static @NonNull BitsCodec of(final Class returnType, final BitsTypeDefinition rootType) + throws ExecutionException { + return CACHE.get(returnType, () -> { final Map getters = new LinkedHashMap<>(); final Set ctorArgs = new TreeSet<>(); @@ -69,7 +85,7 @@ final class BitsCodec extends ReflectionBasedCodec implements SchemaUnawareCodec final MethodHandle ctor = MethodHandles.publicLookup().unreflectConstructor(constructor) .asSpreader(Boolean[].class, ctorArgs.size()).asType(CONSTRUCTOR_INVOKE_TYPE); return new BitsCodec(returnType, ctor, ctorArgs, getters); - }; + }); } @Override diff --git a/binding/mdsal-binding-dom-codec/src/main/java/org/opendaylight/mdsal/binding/dom/codec/impl/EncapsulatedValueCodec.java b/binding/mdsal-binding-dom-codec/src/main/java/org/opendaylight/mdsal/binding/dom/codec/impl/EncapsulatedValueCodec.java index 97ee81ff51..c756c0b8fd 100644 --- a/binding/mdsal-binding-dom-codec/src/main/java/org/opendaylight/mdsal/binding/dom/codec/impl/EncapsulatedValueCodec.java +++ b/binding/mdsal-binding-dom-codec/src/main/java/org/opendaylight/mdsal/binding/dom/codec/impl/EncapsulatedValueCodec.java @@ -10,12 +10,15 @@ package org.opendaylight.mdsal.binding.dom.codec.impl; import static java.util.Objects.requireNonNull; import com.google.common.base.Throwables; +import com.google.common.cache.Cache; +import com.google.common.cache.CacheBuilder; import java.lang.invoke.MethodHandle; import java.lang.invoke.MethodHandles; import java.lang.invoke.MethodHandles.Lookup; import java.lang.invoke.MethodType; import java.lang.reflect.Method; -import java.util.concurrent.Callable; +import java.util.concurrent.ExecutionException; +import org.eclipse.jdt.annotation.NonNull; import org.opendaylight.mdsal.binding.dom.codec.impl.ValueTypeCodec.SchemaUnawareCodec; import org.opendaylight.mdsal.binding.spec.naming.BindingMapping; import org.opendaylight.yangtools.yang.model.api.TypeDefinition; @@ -25,8 +28,20 @@ import org.opendaylight.yangtools.yang.model.api.TypeDefinition; * types, which are same as in NormalizedNode model. */ final class EncapsulatedValueCodec extends ReflectionBasedCodec implements SchemaUnawareCodec { - private static final Lookup LOOKUP = MethodHandles.publicLookup(); + /* + * Use identity comparison for keys and allow classes to be GCd themselves. + * + * Since codecs can (and typically do) hold a direct or indirect strong reference to the class, they need to be also + * accessed via reference. Using a weak reference could be problematic, because the codec would quite often be only + * weakly reachable. We therefore use a soft reference, whose implementation guidance is suitable to our use case: + * + * "Virtual machine implementations are, however, encouraged to bias against clearing recently-created or + * recently-used soft references." + */ + private static final Cache, EncapsulatedValueCodec> CACHE = CacheBuilder.newBuilder().weakKeys() + .softValues().build(); private static final MethodType OBJ_METHOD = MethodType.methodType(Object.class, Object.class); + private final MethodHandle constructor; private final MethodHandle getter; private final Class valueType; @@ -39,15 +54,17 @@ final class EncapsulatedValueCodec extends ReflectionBasedCodec implements Schem this.valueType = requireNonNull(valueType); } - static Callable loader(final Class typeClz, final TypeDefinition typeDef) { - return () -> { + static @NonNull EncapsulatedValueCodec of(final Class typeClz, final TypeDefinition typeDef) + throws ExecutionException { + return CACHE.get(typeClz, () -> { final Method m = typeClz.getMethod(BindingMapping.SCALAR_TYPE_OBJECT_GET_VALUE_NAME); - final MethodHandle getter = LOOKUP.unreflect(m).asType(OBJ_METHOD); + final Lookup lookup = MethodHandles.publicLookup(); + final MethodHandle getter = lookup.unreflect(m).asType(OBJ_METHOD); final Class valueType = m.getReturnType(); - final MethodHandle constructor = LOOKUP.findConstructor(typeClz, + final MethodHandle constructor = lookup.findConstructor(typeClz, MethodType.methodType(void.class, valueType)).asType(OBJ_METHOD); return new EncapsulatedValueCodec(typeClz, constructor, getter, valueType); - }; + }); } /** diff --git a/binding/mdsal-binding-dom-codec/src/main/java/org/opendaylight/mdsal/binding/dom/codec/impl/EnumerationCodec.java b/binding/mdsal-binding-dom-codec/src/main/java/org/opendaylight/mdsal/binding/dom/codec/impl/EnumerationCodec.java index 2e16b0c616..076ff1935f 100644 --- a/binding/mdsal-binding-dom-codec/src/main/java/org/opendaylight/mdsal/binding/dom/codec/impl/EnumerationCodec.java +++ b/binding/mdsal-binding-dom-codec/src/main/java/org/opendaylight/mdsal/binding/dom/codec/impl/EnumerationCodec.java @@ -10,13 +10,16 @@ package org.opendaylight.mdsal.binding.dom.codec.impl; import static com.google.common.base.Preconditions.checkArgument; import static java.util.Objects.requireNonNull; +import com.google.common.cache.Cache; +import com.google.common.cache.CacheBuilder; import com.google.common.collect.ImmutableBiMap; import com.google.common.collect.Maps; import java.util.Arrays; import java.util.Map; import java.util.Set; -import java.util.concurrent.Callable; +import java.util.concurrent.ExecutionException; import java.util.stream.Collectors; +import org.eclipse.jdt.annotation.NonNull; import org.opendaylight.mdsal.binding.dom.codec.impl.ValueTypeCodec.SchemaUnawareCodec; import org.opendaylight.yangtools.yang.binding.Enumeration; import org.opendaylight.yangtools.yang.model.api.type.EnumTypeDefinition; @@ -26,19 +29,31 @@ import org.slf4j.LoggerFactory; final class EnumerationCodec extends ReflectionBasedCodec implements SchemaUnawareCodec { private static final Logger LOG = LoggerFactory.getLogger(EnumerationCodec.class); + /* + * Use identity comparison for keys and allow classes to be GCd themselves. + * + * Since codecs can (and typically do) hold a direct or indirect strong reference to the class, they need to be also + * accessed via reference. Using a weak reference could be problematic, because the codec would quite often be only + * weakly reachable. We therefore use a soft reference, whose implementation guidance is suitable to our use case: + * + * "Virtual machine implementations are, however, encouraged to bias against clearing recently-created or + * recently-used soft references." + */ + private static final Cache, @NonNull EnumerationCodec> CACHE = CacheBuilder.newBuilder().weakKeys() + .softValues().build(); private final ImmutableBiMap> nameToEnum; - EnumerationCodec(final Class> enumeration, final Map> nameToEnum) { + private EnumerationCodec(final Class> enumeration, final Map> nameToEnum) { super(enumeration); this.nameToEnum = ImmutableBiMap.copyOf(nameToEnum); } - static Callable loader(final Class returnType, final EnumTypeDefinition def) { - checkArgument(Enum.class.isAssignableFrom(returnType)); - @SuppressWarnings("unchecked") - final Class> enumType = (Class>) returnType; - return () -> { + static @NonNull EnumerationCodec of(final Class returnType, final EnumTypeDefinition def) + throws ExecutionException { + return CACHE.get(returnType, () -> { + final Class> enumType = castType(returnType); + final Map> mapping = Maps.uniqueIndex(Arrays.asList(enumType.getEnumConstants()), value -> { checkArgument(value instanceof Enumeration, @@ -61,7 +76,13 @@ final class EnumerationCodec extends ReflectionBasedCodec implements SchemaUnawa } return new EnumerationCodec(enumType, mapping); - }; + }); + } + + @SuppressWarnings("unchecked") + private static Class> castType(final Class returnType) { + checkArgument(Enum.class.isAssignableFrom(returnType)); + return (Class>) returnType; } @Override diff --git a/binding/mdsal-binding-dom-codec/src/main/java/org/opendaylight/mdsal/binding/dom/codec/impl/ValueTypeCodec.java b/binding/mdsal-binding-dom-codec/src/main/java/org/opendaylight/mdsal/binding/dom/codec/impl/ValueTypeCodec.java index 55e5d1a50c..253fffae40 100644 --- a/binding/mdsal-binding-dom-codec/src/main/java/org/opendaylight/mdsal/binding/dom/codec/impl/ValueTypeCodec.java +++ b/binding/mdsal-binding-dom-codec/src/main/java/org/opendaylight/mdsal/binding/dom/codec/impl/ValueTypeCodec.java @@ -7,9 +7,8 @@ */ package org.opendaylight.mdsal.binding.dom.codec.impl; -import com.google.common.cache.Cache; -import com.google.common.cache.CacheBuilder; -import java.util.concurrent.Callable; +import static java.util.Objects.requireNonNull; + import java.util.concurrent.ExecutionException; import org.opendaylight.mdsal.binding.spec.reflect.BindingReflections; import org.opendaylight.yangtools.concepts.IllegalArgumentCodec; @@ -22,19 +21,6 @@ import org.opendaylight.yangtools.yang.model.api.type.EnumTypeDefinition; */ // FIXME: IllegalArgumentCodec is perhaps not appropriate here due to null behavior abstract class ValueTypeCodec implements IllegalArgumentCodec { - /* - * Use identity comparison for keys and allow classes to be GCd themselves. - * - * Since codecs can (and typically do) hold a direct or indirect strong reference to the class, they need to be also - * accessed via reference. Using a weak reference could be problematic, because the codec would quite often be only - * weakly reachable. We therefore use a soft reference, whose implementation guidance is suitable to our use case: - * - * "Virtual machine implementations are, however, encouraged to bias against clearing recently-created or - * recently-used soft references." - */ - private static final Cache, SchemaUnawareCodec> STATIC_CODECS = CacheBuilder.newBuilder() - .weakKeys().softValues().build(); - /** * Marker interface for codecs, which functionality will not be affected by schema change (introduction of new YANG * modules) they may have one static instance generated when first time needed. @@ -62,40 +48,39 @@ abstract class ValueTypeCodec implements IllegalArgumentCodec { public static SchemaUnawareCodec getCodecFor(final Class typeClz, final TypeDefinition def) { if (BindingReflections.isBindingClass(typeClz)) { - return getCachedSchemaUnawareCodec(typeClz, getCodecLoader(typeClz, def)); + return getCachedSchemaUnawareCodec(typeClz, def); } return NOOP_CODEC; } - private static SchemaUnawareCodec getCachedSchemaUnawareCodec(final Class typeClz, - final Callable loader) { + private static SchemaUnawareCodec getCachedSchemaUnawareCodec(final Class typeClz, final TypeDefinition def) { + // FIXME: extract this only when really needed + var rootType = requireNonNull(def); + while (true) { + final var base = rootType.getBaseType(); + if (base != null) { + rootType = base; + } else { + break; + } + } + try { - return STATIC_CODECS.get(typeClz, loader); + if (rootType instanceof EnumTypeDefinition) { + return EnumerationCodec.of(typeClz, (EnumTypeDefinition) rootType); + } else if (rootType instanceof BitsTypeDefinition) { + return BitsCodec.of(typeClz, (BitsTypeDefinition) rootType); + } else { + return EncapsulatedValueCodec.of(typeClz, def); + } } catch (ExecutionException e) { throw new IllegalStateException(e); } } - private static Callable getCodecLoader(final Class typeClz, - final TypeDefinition def) { - - TypeDefinition rootType = def; - while (rootType.getBaseType() != null) { - rootType = rootType.getBaseType(); - } - if (rootType instanceof EnumTypeDefinition) { - return EnumerationCodec.loader(typeClz, (EnumTypeDefinition) rootType); - } else if (rootType instanceof BitsTypeDefinition) { - return BitsCodec.loader(typeClz, (BitsTypeDefinition) rootType); - } - return EncapsulatedValueCodec.loader(typeClz, def); - } - @SuppressWarnings("rawtypes") static ValueTypeCodec encapsulatedValueCodecFor(final Class typeClz, final TypeDefinition typeDef, final IllegalArgumentCodec delegate) { - SchemaUnawareCodec extractor = getCachedSchemaUnawareCodec(typeClz, - EncapsulatedValueCodec.loader(typeClz, typeDef)); - return new CompositeValueCodec(extractor, delegate); + return new CompositeValueCodec(getCachedSchemaUnawareCodec(typeClz, typeDef), delegate); } } diff --git a/binding/mdsal-binding-dom-codec/src/test/java/org/opendaylight/mdsal/binding/dom/codec/impl/EnumerationCodecTest.java b/binding/mdsal-binding-dom-codec/src/test/java/org/opendaylight/mdsal/binding/dom/codec/impl/EnumerationCodecTest.java index 9b05d53a54..30d17ff6b7 100644 --- a/binding/mdsal-binding-dom-codec/src/test/java/org/opendaylight/mdsal/binding/dom/codec/impl/EnumerationCodecTest.java +++ b/binding/mdsal-binding-dom-codec/src/test/java/org/opendaylight/mdsal/binding/dom/codec/impl/EnumerationCodecTest.java @@ -18,7 +18,6 @@ import org.opendaylight.yangtools.yang.model.api.type.EnumTypeDefinition; import org.opendaylight.yangtools.yang.model.api.type.EnumTypeDefinition.EnumPair; public class EnumerationCodecTest { - private enum TestEnum implements Enumeration { ENUM; @@ -41,7 +40,7 @@ public class EnumerationCodecTest { EnumTypeDefinition definition = mock(EnumTypeDefinition.class); doReturn(ImmutableList.of(pair)).when(definition).getValues(); - final EnumerationCodec codec = EnumerationCodec.loader(TestEnum.class, definition).call(); + final EnumerationCodec codec = EnumerationCodec.of(TestEnum.class, definition); assertEquals(codec.deserialize(codec.serialize(TestEnum.ENUM)), TestEnum.ENUM); assertEquals(codec.serialize(codec.deserialize(TestEnum.ENUM.name())), TestEnum.ENUM.name()); }