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

Configurable futures executor #63

Closed
hecrj opened this issue Nov 18, 2019 · 6 comments · Fixed by #164
Closed

Configurable futures executor #63

hecrj opened this issue Nov 18, 2019 · 6 comments · Fixed by #164
Labels
feature New feature or request question Further information is requested

Comments

@hecrj
Copy link
Member

hecrj commented Nov 18, 2019

I see that you used ThreadPool as the executor. Would you consider making this only the default but allowing to pass a different executor with the help of futures::task::Spawn?

That way a user can implement his own executor or use libraries that support Spawn, hopefully tokio or async-std at some point.

Happy to make a PR for this if you approve.

Originally posted by @daxpedda in #62 (comment)

@hecrj hecrj mentioned this issue Nov 18, 2019
@hecrj
Copy link
Member Author

hecrj commented Nov 18, 2019

That way a user can implement his own executor or use libraries that support Spawn, hopefully tokio or async-std at some point.

What would be some examples of use cases for this?

@hecrj hecrj added feature New feature or request question Further information is requested labels Nov 18, 2019
@daxpedda
Copy link
Contributor

daxpedda commented Nov 18, 2019

As far as I'm aware only performance. One might want to use the executor that is more tailored for a particular use case.

One example would be NUMA awareness, currently not supported by futures but by tokio (couldn't find hard evidence for it).
A good use case for that might be a render application, cache coherence might be quite important for that.

Or simply more peformance, futures and tokio have quite some disparity in different cases: https://vorner.github.io/async-bench.html, https://github.com/stjepang/executor-benchmarks.

Additionally, this should really be completly non-disruptive for iced, because we can simply use futures::task::Spawn, which isn't supported by tokio or other executors to my knowledge yet, but will hopefully soon be.

EDIT: I failed to mention all the other executors, but beside async-std I really have limited knowledge about others.

@hecrj
Copy link
Member Author

hecrj commented Nov 20, 2019

I'd like to gather a bit more of information before comitting to a solution here.

The competition is quite tight. I expected bigger differences. There’s probably no more IO-heavy workload than this (because there’s no other work than IO in the benchmark), so the real-life difference is likely to be even smaller. Therefore, unless you need ridiculous amount of parallel connections (hundreds of thousands), you probably don’t have to care about performance when choosing your library.

The current use case for async actions is mostly light I/O work that would block the GUI if done sequentially. For instance, opening/saving files, performing HTTP requests, etc. Not really meant to spawn hundreds of thousands of concurrent actions. It is hard for me to picture a conventional use case where the executor will matter.

We need to also keep in mind that advanced users will always be able to roll their own event loop and use winit directly with the conversion module in iced_winit (or just use another shell entirely).

Additionally, this should really be completly non-disruptive for iced, because we can simply use futures::task::Spawn

At the very least, we will need to expose some way to configure the executor. While we could do this with a provided method in the Application trait, it would still expose an internal detail to our users and burden them with a choice.

@daxpedda
Copy link
Contributor

daxpedda commented Nov 20, 2019

We need to also keep in mind that advanced users will always be able to roll their own event loop and use winit directly with the conversion module in iced_winit (or just use another shell entirely).

Yeah that's good enough I would say.
Feel free to close the issue.

@Nemo157
Copy link

Nemo157 commented Jan 7, 2020

A major reason to do this is that tokio types will not work unless created on a thread running the Tokio runtime. async-std uses a background reactor so that its IO types can be run on other executors, but in order to use tokio based IO (more specifically tokio 0.1 types via tokio-compat) I had to run a separate runtime myself and spawn tasks on there, using a handle to the result as the Command so that iced's executor can notice its completion. This works, but it would be nice if I could just run my application directly on the tokio-compat runtime and have all background Commands spawned there.

@hecrj
Copy link
Member Author

hecrj commented Jan 8, 2020

@Nemo157 Yesterday, we had a related discussion in our Zulip server.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature New feature or request question Further information is requested
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants