alienator88 / Pearcleaner

A free, source-available and fair-code licensed mac app cleaner
https://itsalin.com/appInfo/?id=pearcleaner
Other
2.71k stars 62 forks source link

[BUG] Use too much CPU. #82

Closed megabitsenmzq closed 1 month ago

megabitsenmzq commented 1 month ago

Hi!

Describe the bug:
The app uses too much CPU while scanning (around 500% on my M1 Macbook Air). It even slows down my mouse movement.

After the initial scan, the CPU usage does drop. But if I do a leftover scan, it will keep the high CPU usage even after the scan is finished.

As a comparison, "App Cleaner and Uninstaller" uses around 180% of CPU while scanning. And it found a lot of leftover files. Pearcleaner found zero. (Maybe I'll create another issue for that.)

Pearcleaner can be a great app, as many "cleaner" apps don't even have the feature to scan leftover files. Hope you can find a way to resolve this bug.

Steps to reproduce:

  1. Just open the app.

  2. Check the CPU usage.

  3. Do a leftover scan.

  4. Check the CPU usage after the scan is finished.

Expected behavior
It should not use that much CPU.

Info:

alienator88 commented 1 month ago

I think there's something else going on there if it makes your computer laggy AND it's finding zero files, maybe a race condition. macOS normally handles the background tasks automatically and should prevent using so much that your computer starts lagging like that. This feels like some type of CPU callback leak or something similar. Unfortunately I can't reproduce this lag/no result issue yet on multiple computers and haven't really seen any other reports yet. Maybe others could chime in as well if it's more widespread.

Aside from that, there might be a misunderstanding on how apps use CPU on macOS.

It should not use that much CPU.

This line isn't entirely accurate since the search function is setup to search files concurrently by offloading the work on multiple threads at the same time. For example, 200% CPU usage would mean it's using 2 cores fully. In your case at 500% it means it's using 5 cores fully out of your 8 during the search. On my M1 Pro with 8 cores also, it hovers around 600-700% for a bit before dropping back down. But I never get any lag and do get search results. I can't speak for "App Cleaner and Uninstaller" since I don't know how they are searching for files. This is not to say that I can't improve my search function.

I could "patch" this by adding a slight delay between tasks, but I will keep messing around with it and see what works best. I might hold off on a patch for now though, as I was in the process of writing a new search function that uses low level APIs which should speed up the search and lower the cpu usage. From my limited testing using this new search function, I'm only getting around 170-180% CPU usage while searching around 215GB of data.

I'll keep this bug open to track the work on this new search function.

megabitsenmzq commented 1 month ago

I checked the CPU usage because the app makes my mouse lag. That's why I say it should not use that much CPU. It's not about the number, it's about the lag. I understand what the number means.

Great to hear about the implementation of the new search function. Tell me anytime if you need me to give it a test. I guess I will also check the code when I have time. Compile it locally to see why the leftover scan is not working.

alienator88 commented 1 month ago

Understood, but your experience of the mouse lag and no files found is not the norm from what has been reported so far since March when this feature was released. There's something else happening there causing that issue, either the app's logic or less likely, something environmental with the OS.

If you do look at the code, check the function here: https://github.com/alienator88/Pearcleaner/blob/2da3da9ce7ea86a5e60dc2282eb18cf9cbf4fb0a/Pearcleaner/Logic/Logic.swift#L112

From profiling the CPU, the bulk of the high usage is coming from the AppPathFinder execution in that DispatchQueue. https://github.com/alienator88/Pearcleaner/blob/2da3da9ce7ea86a5e60dc2282eb18cf9cbf4fb0a/Pearcleaner/Logic/Logic.swift#L119

If I change the existing code block surrounding that function to add a slight delay between tasks, it does bring the CPU usage down like 50% for example, if I add 0.5 to it:

for (index, app) in allApps.enumerated() {
        DispatchQueue.global(qos: .background).asyncAfter(deadline: .now() + Double(index) * 0.5) { // Delay each task slightly from 0.0 - 1.0
            AppPathFinder(appInfo: app, appState: appState, locations: locations, backgroundRun: true, reverseAddon: reverseAddon).findPaths()
            dispatchGroup.leave()
        }
        dispatchGroup.enter()
    }

What it does is it searches all the files related to your currently installed apps using the AppPathFinder function and then it searches for leftover files while excluding all those installed app files using the ReversePathsSearcher function. I'll be working on this to see if I can use the new search function I'm working on for the AppPathFinder logic, maybe I can cut that down. But it will have to be next week probably, going on vacation tomorrow for a few days.

megabitsenmzq commented 1 month ago

I think maybe you can set maxconcurrentoperationcount to control the CPU usage, it’s more precisely than adding delay.

Also rewriting this in Swift Concurrency might make it easier to to manage search tasks.

Like adding 100 tasks into a TaskGroup, then add another 100 after this group is finished. (https://stackoverflow.com/questions/70976323/how-to-constrain-concurrency-like-maxconcurrentoperationcount-with-swift-con)

This can also make it easier to cancel the search in the middle of the task. (Task.canceled() Task.checkCancellation())

alienator88 commented 1 month ago

So I tried that earlier and it didn’t seem to help on the Reverse search function. But that was before I profiled the CPU to see what was taking up the most CPU cycles 😑 When I get back from vacation this weekend, I’ll give it another go on the AppPathFinder function, hopefully it helps there.

Thanks!

On Tue, Jul 23, 2024 at 8:50 PM Jinyu Meng @.***> wrote:

I think maybe you can set maxconcurrentoperationcount to control the CPU usage, it’s more precisely than adding delay.

Also rewriting this in Swift Concurrency might make it easier to to manage search tasks.

Like adding 100 tasks into a TaskGroup, then add another 100 after this group is finished. ( https://stackoverflow.com/questions/70976323/how-to-constrain-concurrency-like-maxconcurrentoperationcount-with-swift-con )

This can also make it easier to cancel the search in the middle of the task. (Task.canceled() Task.checkCancellation())

— Reply to this email directly, view it on GitHub https://github.com/alienator88/Pearcleaner/issues/82#issuecomment-2246760513, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABPZGSREZVAX7DCJ2N7JPFLZN4I6RAVCNFSM6AAAAABLJMRE2WVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDENBWG43DANJRGM . You are receiving this because you commented.Message ID: @.***>

megabitsenmzq commented 1 month ago

I tried to make an opensource app like this because "App Cleaner" can't clean leftovers, and "App Cleaner and Uninstaller" is too expensive. But I didn't have much time so I never finished it. I'm happy to see someone did it.

Have a good vacation!

alienator88 commented 1 month ago

Hey, made some changes to what we talked about last week. Could you let me know how this build runs on your machine? Side note, this one isn't signed/notarized, just archived it real quick so I can debug. Also added a real progress to the function and a way to cancel the operation.

Pearcleaner.zip

megabitsenmzq commented 1 month ago

Hey, made some changes to what we talked about last week. Could you let me know how this build runs on your machine? Side note, this one isn't signed/notarized, just archived it real quick so I can debug. Also added a real progress to the function and a way to cancel the operation.

Pearcleaner.zip

It uses less CPU than before, about 200%. But the app UI still lags a little bit while searching. Search leftover is very slow, but it works now!

alienator88 commented 1 month ago

That's wild, I wonder if it's data size related. For example I only have about 50 apps installed. Takes about 20s or so to do the whole leftover search. How many apps do you have and how long does it take?

Was thinking about this issue trying to see if I can cut down the workload this function is doing and came up with something that might work. Currently I search for all related files for installed apps to exclude those from the leftover search. Instead I might be able to just filter out the currently installed app names and bundle ids since I already have those in memory from generating the app list. It might be a lot lighter this way not having to find all app files for each app beforehand.

megabitsenmzq commented 1 month ago

I have 254 apps 😆. It takes about 3 minutes.

alienator88 commented 1 month ago

Holy crap! No wonder I couldn't reproduce 😂 So my idea from last night I think is working way better than the old logic, at least on my end. I dropped my search from ~20 seconds to about ~1 second 😮 Because it runs so fast, my CPU barely hits 50% for like a second or so. Can you try this debug build for me and see how it runs with all your apps? I predict ~5 seconds but maybe up to 10 seconds max. Pearcleaner.zip

megabitsenmzq commented 1 month ago

Wow! This is the speed of light! 1 second. The process view almost becomes useless 😂.

And for the per app search, I thought maybe you could search lazily if the app is not visible in the scroll view. It might improve the UI performance when the app opens. For now, the whole UI will freeze for 1 second at start.

alienator88 commented 1 month ago

Nice, that's good to hear!

And to confirm, for the app search, you're referring to the actual app list itself in the sidebar? And it only lags when trying to search it, not when it loads initially, yeah?

megabitsenmzq commented 1 month ago

I do mean it lags when it loads initially. It might be due to my large app collection😂. So I think maybe it's better not to do file search for all the apps at first. Just search files for the apps currently visible. And you can search for the rest when the user scrolls the ScrollView and apps appear.

megabitsenmzq commented 1 month ago

https://github.com/user-attachments/assets/643ab826-635c-4b2e-b53a-e76d74f74cb5

Check this video!

alienator88 commented 1 month ago

Ohhh..I wonder if its the size calculation causing it. I disabled the size function for this debug build. Can you see if it still shows the same issue without that running? I want to figure out first if it's the list of apps getting loaded causing the slow down, or if calculating the size for each app. I will also attempt to get the app list to load lazily since currently it waits to gather all the app list details first. Pearcleaner.zip

megabitsenmzq commented 1 month ago

This build seems fine.

alienator88 commented 1 month ago

Good to know..I'll play around with it to see if I can get the sizes to load for each app only if it's in view and as you scroll load the others.

alienator88 commented 1 month ago

Can you give this one a go? I made the app list lazy load so it should only calculate the sizes for whatever is on screen or pre-loaded for scroll. Pearcleaner.zip

megabitsenmzq commented 1 month ago

Got it! I'll try it when I get home. My work Mac doesn't have many apps.

megabitsenmzq commented 1 month ago

It works! Perfect!

alienator88 commented 1 month ago

Nice! Glad to hear it's finally good 😂 Pushing out a new build tonight with the fixes. Thanks so much for QA testing this, I don't have a ton of apps like that so it was helpful.

megabitsenmzq commented 1 month ago

Glad it helped. I can finally recommend this app to people! Great work! 🎉