-
Notifications
You must be signed in to change notification settings - Fork 13
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
Improve kotlinx.reflect.lite and rewrite it to use kotlinx.metadata.jvm under the hood #12
Conversation
Note that some kdocs are copied from https://github.com/JetBrains/kotlin/tree/master/core/builtins/src/kotlin/reflect
…nctions,constructors,properties}
|
Hey any plan to merge this PR? FYI I would be interested into making Spring Framework usable with it instead of the huge full |
Yes, I'll get back to this PR soon. The main reason it was dormant is we were working on the |
Awesome thanks. That will be a great improvement for our users to be able to leverage it! |
Note that as per this comment, I'll remove parts of this pull request that enable to introspect .class files. This feature was requested at the time when there was no option such as |
I am currently updating Spring in order to be more flexible about reflection, partly because we have found that using We are going to support running Spring apps without Do you think |
@sdeleuze Thanks for the flamegraph showing the
Yes, looks like you will be able to use it for these purposes. I'm not sure if the performance would be much better though (although it probably should); in any case, I'd be very interested to take a look at the new flamegraph if you're planning to render it after switching to The one thing that wouldn't be completely straightforward with the API proposed in this pull request is the mapping of Kotlin declarations to Java (such as |
Hey, back from vacation ;-) I guess you are very busy, but have you made any progress on that topic? On my side I am ready to test whatever you have via https://github.com/spring-projects/spring-fu. For your information, I have done following benchmarks that show the cost of current Reactive webapp with Jackson without kotlin-reflect:
Reactive webapp with Jackson with kotlin-reflect:
This confirms what we saw on the flamegraph. Getting Jackson Kotlin module and Spring compatible with a lite Kotlin reflection API would be a huge improvement for our users, especially on the Cloud where these figures will be even worst. cc @apatrida |
@sdeleuze Thanks for the reminder! I've added a change that reimplements |
Awesome! I have tried to create a branch of Spring based on
|
There's
The way this is represented in
There's no API for that in
It was actually specifically not a goal of this library to be compatible with That said, I of course don't have any numbers as to how would the library size increase, had this been implemented as a fully compatible |
Thanks for your feedback, I will try to create a Spring Framework branch dedicated branch asap to check our tests are green. |
Is this PR still something you plan to merge? |
Yes, we do plan to merge it. Thanks for the reminder. This hasn't been a priority for us mostly because of no real-world usages. I'll take a look at it once again soon. |
Still very much interested by this topic, just other priorities right now but that topic could be back for Spring Framework 5.3 so a merge of that PR would be nice indeed (sorry for not having found the time to test it). |
Ok so more efficient Kotlin reflection becomes a priority for Spring Framework 6, but we now have slightly different requirements because we want to make it work on GraalVM native as well, where I have the feeling that what is currently commited in If that's the case, I would be against merging this PR and instead collaborate on maturing what is currently on |
@sdeleuze The way the library is supposed to be used to read the metadata is:
The key point is that |
Thanks for the clarification, and nice design. If that's mean we can continue to load annotation via reflection in |
Superseded by #19 |
This pull request prepares the
kotlinx.reflect.lite
for a new release 1.1.0,the main feature of which is the ability to introspect not only the java.lang.Class objects, but also .class files[1]. Some new API has been added (requested in KT-11386). All of the existing API has been documented (no API docs yet though, unfortunately), and an example has been added to the ReadMe.[1]: UPD: As noted in KT-11386, making
kotlinx.reflect.lite
understand .class files without the context of Java reflection is no longer necessary because it can be achieved viakotlinx.metadata.jvm
. This pull request has since been repurposed to only contain reimplementation ofkotlinx.reflect.lite
based onkotlinx.metadata.jvm
, and requested improvements in the API.