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

Remove kotlin-reflect dependency #141

Closed
richarddd opened this issue Aug 11, 2019 · 6 comments
Closed

Remove kotlin-reflect dependency #141

richarddd opened this issue Aug 11, 2019 · 6 comments

Comments

@richarddd
Copy link

KProperty1 could be used without all the other classes from kotlin-reflect.

We could use standard java reflection to deal with registration of pojos etc.

The reason why this is an issue is that it adds lots of complexity.

It also does not work when compiling and image with GraalVMs native-image makes the native-image huge:

java.lang.IllegalStateException: @NotNull method kotlin/reflect/jvm/internal/impl/builtins/KotlinBuiltIns.getBuiltInClassByFqName must not return null
        at kotlin.reflect.jvm.internal.impl.builtins.KotlinBuiltIns.$$$reportNull$$$0(KotlinBuiltIns.java)
        at kotlin.reflect.jvm.internal.impl.builtins.KotlinBuiltIns.getBuiltInClassByFqName(KotlinBuiltIns.java:357)
@zigzago
Copy link
Member

zigzago commented Aug 14, 2019

  • kmongo-jackson-mapping depends of third party lib jackson-module-kotlin that uses kotlin-reflect
  • kmongo-native-mapping uses a lot kotlin-reflect and it's not so easy to replace it with java reflection

For my point of view, #104 should be implemented first: kotlinx.serialization does not depend of kotlin-reflect

@richarddd
Copy link
Author

That would be awesome! :)

@richarddd
Copy link
Author

@zigzago i saw that you implemented #104 this weekend 🥳🙇‍♂️🙌 cloud we close this issue as well?

@zigzago
Copy link
Member

zigzago commented Oct 14, 2019

@richarddd Not yet. These are still some reflection involved when checking ids and some other corner cases. (see https://github.com/Litote/kmongo/releases/tag/kmongo-3.11.1 ). Expect a workaround in the next version ;)

@zigzago zigzago closed this as completed in 18560a1 Dec 6, 2019
@richarddd
Copy link
Author

@zigzago This should be opened again. All mappers heavily rely on kotlin-reflect. I looked at the source and the main reason why is that all mappers currently need to get the java ower class from a KProperty1. We could avoid this completely by using reified operators for all kmongo operations and pass the reified class instance to the mappers. I have created a proof-of-concept branch in my fork where I'm trying this

@Legion2
Copy link

Legion2 commented Nov 4, 2021

@richarddd what is the status of your efforts to create a native image with graalvm and kmongo?

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

3 participants