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

Inlined FrameworkMarshallers.create spawns more classes in Metaspace than you could imagine #375

Open
jreznot opened this issue Jan 22, 2023 · 2 comments
Assignees

Comments

@jreznot
Copy link
Member

jreznot commented Jan 22, 2023

object FrameworkMarshallers {
    inline fun <reified T : Any> create(crossinline reader: (AbstractBuffer) -> T, crossinline writer: (AbstractBuffer, T) -> Unit, predefinedId: Int? = null): UniversalMarshaller<T> {
        return UniversalMarshaller(T::class, { _, stream -> reader(stream) }, { _, stream, v -> writer(stream, v) }, predefinedId)
    }

Check this:

 { _, stream -> reader(stream) }

This means that for every callsite of FrameworkMarshallers.create there will be separate class created. Our JVM classloading logs for IntelliJ IDA shows clearly:

[1.35760] Class prepared: com/jetbrains/rd/framework/FrameworkMarshallers$special$$inlined$create$39
[1.35764] Loading class: com/jetbrains/rd/framework/FrameworkMarshallers$special$$inlined$create$40 (2724 bytes)
[1.35772] Class prepared: com/jetbrains/rd/framework/FrameworkMarshallers$special$$inlined$create$40
[1.35778] Loading class: com/jetbrains/rd/framework/FrameworkMarshallers$special$$inlined$create$41 (2583 bytes)
[1.35786] Class prepared: com/jetbrains/rd/framework/FrameworkMarshallers$special$$inlined$create$41
[1.35790] Loading class: com/jetbrains/rd/framework/FrameworkMarshallers$special$$inlined$create$42 (2726 bytes)
[1.35797] Class prepared: com/jetbrains/rd/framework/FrameworkMarshallers$special$$inlined$create$42
[1.35803] Loading class: com/jetbrains/rd/framework/FrameworkMarshallers$special$$inlined$create$43 (2585 bytes)
[1.35810] Class prepared: com/jetbrains/rd/framework/FrameworkMarshallers$special$$inlined$create$43
[1.35814] Loading class: com/jetbrains/rd/framework/FrameworkMarshallers$special$$inlined$create$44 (2728 bytes)
[1.35820] Class prepared: com/jetbrains/rd/framework/FrameworkMarshallers$special$$inlined$create$44
[1.35826] Loading class: com/jetbrains/rd/framework/FrameworkMarshallers$special$$inlined$create$45 (2581 bytes)
[1.35832] Class prepared: com/jetbrains/rd/framework/FrameworkMarshallers$special$$inlined$create$45
[1.35836] Loading class: com/jetbrains/rd/framework/FrameworkMarshallers$special$$inlined$create$46 (2724 bytes)
[1.35844] Class prepared: com/jetbrains/rd/framework/FrameworkMarshallers$special$$inlined$create$46
[1.35849] Loading class: com/jetbrains/rd/framework/FrameworkMarshallers$special$$inlined$create$47 (2587 bytes)
[1.35856] Class prepared: com/jetbrains/rd/framework/FrameworkMarshallers$special$$inlined$create$47
[1.35859] Loading class: com/jetbrains/rd/framework/FrameworkMarshallers$special$$inlined$create$48 (2730 bytes)
[1.35866] Class prepared: com/jetbrains/rd/framework/FrameworkMarshallers$special$$inlined$create$48
[1.35900] Loading class: com/jetbrains/rd/framework/FrameworkMarshallers$special$$inlined$create$49 (2667 bytes)
[1.35907] Class prepared: com/jetbrains/rd/framework/FrameworkMarshallers$special$$inlined$create$49
[1.35911] Loading class: com/jetbrains/rd/framework/FrameworkMarshallers$special$$inlined$create$50 (2794 bytes)
[1.35917] Class prepared: com/jetbrains/rd/framework/FrameworkMarshallers$special$$inlined$create$50
@jreznot
Copy link
Member Author

jreznot commented Jan 22, 2023

I would recommend noinline for lambdas, we cannot eliminate them fully anyway, as UniversalMarshaller needs to store some instances:

    inline fun <reified T : Any> create(noinline reader: (AbstractBuffer) -> T, 
                                        noinline writer: (AbstractBuffer, T) -> Unit, 
                                        predefinedId: Int? = null): UniversalMarshaller<T> {
        return UniversalMarshaller(T::class, { _, stream -> reader(stream) }, { _, stream, v -> writer(stream, v) }, predefinedId)
    }

@ForNeVeR ForNeVeR self-assigned this Jan 22, 2023
@jreznot jreznot changed the title Inlined FrameworkMarshallers.create spawns more classes in Metaspace when you could imagine Inlined FrameworkMarshallers.create spawns more classes in Metaspace than you could imagine Jan 23, 2023
@ForNeVeR
Copy link
Collaborator

A proposed solution that should work in Kotlin 1.9 (currently, it generates exactly the same number of classes):

    inline fun <reified T : Any> create(
        noinline reader: (AbstractBuffer) -> T,
        noinline writer: (AbstractBuffer, T) -> Unit,
        predefinedId: Int? = null
    ): UniversalMarshaller<T> {
        return create(T::class, reader, writer, predefinedId)
    }

    /**
     * This is a non-inline companion of the above function, to avoid creating too many class files for crossinline
     * lambdas.
     */
    fun <T : Any> create(
        kClass: KClass<T>,
        reader: (AbstractBuffer) -> T,
        writer: (AbstractBuffer, T) -> Unit,
        predefinedId: Int? = null
    ): UniversalMarshaller<T> {
        return UniversalMarshaller(
            kClass,
            { _, stream -> reader(stream) },
            { _, stream, v -> writer(stream, v) },
            predefinedId
        )
    }

So, to fix this issue, we'll have to wait for Kotlin 1.9 with extended invokedynamic usage.

We will be waiting.

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

No branches or pull requests

2 participants