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

Serializing object classes is 10x slower than data classes #2003

Closed
chrisbanes opened this issue Aug 2, 2022 · 6 comments
Closed

Serializing object classes is 10x slower than data classes #2003

chrisbanes opened this issue Aug 2, 2022 · 6 comments

Comments

@chrisbanes
Copy link

chrisbanes commented Aug 2, 2022

Copying over my Android Microbenchmarks results (ran on Google Pixel 3):

Benchmark Time (ns) Allocations
object_reflection_serializer 56,077 27
object_reflection_json 69,069 40
object_intrinsic_serializer 56,211 30
object_intrinsic_json 118,768 73
dataclass_reflection_serializer 2,450 8
dataclass_reflection_json 6,462 28
dataclass_intrinsic_serializer 2,687 11
dataclass_intrinsic_json 9,927 42

Also interesting is that the intrinsic is usually (marginally) slower than using reflection.

To Reproduce
All benchmarks are here: https://github.com/chrisbanes/kotlin-serialization-object-perf

Expected behavior
I'd expect objects to be faster than data classes (or at least similar).

Environment

  • Kotlin version: 1.7.0
  • Library version: 1.3.3 (1.4-RC is very similar numbers though)
  • Kotlin platforms: JVM/Android
@qwwdfsad
Copy link
Member

qwwdfsad commented Aug 2, 2022

The problem has multiple layers in it.

First of all, intrinsics are not yet implemented (#1348), which is our biggest issue right now. I suggest reading through the issue to understand why it's important.

The second problem is that we have seemingly the same code having a significant difference in its performance characteristics.

All lookups in all benchmarks use reflection under the hood and use basically the same code path (more or less).
Optimistically, we assume that data classes are serialized much more frequently, thus it is prioritized on the serializer() code path and will always be faster.

Though, it shouldn't be ten times faster. I've ran my own benchmarks under the profiler and pin-pointed the root cause:

private fun <T : Any> invokeSerializerOnCompanion(jClass: Class<*>, vararg args: KSerializer<Any?>): KSerializer<T>? {
val companion = jClass.companionOrNull() ?: return null
return try {
val types = if (args.isEmpty()) emptyArray() else Array(args.size) { KSerializer::class.java }
companion.javaClass.getDeclaredMethod("serializer", *types)
.invoke(companion, *args) as? KSerializer<T>
} catch (e: NoSuchMethodException) {
null
} catch (e: InvocationTargetException) {
val cause = e.cause ?: throw e
throw InvocationTargetException(cause, cause.message ?: e.message)
}

The problem here is simple -- we either have to use getDeclaredMethod and pay for it throwing an exception if the companion doesn't have serializer method (which is the case for object!), thus x10 performance loss on "uncommon path".

Or, we can use getDeclaredMethods().find { ... } as we did before (#952). Then, lookup of object serializer will be as good as lookup of regular class serializer. The problem is, regular lookups will become at least 2.5 times slower.

Java API won't let us do any better here, so this is a tradeoff we are willing to accept. Hopefully, proper intrinsics (#1348) will make all these problems disappear, so closing as "won't fix"

@qwwdfsad qwwdfsad closed this as completed Aug 2, 2022
@qwwdfsad
Copy link
Member

qwwdfsad commented Aug 2, 2022

Just in case, flamegraph of object serializer lookup:

image

@chrisbanes
Copy link
Author

Optimistically, we assume that data classes are serialized much more frequently

Yup, makes total sense at the library level. Thanks for attaching the flame graph. Very interesting!

For now, we're going to workaround this by checking for data::class.objectInstance and avoiding any serializer work when there is an instance. That has a cost, but it is miniscule compared to the serializer (370ns vs 56,000ns with my testing)

@qwwdfsad
Copy link
Member

qwwdfsad commented Aug 2, 2022

Thanks for reporting!

This report got #2004 rolling as well as our work on speeding up some [obvious] kotlin-reflect parts

@sandwwraith
Copy link
Member

NB: You can also use generated serializer() method whenever possible (e.g. ObjectArgs.serializer()) to eliminate any reflection at all

@qwwdfsad
Copy link
Member

qwwdfsad commented Aug 9, 2022

I've tweaked reflection here and there (https://youtrack.jetbrains.com/issue/KT-50705/Use-ClassValue-to-cache-KClass-objects-in-kotlin-reflect and subtasks) and now there is no Kotlin-specific reflection whatsoever. The changes are targeting 1.8.0 release.

Along with #2004 (@shanshin works on that right now), we expect the difference between reified and .serialized() version to be almost negligible

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants