Appcleaner takes almost 30 minutes to finish it's job with root privilege while with shizuku takes a few seconds.

raihan-emon commented 1 month ago

Describe the bug Well, the title said pretty much everything.

To Reproduce Steps to reproduce the behavior:

Go to '...'
Click on '....'
Scroll down to '....'
See error

Expected behavior A clear and concise description of what you expected to happen.

Smartphone (please complete the following information):

Device: [Poco M4 Pro 5G]
OS: [Android 13]
Root status [Rooted - Magisk v28]
SD Maid 2/SE version [1.2.8-rc0]

Debug log If you can reproduce the bug, record the event in a debug log.

Screenshots/Video If applicable, add screenshots or a video if it helps to explain your problem.

Additional context Add any other context about the problem here.

d4rken commented 1 month ago

Please provide a debug log

raihan-emon commented 4 weeks ago

Sorry for the late, here's the log. eu.darken.sdmse_logfile_1730345375237.zip

d4rken commented 3 weeks ago

Almost 30min being 19min here 😉, but i agree that it is somewhat slow.

For 2000 files from Chrome it took from 39:25 to 41:18, so almost 2min.

I'll look into speeding this up.

Most time is spend within the deletion routine here:

https://github.com/d4rken-org/sdmaid-se/blob/63f99877a93cfdee4683e2267857d4a799c93cc6/app/src/main/java/eu/darken/sdmse/appcleaner/core/forensics/BaseExpendablesFilter.kt#L32-L36

Not sure yet if the logic there is the issue (maybe could make it more efficient) or if the back and forth between root-nonroot process consumes too much time (could look into batching operations).

owocado commented 2 weeks ago

I also have this same issue. After a day or two of heavy usage of all my apps, I end up accumulating anywhere from 1.2 to 3+ gigabytes of cache and with root based setup in SD Maid SE, it takes about 10 to 20 minutes depending on the total size that is to be cleaned which at times feels painfully slow.

I am not sure if this is possible programtically in kotlin but would it be any faster if it deleted the outer cache folder instead of deleting thousands of files within it individually? because that's what it feels like right now.

One other nitpick that I observe in SD Maid SE vs. SD Maid is that the latter feels much more responsive in the info it shows while it is deleting files as it live displays full filename path of files in a fast manner as it is deleting them while SE only shows the path of outer folder while it is deleting thousands of files inside of it which sometimes feels like it's stuck. It would be great if the old app's behaviour in this context could be introduced in SD Maid SE.

App version: 1.3.0-beta1 OS: Android 13 (MIUI 14)

d4rken commented 2 weeks ago

SD Maid 1 had a whole different logic where everything was done by writing and reading from a shell terminal, so the actual deletion was done via rm ... which is quite fast, but very difficult to maintain and has a few edge cases that don't work well.

I will look into Kotlins File.deleteRecursively() but this will create new edge-cases: Because Android's permission system is so complex, SD Maid needs to reevaluate very often what access level is needed. E.g. You can access /storage/emulated/0 with normal permission, but can't access /storage/emulated/0/Android/data with the same level of permission. Then the recursive deletion could fail, then we have to check what was deleted and what was not, try it with other permissions or access modes.

Another issue is exclusions. If a subfolder is excluded, then we can't run recursive deletion on the parent folder. To know if there is an exclusion that matches, we also need to scan all subfolders.

Maybe the approach could be:

Introduce a APathGateway.deleteRecursive function, that gets called by the AppCleaner only, for folders like Android/data/<pkg> the permission level should be homogen. Internally this would use Kotlin's File.deleteRecursive
Before running it we check if there are exclusions that apply, if there are none then we use deleteRecursive and if there are some, then we fall back to the old behavior.

If Kotlin's File.deleteRecursive is still too slow we could theoretically also look into starting a shell for deletion, but this is very tricky, as the file path between the apps POV and the shells POV might be different.

Finally, if the access is through the SAF, then all of this doesn't work 🫠

In any case, I see that this is definitely annoying and will spend some more dev time on the deletion performance.

One other nitpick that I observe in SD Maid SE vs. SD Maid is that the latter feels much more responsive in the info it shows while it is deleting files as it live displays full filename path of files in a fast manner as it is deleting them while SE only shows the path of outer folder while it is deleting thousands of files inside of it which sometimes feels like it's stuck. It would be great if the old app's behaviour in this context could be introduced in SD Maid SE.

Do-able, but then again will also cost performance, maybe there is a trade off, I'll experiment with this. I didn't add this in SD Maid 2, because most users are confused by what is displayed, they just see random text. I was not aware of "feeling more responsive" being a positive effect that it had.

owocado commented 2 weeks ago

Android's permission system is so complex, SD Maid needs to reevaluate very often what access level is needed. E.g. You can access /storage/emulated/0 with normal permission, but can't access /storage/emulated/0/Android/data with the same level of permission. Then the recursive deletion could fail, then we have to check what was deleted and what was not, try it with other permissions or access modes [...]

I can understand this challenging aspect in normal scenario but ... wouldn't this be a non-issue in root based setup of SD Maid SE? Since the app would have r+w access to /data/data/ & /storage/emulated/0/Android/data etc restricted paths. Unless you were mentioning it in general sense where as I'm thinking of it from rooted POV since that's the context of this issue.

Do-able, but then again will also cost performance, maybe there is a trade off, I'll experiment with this. I didn't add this in SD Maid 2, because most users are confused by what is displayed, they just see random text. I was not aware of "feeling more responsive" being a positive effect that it had.

Well 😂 Maybe I'm the only one who finds that positive/helpful mainly because of my OCD. Though if adding that impacts performance then I would be ok without having it too. ✌🏼

And, thank you for giving this insightful response. It helped me visualize the challenges of this issue from perspectives that I never imagined I would. And thank you for making this super useful app, it has geniunely saved me so much time not have to manually go and delete cache from so many apps one by one. Your work is amazing, so yeah on that note, wish you a great day ahead!

d4rken commented 2 weeks ago

I can understand this challenging aspect in normal scenario but ... wouldn't this be a non-issue in root based setup of SD Maid SE? Since the app would have r+w access to /data/data/ & /storage/emulated/0/Android/data etc restricted paths. Unless you were mentioning it in general sense where as I'm thinking of it from rooted POV since that's the context of this issue.

Your way is possible. My POV might have been a bit too overthought. Best possible compatibility with devices (and future Google changes) is often ad odds with speed. So performance is often not top priority if it means easier maintenance (to avoid the fate of SD Maid 1 becoming unmaintainable).

That doesn't mean when users help me, point things out and give feedback, that there isn't something that can be done :blush:.

Well 😂 Maybe I'm the only one who finds that positive/helpful mainly because of my OCD. Though if adding that impacts performance then I would be ok without having it too. ✌🏼

By moving nested deletion deeper into the stack I could speed it up a lot.

Previously the traversal of directories (to delete dirs with content) happened in the main app process, and only the directly lookup and deletion call (for single items) went deeper into the stack and then (depending on the required permission) into the ADB or the Root process (process here is still SD Maids code, but launched within the ADB or root user to use their elevated permissions). The back and forth between the processes caused most of the delay.

Now with the faster deletion, the traversal directly happens in the extra process running as root or adb user. No more back and forth between the processes. The downside being, the deletion routine can no longer report back at a fine grained level which sub-directory/file is currently being deleted. Passing that progress information back to the normal app process would cost too much time. I think it's an okay trade-off. This specific progress reporting was only used within the CorpseFinder.

I'll create a new beta soon, looking forward to your feedback.

And, thank you for giving this insightful response. It helped me visualize the challenges of this issue from perspectives that I never imagined I would. And thank you for making this super useful app, it has geniunely saved me so much time not have to manually go and delete cache from so many apps one by one. Your work is amazing, so yeah on that note, wish you a great day ahead!

:+1: I love talking about the code :beers:

d4rken-org / sdmaid-se

Appcleaner takes almost 30 minutes to finish it's job with root privilege while with shizuku takes a few seconds. #1443