Skip to content

Commit 03a558b

Browse files
k163377k163377
authored
Merge pull request #44 from ProjectMapK/improve-strict-null-checks-performance
Improve strict null checks performance
2 parents 00c2415 + fd515e5 commit 03a558b

File tree

7 files changed

+112
-66
lines changed

7 files changed

+112
-66
lines changed

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,7 @@ public enum class KotlinFeature(internal val enabledByDefault: Boolean) {
5353
*
5454
* With this disabled, the default, collections which are typed to disallow null members (e.g. `List<String>`)
5555
* may contain null values after deserialization.
56-
* Enabling it protects against this but has significant performance impact.
56+
* Enabling it protects against this but has performance impact.
5757
*/
5858
StrictNullChecks(enabledByDefault = false);
5959

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,7 @@ public class KotlinModule private constructor(
6464
val cache = ReflectionCache(reflectionCacheSize)
6565

6666
context.addValueInstantiators(
67-
KotlinInstantiators(cache, nullToEmptyCollection, nullToEmptyMap, nullIsSameAsDefault, strictNullChecks)
67+
KotlinInstantiators(cache, nullToEmptyCollection, nullToEmptyMap, nullIsSameAsDefault)
6868
)
6969

7070
if (singletonSupport) {
@@ -74,7 +74,7 @@ public class KotlinModule private constructor(
7474
context.insertAnnotationIntrospector(
7575
KotlinAnnotationIntrospector(context, nullToEmptyCollection, nullToEmptyMap, cache)
7676
)
77-
context.appendAnnotationIntrospector(KotlinNamesAnnotationIntrospector(this, cache))
77+
context.appendAnnotationIntrospector(KotlinNamesAnnotationIntrospector(this, strictNullChecks, cache))
7878

7979
context.addDeserializers(KotlinDeserializers())
8080
context.addKeyDeserializers(KotlinKeyDeserializers)

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

Lines changed: 41 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,9 @@ import com.fasterxml.jackson.databind.introspect.AnnotatedMember
1010
import com.fasterxml.jackson.databind.introspect.AnnotatedMethod
1111
import com.fasterxml.jackson.databind.introspect.AnnotatedParameter
1212
import com.fasterxml.jackson.databind.introspect.NopAnnotationIntrospector
13+
import com.fasterxml.jackson.module.kotlin.deser.StrictNullChecksConverter
1314
import com.fasterxml.jackson.module.kotlin.deser.ValueClassUnboxConverter
15+
import com.fasterxml.jackson.module.kotlin.deser.value_instantiator.creator.ValueParameter
1416
import kotlinx.metadata.Flag
1517
import kotlinx.metadata.KmClass
1618
import kotlinx.metadata.KmClassifier
@@ -26,6 +28,7 @@ import java.lang.reflect.Modifier
2628

2729
internal class KotlinNamesAnnotationIntrospector(
2830
val module: KotlinModule,
31+
private val strictNullChecks: Boolean,
2932
private val cache: ReflectionCache
3033
) : NopAnnotationIntrospector() {
3134
// since 2.4
@@ -103,19 +106,51 @@ internal class KotlinNamesAnnotationIntrospector(
103106
.takeIf { ann.annotated.isPrimarilyConstructorOf(kmClass) && !hasCreator(declaringClass, kmClass) }
104107
}
105108

109+
private fun getValueParameter(a: AnnotatedParameter): ValueParameter? =
110+
cache.valueCreatorFromJava(a.owner)?.let { it.valueParameters[a.index] }
111+
106112
// returns Converter when the argument on Java is an unboxed value class
107113
override fun findDeserializationConverter(a: Annotated): Any? = (a as? AnnotatedParameter)?.let { param ->
108-
cache.valueCreatorFromJava(param.owner)?.let { creator ->
109-
(creator.valueParameters[param.index].type.classifier as? KmClassifier.Class)?.let { classifier ->
110-
runCatching { classifier.name.reconstructClass() }
111-
.getOrNull()
112-
?.takeIf { it.isUnboxableValueClass() && it != param.rawType }
113-
?.let { ValueClassUnboxConverter(it) }
114+
getValueParameter(param)?.let { valueParameter ->
115+
val rawType = a.rawType
116+
117+
valueParameter.createValueClassUnboxConverterOrNull(rawType) ?: run {
118+
if (strictNullChecks) {
119+
valueParameter.createStrictNullChecksConverterOrNull(rawType)
120+
} else {
121+
null
122+
}
114123
}
115124
}
116125
}
117126
}
118127

128+
private fun ValueParameter.createValueClassUnboxConverterOrNull(rawType: Class<*>): ValueClassUnboxConverter<*>? {
129+
return (this.type.classifier as? KmClassifier.Class)?.let { classifier ->
130+
runCatching { classifier.name.reconstructClass() }
131+
.getOrNull()
132+
?.takeIf { it.isUnboxableValueClass() && it != rawType }
133+
?.let { ValueClassUnboxConverter(it) }
134+
}
135+
}
136+
137+
// If the collection type argument cannot be obtained, treat it as nullable
138+
// @see com.fasterxml.jackson.module.kotlin._ported.test.StrictNullChecksTest#testListOfGenericWithNullValue
139+
private fun ValueParameter.isNullishTypeAt(index: Int) = arguments.getOrNull(index)?.isNullable ?: true
140+
141+
private fun ValueParameter.createStrictNullChecksConverterOrNull(rawType: Class<*>): StrictNullChecksConverter<*>? {
142+
@Suppress("UNCHECKED_CAST")
143+
return when {
144+
Array::class.java.isAssignableFrom(rawType) && !this.isNullishTypeAt(0) ->
145+
StrictNullChecksConverter.ForArray(rawType as Class<Array<*>>, this)
146+
Iterable::class.java.isAssignableFrom(rawType) && !this.isNullishTypeAt(0) ->
147+
StrictNullChecksConverter.ForIterable(rawType as Class<Iterable<*>>, this)
148+
Map::class.java.isAssignableFrom(rawType) && !this.isNullishTypeAt(1) ->
149+
StrictNullChecksConverter.ForMapValue(rawType as Class<Map<*, *>>, this)
150+
else -> null
151+
}
152+
}
153+
119154
private fun Constructor<*>.isPrimarilyConstructorOf(kmClass: KmClass): Boolean = kmClass.findKmConstructor(this)
120155
?.let { !Flag.Constructor.IS_SECONDARY(it.flags) || kmClass.constructors.size == 1 }
121156
?: false

‎src/main/kotlin/com/fasterxml/jackson/module/kotlin/deser/Converters.kt

Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,64 @@
1+
package com.fasterxml.jackson.module.kotlin.deser
2+
3+
import com.fasterxml.jackson.databind.JavaType
4+
import com.fasterxml.jackson.databind.type.TypeFactory
5+
import com.fasterxml.jackson.databind.util.Converter
6+
import com.fasterxml.jackson.module.kotlin.MissingKotlinParameterException
7+
import com.fasterxml.jackson.module.kotlin.deser.value_instantiator.creator.ValueParameter
8+
9+
internal class ValueClassUnboxConverter<T : Any>(private val valueClass: Class<T>) : Converter<T, Any?> {
10+
private val unboxMethod = valueClass.getDeclaredMethod("unbox-impl").apply {
11+
if (!this.isAccessible) this.isAccessible = true
12+
}
13+
private val outType = unboxMethod.returnType
14+
15+
override fun convert(value: T): Any? = unboxMethod.invoke(value)
16+
17+
override fun getInputType(typeFactory: TypeFactory): JavaType = typeFactory.constructType(valueClass)
18+
override fun getOutputType(typeFactory: TypeFactory): JavaType = typeFactory.constructType(outType)
19+
}
20+
21+
internal sealed class StrictNullChecksConverter<T : Any> : Converter<T, T> {
22+
protected abstract val clazz: Class<T>
23+
protected abstract val valueParameter: ValueParameter
24+
25+
protected abstract fun getValues(value: T): Iterator<*>
26+
27+
override fun convert(value: T): T {
28+
getValues(value).forEach {
29+
if (it == null) {
30+
throw MissingKotlinParameterException(
31+
valueParameter,
32+
null,
33+
"A null value was entered for the parameter ${valueParameter.name}."
34+
)
35+
}
36+
}
37+
38+
return value
39+
}
40+
41+
override fun getInputType(typeFactory: TypeFactory): JavaType = typeFactory.constructType(clazz)
42+
override fun getOutputType(typeFactory: TypeFactory): JavaType = typeFactory.constructType(clazz)
43+
44+
class ForIterable(
45+
override val clazz: Class<Iterable<*>>,
46+
override val valueParameter: ValueParameter
47+
) : StrictNullChecksConverter<Iterable<*>>() {
48+
override fun getValues(value: Iterable<*>): Iterator<*> = value.iterator()
49+
}
50+
51+
class ForArray(
52+
override val clazz: Class<Array<*>>,
53+
override val valueParameter: ValueParameter
54+
) : StrictNullChecksConverter<Array<*>>() {
55+
override fun getValues(value: Array<*>): Iterator<*> = value.iterator()
56+
}
57+
58+
class ForMapValue(
59+
override val clazz: Class<Map<*, *>>,
60+
override val valueParameter: ValueParameter
61+
) : StrictNullChecksConverter<Map<*, *>>() {
62+
override fun getValues(value: Map<*, *>): Iterator<*> = value.values.iterator()
63+
}
64+
}

‎src/main/kotlin/com/fasterxml/jackson/module/kotlin/deser/ValueClassUnboxConverter.kt

Lines changed: 0 additions & 17 deletions
This file was deleted.

‎src/main/kotlin/com/fasterxml/jackson/module/kotlin/deser/value_instantiator/KotlinValueInstantiator.kt

Lines changed: 3 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,6 @@ import com.fasterxml.jackson.databind.deser.std.StdValueInstantiator
1414
import com.fasterxml.jackson.module.kotlin.MissingKotlinParameterException
1515
import com.fasterxml.jackson.module.kotlin.ReflectionCache
1616
import com.fasterxml.jackson.module.kotlin.deser.value_instantiator.creator.ValueCreator
17-
import com.fasterxml.jackson.module.kotlin.deser.value_instantiator.creator.ValueParameter
1817

1918
private fun JsonMappingException.wrapWithPath(refFrom: Any?, refFieldName: String) =
2019
JsonMappingException.wrapWithPath(this, refFrom, refFieldName)
@@ -24,41 +23,11 @@ internal class KotlinValueInstantiator(
2423
private val cache: ReflectionCache,
2524
private val nullToEmptyCollection: Boolean,
2625
private val nullToEmptyMap: Boolean,
27-
private val nullIsSameAsDefault: Boolean,
28-
private val strictNullChecks: Boolean
26+
private val nullIsSameAsDefault: Boolean
2927
) : StdValueInstantiator(src) {
30-
// If the collection type argument cannot be obtained, treat it as nullable
31-
// @see com.fasterxml.jackson.module.kotlin._ported.test.StrictNullChecksTest#testListOfGenericWithNullValue
32-
private fun ValueParameter.isNullishTypeAt(index: Int) = arguments.getOrNull(index)?.isNullable ?: true
33-
3428
private fun JavaType.requireEmptyValue() =
3529
(nullToEmptyCollection && this.isCollectionLikeType) || (nullToEmptyMap && this.isMapLikeType)
3630

37-
private fun strictNullCheck(
38-
ctxt: DeserializationContext,
39-
jsonProp: SettableBeanProperty,
40-
paramDef: ValueParameter,
41-
paramVal: Any
42-
) {
43-
// If an error occurs, Argument.name is always non-null
44-
// @see com.fasterxml.jackson.module.kotlin.deser.value_instantiator.creator.Argument
45-
when {
46-
paramVal is Collection<*> && !paramDef.isNullishTypeAt(0) && paramVal.any { it == null } ->
47-
"collection" to paramDef.arguments[0].name!!
48-
paramVal is Array<*> && !paramDef.isNullishTypeAt(0) && paramVal.any { it == null } ->
49-
"array" to paramDef.arguments[0].name!!
50-
paramVal is Map<*, *> && !paramDef.isNullishTypeAt(1) && paramVal.values.any { it == null } ->
51-
"map" to paramDef.arguments[1].name!!
52-
else -> null
53-
}?.let { (paramType, itemType) ->
54-
throw MissingKotlinParameterException(
55-
parameter = paramDef,
56-
processor = ctxt.parser,
57-
msg = "Instantiation of $itemType $paramType failed for JSON property ${jsonProp.name} due to null value in a $paramType that does not allow null values"
58-
).wrapWithPath(this.valueClass, jsonProp.name)
59-
}
60-
}
61-
6231
override fun createFromObjectWith(
6332
ctxt: DeserializationContext,
6433
props: Array<out SettableBeanProperty>,
@@ -106,8 +75,6 @@ internal class KotlinValueInstantiator(
10675
).wrapWithPath(this.valueClass, jsonProp.name)
10776
}
10877
}
109-
} else {
110-
if (strictNullChecks) strictNullCheck(ctxt, jsonProp, paramDef, paramVal)
11178
}
11279

11380
bucket[idx] = paramVal
@@ -124,8 +91,7 @@ internal class KotlinInstantiators(
12491
private val cache: ReflectionCache,
12592
private val nullToEmptyCollection: Boolean,
12693
private val nullToEmptyMap: Boolean,
127-
private val nullIsSameAsDefault: Boolean,
128-
private val strictNullChecks: Boolean
94+
private val nullIsSameAsDefault: Boolean
12995
) : ValueInstantiators {
13096
override fun findValueInstantiator(
13197
deserConfig: DeserializationConfig,
@@ -139,8 +105,7 @@ internal class KotlinInstantiators(
139105
cache,
140106
nullToEmptyCollection,
141107
nullToEmptyMap,
142-
nullIsSameAsDefault,
143-
strictNullChecks
108+
nullIsSameAsDefault
144109
)
145110
} else {
146111
// TODO: return defaultInstantiator and let default method parameters and nullability go unused?

‎src/test/kotlin/com/fasterxml/jackson/module/kotlin/_ported/KotlinInstantiatorsTest.kt

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,8 +18,7 @@ class KotlinInstantiatorsTest {
1818
ReflectionCache(10),
1919
nullToEmptyCollection = false,
2020
nullToEmptyMap = false,
21-
nullIsSameAsDefault = false,
22-
strictNullChecks = false
21+
nullIsSameAsDefault = false
2322
)
2423

2524
@Test

0 commit comments

Comments
 (0)