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

Dispatchers.Default is not installed in coroutine builders when there is no ContinuationInterceptor in native #4074

Open
whyoleg opened this issue Mar 20, 2024 · 3 comments
Labels

Comments

@whyoleg
Copy link
Contributor

whyoleg commented Mar 20, 2024

Describe the bug

When using one of the coroutine builders (like launch) if there is no ContinuationInterceptor/CoroutineDispatcher in coroutineContext coroutine should start on Dispatchers.Default as specified in documentation of: Dispatchers.Default, CoroutineScope.newCoroutineContext (which is used under the hood of coroutine builders) and in CoroutineScope.launch itself.

This contract is implemented in actual implementations of CoroutineScope.newCoroutineContext for all platforms (jvm, js, wasmJs) except native, where single-threaded DefaultExecutor is used instead.

Provide a Reproducer

In following example on jvm, js, wasmJs the same dispatcher will be printed (it's name depends on target, but it is Dispatchers.Default`), but on native dispatchers are different:

val job = Job()
val scope = CoroutineScope(job) // no dispatcher provided
scope.launch(Dispatchers.Default) {
    println("default=${coroutineContext[ContinuationInterceptor]}")
}
scope.launch {
    println("empty=${coroutineContext[ContinuationInterceptor]}")
}
job.complete()
job.join()

At my side when running test on macosArm64:

empty=DefaultExecutor@2c70160
default=DarwinGlobalQueueDispatcher@2c30410

Expected behaviour

Dispatchers.Default should be installed on native.
I found this issue during running some tests, after refactoring tests execution time was increased. I had no idea why, but then when I checked dispatcher in some test and found that it's different - just because before it was just EmptyCoroutineContext but now it's Dispatchers.Default`.

Note: test execution time become like several times slower when using Dispatchers.Default - not sure if it's expected or not...

@whyoleg whyoleg added the bug label Mar 20, 2024
@dkhalanskyjb
Copy link
Collaborator

This looks like it's an overlooked artifact of the time the Native implementation was single-threaded.

Note: test execution time become like several times slower when using Dispatchers.Default - not sure if it's expected or not...

On Apple targets, we use our own implementation of scheduling on top of a native thread for DefaultDelay, but delegate to Apple's event queue for Dispatchers.Default. This could be the reason.

@whyoleg
Copy link
Contributor Author

whyoleg commented Mar 21, 2024

Are you willing to accept PR for this? This looks rather straightforward from first look.

@dkhalanskyjb
Copy link
Collaborator

Sure, please go ahead.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants