immich
immich copied to clipboard
WIP: asset/album sync within an isolate
Hi,
I've had a problem with my phone locking up, and suspected it's the number of assets being synced - #10030. Looking at the code, it seems to be done on the UI thread. Isar has a recipe for multi isolate usage
I played around a bit with isolates (first time using it) and uploaded a profile build to my phone.
There's a lot less visual jank whilst the assets/albums are syncing. Importantly, the app doesn't permanently freeze anymore.
Have to say it's not without problems, the search page does some kind of syncing and it doesn't display properly at the moment, but i'm sure that's fixable.
Working in isolates is a bit challenging as the app has shared states. To that extent, I wasn't sure if the hashingService does something sequentially, which might pose a roadblock to this PR.
The demo server doesn't reproduce this problem, otherwise I would have made a demo video comparing before/after. I wonder if you could run the code and use the dart tools to measure FPS on a larger(than the demo) server so that you can see the impact.
It's currently a bit all over the place, just to test/validate the problem...
One nitpick about your PRs is that I see you often use abbreviations for variable names. Can you please write it out entirely instead? For example, receiver instead of rcv?
Thank you for this PR! I think this is a valuable short-term approach that should also make it easier to run these tasks in the background engine as well (mid-long term). I wanted to look into this already for some time :-)
Some important information on engines/isolates: We run two flutter engines (with currently 1 isolate per engine, but possible multiple). One engine is the foreground visible app, the other engine is invoked by the system to perform background backup (we plan to also put the syncing there).
We need to test the following:
- The app (both background backup and foreground sync) still works correctly with this additional foreground isolate.
- The main isolate is notified and re-renders if DB state changes in the DB isolate (e.g. syncing new photos causes the UI to rebuild and show them)
When moving all those DB operations to a separate isolate, we can also change any async DB ops to be synchronous (faster according to Isar docs and simplifies code). We need to be sure that any synchronous DB ops are only called from a non UI isolate, though.
The SyncService uses a locking mechanism to make sure operations are not running interleaved; with this PR we need to be sure that the SyncService methods are only ever called from the DB isolate.
Immediate feedback to your PR: Please move all DB isolate related code to a new service (instead of using the asset service). Please cleanup/remove unrelated changes to allow an unaltered comparison to check the functionality and review the code easier. Thank you!
Hi, will do, many thanks for having a look.
This PR is work in progress and I did this roughly to debug a problem I was having #10030.
The issue doesn't happen anymore when using isolates, but I suspect it might be unrelated actually. I'll add what I found into the issue itself and do some more exploring on isolates separately
We are reworking the sync process and will move the majority of the processing to the server to reduce the computing time on the mobile app. I am closing the PR for that direction. Please let me know if you have any question