Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Don't copy archives to temp files in local source #326

Merged
merged 1 commit into from
Feb 3, 2024

Conversation

FooIbar
Copy link
Contributor

@FooIbar FooIbar commented Feb 2, 2024

Closes #134

@AntsyLich
Copy link
Member

Does it not make loading slower due to SAF?

@FooIbar
Copy link
Contributor Author

FooIbar commented Feb 2, 2024

I mean, we are already using SAF.
It's not like we can bypass it unless requesting MANAGE_EXTERNAL_STORAGE.

@FooIbar
Copy link
Contributor Author

FooIbar commented Feb 2, 2024

Benchmark for a ~340 MB archive to load on test device

Before: 4000~5000 ms (~80% of time on copy to temp file)
After: 1000~1500 ms

@AntsyLich
Copy link
Member

That's quite a bit of improvement. Seems okay to merge I guess. For the records can you also do a benchmark for some smaller sizes like ~20mb ~50mb ~100mb

@FooIbar
Copy link
Contributor Author

FooIbar commented Feb 2, 2024

Archive size [MB] Avg. before [ms] Copy time Avg. after [ms] Diff
3.27 1245 13% 1282 +2.9%
46.8 1387 20% 1435 +3.5%
103 1991 29% 1789 -10%
194 3514 74% 1238 -65%
339 6829 85% 1408 -79%

@AntsyLich
Copy link
Member

Everything looks fine tho how is the performance on lower ram devices 🤔

@FooIbar
Copy link
Contributor Author

FooIbar commented Feb 2, 2024

Will do more tomorrow.

@FooIbar
Copy link
Contributor Author

FooIbar commented Feb 3, 2024

The results above were obtained from a 2GB RAM AVD using debug build.
It may not represent real user experience, so I did another one on a 12GB RAM physical device with release build.

Archive size [MB] Avg. before [ms] Copy time Avg. after [ms] Diff
46.8 110 26% 96 -13%
103 150 36% 96 -36%
194 199 60% 95 -52%
339 280 72% 80 -71%

@AntsyLich AntsyLich merged commit 0da7ad6 into mihonapp:main Feb 3, 2024
1 check passed
@FooIbar FooIbar deleted the archive branch February 4, 2024 05:07
@AntsyLich
Copy link
Member

So apparently this doesn't work on Fold 5 and S24 Ultra

@FooIbar
Copy link
Contributor Author

FooIbar commented Feb 25, 2024

So apparently this doesn't work on Fold 5 and S24 Ultra

Could you provide more details to illustrate it?

Shamicen added a commit to Shamicenso/TachiyomiSY that referenced this pull request Mar 6, 2024
Co-authored-by: FooIbar <118464521+FooIbar@users.noreply.github.com>
Co-authored-by: AntsyLich <59261191+AntsyLich@users.noreply.github.com>
(cherry picked from commit 0da7ad6)
Shamicen added a commit to Shamicenso/TachiyomiSY that referenced this pull request Mar 6, 2024
Co-authored-by: FooIbar <118464521+FooIbar@users.noreply.github.com>

Co-authored-by: AntsyLich <59261191+AntsyLich@users.noreply.github.com>

(cherry picked from commit 0da7ad6)
Shamicen added a commit to Shamicenso/TachiyomiSY that referenced this pull request Mar 6, 2024
Co-authored-by: FooIbar <118464521+FooIbar@users.noreply.github.com>
Co-authored-by: AntsyLich <59261191+AntsyLich@users.noreply.github.com>
(cherry picked from commit 0da7ad6)
Shamicen added a commit to Shamicenso/TachiyomiSY that referenced this pull request Mar 6, 2024
Archives are now being read from channels

Co-authored-by: FooIbar <118464521+FooIbar@users.noreply.github.com>
Co-authored-by: AntsyLich <59261191+AntsyLich@users.noreply.github.com>
(cherry picked from commit 0da7ad6)
Shamicen added a commit to Shamicenso/TachiyomiSY that referenced this pull request Mar 6, 2024
Archives are now being read from channels

Co-authored-by: FooIbar <118464521+FooIbar@users.noreply.github.com>
Co-authored-by: AntsyLich <59261191+AntsyLich@users.noreply.github.com>
Shamicen added a commit to Shamicenso/TachiyomiSY that referenced this pull request Mar 6, 2024
Archives are now being read from channels

Co-authored-by: FooIbar <118464521+FooIbar@users.noreply.github.com>
Co-authored-by: AntsyLich <59261191+AntsyLich@users.noreply.github.com>
jobobby04 pushed a commit to jobobby04/TachiyomiSY that referenced this pull request Mar 16, 2024
* implement mihonapp/mihon#326

Archives are now being read from channels

Co-authored-by: FooIbar <118464521+FooIbar@users.noreply.github.com>
Co-authored-by: AntsyLich <59261191+AntsyLich@users.noreply.github.com>

* disable parallelisms for loading into memory

* switched to mutex

* detekt changes

* more detekt baseline changes

---------

Co-authored-by: FooIbar <118464521+FooIbar@users.noreply.github.com>
Co-authored-by: AntsyLich <59261191+AntsyLich@users.noreply.github.com>
cuong-tran pushed a commit to komikku-app/komikku that referenced this pull request Mar 18, 2024
* implement mihonapp/mihon#326

Archives are now being read from channels

Co-authored-by: FooIbar <118464521+FooIbar@users.noreply.github.com>
Co-authored-by: AntsyLich <59261191+AntsyLich@users.noreply.github.com>

* disable parallelisms for loading into memory

* switched to mutex

* detekt changes

* more detekt baseline changes

---------

Co-authored-by: FooIbar <118464521+FooIbar@users.noreply.github.com>
Co-authored-by: AntsyLich <59261191+AntsyLich@users.noreply.github.com>
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 24, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Local archives are being fully copied to temp folder every time they're opened for reading
2 participants