Use "call" instead of "callBy" as much as possible to speed up the call to "KFunction".

[k163377]

Use case
If the arguments are fully specified (= don't have to use Default arguments), can speed up the call to jackson-module-kotlin by making a call with KFunction.call.

Describe the solution you'd like
Currently jackson-module-kotlin does mapping by calling KFunction.callBy.
On the other hand, KFunction.callBy has about 1/5 the performance of KFunction.call, so you can expect speedup by doing KFunction.call as much as possible.

Describe alternatives you've considered
For example, you can switch between call and callBy with low overhead by using the following data structure.

import kotlin.reflect.KParameter

internal class ArgumentBucket(private val parameters: List<KParameter>) : Map<KParameter, Any?> {
    val valueArray: Array<Any?> = arrayOfNulls(parameters.size)
    private val initializationStatuses: MutableSet<Int> = HashSet(parameters.size)

    class Entry internal constructor(
            override val key: KParameter,
            override var value: Any?
    ) : Map.Entry<KParameter, Any?>

    operator fun set(key: KParameter, value: Any?): Any? {
        return valueArray[key.index].apply {
            valueArray[key.index] = value
            initializationStatuses.add(key.index)
        }
    }

    fun isFullInitialized(): Boolean = parameters.size == initializationStatuses.size

    override val entries: Set<Map.Entry<KParameter, Any?>>
        get() = valueArray.withIndex()
                .filter { (i, _) -> initializationStatuses.contains(i) }
                .map { (i, arg) -> Entry(parameters[i], arg) }
                .toSet()
    override val keys: Set<KParameter>
        get() = parameters
                .filterIndexed { i, _ -> initializationStatuses.contains(i) }
                .toSet()
    override val size: Int
        get() = initializationStatuses.size
    override val values: Collection<Any?>
        get() = valueArray.filterIndexed { i, _ -> initializationStatuses.contains(i) }

    override fun containsKey(key: KParameter): Boolean = initializationStatuses.contains(key.index)

    override fun containsValue(value: Any?): Boolean = valueArray.withIndex()
            .any { (i, arg) -> initializationStatuses.contains(i) && value == arg }

    override fun get(key: KParameter): Any? = valueArray[key.index]

    override fun isEmpty(): Boolean = initializationStatuses.isEmpty()
}

This is used as follows (rewriting from line 134 of KotlinValueInstantiator.kt).

ArgumentBucket(callable.parameters).apply {
    callableParameters.forEachIndexed { idx, paramDef ->
        if (paramDef != null) {
            this[paramDef] = jsonParamValueList[idx]
        }
    }
}.let {
    if (it.isFullInitialized())
        callable.call(*it.valueArray)
    else
        callable.callBy(it)
}

Assuming that there are few situations where Default arguments is needed, I think that this alone can speed up the process.

Additional context
I have published the contents as ProjectMapK/FastKFunction that incorporates further speed-up methods such as avoiding the spread operator and calling Java reflection directly, so I hope you can refer to that as well.

This is my first time posting issue and I am not good at English, so I'm sorry if there is something wrong with it.

[dinomite]

Great idea and your English is perfectly easy to understand.

We've got some performance tests for Jackson and some third party performance tests, neither of which I've ever run but would be interesting to see how this changes affects them.

[cowtowncoder]

@dinomite It would be interesting to create Kotlin variant(s) of tests for jackson-benchmarks.
The simplest possible case would just register module for regular POJOs, but that would probably not be very useful as most usage is for Kotlin value types (and conversely performance of just having module should be close to or identical to "plain Java" case).
But it should not be super difficult to create parallel type hierarchy for MediaItem, although some refactoring of types might make sense to reduce duplication: tests themselves are quite adaptable.

I think having such variants would be quite useful and I can help if you think that makes sense.

[k163377]

@dinomite
Thanks for listening to my opinion.

What evidence do I need to incorporate this change?

If needs only the simple JMH Benchmark (e.g. deserializing flat JSON), I'll create it soon and show it with a pull request.
But, it is difficult to create an exhaustive Benchmark.

[dinomite]

Running jackson-benchmarks with the Kotlin module involved (which will require writing a new test in that project) before & after the change is probably sufficient to demonstrate this. I'm open to other suggestions as well, perhaps a performance test akin to those in jackson-benchmarks but in this module?

[cowtowncoder]

For jmh tests I would suggest running under different profile (if in main repo), or changing this to multi-Maven module repo.

On jackson-benchmarks: adding a simple test with Kotlin module is enabled is easy, but probably would not exercise much of module functionality if using POJOs. So alternative with... POKOs? .. would probably make sense. Just need Kotlin-native variant with equivalent content, but also some changes to pom.xml probably. Not trivial task but quite doable.
Would be great to have, too, I wish I had time to tackle it. But I can at least help.

[k163377]

@dinomite @cowtowncoder
For verification purposes, I created a simple Benchmark for jackson 2.12.0 at the beginning (apologies if I misread the message).

• k163377/jackson-kotlin-benchmark-sample at 5c3ac857ad221949b2d59029013f51185a382478

data class TargetClazz(val foo: Int, val bar: Int, val baz: Int, val qux: Int, val quux: Int) {
    companion object {
        @JvmStatic
        @JsonCreator
        fun creator(
            @JsonProperty("foo") foo: Int,
            @JsonProperty("bar") bar: Int,
            @JsonProperty("baz") baz: Int,
            @JsonProperty("qux") qux: Int,
            @JsonProperty("quux") quux: Int
        ) = TargetClazz(foo, bar, baz, qux, quux)
    }
}

@State(Scope.Benchmark)
open class Benchmarks {
    private val javaMapper = ObjectMapper()
    private val kotlinMapper = ObjectMapper().registerKotlinModule()

    private val expected = TargetClazz(1, 2, 3, 4, 5)
    private val input: ByteArray = javaMapper.writeValueAsBytes(expected)

    @Benchmark
    fun useJavaMapper(): TargetClazz = javaMapper.readValue(input, TargetClazz::class.java)

    @Benchmark
    fun useKotlinMapper(): TargetClazz = kotlinMapper.readValue(input, TargetClazz::class.java)
}

The comparison is a slightly modified version of what I posted in this issue.

• k163377/jackson-module-kotlin at argument-bucket-feature
• diff

The results of the Benchmark are as follows(Higher is better).
This is the results of running it on Windows 10 on Ryzen7 3700X.

before:

Benchmark                    Mode  Cnt        Score       Error  Units
Benchmarks.useJavaMapper    thrpt    9  3001549.012 ± 81703.791  ops/s
Benchmarks.useKotlinMapper  thrpt    9   653588.903 ± 15125.254  ops/s

after:

Benchmark                    Mode  Cnt        Score        Error  Units
Benchmarks.useJavaMapper    thrpt    9  3095619.511 ± 103716.171  ops/s
Benchmarks.useKotlinMapper  thrpt    9   807449.049 ±  10641.585  ops/s

options

jmh {
    fork = 3
    iterations = 3
    warmupBatchSize = 3
    warmupIterations = 3

    failOnError = true
    isIncludeTests = false

    resultFormat = "CSV"
}

(I thought this solution would work for the whole mapping, but I found out that it actually only works for patterns like reading the Factory Method in JsonCreator.)

[k163377]

I looked through the jackson-benchmarks, but there is no evidence of using JsonCreator, and I don't know if it can verify what I suggested.

(Although it is off the subject, by converting com.fasterxml.jackson.perf.model of jackson-benchmarks to Kotlin and using Kotlin no-arg plugin, I thought I could use the same POKO for both Java and Kotlin Benchmarks.)

[cowtowncoder]

@k163377 correct: there isn't yet any use of @JsonCreator since that has performance overhead compared to the original setter or field-based method (which used to be the main mechanism with Java, years ago; lately there has been movement towards constructor-based variants). But it would be good to add alternative mechanisms: I just do not want to only use constructor-passing since for the maximum performance users probably want to know both the fastest variant and relative performance overhead with constructor passing.
But for Kotlin usage it is definitely important to have the common mechanism available for benchmarking.

Overhead for constructor-passing variant comes from couple of things, including need to buffer values (instead directly assigning), and in case of Afterburner/Blackburner their inability (for now, at least) to optimize call of non-default constructors.

[k163377]

Created a temporary pull request.
#408

[k163377]

@cowtowncoder @dinomite
I found a way to improve performance over this suggestion, and I created a PR.
#439

Would it be better to close this issue?

[dinomite]

I'll go ahead and close this, we can track in the PRs.