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

Android: Use storage access framework for game list #9318

Merged
merged 11 commits into from Dec 30, 2020

Conversation

JosJuice
Copy link
Member

@JosJuice JosJuice commented Dec 10, 2020

This is part of my efforts to add support for scoped storage.

A commit from PR #9167 is included in this PR. It should be merged as part of this PR to avoid users getting old-style paths in Dolphin.ini.

Copy link
Member

@leoetlino leoetlino left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The amount of new code and hacky workarounds we have to add just for Android is very unfortunate, but I guess we don't have a better solution :/

Source/Core/Common/FileUtil.h Show resolved Hide resolved
@JosJuice
Copy link
Member Author

JosJuice commented Dec 25, 2020

We've started getting reviews on Google Play saying that you can't run games from the SD card anymore on certain phones after updating to Android 11. Merging this PR should solve that, though I can't say for sure since I don't have any such phone to test it on. (As far as I know, this problem is not related to the scoped storage requirement for apps that target Android 11, which was the original reason why I made the PR.)

@JosJuice
Copy link
Member Author

The issue (which now has been filed as https://bugs.dolphin-emu.org/issues/12364) has been confirmed fixed by this PR.

@Ebola16
Copy link
Member

Ebola16 commented Dec 28, 2020

Sorry I got really behind. I'll be able to review this within the next 12 hours but if an emergency merge is needed, go ahead.

Copy link
Member

@Ebola16 Ebola16 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I noticed that changes detected after rescanning the library now take ~6 seconds to update with content provider paths. Old paths take less than 1 second to update. Any ideas on improving this? Otherwise LGTM

@JosJuice
Copy link
Member Author

JosJuice commented Dec 28, 2020

I'll try to reimplement DoFileSearch in Java and see how much it helps. Due to my approach of wanting to make as few changes as possible to the existing C++ code and due to the SAF APIs not being very good, the current implementation of ScanDirectoryTree for SAF has O(n^2 + nm) behavior for a non-recursive scan (n being the number of files in a folder and m being how deep the "mangled" part of the path is), and doing this should let us bypass ScanDirectoryTree and get O(nm).

@JosJuice
Copy link
Member Author

Done. Please re-test the performance. The game list seems reasonably fast (as long as you're not using DirectoryBlob, which is still using File::ScanDirectoryTree and which I will not attempt to make any changes to right now).

@Ebola16
Copy link
Member

Ebola16 commented Dec 28, 2020

Unfortunately that did not improve library scanning times. I'm building a debug PR right now to confirm if DirectoryBlob's File::ScanDirectoryTree is being hit upon library rescan.

@JosJuice
Copy link
Member Author

Ah wait, I made a little mistake in FileSearch.cpp that made it so that DoFileSearch would use both the C++ and Java implementations when it was supposed to use only the Java implementation. Let me fix this.

@JosJuice
Copy link
Member Author

It's fixed now.

Copy link
Member

@Ebola16 Ebola16 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Library rescan takes about 4 seconds now. Good enough given what this PR fixes.

@JosJuice JosJuice merged commit c1d041b into dolphin-emu:master Dec 30, 2020
10 checks passed
@JosJuice JosJuice deleted the android-saf-games branch December 30, 2020 10:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
3 participants