From 95e6b211927ce45d0bcafd9e3bcbf46461094a42 Mon Sep 17 00:00:00 2001 From: wrongwrong Date: Sun, 16 Jul 2023 00:04:05 +0900 Subject: [PATCH 1/4] Optimization by organizing conditions - Changed so that null check of paramVal is done only once - Skip MismatchedInput check when paramVal is non-null - Skip strictNullCheck for empty collections completed by requireEmptyValue --- .../module/kotlin/KotlinValueInstantiator.kt | 40 ++++++++++--------- 1 file changed, 22 insertions(+), 18 deletions(-) diff --git a/src/main/kotlin/com/fasterxml/jackson/module/kotlin/KotlinValueInstantiator.kt b/src/main/kotlin/com/fasterxml/jackson/module/kotlin/KotlinValueInstantiator.kt index 98dbdd77..efb4cd50 100644 --- a/src/main/kotlin/com/fasterxml/jackson/module/kotlin/KotlinValueInstantiator.kt +++ b/src/main/kotlin/com/fasterxml/jackson/module/kotlin/KotlinValueInstantiator.kt @@ -3,6 +3,7 @@ package com.fasterxml.jackson.module.kotlin import com.fasterxml.jackson.databind.BeanDescription import com.fasterxml.jackson.databind.DeserializationConfig import com.fasterxml.jackson.databind.DeserializationContext +import com.fasterxml.jackson.databind.JavaType import com.fasterxml.jackson.databind.deser.SettableBeanProperty import com.fasterxml.jackson.databind.deser.ValueInstantiator import com.fasterxml.jackson.databind.deser.ValueInstantiators @@ -23,6 +24,9 @@ internal class KotlinValueInstantiator( private val nullIsSameAsDefault: Boolean, private val strictNullChecks: Boolean ) : StdValueInstantiator(src) { + private fun JavaType.requireEmptyValue() = + (nullToEmptyCollection && this.isCollectionLikeType) || (nullToEmptyMap && this.isMapLikeType) + override fun createFromObjectWith( ctxt: DeserializationContext, props: Array, @@ -74,24 +78,24 @@ internal class KotlinValueInstantiator( } } - if (paramVal == null && ((nullToEmptyCollection && jsonProp.type.isCollectionLikeType) || (nullToEmptyMap && jsonProp.type.isMapLikeType))) { - paramVal = NullsAsEmptyProvider(jsonProp.valueDeserializer).getNullValue(ctxt) - } - - val isGenericTypeVar = paramDef.type.javaType is TypeVariable<*> - val isMissingAndRequired = paramVal == null && isMissing && jsonProp.isRequired - if (isMissingAndRequired || - (!isGenericTypeVar && paramVal == null && !paramDef.type.isMarkedNullable)) { - throw MismatchedInputException.from( - ctxt.parser, - jsonProp.type, - "Instantiation of $valueTypeDesc value failed for JSON property ${jsonProp.name} " + - "due to missing (therefore NULL) value for creator parameter ${paramDef.name} " + - "which is a non-nullable type" - ).wrapWithPath(this.valueClass, jsonProp.name) - } - - if (strictNullChecks && paramVal != null) { + if (paramVal == null) { + if (jsonProp.type.requireEmptyValue()) { + paramVal = NullsAsEmptyProvider(jsonProp.valueDeserializer).getNullValue(ctxt) + } else { + val isMissingAndRequired = isMissing && jsonProp.isRequired + val isGenericTypeVar = paramDef.type.javaType is TypeVariable<*> + + if (isMissingAndRequired || (!isGenericTypeVar && !paramDef.type.isMarkedNullable)) { + throw MismatchedInputException.from( + ctxt.parser, + jsonProp.type, + "Instantiation of $valueTypeDesc value failed for JSON property ${jsonProp.name} " + + "due to missing (therefore NULL) value for creator parameter ${paramDef.name} " + + "which is a non-nullable type" + ).wrapWithPath(this.valueClass, jsonProp.name) + } + } + } else if (strictNullChecks) { var paramType: String? = null var itemType: KType? = null if (jsonProp.type.isCollectionLikeType && paramDef.type.arguments.getOrNull(0)?.type?.isMarkedNullable == false && (paramVal as Collection<*>).any { it == null }) { From 075e1335db4a3b8f4c0658e7239b009b9d1f4642 Mon Sep 17 00:00:00 2001 From: wrongwrong Date: Sun, 16 Jul 2023 00:09:43 +0900 Subject: [PATCH 2/4] Modified isGenericTypeVar to be determined at the last --- .../jackson/module/kotlin/KotlinValueInstantiator.kt | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/main/kotlin/com/fasterxml/jackson/module/kotlin/KotlinValueInstantiator.kt b/src/main/kotlin/com/fasterxml/jackson/module/kotlin/KotlinValueInstantiator.kt index efb4cd50..4e78ea72 100644 --- a/src/main/kotlin/com/fasterxml/jackson/module/kotlin/KotlinValueInstantiator.kt +++ b/src/main/kotlin/com/fasterxml/jackson/module/kotlin/KotlinValueInstantiator.kt @@ -27,6 +27,8 @@ internal class KotlinValueInstantiator( private fun JavaType.requireEmptyValue() = (nullToEmptyCollection && this.isCollectionLikeType) || (nullToEmptyMap && this.isMapLikeType) + private fun KType.isGenericTypeVar() = javaType is TypeVariable<*> + override fun createFromObjectWith( ctxt: DeserializationContext, props: Array, @@ -83,9 +85,9 @@ internal class KotlinValueInstantiator( paramVal = NullsAsEmptyProvider(jsonProp.valueDeserializer).getNullValue(ctxt) } else { val isMissingAndRequired = isMissing && jsonProp.isRequired - val isGenericTypeVar = paramDef.type.javaType is TypeVariable<*> - if (isMissingAndRequired || (!isGenericTypeVar && !paramDef.type.isMarkedNullable)) { + // Since #310 reported that the calculation cost is high, isGenericTypeVar is determined last. + if (isMissingAndRequired || (!paramDef.type.isMarkedNullable && !paramDef.type.isGenericTypeVar())) { throw MismatchedInputException.from( ctxt.parser, jsonProp.type, From e6fe727374f21ccbf6cc6e55c1ec7626f14aa83a Mon Sep 17 00:00:00 2001 From: wrongwrong Date: Sun, 16 Jul 2023 00:42:00 +0900 Subject: [PATCH 3/4] Variableize properties that are accessed many times --- .../module/kotlin/KotlinValueInstantiator.kt | 43 +++++++++++-------- 1 file changed, 26 insertions(+), 17 deletions(-) diff --git a/src/main/kotlin/com/fasterxml/jackson/module/kotlin/KotlinValueInstantiator.kt b/src/main/kotlin/com/fasterxml/jackson/module/kotlin/KotlinValueInstantiator.kt index 4e78ea72..47f3683c 100644 --- a/src/main/kotlin/com/fasterxml/jackson/module/kotlin/KotlinValueInstantiator.kt +++ b/src/main/kotlin/com/fasterxml/jackson/module/kotlin/KotlinValueInstantiator.kt @@ -14,6 +14,7 @@ import com.fasterxml.jackson.databind.exc.MismatchedInputException import java.lang.reflect.TypeVariable import kotlin.reflect.KParameter import kotlin.reflect.KType +import kotlin.reflect.KTypeProjection import kotlin.reflect.jvm.javaType internal class KotlinValueInstantiator( @@ -29,6 +30,8 @@ internal class KotlinValueInstantiator( private fun KType.isGenericTypeVar() = javaType is TypeVariable<*> + private fun List.markedNonNullAt(index: Int) = getOrNull(index)?.type?.isMarkedNullable == false + override fun createFromObjectWith( ctxt: DeserializationContext, props: Array, @@ -64,6 +67,7 @@ internal class KotlinValueInstantiator( return@forEachIndexed } + val paramType = paramDef.type var paramVal = if (!isMissing || paramDef.isPrimitive() || jsonProp.hasInjectableValueId()) { val tempParamVal = buffer.getParameter(jsonProp) if (nullIsSameAsDefault && tempParamVal == null && paramDef.isOptional) { @@ -71,7 +75,7 @@ internal class KotlinValueInstantiator( } tempParamVal } else { - if(paramDef.type.isMarkedNullable) { + if(paramType.isMarkedNullable) { // do not try to create any object if it is nullable and the value is missing null } else { @@ -80,17 +84,19 @@ internal class KotlinValueInstantiator( } } + val propType = jsonProp.type + if (paramVal == null) { - if (jsonProp.type.requireEmptyValue()) { + if (propType.requireEmptyValue()) { paramVal = NullsAsEmptyProvider(jsonProp.valueDeserializer).getNullValue(ctxt) } else { val isMissingAndRequired = isMissing && jsonProp.isRequired // Since #310 reported that the calculation cost is high, isGenericTypeVar is determined last. - if (isMissingAndRequired || (!paramDef.type.isMarkedNullable && !paramDef.type.isGenericTypeVar())) { + if (isMissingAndRequired || (!paramType.isMarkedNullable && !paramType.isGenericTypeVar())) { throw MismatchedInputException.from( ctxt.parser, - jsonProp.type, + propType, "Instantiation of $valueTypeDesc value failed for JSON property ${jsonProp.name} " + "due to missing (therefore NULL) value for creator parameter ${paramDef.name} " + "which is a non-nullable type" @@ -98,28 +104,31 @@ internal class KotlinValueInstantiator( } } } else if (strictNullChecks) { - var paramType: String? = null + val arguments = paramType.arguments + + var paramTypeStr: String? = null var itemType: KType? = null - if (jsonProp.type.isCollectionLikeType && paramDef.type.arguments.getOrNull(0)?.type?.isMarkedNullable == false && (paramVal as Collection<*>).any { it == null }) { - paramType = "collection" - itemType = paramDef.type.arguments[0].type + + if (propType.isCollectionLikeType && arguments.markedNonNullAt(0) && (paramVal as Collection<*>).any { it == null }) { + paramTypeStr = "collection" + itemType = arguments[0].type } - if (jsonProp.type.isMapLikeType && paramDef.type.arguments.getOrNull(1)?.type?.isMarkedNullable == false && (paramVal as Map<*, *>).any { it.value == null }) { - paramType = "map" - itemType = paramDef.type.arguments[1].type + if (propType.isMapLikeType && arguments.markedNonNullAt(1) && (paramVal as Map<*, *>).any { it.value == null }) { + paramTypeStr = "map" + itemType = arguments[1].type } - if (jsonProp.type.isArrayType && paramDef.type.arguments.getOrNull(0)?.type?.isMarkedNullable == false && (paramVal as Array<*>).any { it == null }) { - paramType = "array" - itemType = paramDef.type.arguments[0].type + if (propType.isArrayType && arguments.markedNonNullAt(0) && (paramVal as Array<*>).any { it == null }) { + paramTypeStr = "array" + itemType = arguments[0].type } - if (paramType != null && itemType != null) { + if (paramTypeStr != null && itemType != null) { throw MismatchedInputException.from( ctxt.parser, - jsonProp.type, - "Instantiation of $itemType $paramType failed for JSON property ${jsonProp.name} due to null value in a $paramType that does not allow null values" + propType, + "Instantiation of $itemType $paramTypeStr failed for JSON property ${jsonProp.name} due to null value in a $paramTypeStr that does not allow null values" ).wrapWithPath(this.valueClass, jsonProp.name) } } From 578ce844eeb84e4e1a721ce772dc7e71e064770e Mon Sep 17 00:00:00 2001 From: wrongwrong Date: Sun, 16 Jul 2023 00:54:05 +0900 Subject: [PATCH 4/4] Update release notes wrt #687 --- release-notes/CREDITS-2.x | 1 + release-notes/VERSION-2.x | 1 + 2 files changed, 2 insertions(+) diff --git a/release-notes/CREDITS-2.x b/release-notes/CREDITS-2.x index ff7a5c22..9f8b89aa 100644 --- a/release-notes/CREDITS-2.x +++ b/release-notes/CREDITS-2.x @@ -18,6 +18,7 @@ Contributors: # 2.16.0 (not yet released) WrongWrong (@k163377) +* #687: Optimize and Refactor KotlinValueInstantiator.createFromObjectWith * #685: Streamline default value management for KotlinFeatures * #684: Update Kotlin Version to 1.6 * #682: Remove MissingKotlinParameterException and replace with MismatchedInputException diff --git a/release-notes/VERSION-2.x b/release-notes/VERSION-2.x index 35297bb3..5aa73dd1 100644 --- a/release-notes/VERSION-2.x +++ b/release-notes/VERSION-2.x @@ -18,6 +18,7 @@ Co-maintainers: 2.16.0 (not yet released) +#687: Optimize and Refactor KotlinValueInstantiator.createFromObjectWith. #685: Streamline default value management for KotlinFeatures. This improves the initialization cost of kotlin-module a little. #684: Kotlin 1.5 has been deprecated and the minimum supported Kotlin version will be updated to 1.6.