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

Change priority for PolymorphicSerializer inside serializer() function #2565

Merged
merged 2 commits into from
Mar 19, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions core/api/kotlinx-serialization-core.api
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,8 @@ public abstract interface annotation class kotlinx/serialization/Serializer : ja
}

public final class kotlinx/serialization/SerializersKt {
public static final fun moduleThenPolymorphic (Lkotlinx/serialization/modules/SerializersModule;Lkotlin/reflect/KClass;)Lkotlinx/serialization/KSerializer;
public static final fun moduleThenPolymorphic (Lkotlinx/serialization/modules/SerializersModule;Lkotlin/reflect/KClass;[Lkotlinx/serialization/KSerializer;)Lkotlinx/serialization/KSerializer;
public static final fun noCompiledSerializer (Ljava/lang/String;)Lkotlinx/serialization/KSerializer;
public static final fun noCompiledSerializer (Lkotlinx/serialization/modules/SerializersModule;Lkotlin/reflect/KClass;)Lkotlinx/serialization/KSerializer;
public static final fun noCompiledSerializer (Lkotlinx/serialization/modules/SerializersModule;Lkotlin/reflect/KClass;[Lkotlinx/serialization/KSerializer;)Lkotlinx/serialization/KSerializer;
Expand Down
60 changes: 51 additions & 9 deletions core/commonMain/src/kotlinx/serialization/Serializers.kt
Original file line number Diff line number Diff line change
Expand Up @@ -189,24 +189,45 @@ private fun SerializersModule.serializerByKTypeImpl(
val isNullable = type.isMarkedNullable
val typeArguments = type.arguments.map(KTypeProjection::typeOrThrow)

val cachedSerializer = if (typeArguments.isEmpty()) {
findCachedSerializer(rootClass, isNullable)
val cachedSerializer = if (typeArguments.isEmpty()) {
if (rootClass.isInterface() && getContextual(rootClass) != null) {
// We cannot use cache because it may be contextual non-sealed interface serializer,
// but we cannot return result of getContextual() directly either, because rootClass
// can be a sealed interface as well (in that case, rootClass.serializerOrNull() should have priority over getContextual()).
// If we had function like KClass.isNonSealedInterface() we could optimize this place,
// but Native does not provide enough reflection for that. (https://youtrack.jetbrains.com/issue/KT-41339)
null
} else {
findCachedSerializer(rootClass, isNullable)
}
} else {
findParametrizedCachedSerializer(rootClass, typeArguments, isNullable).getOrNull()
// We cannot enable cache even if the current class is non-interface, as it may have interface among type arguments
// and we do not want to waste time scanning them all.
if (hasInterfaceContextualSerializers) {
null
} else {
findParametrizedCachedSerializer(
rootClass,
typeArguments,
isNullable
).getOrNull()
}
}
cachedSerializer?.let { return it }

if (cachedSerializer != null) return cachedSerializer

// slow path to find contextual serializers in serializers module
val contextualSerializer: KSerializer<out Any?>? = if (typeArguments.isEmpty()) {
getContextual(rootClass)
rootClass.serializerOrNull()
?: getContextual(rootClass)
?: rootClass.polymorphicIfInterface()
} else {
val serializers = serializersForParameters(typeArguments, failOnMissingTypeArgSerializer) ?: return null
// first, we look among the built-in serializers, because the parameter could be contextual
rootClass.parametrizedSerializerOrNull(serializers) { typeArguments[0].classifier }
?: getContextual(
rootClass,
serializers
)
?: getContextual(rootClass, serializers)
// PolymorphicSerializer always is returned even for Interface<T>, although it rarely works as expected.
?: rootClass.polymorphicIfInterface()
}
return contextualSerializer?.cast<Any>()?.nullable(isNullable)
}
Expand Down Expand Up @@ -376,3 +397,24 @@ internal fun noCompiledSerializer(
): KSerializer<*> {
return module.getContextual(kClass, argSerializers.asList()) ?: kClass.serializerNotRegistered()
}

/**
* Overloads of [moduleThenPolymorphic] should never be called directly.
* Instead, compiler inserts calls to them when intrinsifying [serializer] function.
*
* If no request KClass is an interface, plugin performs call to [moduleThenPolymorphic] to achieve special behavior for interface serializers.
* (They are only serializers that have module priority over default [PolymorphicSerializer]).
*/
@OptIn(ExperimentalSerializationApi::class)
@Suppress("unused")
@PublishedApi
internal fun moduleThenPolymorphic(module: SerializersModule, kClass: KClass<*>): KSerializer<*> {
sandwwraith marked this conversation as resolved.
Show resolved Hide resolved
return module.getContextual(kClass) ?: PolymorphicSerializer(kClass)
}

@OptIn(ExperimentalSerializationApi::class)
@Suppress("unused")
@PublishedApi
internal fun moduleThenPolymorphic(module: SerializersModule, kClass: KClass<*>, argSerializers: Array<KSerializer<*>>): KSerializer<*> {
return module.getContextual(kClass, argSerializers.asList()) ?: PolymorphicSerializer(kClass)
}
8 changes: 6 additions & 2 deletions core/commonMain/src/kotlinx/serialization/SerializersCache.kt
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
package kotlinx.serialization

import kotlinx.serialization.builtins.nullable
import kotlinx.serialization.internal.*
import kotlinx.serialization.internal.cast
import kotlinx.serialization.internal.createCache
import kotlinx.serialization.internal.createParametrizedCache
Expand All @@ -18,13 +19,13 @@ import kotlin.reflect.KType
* Cache for non-null non-parametrized and non-contextual serializers.
*/
@ThreadLocal
private val SERIALIZERS_CACHE = createCache { it.serializerOrNull() }
private val SERIALIZERS_CACHE = createCache { it.serializerOrNull() ?: it.polymorphicIfInterface() }
sandwwraith marked this conversation as resolved.
Show resolved Hide resolved

/**
* Cache for nullable non-parametrized and non-contextual serializers.
*/
@ThreadLocal
private val SERIALIZERS_CACHE_NULLABLE = createCache<Any?> { it.serializerOrNull()?.nullable?.cast() }
private val SERIALIZERS_CACHE_NULLABLE = createCache<Any?> { (it.serializerOrNull() ?: it.polymorphicIfInterface())?.nullable?.cast() }

/**
* Cache for non-null parametrized and non-contextual serializers.
Expand Down Expand Up @@ -72,3 +73,6 @@ internal fun findParametrizedCachedSerializer(
PARAMETRIZED_SERIALIZERS_CACHE_NULLABLE.get(clazz, types)
}
}

@Suppress("NOTHING_TO_INLINE")
internal inline fun KClass<*>.polymorphicIfInterface() = if (this.isInterface()) PolymorphicSerializer(this) else null
Original file line number Diff line number Diff line change
Expand Up @@ -141,6 +141,8 @@ internal expect fun BooleanArray.getChecked(index: Int): Boolean

internal expect fun <T : Any> KClass<T>.compiledSerializerImpl(): KSerializer<T>?

internal expect fun <T: Any> KClass<T>.isInterface(): Boolean

/**
* Create serializers cache for non-parametrized and non-contextual serializers.
* The activity and type of cache is determined for a specific platform and a specific environment.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,9 @@ public sealed class SerializersModule {
*/
@ExperimentalSerializationApi
public abstract fun dumpTo(collector: SerializersModuleCollector)

@InternalSerializationApi
internal abstract val hasInterfaceContextualSerializers: Boolean
}

/**
Expand All @@ -76,7 +79,14 @@ public sealed class SerializersModule {
level = DeprecationLevel.WARNING,
replaceWith = ReplaceWith("EmptySerializersModule()"))
@JsName("EmptySerializersModuleLegacyJs") // Compatibility with JS
public val EmptySerializersModule: SerializersModule = SerialModuleImpl(emptyMap(), emptyMap(), emptyMap(), emptyMap(), emptyMap())
public val EmptySerializersModule: SerializersModule = SerialModuleImpl(
emptyMap(),
emptyMap(),
emptyMap(),
emptyMap(),
emptyMap(),
false
)

/**
* Returns a combination of two serial modules
Expand Down Expand Up @@ -147,7 +157,8 @@ internal class SerialModuleImpl(
@JvmField val polyBase2Serializers: Map<KClass<*>, Map<KClass<*>, KSerializer<*>>>,
private val polyBase2DefaultSerializerProvider: Map<KClass<*>, PolymorphicSerializerProvider<*>>,
private val polyBase2NamedSerializers: Map<KClass<*>, Map<String, KSerializer<*>>>,
private val polyBase2DefaultDeserializerProvider: Map<KClass<*>, PolymorphicDeserializerProvider<*>>
private val polyBase2DefaultDeserializerProvider: Map<KClass<*>, PolymorphicDeserializerProvider<*>>,
internal override val hasInterfaceContextualSerializers: Boolean
) : SerializersModule() {

override fun <T : Any> getPolymorphic(baseClass: KClass<in T>, value: T): SerializationStrategy<T>? {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ public class SerializersModuleBuilder @PublishedApi internal constructor() : Ser
private val polyBase2DefaultSerializerProvider: MutableMap<KClass<*>, PolymorphicSerializerProvider<*>> = hashMapOf()
private val polyBase2NamedSerializers: MutableMap<KClass<*>, MutableMap<String, KSerializer<*>>> = hashMapOf()
private val polyBase2DefaultDeserializerProvider: MutableMap<KClass<*>, PolymorphicDeserializerProvider<*>> = hashMapOf()
private var hasInterfaceContextualSerializers: Boolean = false

/**
* Adds [serializer] associated with given [kClass] for contextual serialization.
Expand Down Expand Up @@ -155,6 +156,7 @@ public class SerializersModuleBuilder @PublishedApi internal constructor() : Ser
}
}
class2ContextualProvider[forClass] = provider
if (forClass.isInterface()) hasInterfaceContextualSerializers = true
}

@JvmName("registerDefaultPolymorphicSerializer") // Don't mangle method name for prettier stack traces
Expand Down Expand Up @@ -229,7 +231,7 @@ public class SerializersModuleBuilder @PublishedApi internal constructor() : Ser

@PublishedApi
internal fun build(): SerializersModule =
SerialModuleImpl(class2ContextualProvider, polyBase2Serializers, polyBase2DefaultSerializerProvider, polyBase2NamedSerializers, polyBase2DefaultDeserializerProvider)
SerialModuleImpl(class2ContextualProvider, polyBase2Serializers, polyBase2DefaultSerializerProvider, polyBase2NamedSerializers, polyBase2DefaultDeserializerProvider, hasInterfaceContextualSerializers)
}

/**
Expand Down
Loading