CSCI-SHU-410-SE-Project / Deskulpt

Deskulpt (Desktop + Sculpt, pronounced /'deskʌlpt/), a cross-platform desktop customization tool.
https://csci-shu-410-se-project.github.io/Deskulpt/
MIT License
2 stars 0 forks source link

ENH make all Tauri commands async #218

Open Charlie-XIAO opened 2 weeks ago

Charlie-XIAO commented 2 weeks ago

See Tauri documentation on async commands. Synchronous commands will run on the main thread, which, noticeable or not, blocks UI rendering and other things that run on the main thread. See also the YouTube video made by one of the Tauri maintainers.

The things is, there is no reason to use synchronous commands. We currently call them only from the frontend, which is already an asynchronous behavior. By running on a separate thread we avoid freezing the UI and the theoretical performance would always be better. Though we may not notice currently, when developing #66 I made a very long-running command bundle_external, and if not made async, it makes the manager window completely unresponsive during its execution.

This is a major change so ping @Xinyu-Li-123 if you are interested.

github-actions[bot] commented 2 weeks ago

✔️ Deskulpt Built Successfully!

Deskulpt binaries have been built successfully on all supported platforms. Your pull request is in excellent shape! You may check the built Deskulpt binaries here and download them to test locally.

Workflow file: .github/workflows/build.yaml. Generated for commit: 8b0a91a.

Xinyu-Li-123 commented 2 weeks ago

I agree that we should make the corresponding rust apis async. But I do have a question. Are you sure synchronous tauri command will block the UI rendering? According to the Tauri process model, the (one and only) core process is responsible for executing the command, while the webview processes are responsible for rendering the UI.

Charlie-XIAO commented 2 weeks ago

That's interesting. Quote Tauri docs:

Asynchronous functions are benefical in Tauri to perform heavy work in a manner that doesn't result in UI freezes or slowdowns.

So I think UI rendering will indeed suffer from long-running synchronous commands. I'm thinking that though the core process is not in charge of the actual rendering, it still needs to do something to trigger the rendering in the Webview process and perhaps it's this thing that gets blocked. I mean, the Webview processes may be able to do only limited things, and those that require collaboration/communication with the core process will still freeze. Wild guess though 🤔

Xinyu-Li-123 commented 2 weeks ago

I saw that you made all internal commands async, but you leave the widget apis sync. I wonder if this is a design choice or is it just unfinished work?

Charlie-XIAO commented 2 weeks ago

Thanks for noticing that - it's my oversight and I'll fix soon :)

Xinyu-Li-123 commented 2 weeks ago

Could you elaborate on how the manager window freezes in #66? The frontend command for widget bundling is already async, so the manager window shouldn't freeze unless there is extensive resource usage, in which case it's cpu-intensive rather than io-intensive and concurrency shouldn't be helpful.

Xinyu-Li-123 commented 2 weeks ago

Also, I'd like to point out that, although less efficient, synchronous Rust apis makes programming easier on Rust side. For example, if we make the fs api asynchronous, we will need to consider concurrent access to the same file. Even each widget can only access its own resource, this could still happen if one widget invokes multiple apis in a short period, such as quickly creating multiple todolist items. I'm not sure if our use case could really gain benefit from asynchronous Rust apis, given our frontend apis are already asynchronous.

Charlie-XIAO commented 2 weeks ago

Also, I'd like to point out that, although less efficient, synchronous Rust apis makes programming easier on Rust side. For example, if we make the fs api asynchronous, we will need to consider concurrent access to the same file, which could happen if one widget invokes multiple apis in a short period. I'm not sure if our use case could really gain benefit from asynchronous Rust apis, given our frontend apis are already asynchronous.

I assume you are speaking of cases where we want to call the commands internally in Rust code. In widget APIs our endpoints will finally all be async (as they are meant to be called from the frontend); in this case we just need to add .await and that's not complicated. In the core things could become trickier, but we have the tauri::async_runtime::block_on as the last resolution. Another way is to make a separate sync function and wrap it into an async command.

Could you elaborate on how the manager window freezes in #66? The frontend command for widget bundling is already async, so the manager window shouldn't freeze unless there is extensive resource usage, in which case it's cpu-intensive rather than io-intensive and concurrency shouldn't be helpful.

I will try to make a minimal repo to see if I can showcase the problem. Hopefully that would provide more insight.

Charlie-XIAO commented 2 weeks ago

@Xinyu-Li-123 Try this: https://github.com/Charlie-XIAO/why-tauri-async-commands