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 Kotlin reflection runtime efficiency #21546

Open
spring-projects-issues opened this issue Jul 4, 2018 · 30 comments
Open

Optimize Kotlin reflection runtime efficiency #21546

spring-projects-issues opened this issue Jul 4, 2018 · 30 comments
Assignees
Labels
in: core Issues in core modules (aop, beans, core, context, expression) theme: kotlin An issue related to Kotlin support type: enhancement A general enhancement

Comments

@spring-projects-issues
Copy link
Collaborator

spring-projects-issues commented Jul 4, 2018

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 spring-projects-issues added type: enhancement A general enhancement in: core Issues in core modules (aop, beans, core, context, expression) labels Jan 11, 2019
@spring-projects-issues spring-projects-issues added this to the 5.2 RC1 milestone Jan 11, 2019
@sdeleuze sdeleuze modified the milestones: 5.2 M1, 5.x Backlog Jan 30, 2019
@sdeleuze sdeleuze changed the title Support for kotlinx.reflect.lite [SPR-17008] Use kotlinx-metadata-jvm at build time instead of kotlin-reflect at runtime Dec 18, 2020
@sdeleuze
Copy link
Contributor

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 added a commit to spring-attic/spring-native that referenced this issue Feb 24, 2021
Kotlin Coroutines are now supported but require additional
reflection entries due to how Coroutines generates bytecode
with an `Object` return type, see
spring-projects/spring-framework#21546
for a potential future fix.

Closes gh-409
@sdeleuze sdeleuze changed the title Use kotlinx-metadata-jvm at build time instead of kotlin-reflect at runtime Optimize Kotlin reflection runtime efficiency Oct 28, 2021
@sdeleuze
Copy link
Contributor

sdeleuze commented Oct 28, 2021

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
Copy link
Contributor

sdeleuze commented Oct 29, 2021

See also this related comment Kotlin/kotlinx.reflect.lite#12 (comment) and this one on KT-11386.

@jhoeller jhoeller modified the milestones: 5.x, 6.0.x Nov 1, 2021
@sdeleuze sdeleuze added theme: kotlin An issue related to Kotlin support and removed theme: native in: core Issues in core modules (aop, beans, core, context, expression) labels Jan 19, 2022
@poutsma poutsma self-assigned this May 10, 2022
poutsma pushed a commit to poutsma/spring-framework that referenced this issue Sep 7, 2022
This commit migrates Spring Framework from using the "full" Kotlin
reflection API to the "light" reflection API.

Closes spring-projectsgh-21546
@k163377
Copy link

k163377 commented Jan 23, 2023

@cowtowncoder

you would be interested in becoming a maintainer?

Yes.

@cowtowncoder
Copy link

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

@sdeleuze
Copy link
Contributor

sdeleuze commented Apr 3, 2023

@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?

@cowtowncoder
Copy link

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.

@k163377
Copy link

k163377 commented Apr 4, 2023

@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.

@sdeleuze
Copy link
Contributor

sdeleuze commented Apr 5, 2023

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.

@k163377
Copy link

k163377 commented Apr 5, 2023

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(?).

@sdeleuze
Copy link
Contributor

sdeleuze commented Apr 5, 2023

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

Any thoughts @cowtowncoder?

@cowtowncoder
Copy link

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

@sdeleuze sdeleuze modified the milestones: 6.1.0-M1, 6.x Backlog Apr 6, 2023
@sdeleuze
Copy link
Contributor

sdeleuze commented Apr 6, 2023

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.

@poutsma poutsma removed their assignment Dec 12, 2023
@jhoeller jhoeller added the in: core Issues in core modules (aop, beans, core, context, expression) label Jan 11, 2024
@sdeleuze
Copy link
Contributor

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 sdeleuze added the status: waiting-for-feedback We need additional information before we can continue label Jan 12, 2024
@k163377
Copy link

k163377 commented Jan 12, 2024

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).

@spring-projects-issues spring-projects-issues added status: feedback-provided Feedback has been provided and removed status: waiting-for-feedback We need additional information before we can continue labels Jan 12, 2024
@sdeleuze
Copy link
Contributor

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

@k163377
Copy link

k163377 commented Mar 2, 2024

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.

@sdeleuze
Copy link
Contributor

sdeleuze commented Mar 3, 2024

Let me check with the Kotlin team.

@sdeleuze
Copy link
Contributor

sdeleuze commented Mar 4, 2024

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 sdeleuze modified the milestones: 6.x Backlog, General Backlog Mar 4, 2024
@k163377
Copy link

k163377 commented Mar 7, 2024

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

@sdeleuze
Copy link
Contributor

sdeleuze commented Mar 8, 2024

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 sdeleuze removed the status: feedback-provided Feedback has been provided label May 16, 2024
@sdeleuze
Copy link
Contributor

See related interesting comment here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: core Issues in core modules (aop, beans, core, context, expression) theme: kotlin An issue related to Kotlin support type: enhancement A general enhancement
Projects
None yet
Development

No branches or pull requests

6 participants