Repository navigation
Replies: 2 comments
|
@ManfredFr This is the most useful report anyone has sent this project, and I am sorry it sat for two weeks. I have read all six findings against the current code and every one of them lands on something real, so let me answer them individually rather than thank you generically. The Cache writes on the scan path (2). Correct,
Cache admission (3). Worth re-measuring, and 4,430 admitted entries in one Spotlight (5). You measured it at 0.05s and said so rather than presenting it as a win. That is a large part of why I trust the rest of the report. Correct, but not worth the concurrency. Leave it.
On brrw: thank you for reading TRADEMARK.md, and for the separate name, separate icon and explicit non-affiliation notice. That is exactly the right way to do it and I have no objection. Publish it. Please do open the PRs. Start with |
|
@ManfredFr Finding 3 measured, since that one was mine to answer rather than yours to patch. Cold scan of
38% of admitted entries (290) hold fewer than 100 files; they cleared the size threshold alone. Re-applying stricter rules to the same population: I am leaving the constants alone. The thresholds already did the job they were written for, from the 1.88M files / 7.82 GB in the One caveat on my own number: you measured 4,430 entries on 616k files, I measured 764 on 255k. That is 2.4x the entries producing 5.8x the cache files, so tree shape drives this more than tree size and neither of our single samples generalises. Status of the rest, so nothing is left implied: Closing this one, since every finding now has an answer. Open the PRs as PRs; they do not need this thread. |
Uh oh!
There was an error while loading. Please reload this page.
Hi tw93 and everyone,
First: thank you for Mole. I have been using it for a while and it is the
first Mac cleanup tool I actually trust, mostly because of how carefully the
delete paths and the protection lists are written. The amount of macOS
edge-case knowledge in this repo is remarkable.
I want to be upfront about two things: I am not proposing that Mole adopt
Rust, and I am not asking anyone to review or maintain anything. I did a
port for my own use, learned some things while profiling it, and the findings
are language-independent — so they may be worth something to you even though
the port itself probably isn't.
What I did
I ported the
analyzemodule (cmd/analyze/*.go) to Rust as an exercise, onemodule at a time, treating your Go code as the specification. Every file in
the port names the Go file it came from.
The
--jsonmode turned out to be the thing that made this feasible: I couldrun
analyze-go --jsonand my binary over the same tree and diff the outputstructurally, so "is this correct?" had an actual answer instead of a vibe.
Whatever it was originally added for, it is an excellent testability
decision — thank you for it.
Current state: on
/Applications(227 GB, 616k files) and~/Library/Application Support(64 GB, 61k files), both binaries report a0-byte difference in total size and identical file counts.
Findings that could apply to the Go code
All measurements are from an Apple Silicon MacBook on APFS, cold scans (fresh
HOME, so no cache reuse), binaries run sequentially and alternating. Bigcaveat: these were measured in a Rust reimplementation, so the numbers would
land differently in Go. The directions should hold; the magnitudes may not.
1.
getattrlistbulk(2)instead ofreaddir+ per-filelstat— biggest single winReplacing directory enumeration with
getattrlistbulk(one syscall returnsnames plus type, sizes, link count, dev/ino and atime for batches of entries)
was worth roughly 1.4x on its own:
/ApplicationsTwo caveats I paid for by measuring:
pure sizing walk (where you only want bytes), plain
readdir+lstatmeasured about 2x faster than bulk — the per-entry kernel work for name
resolution and attribute packing is not free. I ended up using bulk in the
scanner and deliberately keeping
readdirin the sizing walk.fresh 256 KB buffer per directory ate a visible chunk of the win on trees
with many small directories.
ATTR_FILE_TOTALSIZE, which sums all forks, and it silently inflatedevery resource-forked file (audio components, plugins) — a 52 MB gap
against
analyze-goon a/Libraryscan.stat(2)is asymmetric and themapping has to match it exactly:
st_size= data fork =ATTR_FILE_DATALENGTH, butst_blocks= all forks allocated =ATTR_FILE_ALLOCSIZE.Ordinary test fixtures have no resource forks, so this hides. If you try
it, put a file with a resource fork in the fixture.
In Go this needs a raw
SYS_GETATTRLISTBULKsyscall rather than a ready-madewrapper, plus record parsing, so it is a real chunk of work — but it is the
single highest-leverage change I found, and it is entirely independent of
implementation language. Keeping
readdiras a runtime fallback (and a testasserting both enumerators return identical attributes) made it safe to
adopt.
2. Cache writes sit on the scan's critical path
saveCacheToDiskWithOptionsis called inline fromscanSubdirWithCache, soscan workers serialize, write and rename cache files while the scan is still
running. One cold
/Applicationsscan writes 4,430 cache files.Measured in my port: 2.45s with inline cache writes vs 2.0s with cache
I/O disabled entirely — so roughly 18–20% of a cold scan is cache
persistence. Moving it to a background writer drained after the scan brought
it to 2.19–2.26s.
The counterintuitive part, and the reason I mention it: more writer threads
did not help. Four writers performed the same as writing inline (~2.45s); a
single background writer was the fast configuration. The cost is
filesystem-layer contention with the scan's own metadata reads, not worker CPU
time. If you try this in Go, a buffered channel plus one writer goroutine
(drained before exit) looks like the shape that wins.
3. Cache admission may be looser than the design intent
The budget comment in
constants.goreasons that the thresholds keep "the~1.5% of entries that carry the reuse value". With
subdirCacheMinFiles = 100/subdirCacheMinSize = 10 << 20, a single/Applicationspass still admitted 4,430 entries on my machine. That maybe entirely intended, but given the history described in that comment (1.88M
files / 7.82 GB for one user) it might be worth re-measuring the distribution
against the current thresholds.
4. The
dusubprocess for folded directories can be replaced in-processI replaced the
du -skPxsubprocess with an in-process parallel walkreproducing its semantics exactly — physical size (
st_blocks * 512) forevery entry including directories, hardlinks counted once per tree by
(dev, ino), symlinks counted as the link, no device crossing,
-Ibasenameignores. Unit tests assert it matches real
duoutput byte-for-byte onfixture trees.
Besides removing the fork/exec, this dissolves the
duSem/duQueueSemcontention and the "du timed out on a huge tree, now fall back to a slow
re-walk" failure mode.
5. Spotlight runs after the walk instead of alongside it
findLargeFilesWithSpotlightis called once the walk has finished, so itslatency is fully additive. Starting the
mdfindquery when the scan startsand joining at the end would hide it. Honest disclosure: on my machine those
queries measured only ~0.05–0.06s, so this is small — but it is close to free.
6.
~/Library/CloudStoragemay deserve theMobile DocumentstreatmentA full home-directory scan took ~67s here, and the overwhelming majority was
macOS File Provider enumeration of Dropbox placeholders — neither
implementation was meaningfully faster, because it is all waiting on the OS.
You already special-case
Mobile DocumentsinoverviewDuIgnoreNamesforexactly this reason on the iCloud side.
~/Library/CloudStorage(Dropbox,OneDrive, Google Drive via File Provider) has the same characteristic. Clearly
a product decision rather than a bug, but the parallel seemed worth pointing
out.
On the port itself
I am publishing it as brrw, with its own name and icon, no Mole branding,
and a clear "unofficial, not affiliated or endorsed" notice, per your
TRADEMARK.md. It stays GPL-3.0, credits Mole prominently as the origin, and is
explicit that it covers only the
analyzemodule — Mole remains the completeand mature tool.
If any of the findings above are interesting, I am happy to open individual
issues with the detail, or to attempt them as Go PRs against Mole — I would
rather contribute upstream than fork quietly. Equally, if you would prefer I
just leave you to it, that is a completely fine answer and I will not be
offended.
Thanks again for the tool and for making it open source.
All reactions