Merge pull request #885 from k163377/719_snc

Performance improvement of `strictNullChecks`

Author: k163377

pom.xml

@@ -254,7 +254,9 @@
                             <exclude>com.fasterxml.jackson.module.kotlin.KotlinModule#getSingletonSupport()</exclude>
                             <exclude>com.fasterxml.jackson.module.kotlin.SingletonSupport</exclude>
                             <!-- internal -->
-
+                            <exclude>
+                                com.fasterxml.jackson.module.kotlin.KotlinNamesAnnotationIntrospector#KotlinNamesAnnotationIntrospector(com.fasterxml.jackson.module.kotlin.ReflectionCache,boolean)
+                            </exclude>
                         </excludes>
                     </parameter>
                 </configuration>

release-notes/CREDITS-2.x

@@ -18,6 +18,7 @@ Contributors:
 # 2.19.0 (not yet released)
 
 WrongWrong (@k163377)
+* #885: Performance improvement of strictNullChecks
 * #884: Changed the base class of MissingKotlinParameterException to InvalidNullException
 * #878: Fix for #876
 * #868: Added test case for FAIL_ON_NULL_FOR_PRIMITIVES

release-notes/VERSION-2.x

@@ -17,6 +17,10 @@ Co-maintainers:
 ------------------------------------------------------------------------
 
 2.19.0 (not yet released)
+#885: A new `StrictNullChecks` option(KotlinFeature.NewStrictNullChecks) has been added which greatly improves throughput.
+ Benchmarks show a consistent throughput drop of less than 2% when enabled (prior to the improvement, the worst throughput drop was more than 30%).
+ Note that the new backend changes the exception thrown to `InvalidNullException` and with it the error message.
+ Also note that the base class for `MissingKotlinParameterException` was changed to `InvalidNullException` in #884.
 #884: The base class for `MissingKotlinParameterException` has been changed to `InvalidNullException`.
  If you do not catch this exception or catch `MismatchedInputException`, the behavior is unchanged.
  If you are catching both `MismatchedKotlinParameterException` and `InvalidNullException`, you must catch `MismatchedKotlinParameterException` first.

src/main/kotlin/com/fasterxml/jackson/module/kotlin/KotlinFeature.kt

@@ -1,5 +1,7 @@
 package com.fasterxml.jackson.module.kotlin
 
+import com.fasterxml.jackson.annotation.JsonSetter
+import com.fasterxml.jackson.databind.exc.InvalidNullException
 import java.util.BitSet
 
 /**
@@ -40,6 +42,11 @@ enum class KotlinFeature(internal val enabledByDefault: Boolean) {
      * may contain null values after deserialization.
      * Enabling it protects against this but has significant performance impact.
      */
+    @Deprecated(
+        level = DeprecationLevel.WARNING,
+        message = "This option will be migrated to the new backend in 2.21.",
+        replaceWith = ReplaceWith("NewStrictNullChecks")
+    )
     StrictNullChecks(enabledByDefault = false),
 
     /**
@@ -66,7 +73,23 @@ enum class KotlinFeature(internal val enabledByDefault: Boolean) {
      * `@JsonFormat` annotations need to be declared either on getter using `@get:JsonFormat` or field using `@field:JsonFormat`.
      * See [jackson-module-kotlin#651] for details.
      */
-    UseJavaDurationConversion(enabledByDefault = false);
+    UseJavaDurationConversion(enabledByDefault = false),
+
+    /**
+     * New [StrictNullChecks] feature with improved throughput.
+     * Internally, it will be the same as if [JsonSetter] (contentNulls = FAIL) had been granted.
+     * Benchmarks show that it can check for illegal nulls with throughput nearly identical to the default (see [jackson-module-kotlin#719]).
+     *
+     * Note that in the new backend, the exception thrown has changed from [MissingKotlinParameterException] to [InvalidNullException].
+     * The message will be changed accordingly.
+     * Since 2.19, the base class of [MissingKotlinParameterException] has also been changed to [InvalidNullException],
+     * so be careful when catching it.
+     *
+     * This is a temporary option for a phased backend migration,
+     * which will eventually be merged into [StrictNullChecks].
+     * Also, specifying both this and [StrictNullChecks] is not permitted.
+     */
+    NewStrictNullChecks(enabledByDefault = false);
 
     internal val bitSet: BitSet = (1 shl ordinal).toBitSet()
 

src/main/kotlin/com/fasterxml/jackson/module/kotlin/KotlinModule.kt

@@ -7,6 +7,7 @@ import com.fasterxml.jackson.module.kotlin.KotlinFeature.NullToEmptyCollection
 import com.fasterxml.jackson.module.kotlin.KotlinFeature.NullToEmptyMap
 import com.fasterxml.jackson.module.kotlin.KotlinFeature.SingletonSupport
 import com.fasterxml.jackson.module.kotlin.KotlinFeature.StrictNullChecks
+import com.fasterxml.jackson.module.kotlin.KotlinFeature.NewStrictNullChecks
 import com.fasterxml.jackson.module.kotlin.KotlinFeature.KotlinPropertyNameAsImplicitName
 import com.fasterxml.jackson.module.kotlin.KotlinFeature.UseJavaDurationConversion
 import java.util.*
@@ -42,9 +43,10 @@ class KotlinModule private constructor(
     val nullToEmptyMap: Boolean = NullToEmptyMap.enabledByDefault,
     val nullIsSameAsDefault: Boolean = NullIsSameAsDefault.enabledByDefault,
     val singletonSupport: Boolean = SingletonSupport.enabledByDefault,
-    val strictNullChecks: Boolean = StrictNullChecks.enabledByDefault,
+    strictNullChecks: Boolean = StrictNullChecks.enabledByDefault,
     val kotlinPropertyNameAsImplicitName: Boolean = KotlinPropertyNameAsImplicitName.enabledByDefault,
     val useJavaDurationConversion: Boolean = UseJavaDurationConversion.enabledByDefault,
+    private val newStrictNullChecks: Boolean = NewStrictNullChecks.enabledByDefault,
 ) : SimpleModule(KotlinModule::class.java.name, PackageVersion.VERSION) {
     /*
      * Prior to 2.18, an older Enum called SingletonSupport was used to manage feature.
@@ -64,6 +66,19 @@ class KotlinModule private constructor(
     )
     val enabledSingletonSupport: Boolean get() = singletonSupport
 
+    private val oldStrictNullChecks: Boolean = strictNullChecks
+
+    // To reduce the amount of destructive changes, no properties will be added to the public.
+    val strictNullChecks: Boolean = if (strictNullChecks) {
+        if (newStrictNullChecks) {
+            throw IllegalArgumentException("Enabling both StrictNullChecks and NewStrictNullChecks is not permitted.")
+        }
+
+        true
+    } else {
+        newStrictNullChecks
+    }
+
     companion object {
         // Increment when option is added
         private const val serialVersionUID = 3L
@@ -84,6 +99,7 @@ class KotlinModule private constructor(
         builder.isEnabled(StrictNullChecks),
         builder.isEnabled(KotlinPropertyNameAsImplicitName),
         builder.isEnabled(UseJavaDurationConversion),
+        builder.isEnabled(NewStrictNullChecks),
     )
 
     override fun setupModule(context: SetupContext) {
@@ -95,7 +111,7 @@ class KotlinModule private constructor(
 
         val cache = ReflectionCache(reflectionCacheSize)
 
-        context.addValueInstantiators(KotlinInstantiators(cache, nullToEmptyCollection, nullToEmptyMap, nullIsSameAsDefault, strictNullChecks))
+        context.addValueInstantiators(KotlinInstantiators(cache, nullToEmptyCollection, nullToEmptyMap, nullIsSameAsDefault, oldStrictNullChecks))
 
         if (singletonSupport) {
             context.addBeanDeserializerModifier(KotlinBeanDeserializerModifier)
@@ -109,7 +125,9 @@ class KotlinModule private constructor(
             nullIsSameAsDefault,
             useJavaDurationConversion
         ))
-        context.appendAnnotationIntrospector(KotlinNamesAnnotationIntrospector(cache, kotlinPropertyNameAsImplicitName))
+        context.appendAnnotationIntrospector(
+            KotlinNamesAnnotationIntrospector(cache, newStrictNullChecks, kotlinPropertyNameAsImplicitName)
+        )
 
         context.addDeserializers(KotlinDeserializers(cache, useJavaDurationConversion))
         context.addKeyDeserializers(KotlinKeyDeserializers)

src/main/kotlin/com/fasterxml/jackson/module/kotlin/KotlinNamesAnnotationIntrospector.kt

@@ -1,6 +1,8 @@
 package com.fasterxml.jackson.module.kotlin
 
 import com.fasterxml.jackson.annotation.JsonProperty
+import com.fasterxml.jackson.annotation.JsonSetter
+import com.fasterxml.jackson.annotation.Nulls
 import com.fasterxml.jackson.databind.JavaType
 import com.fasterxml.jackson.databind.cfg.MapperConfig
 import com.fasterxml.jackson.databind.introspect.Annotated
@@ -12,8 +14,10 @@ import com.fasterxml.jackson.databind.introspect.NopAnnotationIntrospector
 import com.fasterxml.jackson.databind.introspect.PotentialCreator
 import java.lang.reflect.Constructor
 import java.util.Locale
+import kotlin.collections.getOrNull
 import kotlin.reflect.KClass
 import kotlin.reflect.KFunction
+import kotlin.reflect.KParameter
 import kotlin.reflect.full.hasAnnotation
 import kotlin.reflect.full.memberProperties
 import kotlin.reflect.full.primaryConstructor
@@ -22,6 +26,7 @@ import kotlin.reflect.jvm.javaType
 
 internal class KotlinNamesAnnotationIntrospector(
     private val cache: ReflectionCache,
+    private val strictNullChecks: Boolean,
     private val kotlinPropertyNameAsImplicitName: Boolean
 ) : NopAnnotationIntrospector() {
     private fun getterNameFromJava(member: AnnotatedMethod): String? {
@@ -73,16 +78,26 @@ internal class KotlinNamesAnnotationIntrospector(
     }
 
     override fun refineDeserializationType(config: MapperConfig<*>, a: Annotated, baseType: JavaType): JavaType =
-        (a as? AnnotatedParameter)?.let { _ ->
-            cache.findKotlinParameter(a)?.let { param ->
-                val rawType = a.rawType
-                (param.type.classifier as? KClass<*>)
-                    ?.java
-                    ?.takeIf { it.isUnboxableValueClass() && it != rawType }
-                    ?.let { config.constructType(it) }
-            }
+        findKotlinParameter(a)?.let { param ->
+            val rawType = a.rawType
+            (param.type.classifier as? KClass<*>)
+                ?.java
+                ?.takeIf { it.isUnboxableValueClass() && it != rawType }
+                ?.let { config.constructType(it) }
         } ?: baseType
 
+    override fun findSetterInfo(ann: Annotated): JsonSetter.Value = ann.takeIf { strictNullChecks }
+        ?.let { _ ->
+            findKotlinParameter(ann)?.let { param ->
+                if (param.requireStrictNullCheck(ann.type)) {
+                    JsonSetter.Value.forContentNulls(Nulls.FAIL)
+                } else {
+                    null
+                }
+            }
+        }
+        ?: super.findSetterInfo(ann)
+
     override fun findDefaultCreator(
         config: MapperConfig<*>,
         valueClass: AnnotatedClass,
@@ -106,8 +121,18 @@ internal class KotlinNamesAnnotationIntrospector(
     }
 
     private fun findKotlinParameterName(param: AnnotatedParameter): String? = cache.findKotlinParameter(param)?.name
+
+    private fun findKotlinParameter(param: Annotated) = (param as? AnnotatedParameter)
+        ?.let { cache.findKotlinParameter(it) }
 }
 
+private fun KParameter.markedNonNullAt(index: Int) = type.arguments.getOrNull(index)?.type?.isMarkedNullable == false
+
+private fun KParameter.requireStrictNullCheck(type: JavaType): Boolean =
+    ((type.isArrayType || type.isCollectionLikeType) && this.markedNonNullAt(0)) ||
+            (type.isMapLikeType && this.markedNonNullAt(1))
+
+
 // If it is not a Kotlin class or an Enum, Creator is not used
 private fun AnnotatedClass.creatableKotlinClass(): KClass<*>? = annotated
     .takeIf { it.isKotlinClass() && !it.isEnum }

src/test/kotlin/com/fasterxml/jackson/module/kotlin/DslTest.kt

@@ -6,7 +6,7 @@ import com.fasterxml.jackson.module.kotlin.KotlinFeature.NullIsSameAsDefault
 import com.fasterxml.jackson.module.kotlin.KotlinFeature.NullToEmptyCollection
 import com.fasterxml.jackson.module.kotlin.KotlinFeature.NullToEmptyMap
 import com.fasterxml.jackson.module.kotlin.KotlinFeature.SingletonSupport
-import com.fasterxml.jackson.module.kotlin.KotlinFeature.StrictNullChecks
+import com.fasterxml.jackson.module.kotlin.KotlinFeature.NewStrictNullChecks
 import org.junit.jupiter.api.Assertions.assertNotNull
 import org.junit.jupiter.api.Test
 import kotlin.test.assertEquals
@@ -35,7 +35,7 @@ class DslTest {
             enable(NullToEmptyMap)
             enable(NullIsSameAsDefault)
             enable(SingletonSupport)
-            enable(StrictNullChecks)
+            enable(NewStrictNullChecks)
         }
 
         assertNotNull(module)

src/test/kotlin/com/fasterxml/jackson/module/kotlin/KotlinModuleTest.kt

@@ -6,9 +6,24 @@ import org.junit.jupiter.api.Assertions.assertEquals
 import org.junit.jupiter.api.Assertions.assertFalse
 import org.junit.jupiter.api.Assertions.assertTrue
 import org.junit.jupiter.api.Test
+import org.junit.jupiter.api.assertThrows
 import kotlin.test.assertNotNull
 
 class KotlinModuleTest {
+    // After the final migration is complete, this test will be removed.
+    @Test
+    fun strictNullChecksTests() {
+        assertTrue(kotlinModule { enable(StrictNullChecks) }.strictNullChecks)
+        assertTrue(kotlinModule { enable(NewStrictNullChecks) }.strictNullChecks)
+
+        assertThrows<IllegalArgumentException> {
+            kotlinModule {
+                enable(StrictNullChecks)
+                enable(NewStrictNullChecks)
+            }
+        }
+    }
+
     @Test
     fun builder_Defaults() {
         val module = KotlinModule.Builder().build()