InsertKoinIO / koin

Koin - a pragmatic lightweight dependency injection framework for Kotlin & Kotlin Multiplatform
https://insert-koin.io
Apache License 2.0
9.07k stars 719 forks source link

Coroutine support #388

Open Pitel opened 5 years ago

Pitel commented 5 years ago

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 returns Deferred<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

Pitel commented 5 years ago

@arnaudgiuliani Resolved or wontfix?

arnaudgiuliani commented 5 years ago

Would make it wont-fix but if you have a clear use-case, let check that

Pitel commented 5 years ago

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 suspend function, you have to be inside some coroutine builder. And because Koin doen't support that directly, we have to use async. So let's make a singleton:

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 async, we have to use await. And in order to use await, we have to be in a coroutine. So wrap it in async again. But now we have another problem, because async returns Deferred<T> and Koin isn't very good with generics (U understand why, don't take it as a complaint). So we have to use named, ending up with this:

single(named("settings")) {
    GlobalScope.async() {
        getMqttSettings()
    }
}

single(named("mqtt")) {
    GlobalScope.async() {
        MqttClient(
            get<Deferred<Settings>>(named("settings")).await()
        )
    }
}

And you have to this named/async/await/Deferred dance every time you start using coroutines.

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

Swordsman-Inaction commented 4 years ago

IMHO, this is out of the scope of a dependency injection library.

marcardar commented 4 years ago

This is how I deal with this (feedback much appreciated):

single(named("instanceProvider")) {
    suspendableLazy {
        createAndInitInstance() // returns instance of MyClass
    }
}

and then:

private val instanceProvider by lazy {
   get<SuspendableProvider<MyClass>>(named("instanceProvider"))
}
suspend fun getInstance(): MyClass? = try {
    instanceProvider.get()
} catch (e: Exception) {
    Log.e("di", "unable to create instance", e)
    null
}

where using some common code:

interface SuspendableProvider<T> {
    suspend fun get(): T
    val isCompleted: Boolean
}
fun <T> suspendableLazy(provider: suspend () -> T) = object : SuspendableProvider<T> {
    private val computed = GlobalScope.async(start = CoroutineStart.LAZY) { provider() }
    override val isCompleted: Boolean
        get() = computed.isCompleted
    override suspend fun get() = computed.await()
}
gorgexec commented 4 years ago

I think, this is really not the responsibility of Koin. In this case, coroutine need is because of complexity of object building, but Koin deals with another activities: object resolving and injecting. First of all, the object building depends on app logic and may be handled by different ways and vary from app to app. DI library shouldn't know about such details. So, as example, you may initially preconstruct your complex object and then load Koin feature module that provides required object without any coroutine usage in Koin.

Pitel commented 4 years ago

@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 startKoin(), get()/inject()/etc. methods suspending (and asynchronous), so the whole dependency graph creation and crawling won't block the main thread. But I understand it's not an easy task.

fmatosqg commented 4 years ago

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

 val weekdayFormatter: DateTimeFormatter = DateTimeFormatter.ofPattern("EEEE")

that I put together in a dedicated FixtureDateFormatterUtil class, and I'd love to make something like

    class FixtureDateFormatterUtil {

        private constructor()

        companion object {
            suspend
            fun factory(): FixtureDateFormatterUtil =
                  withContext(Dispatchers.Default) {
                       FixtureDateFormatterUtil()
                }
        }

But then I can't easily use it with koin.

fmatosqg commented 4 years ago

@marcardar I have a proof of concept code (still needs quite some cleaning at https://github.com/fmatosqg/koin/tree/factory_suspend).

The basic idea is having a new dsl factorySlow (yep it needs a better name, or it could be like suspendableLazy suggested above) that will tag that Bean with options.coroutine = true, in a similar fashion as options.isCreatedAtStart for eager modules. Then all of those beans will be initialized in parallel when the app starts.

I made a number of assumptions so far

also

The heart of it is at a variation of ::createEagerInstances that looks for options.coroutine and runs things in parallel. get2 here is a suspending sibling of get, who calls a suspending version of create and so forth.

        instances.values.filterIsInstance<SingleInstanceFactory<*>>()
            .filter { instance -> instance.beanDefinition.options.coroutine }
            .map { instance ->
                CoroutineScope(Dispatchers.Default)
                    .async {
                        instance.get2(InstanceContext(_koin, _scope))
                    }
            }
            .let {

                runBlocking {
                    it.forEach {
                        it.await()
                    }

                }                   }
            }

The current version adds a typealias SuspendDefinition<T> = suspend Scope.(DefinitionParameters) -> T that gets passed as a parameter along the current definition object through all the layers from module to InstanceFactory. This def has lots of room for improvement, but not worried about that now.

I'm interested in knowing about the performance of running runBlocking lots of times, that may help simplify the internal API. But again, not worring about this yet.

Keen to hear ideas or feedback.

Pitel commented 4 years ago

Great, I see it moves forward! :+1:

fmatosqg commented 4 years ago

Yes, the runBlocking and the await or awaitAll is at the heart of the controversy. They may disapear from code, but another form of waiting would need to replace them.

jillesvangurp commented 2 years ago

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. runBlocking is not actually a thing in kotlin-js so you would not be able to use that. And threads are not a thing either so you can't await a co-routine from the main thread without blocking that thread (which is why runBlocking is not a thing.

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.

stale[bot] commented 1 year ago

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.

jillesvangurp commented 1 year ago

@arnaudgiuliani it seems the stale bot just added the wontfix label. Seems a bit harsh?

arnaudgiuliani commented 1 year ago

yes weird.

jillesvangurp commented 1 year ago

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.

fmatosqg commented 1 year ago

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 https://github.com/InsertKoinIO/koin/issues/388#issuecomment-1400503720, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABI4Y4OJPGOUHPHYD3TNSBLWT2NDBANCNFSM4G4B2WOA . You are receiving this because you commented.Message ID: @.***>

jillesvangurp commented 1 year ago

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 runBlocking {}. Nothing happens in any case before onCreate finishes. All runBlockingwould do there is give you the chance to call some suspending things. Of course you do have to be careful with doing slow things there. But that is true regardless. Likely people are possibly still using some blocking IO on Android still? That would make sense given the Java legacy.

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 GlobalScope, which works fine. It's still sequential but it gives us the possibility to do non blocking IO as part of our initialization.

shubhamsinghshubham777 commented 1 year ago

Another common use case for having coroutine support in Koin would be instantiating SQLDriver for JS in SQLDelight. Ref: https://cashapp.github.io/sqldelight/1.5.4/js_sqlite/

Code sample in the ref link above:

// As a Promise
val promise: Promise<SqlDriver> = initSqlDriver(Database.Schema)
promise.then { driver -> /* ... */ }

// In a coroutine
suspend fun createDriver() {
    val driver: SqlDriver = initSqlDriver(Database.Schema).await()
    /* ... */
}
fmatosqg commented 1 year ago

Nothing happens in any case before onCreate finishes. All runBlocking would do there is give you the chance to call some suspending things. Of course you do have to be careful with doing slow things there. But that is true regardless.

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.

Likely people are possibly still using some blocking IO on Android still?

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.

arnaudgiuliani commented 1 year ago

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

arnaudgiuliani commented 1 year ago

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:

jillesvangurp commented 1 year ago

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 GlobalScope.launch {...} ugly but effective.

arnaudgiuliani commented 1 year ago

The issue that I had with koin was simply that the koin dsl doesn't support suspending code blocks

We also need API to request dependencies in async way then, to have the whole "suspend" stuff working

arnaudgiuliani commented 1 year ago

I've already added support for the Koin extension for modules loading via coroutines. This is a beginning 👍

amal commented 1 year ago

Neat support for suspending is available in kotlin-inject. Maybe it can be helpful.

arnaudgiuliani commented 1 year ago

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

wreedamz commented 5 months ago

@arnaudgiuliani any updates on your work here?

haydenkaizeta commented 5 months ago

I am interested in the current progress and the suggested pattern in the meantime

aaziz993 commented 2 months ago

I've already added support for the Koin extension for modules loading via coroutines. This is a beginning 👍

Can you please provide example on how to use it to register coroutine dependency.

arnaudgiuliani commented 2 months ago

I believe your are looking for coroutines based definition running, no?

aaziz993 commented 1 month ago

I believe your are looking for coroutines based definition running, no?

I create compose multiplatform application where I use Res.readBytes to read global configuration from composeResources/file which is suspend method. I use multiplatform runBlocking to declare singleton of my Config class with koin annotations, but it doesn't work in browser.

shubhamsinghshubham777 commented 5 days ago

Any updates on this @arnaudgiuliani? This will be a really neat feature to have for many.