Skip to content

Create a thread for network events#322

Merged
Rigellute merged 1 commit into
masterfrom
async-await-tokio-3
Mar 9, 2020
Merged

Create a thread for network events#322
Rigellute merged 1 commit into
masterfrom
async-await-tokio-3

Conversation

@Rigellute
Copy link
Copy Markdown
Owner

This is a huge PR that changes the architecture of the app considerably.

The idea is to create thread that will handle network events, while the main thread will handle the UI rendering loop.

The implication of this change is that the UI thread will disptach (or send) messages to the network thread for it then to execute the async action.

Thus, slow network calls will no longer block the UI thread meaning the app should feel more responsive on slow connections.

I have tested on very slow internet connections and can see these changes will drastically improve both #92 and #207.

Given the scale of these changes, I encourage anyone to checkout this branch and test!

@Rigellute Rigellute mentioned this pull request Mar 5, 2020
@Rigellute Rigellute force-pushed the async-await-tokio-3 branch from c40ea9f to 92ed03c Compare March 5, 2020 12:55
@Rigellute Rigellute mentioned this pull request Mar 5, 2020
@Rigellute Rigellute force-pushed the async-await-tokio-3 branch from 52a9b35 to 82b0804 Compare March 5, 2020 14:52
@ericonr
Copy link
Copy Markdown
Contributor

ericonr commented Mar 7, 2020

Just started trying it out, thanks for all the hard work! It seems to be much more responsive.

Should I make issues in the issue tracker for this?

When I press v to open the audio visualizer, if I press q before it loads the visualizer, the quit action is passed to whatever context I'm in, meaning it can even close the application. Testing in the previous version, pressing q before the visualizer opened just means that the visualizer will quickly flash on the screen, because it's closed right after.

Perhaps you can announce this update somewhere, so people come try it out? Somewhere like r/unixporn or r/rust.

@Rigellute
Copy link
Copy Markdown
Owner Author

Thanks @ericonr.

Please do create an issue for that!

I think we should navigate before the async action has completed to make it feel more responsive. I also think there will be plenty of other issues caused by the move to full async 😄 .

And absolutely, once this is released I intend to announce on r/rust and r/unixporn!

@Rigellute Rigellute force-pushed the async-await-tokio-3 branch from d45c439 to 5dda717 Compare March 9, 2020 09:07
Create a thread that will handle network events, while the main thread will handle the UI rendering loop.

The implication of this change is that the UI thread will disptach (or send) messages to the network thread for it then to execute the async action.

Thus, slow network calls will no longer block the UI thread meaning the app should feel more responsive on slow connections.

Closes #92.
Closes #207.

Use released version of rspotify

Make it work

Aquire lock after network request

Continue migrating async jobs to other thread

Move get saved tracks to network thread

Move start_playback to network thread

Update search limits in network

Spawn a new tokio task for each network event

Not sure if this is good or not? Seemed to work fine without spawning on
each event

Show loading indicator in UI

Move more network events to network thread

Complete network migration

Move app lock outside of loop

Fix polling

Fix search performance

Clean up country fetching code

Add app to network struct

Move start UI to another function

Dispatch async events from within network

Remove futures

Implement default for app

Fix tests

Fix clippy suggestions

Move refresh auth function to method

Box up large variant and fix clippy

Use Box for large variant GetAlbumTracks arg

Ignore clippy complexity for handle_network_event

This warning is unhelpful in this case since it's easier to see all the
io_event variants matched in one place

Correctly update polling status

Regenerate lock file
@Rigellute Rigellute force-pushed the async-await-tokio-3 branch from 5dda717 to dae20e1 Compare March 9, 2020 10:14
@Rigellute Rigellute merged commit 3f19eb7 into master Mar 9, 2020
@Rigellute Rigellute deleted the async-await-tokio-3 branch March 9, 2020 12:13
@netishix
Copy link
Copy Markdown

netishix commented Mar 9, 2020

Amazing. Just tried it out. It works like a charm. Thank you very much!

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