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

Significant improvement in deserialization speed #439

Open
wants to merge 15 commits into
base: 2.13
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 11 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
37 changes: 36 additions & 1 deletion pom.xml
Expand Up @@ -114,7 +114,6 @@
</dependencies>

<build>
<sourceDirectory>${project.basedir}/src/main/kotlin</sourceDirectory>
<testSourceDirectory>${project.basedir}/src/test/kotlin</testSourceDirectory>
<plugins>
<plugin>
Expand All @@ -129,6 +128,13 @@
<goals>
<goal>compile</goal>
</goals>
<configuration>
dinomite marked this conversation as resolved.
Show resolved Hide resolved
<sourceDirs>
<source>${project.basedir}/target/generated-sources</source>
<source>${project.basedir}/src/main/java</source>
<source>${project.basedir}/src/main/kotlin</source>
</sourceDirs>
</configuration>
</execution>

<execution>
Expand All @@ -145,10 +151,12 @@
</execution>
</executions>
</plugin>

<plugin>
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-surefire-plugin</artifactId>
</plugin>

<plugin>
<!-- Inherited from oss-base. Generate PackageVersion.java.-->
<groupId>com.google.code.maven-replacer-plugin</groupId>
Expand All @@ -160,6 +168,7 @@
</execution>
</executions>
</plugin>

<plugin>
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-compiler-plugin</artifactId>
Expand All @@ -168,6 +177,32 @@
<showDeprecation>true</showDeprecation>
<showWarnings>true</showWarnings>
</configuration>
<executions>
<!-- Replacing default-compile as it is treated specially by maven -->
<execution>
<id>default-compile</id>
<phase>none</phase>
</execution>
<!-- Replacing default-testCompile as it is treated specially by maven -->
<execution>
<id>default-testCompile</id>
<phase>none</phase>
</execution>
<execution>
<id>java-compile</id>
<phase>compile</phase>
<goals>
<goal>compile</goal>
</goals>
</execution>
<execution>
<id>java-test-compile</id>
<phase>test-compile</phase>
<goals>
<goal>testCompile</goal>
</goals>
</execution>
</executions>
</plugin>

<!-- 02-Nov-2020, tatu: Add JDK9+ module info with Moditect -->
Expand Down
@@ -0,0 +1,47 @@
package com.fasterxml.jackson.module.kotlin;

import org.jetbrains.annotations.NotNull;
cowtowncoder marked this conversation as resolved.
Show resolved Hide resolved
import org.jetbrains.annotations.Nullable;

import java.lang.reflect.Constructor;
import java.lang.reflect.InvocationTargetException;
import java.lang.reflect.Method;

/**
* Wrapper to avoid costly calls using spread operator.
* @since 2.13
*/
dinomite marked this conversation as resolved.
Show resolved Hide resolved
class SpreadWrapper {
public static <T> Constructor<T> getConstructor(
@NotNull Class<T> clazz,
@NotNull Class<?>[] parameterTypes
) throws NoSuchMethodException {
return clazz.getConstructor(parameterTypes);
}

public static <T> T newInstance(
@NotNull Constructor<T> constructor,
@NotNull Object[] initargs
) throws InvocationTargetException, InstantiationException, IllegalAccessException {
return constructor.newInstance(initargs);
}

public static Method getDeclaredMethod(
@NotNull Class<?> clazz,
@NotNull String name,
@NotNull Class<?>[] parameterTypes
) throws NoSuchMethodException {
return clazz.getDeclaredMethod(name, parameterTypes);
}

/**
* Instance is null on static method
*/
public static Object invoke(
@NotNull Method method,
@Nullable Object instance,
@NotNull Object[] args
) throws InvocationTargetException, IllegalAccessException {
return method.invoke(instance, args);
}
}
@@ -0,0 +1,86 @@
package com.fasterxml.jackson.module.kotlin

import kotlin.reflect.KParameter

internal class BucketGenerator(parameters: List<KParameter>) {
private val paramSize: Int = parameters.size
val maskSize = (paramSize / Int.SIZE_BITS) + 1
// For Optional and Primitive types, set the initial value because the function cannot be called if the argument is null.
private val originalValues: Array<Any?> = Array(paramSize) {
val param = parameters[it]

if (param.isOptional) {
ABSENT_VALUE[param.type.erasedType()]
} else {
null
}
}
private val originalMasks: IntArray = IntArray(maskSize) { FILLED_MASK }

fun generate() = ArgumentBucket(paramSize, originalValues.clone(), originalMasks.clone())

companion object {
private const val FILLED_MASK = -1

private val ABSENT_VALUE: Map<Class<*>, Any> = mapOf(
Boolean::class.javaPrimitiveType!! to false,
Char::class.javaPrimitiveType!! to Char.MIN_VALUE,
Byte::class.javaPrimitiveType!! to Byte.MIN_VALUE,
dinomite marked this conversation as resolved.
Show resolved Hide resolved
Short::class.javaPrimitiveType!! to Short.MIN_VALUE,
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why use MIN_VALUE instead of zeros?

Int::class.javaPrimitiveType!! to Int.MIN_VALUE,
Long::class.javaPrimitiveType!! to Long.MIN_VALUE,
Float::class.javaPrimitiveType!! to Float.MIN_VALUE,
Double::class.javaPrimitiveType!! to Double.MIN_VALUE
)
}
}

/**
* Class for managing arguments and their initialization state.
* [masks] is used to manage the initialization state of arguments, and is also a mask to indicate whether to use default arguments in Kotlin.
* For the [masks] bit, 0 means initialized and 1 means uninitialized.
*
* @property values Arguments arranged in order in the manner of a bucket sort.
*/
internal class ArgumentBucket(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this class is complex enough that a unit tests would be helpful, it'd also be great for documenting what it does and how it works

private val paramSize: Int,
val values: Array<Any?>,
private val masks: IntArray
Comment on lines +46 to +48
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing a couple of properties in the javadoc

) {
private var initializedCount: Int = 0

private fun getMaskAddress(index: Int): Pair<Int, Int> = (index / Int.SIZE_BITS) to (index % Int.SIZE_BITS)

/**
* Set the argument. The second and subsequent inputs for the same `index` will be ignored.
*/
operator fun set(index: Int, value: Any?) {
val maskAddress = getMaskAddress(index)

val updatedMask = masks[maskAddress.first] and BIT_FLAGS[maskAddress.second]

if (updatedMask != masks[maskAddress.first]) {
values[index] = value
masks[maskAddress.first] = updatedMask
initializedCount++
}
}

fun isFullInitialized(): Boolean = initializedCount == paramSize

/**
* An array of values to be used when making calls with default arguments.
* The null at the end is a marker for synthetic method.
* @return arrayOf(*values, *masks, null)
*/
fun getValuesOnDefault(): Array<Any?> = values.copyOf(values.size + masks.size + 1).apply {
masks.forEachIndexed { i, mask ->
this[values.size + i] = mask
}
}

companion object {
// List of Int with only 1 bit enabled.
private val BIT_FLAGS: List<Int> = IntArray(Int.SIZE_BITS) { (1 shl it).inv() }.asList()
}
}
@@ -0,0 +1,61 @@
package com.fasterxml.jackson.module.kotlin

import com.fasterxml.jackson.databind.DeserializationContext
import com.fasterxml.jackson.databind.MapperFeature
import com.fasterxml.jackson.module.kotlin.Instantiator.Companion.INT_PRIMITIVE_CLASS
import java.lang.reflect.Constructor
import kotlin.reflect.KFunction
import kotlin.reflect.KParameter

// This class does not support inner constructor.
dinomite marked this conversation as resolved.
Show resolved Hide resolved
internal class ConstructorInstantiator<T>(
kConstructor: KFunction<T>, private val constructor: Constructor<T>
) : Instantiator<T> {
// Top level constructor does not require any instance parameters.
override val hasInstanceParameter: Boolean = false
override val valueParameters: List<KParameter> = kConstructor.parameters
private val accessible: Boolean = constructor.isAccessible
private val bucketGenerator = BucketGenerator(valueParameters)
// This initialization process is heavy and will not be done until it is needed.
private val localConstructor: Constructor<T> by lazy {
val parameterTypes = arrayOf(
*constructor.parameterTypes,
*Array(bucketGenerator.maskSize) { INT_PRIMITIVE_CLASS },
DEFAULT_CONSTRUCTOR_MARKER
)

SpreadWrapper.getConstructor(constructor.declaringClass, parameterTypes)
.apply { isAccessible = true }
}

init {
// Preserve the initial value of Accessibility, and make the entity Accessible.
constructor.isAccessible = true
}

override fun checkAccessibility(ctxt: DeserializationContext) {
if ((!accessible && ctxt.config.isEnabled(MapperFeature.CAN_OVERRIDE_ACCESS_MODIFIERS)) ||
(accessible && ctxt.config.isEnabled(MapperFeature.OVERRIDE_PUBLIC_ACCESS_MODIFIERS))) {
return
}

throw IllegalAccessException("Cannot access to Constructor, instead found ${constructor.declaringClass.name}")
}

override fun generateBucket() = bucketGenerator.generate()

override fun callBy(bucket: ArgumentBucket): T = when (bucket.isFullInitialized()) {
true -> SpreadWrapper.newInstance(constructor, bucket.values)
false -> SpreadWrapper.newInstance(localConstructor, bucket.getValuesOnDefault())
}

companion object {
private val DEFAULT_CONSTRUCTOR_MARKER: Class<*> = try {
Class.forName("kotlin.jvm.internal.DefaultConstructorMarker")
dinomite marked this conversation as resolved.
Show resolved Hide resolved
} catch (ex: ClassNotFoundException) {
throw IllegalStateException(
"DefaultConstructorMarker not on classpath. Make sure the Kotlin stdlib is on the classpath."
)
}
}
}
@@ -0,0 +1,34 @@
package com.fasterxml.jackson.module.kotlin

import com.fasterxml.jackson.databind.DeserializationContext
import kotlin.reflect.KParameter

internal interface Instantiator<T> {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A javadoc explaining what Instantiators are for would be good

val hasInstanceParameter: Boolean

/**
* ValueParameters of the KFunction to be called.
*/
val valueParameters: List<KParameter>

/**
* Checking process to see if access from context is possible.
* @throws IllegalAccessException
*/
fun checkAccessibility(ctxt: DeserializationContext)

/**
* The process of getting the target bucket to set the value.
*/
fun generateBucket(): ArgumentBucket

/**
* Function call from bucket.
* If there are uninitialized arguments, the call is made using the default function.
*/
fun callBy(bucket: ArgumentBucket): T

companion object {
val INT_PRIMITIVE_CLASS: Class<Int> = Int::class.javaPrimitiveType!!
}
}
Expand Up @@ -8,7 +8,8 @@ enum class KotlinFeature(val enabledByDefault: Boolean) {
NullToEmptyMap(enabledByDefault = false),
NullIsSameAsDefault(enabledByDefault = false),
SingletonSupport(enabledByDefault = false),
StrictNullChecks(enabledByDefault = false);
StrictNullChecks(enabledByDefault = false),
ExperimentalDeserializationBackend(enabledByDefault = false);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A quick javadoc for this would be good


internal val bitSet: BitSet = 2.0.pow(ordinal).toInt().toBitSet()
}
Expand Up @@ -31,14 +31,18 @@ fun Class<*>.isKotlinClass(): Boolean {
* the default, collections which are typed to disallow null members
* (e.g. List<String>) may contain null values after deserialization. Enabling it
* protects against this but has significant performance impact.
* @param experimentalDeserializationBackend
* Default: false. Whether to enable experimental deserialization backend. Enabling
* it significantly improve performance in certain use cases.
*/
class KotlinModule @Deprecated(level = DeprecationLevel.WARNING, message = "Use KotlinModule.Builder") constructor(
val reflectionCacheSize: Int = 512,
val nullToEmptyCollection: Boolean = false,
val nullToEmptyMap: Boolean = false,
val nullIsSameAsDefault: Boolean = false,
val singletonSupport: SingletonSupport = DISABLED,
val strictNullChecks: Boolean = false
val strictNullChecks: Boolean = false,
val experimentalDeserializationBackend: Boolean = false
dinomite marked this conversation as resolved.
Show resolved Hide resolved
dinomite marked this conversation as resolved.
Show resolved Hide resolved
) : SimpleModule(PackageVersion.VERSION) {
@Deprecated(level = DeprecationLevel.HIDDEN, message = "For ABI compatibility")
constructor(
Expand Down Expand Up @@ -77,7 +81,8 @@ class KotlinModule @Deprecated(level = DeprecationLevel.WARNING, message = "Use
builder.isEnabled(KotlinFeature.SingletonSupport) -> CANONICALIZE
else -> DISABLED
},
builder.isEnabled(StrictNullChecks)
builder.isEnabled(StrictNullChecks),
builder.isEnabled(KotlinFeature.ExperimentalDeserializationBackend)
)

companion object {
Expand All @@ -95,7 +100,14 @@ class KotlinModule @Deprecated(level = DeprecationLevel.WARNING, message = "Use

val cache = ReflectionCache(reflectionCacheSize)

context.addValueInstantiators(KotlinInstantiators(cache, nullToEmptyCollection, nullToEmptyMap, nullIsSameAsDefault, strictNullChecks))
context.addValueInstantiators(KotlinInstantiators(
cache,
nullToEmptyCollection,
nullToEmptyMap,
nullIsSameAsDefault,
strictNullChecks,
experimentalDeserializationBackend
))

when (singletonSupport) {
DISABLED -> Unit
Expand Down