spring-framework icon indicating copy to clipboard operation
spring-framework copied to clipboard

Optimize Kotlin reflection runtime efficiency

Open spring-projects-issues opened this issue 7 years ago • 33 comments
trafficstars

kotlin-reflect is a big 2 MB JAR, producing important CPU and memory spikes at startup on the JVM.

It would be interesting to explore if it could be replaced by a lighter incarnation like https://github.com/Kotlin/kotlinx.reflect.lite or use kotlinx-metadata-jvm at build time to generate some kind of reflection index. jackson-module-kotlin should also work with similar approach (not blocking now that we have Kotlin Serialization support).

spring-projects-issues avatar Jul 04 '18 15:07 spring-projects-issues

Short term kotlin-reflect usage could be optimized, especially for native use case, by KT-44594 so feel free to vote for it on Kotlin issue tracker.

sdeleuze avatar Jan 28 '21 14:01 sdeleuze

After a deeper look, I am not sure kotlinx-metadata-jvm is suitable for our use case since it would require an alternative reflection metadata index. Also kotlin-reflect is conceptually ok for native since it is using Java reflection (which can be quite cheap for some use cases with GraalVM 21.3+) and not class resources (not available on native at runtime).

So my hope for the future is that we could collaborate with Kotlin team (cc @udalov @elizarov) to have a more optimized version of kotlin-reflect.jar. I does not have to be the full beast, maybe a lighter implementation of a subset (we only use a very small subset of it, see https://github.com/spring-projects/spring-framework/search?q=ReflectJvmMapping) inspired from https://github.com/Kotlin/kotlinx.reflect.lite. Notice that should use regular reflection not *.class resources because those are not available on native.

Could be useful for Jackson Kotlin support as well.

That would be a good fit with Spring Framework 6.

sdeleuze avatar Oct 28 '21 09:10 sdeleuze

See also this related comment https://github.com/Kotlin/kotlinx.reflect.lite/pull/12#issuecomment-954900640 and this one on KT-11386.

sdeleuze avatar Oct 29 '21 17:10 sdeleuze

I have released a project to remove kotlin-reflect from jackson-module-kotlin and replace it with kotlinx.metadata.jvm and will share it here as well. https://github.com/FasterXML/jackson-module-kotlin/issues/450#issuecomment-1382827177

k163377 avatar Jan 14 '23 16:01 k163377

I guess it would be interesting to create a Spring Framework branch that uses this experimental library and migrate Spring own usage of kotlin-reflect to use kotlinx.metadata.jvm to measure the gains. If the results are good, we could then sync with @cowtowncoder to see if we could target something non experimental releases of Jackson and Spring that uses the new library. Open to get some help on this.

sdeleuze avatar Jan 16 '23 18:01 sdeleuze

I would be interested in this as well, the problem being that right now none of official Kotlin module maintainers appears to be active. But if @k163377 was interested in becoming one, perhaps we could get this integrated, assuming that constraints for it (wrt JDK compatibility level etc) are reasonable (which is probably something to discuss with community via jackson-dev mailing list).

cowtowncoder avatar Jan 16 '23 23:01 cowtowncoder

Before we proceed with the migration, do we need to decide on a policy for how Spring will remove kotlin-reflect? Currently, I think there are two options: use kotlin-reflect-lite or use kotlinx-metadata-jvm.

Also, even if we use kotlinx-metadata-jvm, we need to consider the timing. The size of kotlinx-metadata-jvm is 1MB, which is not light. Even if we migrate only one of them, I think the total size will only increase by 1MB. I think it is necessary to migrate at least one of them at the same time, or at least the Spring side should be migrated first.

But if @k163377 was interested in becoming one

I am interested in becoming a maintainer, but I don't know what to do. I can only write in English with the help of machine translation, so I am particularly anxious about communication.

k163377 avatar Jan 17 '23 02:01 k163377

Oops, I missed that kotlin-reflect-lite uses kotlinx-metadata-jvm. https://github.com/Kotlin/kotlinx.reflect.lite/blob/93850b23d20242919c37c603b7f52b4535d5d907/build.gradle.kts#L26

Am I correct in assuming that importing 39bed4d will solve this problem?

k163377 avatar Jan 17 '23 14:01 k163377

@k163377 Indeed kotlin-reflect-lite leverages kotlinx-metadata-jvm and have an API pretty close to kotlin-reflect. I have rebased the original commit from @mvicsokolova you mentioned on top of main and solved related conflict, so you can use https://github.com/sdeleuze/spring-framework/commit/gh-21546 for your tests.

The question is to know if JetBrains intends to go out of experimental with kotlin-reflect-lite or not short term, since on https://github.com/Kotlin/kotlinx.reflect.lite/ it is mentioned "The libary is in purely experimental pre-alpha state.". I will reach them to discuss if we should leverage kotlin-reflect-lite (easier migration but currently alpha) or directly kotlinx-metadata-jvm (on its way to become stable but more work to do to migrate).

sdeleuze avatar Jan 18 '23 08:01 sdeleuze

@cowtowncoder

But if @k163377 was interested in becoming one

I have posted a reply on the jackson mailing list, could you please check?

k163377 avatar Jan 21 '23 07:01 k163377

@k163377 Ok, I was not 100% sure about what to discuss, but given context I hope you were indicating you would be interested in becoming a maintainer? I will send a response in that case. :)

cowtowncoder avatar Jan 23 '23 00:01 cowtowncoder

@cowtowncoder

you would be interested in becoming a maintainer?

Yes.

k163377 avatar Jan 23 '23 03:01 k163377

Ok let's make that happen then. I think you have contributed a lot and have a lot more to contribute!

cowtowncoder avatar Jan 24 '23 04:01 cowtowncoder

@cowtowncoder @k163377 Could you please share a status on Jackson side related to this topic to allow us to evaluate our options for Spring Framework 6.1?

sdeleuze avatar Apr 03 '23 08:04 sdeleuze

I will let @k163377 comment.

But fwtw, Jackson 2.15.0 is getting ready for release so whatever jackson-module-kotlin currently has is likely to be what is in 2.15.0 -- other developments past 2.15.

cowtowncoder avatar Apr 03 '23 23:04 cowtowncoder

@sdeleuze @cowtowncoder As for jackson-module-kotlin, I will not merge that change until 2.16 at the earliest. I don't know when Jackson and Spring Framework are scheduled to be released, but will Jackson 2.16 be ready for Spring Framework 6.1?

The jackson-module-kogera is always available, but there are still some untested items and it has not been deployed to Central.

k163377 avatar Apr 04 '23 11:04 k163377

Given our development cycle, I think we would need the Jackson support in June, first half of July the latest, to make it happen. If you think that's too short, we can move that feature to the next Framework release.

sdeleuze avatar Apr 05 '23 08:04 sdeleuze

I am not aware of jacksons release cycle, but perhaps 2.16 will not be ready in time for July. Please let me aim to make it in time for 6.2(?).

k163377 avatar Apr 05 '23 13:04 k163377

Yeah looks more reasonable to me I don't want to rush it.

Any thoughts @cowtowncoder?

sdeleuze avatar Apr 05 '23 15:04 sdeleuze

@sdeleuze Yeah I do not think Jackson 2.16 will be ready by July.

cowtowncoder avatar Apr 05 '23 21:04 cowtowncoder

Ok, I moved this issue to the 6.x backlog, that will let the time to Jackson to evolve to support the new Kotlin reflection, then we will re-evaluate this after Spring Framewor 6.1.

sdeleuze avatar Apr 06 '23 08:04 sdeleuze

After more thoughts, I begin to be increasingly convinced that if we do something here, I think that should be with kotlinx.reflect.lite using a subset of the current kotlin-reflect otherwise the migration pain portfolio wide will be too hard.

Would somebody from the community have the bandwidth and be interested to help moving that topic forward by replacing kotlin-reflect by org.jetbrains.kotlinx:kotlinx.reflect.lite:1.1.0 in a basic Spring + Kotlin webapp, and provide a feedback on what works/does not work, as well as provide data points to see if that provide a real footprint improvement.

Those information would be useful feedback for Kotlin team to make kotlinx.reflect.lite more first class. If there is no interest, maybe we should just close this issue. Let see!

sdeleuze avatar Jan 12 '24 10:01 sdeleuze

I would like to work on this issue. However, I am working on value class support and other updates to jackson-module-kotlin and don't have time right now. I will be back when development for jackson 2.17 is settled (probably around March).

k163377 avatar Jan 12 '24 13:01 k163377

Great, I will let this issue open then, and wait for your feedback.

sdeleuze avatar Jan 12 '24 13:01 sdeleuze

I have checked on kotlinx.reflect.lite and it appears that a simple replacement is not possible. This is due to lack of support for value class and features like callSuspendBy.

Personally, I find it more practical to use kotlinx-metadata-jvm to implement an optimized reflection process for Spring.

k163377 avatar Mar 02 '24 05:03 k163377

Let me check with the Kotlin team.

sdeleuze avatar Mar 03 '24 17:03 sdeleuze

After discussing with the Kotlin team, it looks like to path to move forward it to use kotlinx-metadata-jvm (about to be stabilised in Kotlin 2.0) to read metadata and keep using kotlin-reflect for reflective invocations.

@k163377 If you want to work on such branch, I could then run my benchmarks on it to share data points and see how much we gain in term of efficiency (CPU and memory overhead).

sdeleuze avatar Mar 04 '24 14:03 sdeleuze

it looks like to path to move forward it to use kotlinx-metadata-jvm (about to be stabilised in Kotlin 2.0) to read metadata and keep using kotlin-reflect for reflective invocations

Do kotlin-reflect and kotlinx-metadata-jvm have many overlapping dependencies? Since kotlinx-metadata-jvm also has a large size of 1MB, I am wondering if it would be better to simply use kotlin-reflect only (I don't know how to test this hypothesis).


By the way, I would appreciate it if you could vote for the following ticket to improve the performance of metadata readout. https://youtrack.jetbrains.com/issue/KT-63391

k163377 avatar Mar 07 '24 13:03 k163377

Do kotlin-reflect and kotlinx-metadata-jvm have many overlapping dependencies?

I think the answer is no as kotlinx-metadata-jvm has no dependency except the standard library.

Since kotlinx-metadata-jvm also has a large size of 1MB, I am wondering if it would be better to simply use kotlin-reflect only (I don't know how to test this hypothesis).

Wondering the same especially given the recent improvement in Spring Framework 6.1.5-SNAPSHOT via #32390 and #32334. To check, you can start from that (6.1.x or mainbranch), replace the metadata read by kotlinx-metadata-jvm and check if you see significant differences on flamegraphs generated via https://github.com/async-profiler/async-profiler. I suspect most of the ovehead is due to metadata reading, so that's IMO worth to try. If we don't see an improvement, I guess we should just close this issue and just live with the optimizations I mentioned above. If you find new optimizatons with kotlin-reflect, feel free to create related issues/PRs that could even ship in 6.1.

By the way, I would appreciate it if you could vote for the following ticket to improve the performance of metadata readout. https://youtrack.jetbrains.com/issue/KT-63391

I will but be aware that kotlinx-metadata-jvm API is about to be stabilized for the upcoming Kotlin 2.0 release so there is little chance they will change the design significantly. The structure and parsing of the metadata may also put some constraints on their API design.

sdeleuze avatar Mar 08 '24 08:03 sdeleuze

See related interesting comment here.

sdeleuze avatar May 16 '24 10:05 sdeleuze