Skip to content

Use Dispatchers.IO by default on multi-threaded platforms #905

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

Merged
merged 9 commits into from
Apr 27, 2025

Conversation

sproctor
Copy link
Contributor

and allow dispatcher to be configured in realtime and storage

What kind of change does this PR introduce?

Bug fix/feature

What is the current behavior?

Dispatchers.Default is used for all operations, but configurable in Auth.

What is the new behavior?

Dispatchers.IO is used by default on JVM and K/N targets, Dispatchers.Default is still used on JS and WASM targets. All invocations are configurable by an appropriate config.

Additional context

Dispatchers.Default is intended for CPU bound operations and is limited by the number of CPU cores. IO operations should use Dispatchers.IO instead.

…atcher to be configured in realtime and storage
@sproctor
Copy link
Contributor Author

I think it would be cleaner if defaultDispatcher were made internal and set in the top level config rather than in each module that uses dispatchers. I'm just not sure how to access that config from the other modules. I guess it can be a property of SupabaseClient. Let me know what you think of these changes and if you'd prefer that and what to do about the existing coroutineDispatcher property in Auth.

@jan-tennert
Copy link
Collaborator

jan-tennert commented Apr 18, 2025

We could do something similar to the serializers. So you can set a default dispatcher in the SupabaseClientBuilder:

val supabase = createSupabaseClient(url, key) {
     defaultCoroutineDispatcher = ... //By default set to the corresponding platform dispatcher
}

.. maybe even defaultIoDispatcher or a similar name so its clear that it's not for CPU bound operations?
Introduce a CustomDispatcherConfig similar to that interface:

/**
* A configuration for a plugin, which allows to customize the serialization
*/
interface CustomSerializationConfig {
/**
* The serializer used for this module. Defaults to [SupabaseClientBuilder.defaultSerializer], when null.
*/
var serializer: SupabaseSerializer?
}

Then plugins which actually use a scope override this interface in their config, defaulting to null. And in the plugin implementation you have something like this:

val dispatcher = config.ioDispatcher ?: supabase.defaultIoDispatcher //defaultIoDispatcher is obviously non-nullable
val scope = CoroutineScope(dispatcher...)

For Auth, if we go with ioDispatcher as the config name, you could probably deprecate the coroutineDispatcher property and use this in auth:

val dispatcher = config.ioDispatcher ?: config.coroutineDispatcher ?: supabase.defaultIoDispatcher
val scope = CoroutineScope(dispatcher...)

But we can also leave the name and move up the coroutineDispatcher config property and use the interface

@jan-tennert
Copy link
Collaborator

@sproctor What do you think? I'd include this in the next release candidate.

@sproctor
Copy link
Contributor Author

Sorry, I missed the notification on the initial comment. I'm not sure there's a ton of value to being able to set the dispatcher per module. It probably makes the most sense to just put the dispatcher in SupabaseClientBuilder defaulting to Dispatchers.IO on targets that have it and deprecate the config in Auth.

@jan-tennert
Copy link
Collaborator

Yea then lets do that.

@sproctor
Copy link
Contributor Author

Okay, I'll re-work this now

@jan-tennert jan-tennert merged commit d0d4710 into supabase-community:master Apr 27, 2025
13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants