[Improvement] Async bottle loading #574
Conversation
| programs.append(contentsOf: favourites) | ||
| programs.append(contentsOf: nonFavourites) | ||
| private func sortPrograms() { | ||
| DispatchQueue(label: "whisky.lock.queue").async { |
There was a problem hiding this comment.
In general, we don't use GCD in Whisky, using structured concurrency would be preferable.
There was a problem hiding this comment.
I opted for using a DispatchQueue in this specific scenario to ensure bottle loading operations get queued up in the same thread. Structured concurrency would not have this guarantee (at least as far as I know), and in general I avoided going down the route of rewriting Bottle to be an Actor.
There was a problem hiding this comment.
Actor based approach might be preferred here.
| pin: pin, | ||
| loadStartMenu: $loadStartMenu, | ||
| path: $path) | ||
| ZStack { |
There was a problem hiding this comment.
I don't think this ZStack is needed. Why block the user from accessing loaded pins until they've all loaded?
There was a problem hiding this comment.
Hmmm, I see what you mean - yeah, maybe a less fancy but less obstructive way of displaying the indicator would be better indeed. I'll change that, thanks for the feedback :)
|
@vcsoares Rebase needed after recent reorganisation |
|
Hi - just chiming in that life got a little hectic these weeks and I haven't had much time to resume work on this. As soon as I get some spare time I'll address your comments @IsaacMarovitz. Thanks! |
What?
This PR makes the methods responsible for loading and updating a bottle's program list run asynchronously. It also adds indicators in the Bottle and Program views to make the user aware there's a loading operation in progress.
Why?
With relatively large bottles, loading the programs list can sometimes take a long while. Whisky used to do this synchronously, which in turn would lock the app until the operation finished, leading to degraded performance every time you switched bottles or changed pinned programs, as well as slow startup times.
How?
By moving execution of methods that load a bottle's program list to a dedicated DispatchQueue and adding a lock to the Bottles struct to protect accesses as needed, since there's now a chance for concurrent load operations to happen. Besides that, calls to
updateInstalledPrograms()have been reduced, and this method will only run if the programs list for the current bottle is empty. This means that pinning/unpinning programs in the programs list won't trigger a full reload, for instance.UI affordances were implemented using common SwiftUI components.