Merge pull request #164 from ProjectMapK/fix/cache

Combined the cache LRUMap into one and changed it so that both the initial and maximum sizes can be specified.

Author: k163377

src/main/kotlin/io/github/projectmapk/jackson/module/kogera/KotlinModule.kt

@@ -20,26 +20,30 @@ import io.github.projectmapk.jackson.module.kogera.ser.serializers.KotlinSeriali
 import java.util.*
 
 /**
- * @param reflectionCacheSize     Default: 512.  Size, in items, of the caches used for mapping objects.
- * @param nullToEmptyCollection   Default: false.  Whether to deserialize null values for collection properties as
- *                                      empty collections.
- * @param nullToEmptyMap          Default: false.  Whether to deserialize null values for a map property to an empty
- *                                      map object.
- * @param nullIsSameAsDefault     Default false.  Whether to treat null values as absent when deserializing, thereby
- *                                      using the default value provided in Kotlin.
- * @param singletonSupport        Default: DISABLED.  Mode for singleton handling.
- *                                      See {@link io.github.projectmapk.jackson.module.kogera.SingletonSupport label}
- * @param strictNullChecks        Default: false.  Whether to check deserialized collections.  With this disabled,
- *                                      the default, collections which are typed to disallow null members
- *                                      (e.g. List<String>) may contain null values after deserialization.  Enabling it
- *                                      protects against this but has significant performance impact.
- * @param useJavaDurationConversion Default: false.  Whether to use [java.time.Duration] as a bridge for [kotlin.time.Duration].
- *                                      This allows use Kotlin Duration type with [com.fasterxml.jackson.datatype.jsr310.JavaTimeModule].
+ * @param initialCacheSize
+ *   Default: [Builder.DEFAULT_CACHE_SIZE]. See [Builder.withInitialCacheSize] for details.
+ * @param maxCacheSize
+ *   Default: [Builder.DEFAULT_CACHE_SIZE]. See [Builder.withMaxCacheSize] for details.
+ * @param nullToEmptyCollection
+ *   Default: false. See [KotlinFeature.NullToEmptyCollection] for details.
+ * @param nullToEmptyMap
+ *   Default: false. See [KotlinFeature.NullToEmptyMap] for details.
+ * @param nullIsSameAsDefault
+ *   Default: false. See [KotlinFeature.NullIsSameAsDefault] for details.
+ * @param singletonSupport
+ *   Default: false. See [KotlinFeature.SingletonSupport] for details.
+ * @param strictNullChecks
+ *   Default: false. See [KotlinFeature.StrictNullChecks] for details.
+ * @param copySyntheticConstructorParameterAnnotations
+ *   Default false.  See [KotlinFeature.CopySyntheticConstructorParameterAnnotations] for details.
+ * @param useJavaDurationConversion
+ *   Default: false. See [KotlinFeature.UseJavaDurationConversion] for details.
  */
 // Do not delete default arguments,
 // as this will cause an error during initialization by Spring's Jackson2ObjectMapperBuilder.
 public class KotlinModule private constructor(
-    public val reflectionCacheSize: Int = Builder.DEFAULT_CACHE_SIZE,
+    public val initialCacheSize: Int = Builder.DEFAULT_CACHE_SIZE,
+    public val maxCacheSize: Int = Builder.DEFAULT_CACHE_SIZE,
     public val nullToEmptyCollection: Boolean = NullToEmptyCollection.enabledByDefault,
     public val nullToEmptyMap: Boolean = NullToEmptyMap.enabledByDefault,
     public val nullIsSameAsDefault: Boolean = NullIsSameAsDefault.enabledByDefault,
@@ -50,7 +54,8 @@ public class KotlinModule private constructor(
     public val useJavaDurationConversion: Boolean = UseJavaDurationConversion.enabledByDefault
 ) : SimpleModule(KotlinModule::class.java.name, kogeraVersion) { // kogeraVersion is generated by building.
     private constructor(builder: Builder) : this(
-        builder.reflectionCacheSize,
+        builder.initialCacheSize,
+        builder.maxCacheSize,
         builder.isEnabled(NullToEmptyCollection),
         builder.isEnabled(NullToEmptyMap),
         builder.isEnabled(NullIsSameAsDefault),
@@ -66,8 +71,26 @@ public class KotlinModule private constructor(
     )
     public constructor() : this(Builder())
 
+    init {
+        checkMaxCacheSize(maxCacheSize)
+        checkCacheSize(initialCacheSize, maxCacheSize)
+    }
+
     public companion object {
-        private const val serialVersionUID = 1L
+        @Suppress("ConstPropertyName")
+        private const val serialVersionUID = 2L
+
+        private fun checkMaxCacheSize(maxCacheSize: Int) {
+            if (maxCacheSize < 16) throw IllegalArgumentException("16 or higher must be specified")
+        }
+
+        private fun checkCacheSize(initialCacheSize: Int, maxCacheSize: Int) {
+            if (maxCacheSize < initialCacheSize) {
+                throw IllegalArgumentException(
+                    "maxCacheSize($maxCacheSize) was less than initialCacheSize($initialCacheSize)."
+                )
+            }
+        }
     }
 
     override fun setupModule(context: SetupContext) {
@@ -79,7 +102,7 @@ public class KotlinModule private constructor(
             )
         }
 
-        val cache = ReflectionCache(reflectionCacheSize)
+        val cache = ReflectionCache(initialCacheSize, maxCacheSize)
 
         context.addValueInstantiators(
             KotlinInstantiators(cache, nullToEmptyCollection, nullToEmptyMap, nullIsSameAsDefault)
@@ -115,13 +138,35 @@ public class KotlinModule private constructor(
             internal const val DEFAULT_CACHE_SIZE = 512
         }
 
-        public var reflectionCacheSize: Int = DEFAULT_CACHE_SIZE
+        public var initialCacheSize: Int = DEFAULT_CACHE_SIZE
+            private set
+        public var maxCacheSize: Int = DEFAULT_CACHE_SIZE
             private set
 
         private val bitSet: BitSet = KotlinFeature.defaults
 
-        public fun withReflectionCacheSize(reflectionCacheSize: Int): Builder = apply {
-            this.reflectionCacheSize = reflectionCacheSize
+        /**
+         * @throws IllegalArgumentException A value less than [maxCacheSize] was entered for [initialCacheSize].
+         */
+        public fun withInitialCacheSize(initialCacheSize: Int): Builder = apply {
+            checkCacheSize(initialCacheSize, maxCacheSize)
+
+            this.initialCacheSize = initialCacheSize
+        }
+
+        /**
+         * Kogera's internal processing requires a certain cache size.
+         * The lower limit of [maxCacheSize] is set to 16 for extreme use cases,
+         * but it is recommended to set it to 100 or more unless there is a very clear reason.
+         *
+         * @throws IllegalArgumentException Specified size was less than the 16.
+         * @throws IllegalArgumentException A value less than [initialCacheSize] was entered for maxCacheSize.
+         */
+        public fun withMaxCacheSize(maxCacheSize: Int): Builder = apply {
+            checkMaxCacheSize(maxCacheSize)
+            checkCacheSize(initialCacheSize, maxCacheSize)
+
+            this.maxCacheSize = maxCacheSize
         }
 
         public fun enable(feature: KotlinFeature): Builder = apply {

src/main/kotlin/io/github/projectmapk/jackson/module/kogera/ReflectionCache.kt

@@ -1,34 +1,53 @@
 package io.github.projectmapk.jackson.module.kogera
 
-import com.fasterxml.jackson.databind.introspect.AnnotatedMethod
 import com.fasterxml.jackson.databind.util.LRUMap
 import java.io.Serializable
 import java.lang.reflect.Method
 import java.util.Optional
 
-internal class ReflectionCache(reflectionCacheSize: Int) : Serializable {
+// For ease of testing, maxCacheSize is limited only in KotlinModule.
+internal class ReflectionCache(initialCacheSize: Int, maxCacheSize: Int) : Serializable {
     companion object {
         // Increment is required when properties that use LRUMap are changed.
         @Suppress("ConstPropertyName")
-        private const val serialVersionUID = 3L
+        private const val serialVersionUID = 4L
     }
 
-    // This cache is used for both serialization and deserialization, so reserve a larger size from the start.
-    private val classCache = LRUMap<Class<*>, JmClass>(reflectionCacheSize, reflectionCacheSize)
+    /**
+     * For frequently used JmClass and BoxedReturnType, reduce overhead by using Class and Method directly as key.
+     * For other caches, if the key type overlaps, wrap it.
+     */
+    private sealed class OtherCacheKey<K : Any, V : Any> {
+        abstract val key: K
 
-    // Initial size is 0 because the value class is not always used
-    private val valueClassReturnTypeCache: LRUMap<Method, Optional<Class<*>>> =
-        LRUMap(0, reflectionCacheSize)
+        // The comparison was implemented directly because the decompiled results showed subtle efficiency.
 
-    // TODO: Consider whether the cache size should be reduced more,
-    //       since the cache is used only twice locally at initialization per property.
-    private val valueClassBoxConverterCache: LRUMap<Class<*>, ValueClassBoxConverter<*, *>> =
-        LRUMap(0, reflectionCacheSize)
-    private val valueClassUnboxConverterCache: LRUMap<Class<*>, ValueClassUnboxConverter<*>> =
-        LRUMap(0, reflectionCacheSize)
+        final override fun equals(other: Any?): Boolean =
+            (other as? OtherCacheKey<*, *>)?.let { it::class == this::class && it.key == key } ?: false
+
+        // If the hashCode matches the raw key, the search efficiency is reduced, so it is displaced.
+        final override fun hashCode(): Int = key.hashCode() * 31
+        final override fun toString(): String = key.toString()
+
+        class ValueClassBoxConverter(
+            override val key: Class<*>
+        ) : OtherCacheKey<Class<*>, io.github.projectmapk.jackson.module.kogera.ValueClassBoxConverter<*, *>>()
+        class ValueClassUnboxConverter(
+            override val key: Class<*>
+        ) : OtherCacheKey<Class<*>, io.github.projectmapk.jackson.module.kogera.ValueClassUnboxConverter<*>>()
+    }
+
+    private val cache = LRUMap<Any, Any>(initialCacheSize, maxCacheSize)
+    private fun <T : Any> find(key: Any): T? = cache[key]?.let {
+        @Suppress("UNCHECKED_CAST")
+        it as T
+    }
+
+    @Suppress("UNCHECKED_CAST")
+    private fun <T : Any> putIfAbsent(key: Any, value: T): T = cache.putIfAbsent(key, value) as T? ?: value
 
     fun getJmClass(clazz: Class<*>): JmClass? {
-        return classCache[clazz] ?: run {
+        return find(clazz) ?: run {
             val metadata = clazz.getAnnotation(Metadata::class.java) ?: return null
 
             // Do not parse super class for interfaces.
@@ -43,7 +62,7 @@ internal class ReflectionCache(reflectionCacheSize: Int) : Serializable {
             val interfaceJmClasses = clazz.interfaces.mapNotNull { getJmClass(it) }
 
             val value = JmClass(clazz, metadata, superJmClass, interfaceJmClasses)
-            (classCache.putIfAbsent(clazz, value) ?: value)
+            putIfAbsent(clazz, value)
         }
     }
 
@@ -56,34 +75,29 @@ internal class ReflectionCache(reflectionCacheSize: Int) : Serializable {
     }
 
     // Return boxed type on Kotlin for unboxed getters
-    fun findBoxedReturnType(getter: AnnotatedMethod): Class<*>? {
-        val method = getter.member
-        val optional = valueClassReturnTypeCache.get(method)
+    fun findBoxedReturnType(getter: Method): Class<*>? {
+        val optional = find<Optional<Class<*>>>(getter)
 
         return if (optional != null) {
             optional
         } else {
             // If the return value of the getter is a value class,
             // it will be serialized properly without doing anything.
             // TODO: Verify the case where a value class encompasses another value class.
-            if (method.returnType.isUnboxableValueClass()) return null
+            if (getter.returnType.isUnboxableValueClass()) return null
 
-            val value = Optional.ofNullable(method.getValueClassReturnType())
-            (valueClassReturnTypeCache.putIfAbsent(method, value) ?: value)
+            val value = Optional.ofNullable(getter.getValueClassReturnType())
+            putIfAbsent(getter, value)
         }.orElse(null)
     }
 
-    fun getValueClassBoxConverter(unboxedClass: Class<*>, valueClass: Class<*>): ValueClassBoxConverter<*, *> =
-        valueClassBoxConverterCache.get(valueClass) ?: run {
-            val value = ValueClassBoxConverter(unboxedClass, valueClass)
-
-            (valueClassBoxConverterCache.putIfAbsent(valueClass, value) ?: value)
-        }
-
-    fun getValueClassUnboxConverter(valueClass: Class<*>): ValueClassUnboxConverter<*> =
-        valueClassUnboxConverterCache.get(valueClass) ?: run {
-            val value = ValueClassUnboxConverter(valueClass)
+    fun getValueClassBoxConverter(unboxedClass: Class<*>, valueClass: Class<*>): ValueClassBoxConverter<*, *> {
+        val key = OtherCacheKey.ValueClassBoxConverter(valueClass)
+        return find(key) ?: putIfAbsent(key, ValueClassBoxConverter(unboxedClass, valueClass))
+    }
 
-            (valueClassUnboxConverterCache.putIfAbsent(valueClass, value) ?: value)
-        }
+    fun getValueClassUnboxConverter(valueClass: Class<*>): ValueClassUnboxConverter<*> {
+        val key = OtherCacheKey.ValueClassUnboxConverter(valueClass)
+        return find(key) ?: putIfAbsent(key, ValueClassUnboxConverter(valueClass))
+    }
 }

src/main/kotlin/io/github/projectmapk/jackson/module/kogera/annotation_introspector/KotlinFallbackAnnotationIntrospector.kt

@@ -68,7 +68,7 @@ internal class KotlinFallbackAnnotationIntrospector(
 
     override fun findSerializationConverter(a: Annotated): Converter<*, *>? = when (a) {
         // Find a converter to handle the case where the getter returns an unboxed value from the value class.
-        is AnnotatedMethod -> cache.findBoxedReturnType(a)?.let {
+        is AnnotatedMethod -> cache.findBoxedReturnType(a.member)?.let {
             if (useJavaDurationConversion && it == KotlinDuration::class.java) {
                 if (a.rawReturnType == KotlinDuration::class.java) {
                     KotlinToJavaDurationConverter
@@ -102,7 +102,7 @@ internal class KotlinFallbackAnnotationIntrospector(
     // Perform proper serialization even if the value wrapped by the value class is null.
     // If value is a non-null object type, it must not be reboxing.
     override fun findNullSerializer(am: Annotated): JsonSerializer<*>? = (am as? AnnotatedMethod)?.let { _ ->
-        cache.findBoxedReturnType(am)?.let {
+        cache.findBoxedReturnType(am.member)?.let {
             if (it.requireRebox()) cache.getValueClassBoxConverter(am.rawReturnType, it).delegatingSerializer else null
         }
     }

src/test/kotlin/io/github/projectmapk/jackson/module/kogera/KotlinModuleTest.kt

@@ -4,8 +4,10 @@ import org.junit.jupiter.api.Assertions.assertEquals
 import org.junit.jupiter.api.Assertions.assertFalse
 import org.junit.jupiter.api.Assertions.assertNotNull
 import org.junit.jupiter.api.Assertions.assertTrue
+import org.junit.jupiter.api.Nested
 import org.junit.jupiter.api.Test
 import org.junit.jupiter.api.assertDoesNotThrow
+import org.junit.jupiter.api.assertThrows
 
 class KotlinModuleTest {
     @Test
@@ -35,10 +37,52 @@ class KotlinModuleTest {
         assertFalse(builder.isEnabled(KotlinFeature.UseJavaDurationConversion))
     }
 
+    @Nested
+    inner class SetCacheSizesTest {
+        @Test
+        fun `Cannot set initialCacheSize to a value larger than maxCacheSize`() {
+            assertThrows<IllegalArgumentException> {
+                KotlinModule.Builder().apply {
+                    withInitialCacheSize(maxCacheSize + 1)
+                }
+            }
+        }
+
+        @Test
+        fun `Cannot set maxCacheSize to a value smaller than initialCacheSize`() {
+            assertThrows<IllegalArgumentException> {
+                KotlinModule.Builder().apply {
+                    withMaxCacheSize(initialCacheSize - 1)
+                }
+            }
+        }
+
+        @Test
+        fun `Cannot set maxCacheSize to a value smaller than 15`() {
+            assertThrows<IllegalArgumentException> {
+                KotlinModule.Builder().apply {
+                    withInitialCacheSize(0)
+                    withMaxCacheSize(15)
+                }
+            }
+        }
+
+        @Test
+        fun test() {
+            assertDoesNotThrow {
+                KotlinModule.Builder().apply {
+                    withInitialCacheSize(0)
+                    withMaxCacheSize(16)
+                }
+            }
+        }
+    }
+
     @Test
     fun jdkSerializabilityTest() {
         val module = KotlinModule.Builder().apply {
-            withReflectionCacheSize(123)
+            withInitialCacheSize(123)
+            withMaxCacheSize(321)
 
             KotlinFeature.values().forEach {
                 enable(it)
@@ -51,7 +95,8 @@ class KotlinModuleTest {
             val deserialized = jdkDeserialize<KotlinModule>(serialized)
 
             assertNotNull(deserialized)
-            assertEquals(123, deserialized.reflectionCacheSize)
+            assertEquals(123, deserialized.initialCacheSize)
+            assertEquals(321, deserialized.maxCacheSize)
             assertEquals(module.nullToEmptyCollection, deserialized.nullToEmptyCollection)
             assertEquals(module.nullToEmptyMap, deserialized.nullToEmptyMap)
             assertEquals(module.nullIsSameAsDefault, deserialized.nullIsSameAsDefault)