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

Add spawn_future and spawn_future_local convenience functions #1201

Merged
merged 1 commit into from
Oct 9, 2023

Conversation

Hofer-Julian
Copy link
Contributor

A couple of gtk-rs apps currently use utility functions to make spawning futures less verbose.
See:

Exposing these functions via glib should fulfill that need.

sdroege
sdroege previously approved these changes Oct 9, 2023
@sdroege sdroege requested a review from bilelmoussaoui October 9, 2023 09:11
@sdroege
Copy link
Member

sdroege commented Oct 9, 2023

@bilelmoussaoui What do you think?

@pbor
Copy link
Contributor

pbor commented Oct 9, 2023

Shall these use glib::MainContext::ref_thread_default().spawn_local etc?

@sdroege
Copy link
Member

sdroege commented Oct 9, 2023

Shall these use glib::MainContext::ref_thread_default().spawn_local etc?

The goal here seems to be to actually get the global default main context. But you're right, from the function names I would also expect it to work on the thread default one.

@Hofer-Julian
Copy link
Contributor Author

The goal here seems to be to actually get the global default main context. But you're right, from the function names I would also expect it to work on the thread default one.

I have to admit that I don't know what the difference would be in practice.

@sdroege
Copy link
Member

sdroege commented Oct 9, 2023

None at all unless you create other main contexts in your application and run those from other threads. That seems very uncommon in GTK applications, but for example is how the GDBus thread and some other helper threads inside GIO work. It's more common a pattern in libraries.

@Hofer-Julian
Copy link
Contributor Author

None at all unless you create other main contexts in your application and run those from other threads. That seems very uncommon in GTK applications, but for example is how the GDBus thread and some other helper threads inside GIO work. It's more common a pattern in libraries.

I guess, in that case, I wouldn't mind changing it to glib::MainContext::ref_thread_default().spawn* if that makes stuff clearer and/or makes the function applicable for more use cases.

@sdroege
Copy link
Member

sdroege commented Oct 9, 2023

It would only behave differently if you run it from a thread that explictly set another main context as the thread default one. It would behave exactly the same as GIO async operations (and GTask) in that regard.

@Hofer-Julian
Copy link
Contributor Author

It would only behave differently if you run it from a thread that explictly set another main context as the thread default one. It would behave exactly the same as GIO async operations (and GTask) in that regard.

I understand :)
Which option is more sensible in your opinion?

@sdroege
Copy link
Member

sdroege commented Oct 9, 2023

The behaviour of GTask, using the thread default one. The main reason why some APIs (g_idle_add() etc) don't do that is for backwards compatibility reason.

A couple of gtk-rs apps currently use utility functions to make spawning futures less verbose.
See:
- https://docs.rs/gtk-macros/0.3.0/gtk_macros/macro.spawn.html
- https://gitlab.gnome.org/GNOME/loupe/-/blob/main/src/util/mod.rs#L251

Exposing these functions via glib should fulfill that need.
@Hofer-Julian
Copy link
Contributor Author

The behaviour of GTask, using the thread default one

Okay! I've adapted the PR accordingly.

@sdroege sdroege merged commit 9125737 into gtk-rs:master Oct 9, 2023
48 checks passed
@Hofer-Julian Hofer-Julian deleted the add-spawn-functions branch October 9, 2023 14:28
alatiera added a commit to alatiera/gtk-rs-core that referenced this pull request Oct 29, 2023
Followup to gtk-rs#1201

Allow to set custom priority for the spawned futures.
alatiera added a commit to alatiera/gtk-rs-core that referenced this pull request Oct 30, 2023
Followup to gtk-rs#1201

Allow to set custom priority for the spawned futures.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants