haecker-felix / Gradio

GNU General Public License v3.0
326 stars 51 forks source link

Add station url requst to a new thread. #331

Closed sigmaSd closed 6 years ago

sigmaSd commented 6 years ago

clicking play button freeze the whole app for a few seconds , so I offloaded the url request into a new thread. Works pretty well. Not sure if there is a better way though.

sigmaSd commented 6 years ago

I applied a new method but I doubt you will agree with it , so can I ask for some pointers on how to do the url request in an async way , or its too much trouble to add it now ?

haecker-felix commented 6 years ago

Huh... Using AtomicUsize look more complicated than I thought.... I'll look if I can simplify it, otherwise it's better to use Arc/mutex.

sigmaSd commented 6 years ago

Its not actually complicated , the only thing extra job with this approach is to add clone function manually cause AtomicUsize doesn't have clone nor copy trait. Arc<<Mutex has a performance overhead you should consider. But Its true that its still feels like there could be a better approach to this problem.

sigmaSd commented 6 years ago

I'm curious can't we just use a simple u64 instead , why do you need Rc<<RefCell in the first place? I tried it and it works normally .

haecker-felix commented 6 years ago

I'm not sure if u64 directly works, because we need to access the value in the timeout closure, otherwise the old search results will be returned too (see line 128 client.rs)

You can check this, by typing something fast into the searchbox. Now you should see some "Search ID changed -> cancel this search loop..." debug messages (you have to enable debugging).

Short:

sigmaSd commented 6 years ago

You're right , I played around with this , You have to keep track of the current_search with Arc<<Mutex , doesn't work with u64 because it just gets copied around so I end up with search_id and current_search_id always the same and I cant send the original value to the timeout closure.

sigmaSd commented 6 years ago

Guess there is no need for heavy modifications!

haecker-felix commented 6 years ago

It seems everything is fine. I'm currently busy, so i cannot merge it right now. I want to test it before. It's not your fault. I'm sorry!

sigmaSd commented 6 years ago

Don't be mate , I'm just learning and trying out ideas you can accept or not its cool. Best of luck !

haecker-felix commented 6 years ago

Works flawless. Thanks for your work! I'm sorry, that I needed that long to merge it actually.

Feel free to continue to work on Gradio!

I found a pretty interesting piece of software, in the next days I'm going to to test this: https://crates.io/crates/mdl

Maybe it can improve the code structure a bit :-)

sigmaSd commented 6 years ago

Thanks for your words :) It actually turned out to be better that it took so long since PR looks now way better than It looked at first.

https://crates.io/crates/mdl

Looks Interesting , hope it turns out well !

haecker-felix commented 6 years ago

Yeah, looks very intresting, but all objects which should be "cached", needs to implement the serde Deserialize/Serialize trait, which is a bit hard for some objects...

fyi: https://gitlab.gnome.org/danigm/mdl/issues/1

I also think it's better, if something needs longer, but it's well done then.

sigmaSd commented 6 years ago

The truth is ,its not very clear to me what this crate is . Hopefully the author is helpful and will give you at least some guidelines.

haecker-felix commented 6 years ago

@sigmaSd first part of the rework is now pushed to master. The big advantages: