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

Krate#moshiPref - why using typeOf instead of class? #31

Open
mikef-dk opened this issue Sep 8, 2023 · 3 comments
Open

Krate#moshiPref - why using typeOf instead of class? #31

mikef-dk opened this issue Sep 8, 2023 · 3 comments

Comments

@mikef-dk
Copy link

mikef-dk commented Sep 8, 2023

Recently we've done some tracing regarding our app start performance. What we noticed is that the very first time typeOf is invoked it takes a long time (~1 second). While we were able to eliminate those calls in our code base, we were noticing that the Moshi extension for Krate is doing it as well:

@OptIn(ExperimentalStdlibApi::class)
public inline fun <reified T : Any> Krate.moshiPref(
    key: String? = null,
): KeyedKratePropertyProvider<T?> {
    return moshiPrefImpl(key, typeOf<T>().javaType)
}

I'm wondering: Is there any good reason to use typeOf<T>().javaType over T::class.java?

Lastly a screenshot of the trace proving the point that typeOf really takes that long:

SCR-20230908-mqwq
@zsmb13
Copy link
Collaborator

zsmb13 commented Sep 9, 2023

Unfortunately there is a good reason. typeOf can handle nested types being input as the T type parameter (such as List<String>), while ::class.java would erase that to just the List class with its nested parameter lost.

I go into detail on this in this talk: https://zsmb.co/talks/krate-ft-moshi/

That being said, if there are alternatives to typeOf that preserve full types (or if I'm doing something wrong in the library that's causing it to be this slow) I'd be open to improving this.

@mikef-dk
Copy link
Author

Thank you very much for the explanation and the link to your talk - very helpful. As far as I'm aware there is nothing wrong that the library is doing - the first invocation of typeOf just seems to be painfully slow no matter where it happens in the code and I'm not aware of any alternative unfortunately.

@mikef-dk
Copy link
Author

@zsmb13 After watching your talk and checking your slides I'm wondering whether it would be an option to bring back the old implementation that you were using before switching to typeOf?

We've compared both implementations and those are the results:

typeOf
typeOf

makeType
makeType

@mikef-dk mikef-dk reopened this Sep 19, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants