-
-
Notifications
You must be signed in to change notification settings - Fork 699
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
Coroutine support #388
Comments
@arnaudgiuliani Resolved or wontfix? |
Would make it |
Alright. We use MQTT and for some reasons, we have the server url and credentials accesible via internal REST API, so in case of some change, we won't have to re-release our apps. Now, for REST, we use Retrofit. Retrofit recently got support for coroutines, so my Retrofit interface looks like this: suspend fun getMqttSettings(): Settings In order to call the single {
GlobalScope.async() {
getMqttSettings()
}
} We can also make a singleton for our MQTT client. But we can't do that directly! In order to get result from single(named("settings")) {
GlobalScope.async() {
getMqttSettings()
}
}
single(named("mqtt")) {
GlobalScope.async() {
MqttClient(
get<Deferred<Settings>>(named("settings")).await()
)
}
} And you have to this Wouldn't it be nice, if you could just write something this: coSingle {
getMqttSettings()
}
coSingle {
MqttClient(
get()
)
} (The co prefix is what MockK is using for it's coroutine supported functions.) |
IMHO, this is out of the scope of a dependency injection library. |
This is how I deal with this (feedback much appreciated):
and then:
where using some common code:
|
I think, this is really not the responsibility of Koin. |
@gorgexec Ok, that's a valid point, but... You can't preconstruct them for Koin, because the construction is asynchronous. And if any other object would try to get instance of the object before it's constructed, it'll fail and crash. What I think is, that K in Koin means Kotlin. And coroutines are now part of the Koltin's stdlib. So it would be nice if Koin would support them. Heck, you can even make the whole |
I didn't really understand @marcardar suggestion, but it looks reasonable to me that something like that could be inside koin lib and proof-read by other developers instead of sitting inside my project and "working on my test devices". I'd bring my problem that could use a similar solution. I have an object that has a bunch of ThreeTenAbp date formatters, and they are sloooow to instantiate. In a previous shorter version it took 90ms to instantiate. It's just a handful of
that I put together in a dedicated FixtureDateFormatterUtil class, and I'd love to make something like
But then I can't easily use it with koin. |
@marcardar The basic idea is having a new dsl I made a number of assumptions so far
also
The heart of it is at a variation of
The current version adds a I'm interested in knowing about the performance of running Keen to hear ideas or feedback. |
Great, I see it moves forward! 👍
|
Yes, the |
We do the following in our kotlin-js app: GlobalScope.launch {
startKoin {
modules(searchModule)
}
// asynchronously initializes our localization, which does some IO
GlobalContext.get().declare(LocalizationUtil.load())
...
// render the UI
} Basically, this creates a co routine scope. You then initialize koin the normal way, and then you declare some new objects via suspend functions. This works because inside co-routines, you have before/after semantics. So the localization is guaranteed to finish loading before the UI renders. This works fine but it's a bit klunky; IMHO it shouldn't be that hard to support a nicer version of this in Koin. UIs are inherently asynchronous and so is their lazy initialization. |
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. |
@arnaudgiuliani it seems the stale bot just added the wontfix label. Seems a bit harsh? |
yes weird. |
Building on the example I posted above, what's probably needed is a few suspending extension functions that allow people to call suspending logic. I'm not familiar enough with Android to know how you'd do that exactly in that context. I think with ktor and browser based things a variation of what I did should work fine. Either way, the challenge is avoiding duplicating all the functionality in suspending and non suspending ways. Or just make suspending logic the default. |
I don't think that sample will work for Android, at best I'd expect a race
condition that "works".
Android doesn't need to ensure the global scope is finished by the time the
first frame is rendered (and if we were to enforce that we may be killing
the value of the feature),, - what it needs is whatever part of the graph
is instantiated when it wants. But if it did, I would do it with run
Blocking right after launch. Upside: spin a particular function in a
particular dispatcher. But I don't see a strong need for this, I'd let the
app developer do all of this inside his own written method.
It's been very long since I started looking into this and honestly don't
remember the use case I was aiming for.
…On Tue, 24 Jan 2023, 02:07 Jilles van Gurp, ***@***.***> wrote:
Building on the example I posted above, what's probably needed is a few
suspending extension functions that allow people to call suspending logic.
I'm not familiar enough with Android to know how you'd do that exactly in
that context. I think with ktor and browser based things a variation of
what I did should work fine. Either way, the challenge is avoiding
duplicating all the functionality in suspending and non suspending ways. Or
just make suspending logic the default.
—
Reply to this email directly, view it on GitHub
<#388 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ABI4Y4OJPGOUHPHYD3TNSBLWT2NDBANCNFSM4G4B2WOA>
.
You are receiving this because you commented.Message ID:
***@***.***>
|
I think it's not so much a matter of blocking but just guaranteeing that certain things happen before other things. I just looked at the docs, and it seems on Android you just initialize the whole context in an onCreate handler (not suspending). Effectively the application does not start until that finishes and koin has been initialized. So, I don't think it would be a big deal to surround the koin start with a The issue is that the various lambdas in the koin dsl are not suspending. That's what I work around in the code above by calling declare. So, a few variations of the DSL functions that can work with suspending functions would be helpful. In the browser we don't actually have runBlocking, so we just 'launch' the whole application in the |
Another common use case for having coroutine support in Koin would be instantiating SQLDriver for JS in SQLDelight. Code sample in the ref link above:
|
true. Though for android we'd need to use at least 2 CPU cores to take advantage of it, or else there's no upsides.
Let's keep blocking IO out of scope. Android is multi-threaded, and best practice is not do long operations on main thread, IO or not, whether that's coroutines, RxJava or bare threads. |
One of the my point here, is to help also introduce coroutines in the core engine architecture of Koin to avoid pessimist lock under thread (like currently). If I wrap up the need of coroutines :
This will land under the version 4.0 of Koin. I will need beta testers ;) |
One first proposal would be for example to help load koin on another thread than the main one. The idea: unlock the problem of having more and more definitions & modules to read at start. This way, this is not blocking anymore. At least we need to have some minimum definitions ready. Then 2 possibilities:
|
The point of co-routines isn't necessarily doing things on threads but doing asynchronous/non blocking things. The browser actually has no threads (well, it has workers but that's a bit of a niche thing). But everything is promise based in a browser, which is why co-routines are a really nice abstraction there. And if you have a lot of initialization that does things with promises and co-routines, having support for that in Koin is nice. The issue that I had with koin was simply that the koin dsl doesn't support suspending code blocks. I worked around it by calling things on the context directly in a |
We also need API to request dependencies in async way then, to have the whole "suspend" stuff working |
I've already added support for the Koin extension for modules loading via coroutines. This is a beginning 👍 |
Neat support for suspending is available in kotlin-inject. |
yes, they offer the possibility to create a definition within a coroutine context then 🤔 For the first proposal above, it is more about loading content in a coroutine context |
@arnaudgiuliani any updates on your work here? |
I am interested in the current progress and the suggested pattern in the meantime |
Is your feature request related to a problem? Please describe.
I can't write
single { someSuspenfingFunctionReturningObject() // ERROR, can't call outside coroutine scope }
Describe the solution you'd like
Allow the
single
's lambda to be suspending. By adding coroutines to Koin, it might also speed up graph generation by being multithreaded.Describe alternatives you've considered
Workaround I use is to use
GlobalScope.async()
. But it returnsDeferred<Something>
and if you use it multiple times, you also has to deal with generics and use names. And you also has to call.await()
every f**king time you want to get the Something instance.Another possible workaround might be to use
runBlocking
, but it'll degrade performance.Target Koin project
It might be big change, so I suggest 2.x
The text was updated successfully, but these errors were encountered: