Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Performance improvement of strictNullChecks #885

Merged
merged 7 commits into from
Jan 19, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down
1 change: 1 addition & 0 deletions release-notes/CREDITS-2.x
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 4 additions & 0 deletions release-notes/VERSION-2.x
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
Original file line number Diff line number Diff line change
@@ -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

/**
Expand Down Expand Up @@ -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),

/**
Expand All @@ -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()

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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.*
Expand Down Expand Up @@ -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.
Expand All @@ -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
Expand All @@ -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) {
Expand All @@ -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)
Expand All @@ -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)
Expand Down
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -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
Expand All @@ -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? {
Expand Down Expand Up @@ -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,
Expand All @@ -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 }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -35,7 +35,7 @@ class DslTest {
enable(NullToEmptyMap)
enable(NullIsSameAsDefault)
enable(SingletonSupport)
enable(StrictNullChecks)
enable(NewStrictNullChecks)
}

assertNotNull(module)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down
Loading