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

Optimize JsonElement #2791

Open
swankjesse opened this issue Aug 21, 2024 · 1 comment
Open

Optimize JsonElement #2791

swankjesse opened this issue Aug 21, 2024 · 1 comment
Labels

Comments

@swankjesse
Copy link

swankjesse commented Aug 21, 2024

I’m using kotlinx.serialization on Kotlin/JS. I wrote some code that did something like this:

    val value = 1L
    val jsonElement = Json.encodeToJsonElement(value)
    val dynamic = Json.encodeToDynamic(jsonElement)

(In my actual code, each of these 3 lines happen in different parts of the application. And in my actual code, the value is a more sophisticated data class.)

When I ran this code, it was super slow! In particular I found that a severe bottleneck for my entire program was encoding 1L as a string, and then converting it back to a number.

In particular, this code in JsonElementSerializer was a hot spot:

https://github.com/Kotlin/kotlinx.serialization/blob/master/formats/json/commonMain/src/kotlinx/serialization/json/JsonElementSerializers.kt#L120-L122

        // use .content instead of .longOrNull as latter can process exponential notation,
        // and it should be delegated to double when encoding.
        value.content.toLongOrNull()?.let { return encoder.encodeLong(it) }

This is partly because Long is inefficient on Kotlin/JS, and partly because String.toLongOrNull() does a lot of work.

I was able to get dramatically better performance by skipping the JsonElement intermediate model:

    val value = 1L
    val dynamic = Json.encodeToDynamic(value)

Here’s the actual PR if you’re curious.

I would like for JsonElement to be a low-cost abstraction. In particular I believe the before & after samples above should have similar performance.

@sandwwraith
Copy link
Member

Thanks for your investigation! I'll see what I can do with it. Since Double formatting is already different on JS, maybe we'll be able to make this part platform-specific

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

No branches or pull requests

2 participants