Weird proposal: operator invoke extension on context

[elizarov]

Ok. This is a weird, but let me shoot it here. How about instead of writing:

withContext(UI) { 
    updateSomethingInMyUI()
}

we could just write:

UI { 
    updateSomethingInMyUI()
}

Does it make sense? 👍or 👎

[dave08]

Maybe (updated)

IO.launch { 
   val result = IO.async { loadData2() }
   val result2 = loadData()

    UI {
        updateSomethingInMyUI(result.await(), result2)
    }
}

Then we have code complete narrowed down, and the meaning of UI { } is more obvious...

[elizarov]

@dave08 Launching new coroutines in UI is a "global" operation, since you are now responsible for making sure you'll cancel it. That is a patter we'd like to discourage in the future. See my notes in #410. We plan to make launch an extension on CoroutineScope which would also make sure that a launched coroutine has a proper child.

[qwwdfsad]

Not sure if UI {} won't be confused with launch(UI) {}

[dave08]

That was a bit the idea (I just updated it a bit now), and how I would understand your usage of UI @elizarov. I had seen that issue and was wondering if it could mean UI being a shortcut for CoroutineScope + CoroutineContext here... I agree with @qwwdfsad that as long as there are so many usages of CoroutineContext (withContext, launch, broadcast, async...), there might be confusion as to what it means all alone, but with consistency in all the builders, it might be clearer.

I proposed (along the lines of your idea) that UI (or any CoroutineContext) in a global context is a shortcut for GlobalScope(UI) (if I'm understanding properly), which might be a sane default and save extra keystrokes without loosing too much clarity. So there could also be UI.async { } and other coroutine builders on it, but it can't be used on its own as UI { } there.

Inside a CoroutineScope or suspend fun, when alone (like UI { }), a CoroutineContext could be interpreted as a shortcut for a regular withContext, and could have other coroutine builders be called from it UI.launch { }, which would also keep along with the proposed pattern.

[elizarov]

@dave08 I see. That makes sense. However, I don't have a clear picture on how we could implement something like that.

The idea in #410 was the we take CoroutineScope interface and define all coroutine builders as extensions on it. We can do it, because CoroutineScope will be turned into a a very light-weight interface. The original idea was that any class could implement CoroutineScope by overriding a single open val scopeContext: CoroutineContext and providing its context there.

CoroutineContext itself is heavy weight interface (and it is in Kotlin stdlib). We cannot expect 3rd party classes to implement it, so we'd like to avoid defining extensions on CoroutineContext. However, some transformation might do the trick, but the best we can do with transformation would look like this:

UI.scope.launch {  ... }

This does not look nicer to read than:

launch(UI) { ... } // works if you are already in some CoroutineScope, launches child coroutine
GlobalScope.launch(UI) { ... } // works anywhere, launches coroutine without parent

[elizarov]

@qwwdfsad It is indeed a fertile ground for confusion. That's the challenge. However, it is Ok if people use UI { ... } instead of launch(UI) { ... } because the former is much safer than the later.

I was actually thinking about the code that you are would be writing when you are doing UI apps and doing some processing background. Now you have to write something like this to do some background work and update UI:

launch(job) { // don't forget the job, or there is risk of dangling global coroutine
    val result = computeSomethingInBackground()
    withContext(UI) {
        updateUI(result)
    }
}

Given ability to provide your own coroutine scope per #410 and combining it with this proposal, we could simplify this common pattern to:

launch { // just launch in the default background context from the current coroutine scope
    val result = computeSomethingInBackground()
    UI {
        updateUI(result)
    }

So if you follow the pattern of always launching coroutines in background and only switching to UI context to update UI, you would not need to use launch(someContext) { ... }. You would only use launch { ... } and UI { ... }.

Now, if you also follow a pattern of "encapsulating context", you can rewrite it like this:

// encapsulates switch to UI context (notice - it is a suspend fun)
suspend fun updateUI(result: Result) = UI { ... } 

// somewhere in business logic (notice - it is a regular fun)
fun doSomeJob() = launch {
    updateUI(computeSomethingInBackground())
}

This looks very clean and readable to me, especially with proper naming (like adding UI suffix to all functions that encapsulate switch to UI context).

[LittleLightCz]

@elizarov: Hi Roman,

As I checked your latest example, I noticed that you do it exactly the opposite way as I am used to do it :-). What I usualy do is launch(UI) {} in the first place. Then inside of it i write the whole code (sequentially) and after it is done, I go through it and decide what should be done (usually) on the CommonPool - mostly with withContext or async - depending on the use case (rarely even another launch {} as well).

I like this approach more especially when I need to do more updates on UI thread (so I don't need to switch often) and regarding some in-memory operations like List mapping etc., they're usually fast enough not to block the UI noticably for the eye, so I am fine doing it on the UI thread as well without the need to switch to another pool.

So having the ability to write UI {} would be nice, but for my needs I would like it to be a launch instead of withContext 😁.

PS: My use cases are mainly TornadoFX apps

[elizarov]

@LittleLightCz You are right. I've actually played some more with various UIs and, indeed, launch(UI) { ... } is more natural. We plan to address this pattern as a part of structured concurrency project (see #410).

The idea if that in your UI you should be able to define you primary UI classes to implement CoroutineScope and then, from inside of those UI classes, you can just write launch { ... } and the corresponding UI context is going to be used. There will be no ambiguity, since we'll deprecate standalone launch outside of CoroutineScope, so if you try to launch a coroutine somewhere else (say, in a top-level function) you'll have to be explicit about your intent. For example, starting a "global" coroutine in a background context (that is done by a simple launch now) will require an explicit GlobalScope.launch { ... }.

Having, this cleared, we'll need some nice name for various background contexts. We're plan to have IO (see #79) for blocking operations, so with operator invoke (as proposed by this issue) you can write IO { readMyFiles() }. But we still lack a nice and descriptive name for our "default dispatcher" that should be used to offload CPU-intensive computations to background threads. I wonder if something like Compute is going to be good for it:

launch { 
    // read some UI
    // perform some async IO
    Compute { ... } // do heavy computations here
    IO { ... } // do blocking IO here
    // update some UI
   ...
}

What do you think?

[LouisCAD]

This proposal is becoming much more interesting with structured concurrency and default dispatcher that can change according to the coroutine scope (e.g. default to UI in Android LifecycleOwner subclasses like AppCompatActivity or LifecycleService).

[LittleLightCz]

@elizarov I've looked at the related issues and to be honest, I don't feel to be technically skilled enough to fully comprehend the topics discussed there 😁. Anyway just a few points on what I think I understood:

• Automatic child coroutines cancellation might be nice
• Regarding not having the need to call .join() on a Job - in your example I wouldn't mind using .joinAll() on that List, so I don't consider this a big deal
• Regarding Images 1 and 2 failures, I am thinking if loading of Image 1 failed, why would one want to know that also the other one failed as well? In a typical UI app when doing some kind of all-or-nothing parallel job, I am usually OK to know that at least one operation failed and as a result I will e.g. just show only a single error dialog (definitely not as many error dialogs as how many images have failed)

And for the rest I will give it into your hands hoping that you guys know what you're doing by making such redesign 😉. The coroutines, as they are now, are very lovely and I like them very much, so please be careful not to make them less concise or uglier ☺️.

Regarding the namings, generally Compute seems ok to me. But maybe I would also consider camel case namings. Because everything starting with a capital letter feels to me like and instance creation (e.g. compare with interfaces and SAM constructors), whereas lowercase/camel case feel to me more as an invocation of something. Therefore I wouldn't be afraid to consider going with ui {}, io {} or compute {}. TornadoFx has already somewhat similar notation: https://github.com/edvin/tornadofx/wiki/Async-Task-Execution

And if you're asking me for a shorter name for compute {}, then the only thing that comes to my mind right now is cpu {} 😁. If I will come up with something better I will let you know.

PS: There is one advantage for going with lower/camel case: you don't need to hold Shift to type it! ☺️

[LittleLightCz]

Another names that came to my mind: comp (as an abbreviation for compute), or since there is a similar word calculation, it could be also calc, but that sounds a bit weird to me in this context. And last option, since this kind of parallel computations is known as Fork/Join (at least I think so ☺️) then what about naming it fork {}? Funny part would be that you could write .join() right after it, if it was a launch 😁.

[LittleLightCz]

Another few brainstorming names: parallel, core (=pool that utilizes CPU cores a lot) or perf (=pool for high-performance computations)

[LouisCAD]

I think that it now makes sense as we have the Dispatchers prefix.
Here's a small example:

launch {
    touchTheUi()
    Dispatchers.IO {
        touchTheStorage()
    }
    touchTheUiAgain()
    Dispatchers.Default {
        heatUpTheCpu()
    }
    centerTextView.text = "You can now warm your hands on your device"
}

[LouisCAD]

Should I make a PR to add an invoke operator fun to the CoroutineDispatcher class?

I'm proposing to add it to CoroutineDispatcher and not CoroutineContext for several reasons:

1. CoroutineContext is an interface that can't have members added to it as easily regarding binary compatibility.
2. Most current usages of withContext are to pass a CoroutineDispatcher (at least in the projects I work on), so supporting only CoroutineDispatcher is already good enough.
3. When you want to run in a context that is not simply a CoroutineDispatcher, then you would keep using withContext, which is explicit, not something bad. It'd be easy to relate to the shortcut operator invoke syntax for usage only with dispatchers.
4. The idea of an extension for CoroutineContext has been dismissed because it requires an import, and the IDE doesn't import it as easily as when it's a member function, something only properly possible with a class, as it is the case for CoroutineDispatcher.

[elizarov]

@LouisCAD 👍 Let's try it as an extension for CoroutineDispatcher. We should start with an experimental inline function -- it adds less weight to our public API. A member for this purpose is not Kotlinish. If the feedback is positive, then we'll stabilize it after one release cycle.

[ZakTaccardi]

I have a class that represents my dispatchers that I use to ensure my dispatchers are injected. I believe this is a good practice.

class AppDispatchers(
    val main: CoroutineContext = Dispatchers.Main,
    val default: CoroutineContext = Dispatchers.Default,
    val io: CoroutineContext = Dispatchers.IO,
)

By making this an extension for CoroutineDispatcher and not CoroutineContext, we can't leverage this feature unfortunately

[LouisCAD]

@ZakTaccardi You can just expose CoroutineDispatcher instead of CoroutineContext. Do you have other reasons to believe that this operator should be exposed to all CoroutineContext instead?

[qwwdfsad]

Closing because it is fixed in 1.2.0-alpha

[ZakTaccardi]

@LouisCAD You can just expose CoroutineDispatcher instead of CoroutineContext

No, I can't. A CoroutineDispatcher + CoroutineExceptionHandler creates a CoroutineContext.

The following will not compile.

val dispatcher: CoroutineDispatcher = Dispatchers.Default + CoroutineExceptionHandler { _, _ }

[LouisCAD]

@ZakTaccardi Then the name of your property is wrong, it's not a dispatcher, but a CoroutineContext.

Adding an exception handler along with it, and treating it like its just a dispatcher is shady (may introduce unexpected bugs when exception handler is replaced) and lacks explicitness, which I mentioned in my pre-PR comment: #428 (comment)