Skip to content

Commit f761cca

Browse files
k163377k163377
authored
Merge pull request #755 from k163377/argument-bucket
Changes in constructor invocation and argument management
2 parents 2ee58d2 + be688b3 commit f761cca

15 files changed

+4071
-39
lines changed

‎.github/workflows/main.yml

+2-1
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,8 @@ jobs:
2727
fail-fast: false
2828
matrix:
2929
java_version: ['8', '11', '17', '21']
30-
kotlin_version: ['1.7.22', '1.8.22', '1.9.21']
30+
# kotlin-reflect 1.8.2x has a bug and some tests fail, so we are downgrading to 1.8.10.
31+
kotlin_version: ['1.7.22', '1.8.10', '1.9.21']
3132
os: ['ubuntu-20.04']
3233
env:
3334
JAVA_OPTS: "-XX:+TieredCompilation -XX:TieredStopAtLevel=1"

‎pom.xml

+3
Original file line numberDiff line numberDiff line change
@@ -237,6 +237,9 @@
237237
<exclude>com.fasterxml.jackson.module.kotlin.KotlinModule#serialVersionUID</exclude>
238238
<!-- internal -->
239239
<exclude>com.fasterxml.jackson.module.kotlin.KotlinNamesAnnotationIntrospector</exclude>
240+
<exclude>com.fasterxml.jackson.module.kotlin.ValueCreator</exclude>
241+
<exclude>com.fasterxml.jackson.module.kotlin.ConstructorValueCreator</exclude>
242+
<exclude>com.fasterxml.jackson.module.kotlin.MethodValueCreator</exclude>
240243
</excludes>
241244
</parameter>
242245
</configuration>

‎release-notes/CREDITS-2.x

+1
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ Contributors:
1818
# 2.17.0 (not yet released)
1919

2020
WrongWrong (@k163377)
21+
* #755: Changes in constructor invocation and argument management.
2122
* #752: Fix KDoc for KotlinModule.
2223
* #751: Marked useKotlinPropertyNameForGetter as deprecated.
2324
* #747: Improved performance related to KotlinModule initialization and setupModule.

‎release-notes/VERSION-2.x

+2
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,8 @@ Co-maintainers:
1818

1919
2.17.0 (not yet released)
2020

21+
#755: Changes in constructor invocation and argument management.
22+
This change degrades performance in cases where the constructor is called without default arguments, but improves performance in other cases.
2123
#751: The KotlinModule#useKotlinPropertyNameForGetter property was deprecated because it differed from the name of the KotlinFeature.
2224
Please use KotlinModule#kotlinPropertyNameAsImplicitName from now on.
2325
#747: Improved performance related to KotlinModule initialization and setupModule.

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

+68
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,68 @@
1+
package com.fasterxml.jackson.module.kotlin
2+
3+
import kotlin.reflect.KParameter
4+
5+
internal class BucketGenerator private constructor(paramSize: Int, instanceParameter: KParameter?, instance: Any?) {
6+
private val originalParameters = arrayOfNulls<KParameter>(paramSize)
7+
private val originalArguments = arrayOfNulls<Any?>(paramSize)
8+
private val initialCount: Int
9+
10+
init {
11+
if (instance != null) {
12+
originalParameters[0] = instanceParameter
13+
originalArguments[0] = instance
14+
initialCount = 1
15+
} else {
16+
initialCount = 0
17+
}
18+
}
19+
20+
fun generate(): ArgumentBucket = ArgumentBucket(
21+
parameters = originalParameters.clone(),
22+
arguments = originalArguments.clone(),
23+
count = initialCount
24+
)
25+
26+
companion object {
27+
fun forConstructor(paramSize: Int): BucketGenerator = BucketGenerator(paramSize, null, null)
28+
29+
fun forMethod(paramSize: Int, instanceParameter: KParameter, instance: Any): BucketGenerator =
30+
BucketGenerator(paramSize, instanceParameter, instance)
31+
}
32+
}
33+
34+
internal class ArgumentBucket(
35+
private val parameters: Array<KParameter?>,
36+
val arguments: Array<Any?>,
37+
private var count: Int
38+
) : Map<KParameter, Any?> {
39+
operator fun set(key: KParameter, value: Any?) {
40+
arguments[key.index] = value
41+
parameters[key.index] = key
42+
43+
// Multiple calls are not checked because internally no calls are made more than once per argument.
44+
count++
45+
}
46+
47+
val isFullInitialized: Boolean get() = count == arguments.size
48+
49+
private class Entry(override val key: KParameter, override val value: Any?) : Map.Entry<KParameter, Any?>
50+
51+
override val entries: Set<Map.Entry<KParameter, Any?>>
52+
get() = parameters.mapNotNull { key -> key?.let { Entry(it, arguments[it.index]) } }.toSet()
53+
override val keys: Set<KParameter>
54+
get() = parameters.filterNotNull().toSet()
55+
override val size: Int
56+
get() = count
57+
override val values: Collection<Any?>
58+
get() = keys.map { arguments[it.index] }
59+
60+
override fun isEmpty(): Boolean = this.size == 0
61+
62+
// Skip the check here, as it is only called after the check for containsKey.
63+
override fun get(key: KParameter): Any? = arguments[key.index]
64+
65+
override fun containsValue(value: Any?): Boolean = keys.any { arguments[it.index] == value }
66+
67+
override fun containsKey(key: KParameter): Boolean = parameters.any { it?.index == key.index }
68+
}

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

+1
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import kotlin.reflect.jvm.isAccessible
55

66
internal class ConstructorValueCreator<T>(override val callable: KFunction<T>) : ValueCreator<T>() {
77
override val accessible: Boolean = callable.isAccessible
8+
override val bucketGenerator: BucketGenerator = BucketGenerator.forConstructor(callable.parameters.size)
89

910
init {
1011
// To prevent the call from failing, save the initial value and then rewrite the flag.

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

+4-35
Original file line numberDiff line numberDiff line change
@@ -42,24 +42,7 @@ internal class KotlinValueInstantiator(
4242
val valueCreator: ValueCreator<*> = cache.valueCreatorFromJava(_withArgsCreator)
4343
?: return super.createFromObjectWith(ctxt, props, buffer)
4444

45-
val propCount: Int
46-
var numCallableParameters: Int
47-
val callableParameters: Array<KParameter?>
48-
val jsonParamValueList: Array<Any?>
49-
50-
if (valueCreator is MethodValueCreator) {
51-
propCount = props.size + 1
52-
numCallableParameters = 1
53-
callableParameters = arrayOfNulls<KParameter>(propCount)
54-
.apply { this[0] = valueCreator.instanceParameter }
55-
jsonParamValueList = arrayOfNulls<Any>(propCount)
56-
.apply { this[0] = valueCreator.companionObjectInstance }
57-
} else {
58-
propCount = props.size
59-
numCallableParameters = 0
60-
callableParameters = arrayOfNulls(propCount)
61-
jsonParamValueList = arrayOfNulls(propCount)
62-
}
45+
val bucket = valueCreator.generateBucket()
6346

6447
valueCreator.valueParameters.forEachIndexed { idx, paramDef ->
6548
val jsonProp = props[idx]
@@ -131,26 +114,12 @@ internal class KotlinValueInstantiator(
131114
}
132115
}
133116

134-
jsonParamValueList[numCallableParameters] = paramVal
135-
callableParameters[numCallableParameters] = paramDef
136-
numCallableParameters++
117+
bucket[paramDef] = paramVal
137118
}
138119

139-
return if (numCallableParameters == jsonParamValueList.size && valueCreator is ConstructorValueCreator) {
140-
// we didn't do anything special with default parameters, do a normal call
141-
super.createFromObjectWith(ctxt, jsonParamValueList)
142-
} else {
143-
valueCreator.checkAccessibility(ctxt)
144-
145-
val callableParametersByName = linkedMapOf<KParameter, Any?>()
146-
callableParameters.mapIndexed { idx, paramDef ->
147-
if (paramDef != null) {
148-
callableParametersByName[paramDef] = jsonParamValueList[idx]
149-
}
150-
}
151-
valueCreator.callBy(callableParametersByName)
152-
}
120+
valueCreator.checkAccessibility(ctxt)
153121

122+
return valueCreator.callBy(bucket)
154123
}
155124

156125
private fun SettableBeanProperty.hasInjectableValueId(): Boolean = injectableValueId != null

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

+4-2
Original file line numberDiff line numberDiff line change
@@ -9,9 +9,11 @@ import kotlin.reflect.jvm.isAccessible
99
internal class MethodValueCreator<T> private constructor(
1010
override val callable: KFunction<T>,
1111
override val accessible: Boolean,
12-
val companionObjectInstance: Any
12+
companionObjectInstance: Any
1313
) : ValueCreator<T>() {
14-
val instanceParameter: KParameter = callable.instanceParameter!!
14+
override val bucketGenerator: BucketGenerator = callable.parameters.let {
15+
BucketGenerator.forMethod(it.size, it[0], companionObjectInstance)
16+
}
1517

1618
companion object {
1719
fun <T> of(callable: KFunction<T>): MethodValueCreator<T>? {

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

+9-1
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,10 @@ internal sealed class ValueCreator<T> {
2121
*/
2222
protected abstract val accessible: Boolean
2323

24+
protected abstract val bucketGenerator: BucketGenerator
25+
26+
fun generateBucket(): ArgumentBucket = bucketGenerator.generate()
27+
2428
/**
2529
* ValueParameters of the KFunction to be called.
2630
*/
@@ -45,5 +49,9 @@ internal sealed class ValueCreator<T> {
4549
/**
4650
* Function call with default values enabled.
4751
*/
48-
fun callBy(args: Map<KParameter, Any?>): T = callable.callBy(args)
52+
fun callBy(args: ArgumentBucket): T = if (args.isFullInitialized) {
53+
callable.call(*args.arguments)
54+
} else {
55+
callable.callBy(args)
56+
}
4957
}

‎src/test/kotlin/com/fasterxml/jackson/module/kotlin/ArgumentBucketTest.kt

+86
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,86 @@
1+
package com.fasterxml.jackson.module.kotlin
2+
3+
import com.fasterxml.jackson.annotation.JsonCreator
4+
import org.junit.Ignore
5+
import org.junit.experimental.runners.Enclosed
6+
import org.junit.runner.RunWith
7+
import kotlin.reflect.KFunction
8+
import kotlin.reflect.full.functions
9+
import kotlin.reflect.full.hasAnnotation
10+
import kotlin.test.Test
11+
import kotlin.test.assertEquals
12+
import kotlin.test.assertFalse
13+
import kotlin.test.assertTrue
14+
15+
16+
@RunWith(Enclosed::class)
17+
class ArgumentBucketTest {
18+
class Normal {
19+
@Ignore
20+
data class Constructor(val foo: String, val bar: String)
21+
22+
@Test
23+
fun constructorTest() {
24+
val function: KFunction<*> = ::Constructor
25+
val params = function.parameters
26+
val generator = BucketGenerator.forConstructor(params.size)
27+
val bucket = generator.generate()
28+
29+
assertTrue(bucket.isEmpty())
30+
assertEquals(0, bucket.size)
31+
assertFalse(bucket.isFullInitialized)
32+
33+
bucket[params[0]] = "foo"
34+
35+
assertFalse(bucket.isEmpty())
36+
assertEquals(1, bucket.size)
37+
assertFalse(bucket.isFullInitialized)
38+
assertEquals("foo", bucket[params[0]])
39+
40+
bucket[params[1]] = "bar"
41+
42+
assertFalse(bucket.isEmpty())
43+
assertEquals(2, bucket.size)
44+
assertTrue(bucket.isFullInitialized)
45+
assertEquals("bar", bucket[params[1]])
46+
}
47+
48+
@Ignore
49+
data class Method(val foo: String, val bar: String) {
50+
companion object {
51+
@JvmStatic
52+
@JsonCreator
53+
fun of(foo: String, bar: String): Method = Method(foo, bar)
54+
}
55+
}
56+
57+
@Test
58+
fun methodTest() {
59+
val function: KFunction<*> = Method.Companion::class.functions.first { it.hasAnnotation<JsonCreator>() }
60+
val params = function.parameters
61+
val generator = BucketGenerator.forMethod(params.size, params[0], Method)
62+
val bucket = generator.generate()
63+
64+
assertFalse(bucket.isEmpty())
65+
assertEquals(1, bucket.size)
66+
assertEquals(Method.Companion, bucket[params[0]])
67+
assertFalse(bucket.isFullInitialized)
68+
69+
bucket[params[1]] = "foo"
70+
71+
assertFalse(bucket.isEmpty())
72+
assertEquals(2, bucket.size)
73+
assertFalse(bucket.isFullInitialized)
74+
assertEquals("foo", bucket[params[1]])
75+
76+
bucket[params[2]] = "bar"
77+
78+
assertFalse(bucket.isEmpty())
79+
assertEquals(3, bucket.size)
80+
assertTrue(bucket.isFullInitialized)
81+
assertEquals("bar", bucket[params[2]])
82+
}
83+
}
84+
85+
// After support, add a case with a value class.
86+
}

‎src/test/kotlin/com/fasterxml/jackson/module/kotlin/TestCommons.kt

+18
Original file line numberDiff line numberDiff line change
@@ -5,10 +5,28 @@ import com.fasterxml.jackson.core.util.DefaultIndenter
55
import com.fasterxml.jackson.core.util.DefaultPrettyPrinter
66
import com.fasterxml.jackson.databind.ObjectMapper
77
import com.fasterxml.jackson.databind.ObjectWriter
8+
import kotlin.reflect.KParameter
9+
import kotlin.reflect.full.memberProperties
10+
import kotlin.reflect.full.primaryConstructor
11+
import kotlin.test.assertEquals
812

913
// This `printer` is used to match the output from Jackson to the newline char of the source code.
1014
// If this is removed, comparisons will fail in a Windows-like platform.
1115
val LF_PRINTER: PrettyPrinter =
1216
DefaultPrettyPrinter().withObjectIndenter(DefaultIndenter().withLinefeed("\n"))
1317

1418
fun ObjectMapper.testPrettyWriter(): ObjectWriter = this.writer().with(LF_PRINTER)
19+
internal val defaultMapper = jacksonObjectMapper()
20+
21+
internal inline fun <reified T : Any> callPrimaryConstructor(mapper: (KParameter) -> Any? = { it.name }): T =
22+
T::class.primaryConstructor!!.run {
23+
val args = parameters.associateWith { mapper(it) }
24+
callBy(args)
25+
}
26+
27+
// Function for comparing non-data classes.
28+
internal inline fun <reified T : Any> assertReflectEquals(expected: T, actual: T) {
29+
T::class.memberProperties.forEach {
30+
assertEquals(it.get(expected), it.get(actual))
31+
}
32+
}

0 commit comments

Comments
 (0)