861 library loading is blocking the main thread #974
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #974 +/- ##
=======================================
Coverage ? 39.52%
=======================================
Files ? 113
Lines ? 6444
Branches ? 0
=======================================
Hits ? 2547
Misses ? 3897
Partials ? 0 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
LGTM ; very useful improvement I'd say.
My only concern being on the UI. Now that it works well, could we imagine the visual spinner to be bound by the button's dimensions?
In the library card example, it works well because the loader is contained inside the card.
On the open homepage button, it doesn't because it's floating on top with the same (larger than the button) dimensions.
Notye: this comment is based on the screencast.
BPerlakiH
force-pushed
the
861-library-loading-is-blocking-the-main-thread
branch
from
ef8de47
to
81a5a48
Compare
|
@rgaudin the progress bar issue has been addressed: progress_view_small.mov |
|
Yep, looks good now |
|
@BPerlakiH Can you please rebase? |
BPerlakiH
force-pushed
the
861-library-loading-is-blocking-the-main-thread
branch
from
8b7a3d5
to
2a8a61a
Compare
kelson42
deleted the
861-library-loading-is-blocking-the-main-thread
branch
Fixes: #861
Summary:
I measured that the longest operation we have, when loading the library is:
[Archive](zim::Archive archive = zim::Archive([url fileSystemRepresentation]);The way it worked so far:
We opened all the former files that were added to the local library, and passed through this Archive reading and stored it in memory.
After that was done, opening and displaying (any content really from) that ZIM file was quite quick.
The problem:
When we have relatively small files, or not much files added to the library, there's no visible issue.
Problems start when we have large files. Eg. a 100 GB wiki ZIM file, is taking time to open with the aforementioned
Archive.The solution:
Open the
Archiveon demand. Meaning, if we want a content from a ZIM file, only at that point open theArchive, and store it at that point, as opposed to app start. Subsequent requests to the get content (or metadata) from the same ZIM file will be equally quick as before. This makes the app start a lot faster, since we do not need to open those archives anymore on start.Handle the side effects:
The side effect is that when we press on a button (eg. to open the home page of a local library ZIM file), won't open it right away, so we need some UI loading indicator that we are "working" on it.
For this I have added 2 new classes,
AsyncButtonandAsyncButtonView, which can handle alternative loading states, while the underlying task is working.Handle concurrency:
Formerly most of the ZIM file operations were running on the main thread, which was safe in a way that no other threads were accessing the same: list of
Archiveor other IO operations outcomes.The solution for this is a new
@globalActor, that is: single instance, and only allows async access to theZimFileServiceinstance, and this way guarantees that the stored data within is only accessed in a thread safe manner.Preview on mac:
async_loading_mac.mov
Adjusting search:
The other point to solve was searching. By default we can search in any local ZIM file, or to be precise the ones that were selected by the user to search in. Since the Search is initialised on start, it made sense to do the opening of the ZIM files at that point. This way we don't have a race between app start opening the ZIM archives and the user starting to search. This has also other benefits, as if the user excludes a ZIM file from search results (which is saved to the next app session), we do not need to open that archive. As soon as the user is (re)including that file in search results, the archive will be opened. The asynchronous part of this is handled during the search result loader / spinner: