From 1a58b21b21335cde6fadd417c83c917de2cd589f Mon Sep 17 00:00:00 2001 From: Youngsup Oh Date: Wed, 28 Oct 2026 22:12:58 +0900 Subject: [PATCH 1/8] perf: scan fixed cache paths alongside the project walk and split large artifacts across workers Stat scanners (global caches, Xcode, Docker, LLM stores) now run in a second lane next to the project walk instead of after it, and measure walks subdirectories of one artifact in parallel while one of 8 process-wide slots is free, so a lone large artifact such as a pnpm store is no longer sized by one goroutine. Sub-walks share one hard-link and clone tally, so totals are unchanged. Co-Authored-By: Claude Opus 5.5 --- docs/ARCHITECTURE.md | 4 +- internal/scanner/scanner.go | 183 +++++++++++++++++++++---------- internal/scanner/scanner_test.go | 24 ++++ internal/scanner/sizing_test.go | 41 +++++++ internal/scanner/walk_test.go | 17 ++- 5 files changed, 202 insertions(+), 67 deletions(-) diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index 2ccfb92..ed8c2dc 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -10,7 +10,7 @@ Scan → Classify → Filter/Sort → Output/Clean devclean is a monolithic Go CLI binary: every ecosystem scanner is built in, with no external scanner plugins. -1. **Scan**: Registry batches all project (walk-based) scanners into a single filesystem pass, then runs stat scanners (fixed paths) sequentially. Real-time progress via spinner. +1. **Scan**: Registry batches all project (walk-based) scanners into a single filesystem pass and runs stat scanners (fixed paths) sequentially alongside it. Results list the walk's first, then the stat scanners' in registry order. Real-time progress via spinner. 2. **Classify**: activity status + gitignore-aware protection ([SPEC](SPEC.md#classification)) 3. **Filter/Sort**: CLI flags filter by ecosystem, category, status; sort by size/time/name; top N projects 4. **Output**: colored table or JSON (`--json`) @@ -40,7 +40,7 @@ Three invariants keep the figures honest: - **Hugging Face shared blobs** referenced by several models go to `BlobShares` so `DedupedTotal` counts them once. - **Shared traversal**: sizing walks with `fstree`, so it counts what finding reports and deletion removes. -Neither scanner family sizes inline. Each collects its artifacts first — the walk during its single pass, stat scanners via `stat` — then sizes them through one shared bounded worker pool (`sizePending`, `min(NumCPU, 8)`) so the tree-walk I/O overlaps. +Neither scanner family sizes inline. Each collects its artifacts first — the walk during its single pass, stat scanners via `stat` — then sizes them through a bounded worker pool (`sizePending`, `min(NumCPU, 8)`) so the tree-walk I/O overlaps. Inside one artifact, `measure` hands a subdirectory to its own goroutine while one of 8 process-wide slots is free, so a lone large artifact (a pnpm store) is not walked by one goroutine; the sub-walks share one hard-link and clone tally, so the totals do not depend on the split. ### Shared traversal (`fstree`) diff --git a/internal/scanner/scanner.go b/internal/scanner/scanner.go index 882c90b..5319458 100644 --- a/internal/scanner/scanner.go +++ b/internal/scanner/scanner.go @@ -2,8 +2,10 @@ package scanner import ( "context" + "errors" "io/fs" "path/filepath" + "sync" "syscall" "time" @@ -104,47 +106,78 @@ func (r *Registry) ScanWith(ctx context.Context, root string, scanners []Scanner } // ScanWithProgress runs scanners with an optional progress callback. -// Walk-based scanners are partitioned out and executed first as a single -// batched filesystem pass; the remaining scanners run sequentially. +// Walk-based scanners are partitioned out and executed as a single batched +// filesystem pass; the remaining scanners run sequentially alongside it. The +// two lanes share no state, so the scan takes as long as the slower lane. +// Results keep the order walk first, then the remaining scanners in order, and +// onProgress is never called concurrently. func (r *Registry) ScanWithProgress(ctx context.Context, root string, scanners []Scanner, onProgress ProgressFunc) ([]model.ScanResult, error) { root, err := filepath.Abs(root) if err != nil { return nil, err } - var all []model.ScanResult - - // Attach a real-time progress reporter via context - scanCtx := ctx - if onProgress != nil { - scanCtx = WithProgress(ctx, func(found int) { - onProgress("", len(all)+found) - }) - } + ctx, cancel := context.WithCancel(ctx) + defer cancel() walkTables, rest := partitionScanners(scanners) - if len(walkTables) > 0 { - if onProgress != nil { - onProgress("projects", len(all)) - } - results, err := runWalk(scanCtx, root, walkTables) - if err != nil { - return nil, err + // found[0] counts the walk lane, found[1] the other scanners. + var mu sync.Mutex + var found [2]int + progress := func(lane int, eco string, n int) { + if onProgress == nil { + return } - all = append(all, results...) + mu.Lock() + defer mu.Unlock() + found[lane] = n + onProgress(eco, found[0]+found[1]) + } + laneCtx := func(lane int, done func() int) context.Context { + return WithProgress(ctx, func(n int) { progress(lane, "", done()+n) }) } - for _, s := range rest { - if onProgress != nil { - onProgress(s.Name(), len(all)) + var walkResults, restResults []model.ScanResult + var walkErr, restErr error + var wg sync.WaitGroup + if len(walkTables) > 0 { + wg.Go(func() { + progress(0, "projects", 0) + walkResults, walkErr = runWalk(laneCtx(0, func() int { return 0 }), root, walkTables) + if walkErr != nil { + cancel() + } + }) + } + wg.Go(func() { + var all []model.ScanResult + for _, s := range rest { + progress(1, s.Name(), len(all)) + results, err := s.Scan(laneCtx(1, func() int { return len(all) }), root) + if err != nil { + restErr = err + cancel() + return + } + all = append(all, results...) } - results, err := s.Scan(scanCtx, root) - if err != nil { - return nil, err + restResults = all + }) + wg.Wait() + // A lane cancelled by the other lane's failure reports context.Canceled; + // return the error that caused it. + for _, e := range []error{walkErr, restErr} { + if e != nil && !errors.Is(e, context.Canceled) { + return nil, e } - all = append(all, results...) } - return all, nil + if walkErr != nil { + return nil, walkErr + } + if restErr != nil { + return nil, restErr + } + return append(walkResults, restResults...), nil } // partitionScanners splits scanners into walk-engine tables (batched into a @@ -188,10 +221,18 @@ func Measure(path string) SizeStat { return measure(context.Background(), path, nil, "") } +// subWalkTokens bounds the extra goroutines measure starts to walk +// subdirectories of one artifact in parallel. Every measure in the process +// shares them, so once the other artifacts are sized a lone large one (a pnpm +// store) gets them all instead of being walked by one goroutine. +var subWalkTokens = make(chan struct{}, sizeWorkerCap) + // measure is Measure that stops once ctx is done. A non-nil clones makes it // clone-aware (see clone.go), netting out blocks shared with the files under -// source, if given. +// source, if given. A subdirectory is walked in its own goroutine whenever a +// subWalkTokens slot is free; the totals do not depend on how the tree is split. func measure(ctx context.Context, path string, clones *cloneSources, source string) SizeStat { + var mu sync.Mutex // guards everything below, shared by the sub-walks var st SizeStat seen := make(map[model.InodeKey]struct{}) var extents []extent @@ -202,18 +243,37 @@ func measure(ctx context.Context, path string, clones *cloneSources, source stri sys, ok := info.Sys().(*syscall.Stat_t) if !ok { if !info.IsDir() && info.Mode()&fs.ModeSymlink == 0 { + mu.Lock() st.Apparent += info.Size() // non-unix fallback: apparent only + mu.Unlock() } return } blocks := int64(sys.Blocks) * 512 if info.IsDir() { + mu.Lock() st.Disk += blocks + mu.Unlock() return } if info.Mode()&fs.ModeSymlink != 0 { return } + // Read clone attributes and extents before taking the lock: they are + // syscalls, and the lock would serialize them across sub-walks. + var full string + var ci cloneInfo + var ciOK, xsOK bool + var xs []extent + if sys.Nlink <= 1 && clones != nil && blocks > 0 { + full = filepath.Join(dir, name) + ci, ciOK = fileCloneInfo(full) + if !ciOK || (ci.refcnt <= 1 && ci.mayShare) { + xs, xsOK = fileExtents(full, info.Size()) + } + } + mu.Lock() + defer mu.Unlock() if sys.Nlink > 1 { // Count a multiply-linked inode once per artifact for both apparent // and disk (matches `du`/`du -A` within-call dedup), and record it @@ -227,11 +287,9 @@ func measure(ctx context.Context, path string, clones *cloneSources, source stri st.Links = make(map[model.InodeKey]int64) } st.Links[key] = blocks - } else if clones != nil && blocks > 0 { - full := filepath.Join(dir, name) - ci, ok := fileCloneInfo(full) + } else if full != "" { switch { - case ok && ci.refcnt > 1: + case ciOK && ci.refcnt > 1: g := groups[ci.id] if g == nil { if groups == nil { @@ -242,37 +300,52 @@ func measure(ctx context.Context, path string, clones *cloneSources, source stri } g.Seen++ groupBlocks += blocks - case !ok || ci.mayShare: - if xs, ok := fileExtents(full, info.Size()); ok { - extents = append(extents, xs...) - extentBlocks += blocks - } + case xsOK: + extents = append(extents, xs...) + extentBlocks += blocks } } st.Apparent += info.Size() st.Disk += blocks } - w := fstree.Walker{ - Enter: func(dir string, info fs.FileInfo) bool { - if sys, ok := info.Sys().(*syscall.Stat_t); ok && dir == path { - rootDev = uint64(sys.Dev) //nolint:unconvert // Dev is int32 on darwin, uint64 on linux - } - add(dir, "", info) - return true - }, - Entries: func(dir string, entries []fs.DirEntry) bool { - for _, e := range entries { - if e.IsDir() { - continue // counted on Enter, if on this filesystem + var wg sync.WaitGroup + var walk func(root string) + walk = func(root string) { + w := fstree.Walker{ + Enter: func(dir string, info fs.FileInfo) bool { + if dir != root && info.IsDir() { + select { + case subWalkTokens <- struct{}{}: + wg.Go(func() { + defer func() { <-subWalkTokens }() + walk(dir) + }) + return false // the sub-walk counts dir itself + default: + } } - if info, err := e.Info(); err == nil { - add(dir, e.Name(), info) + if sys, ok := info.Sys().(*syscall.Stat_t); ok && dir == path { + rootDev = uint64(sys.Dev) //nolint:unconvert // Dev is int32 on darwin, uint64 on linux } - } - return true - }, + add(dir, "", info) + return true + }, + Entries: func(dir string, entries []fs.DirEntry) bool { + for _, e := range entries { + if e.IsDir() { + continue // counted on Enter, if on this filesystem + } + if info, err := e.Info(); err == nil { + add(dir, e.Name(), info) + } + } + return true + }, + } + _, _ = w.Walk(ctx, root) } - _, _ = w.Walk(ctx, path) + walk(path) + wg.Wait() st.Reclaim = st.Disk if clones == nil { return st diff --git a/internal/scanner/scanner_test.go b/internal/scanner/scanner_test.go index 8e1ef72..7ccb0eb 100644 --- a/internal/scanner/scanner_test.go +++ b/internal/scanner/scanner_test.go @@ -295,3 +295,27 @@ func TestRegistry_ResolvesRelativeRoot(t *testing.T) { t.Errorf("expected only %s, got %+v", ollama, results) } } + +type failingScanner struct{ err error } + +func (f *failingScanner) Name() string { return "failing" } +func (f *failingScanner) Ecosystem() model.Ecosystem { return model.EcoGlobal } + +func (f *failingScanner) Scan(_ context.Context, _ string) ([]model.ScanResult, error) { + return nil, f.err +} + +// TestRegistryScanWith_StatScannerError checks a failing stat scanner's own +// error is returned, not the cancellation it causes in the walk lane. +func TestRegistryScanWith_StatScannerError(t *testing.T) { + root := t.TempDir() + mustMkdir(t, filepath.Join(root, "app")) + mustWriteFile(t, filepath.Join(root, "app", "package.json"), nil) + want := errors.New("boom") + + reg := scanner.DefaultRegistry() + scanners := append(reg.ForEcosystems([]model.Ecosystem{model.EcoNode}), &failingScanner{err: want}) + if _, err := reg.ScanWith(context.Background(), root, scanners); !errors.Is(err, want) { + t.Errorf("ScanWith error = %v, want %v", err, want) + } +} diff --git a/internal/scanner/sizing_test.go b/internal/scanner/sizing_test.go index 32a080f..2587c99 100644 --- a/internal/scanner/sizing_test.go +++ b/internal/scanner/sizing_test.go @@ -260,3 +260,44 @@ func TestDedupedTotalNoLinks(t *testing.T) { t.Errorf("DedupedTotal = %d, want 350", got) } } + +// TestMeasureSplitMatchesSerial sizes a tree whose files are hard-linked +// across many subdirectories twice: with every sub-walk slot taken (one +// goroutine walks it all) and with the slots free (subdirectories are walked +// in parallel). The totals must match, a shared inode counted once. +func TestMeasureSplitMatchesSerial(t *testing.T) { + dir := t.TempDir() + orig := filepath.Join(dir, "orig") + if err := os.WriteFile(orig, make([]byte, 300_000), 0o644); err != nil { + t.Fatal(err) + } + for i := range 32 { + sub := filepath.Join(dir, "d", string(rune('a'+i%26))+string(rune('0'+i/26)), "x") + if err := os.MkdirAll(sub, 0o755); err != nil { + t.Fatal(err) + } + if err := os.Link(orig, filepath.Join(sub, "link")); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(sub, "own"), make([]byte, 5000*(i+1)), 0o644); err != nil { + t.Fatal(err) + } + } + + for range cap(subWalkTokens) { + subWalkTokens <- struct{}{} + } + serial := Measure(dir) + for range cap(subWalkTokens) { + <-subWalkTokens + } + split := Measure(dir) + + if serial.Apparent != split.Apparent || serial.Disk != split.Disk || len(serial.Links) != len(split.Links) { + t.Errorf("split = {%d %d %d links}, serial = {%d %d %d links}", + split.Apparent, split.Disk, len(split.Links), serial.Apparent, serial.Disk, len(serial.Links)) + } + if len(split.Links) != 1 { + t.Errorf("Links = %v, want exactly 1 shared inode", split.Links) + } +} diff --git a/internal/scanner/walk_test.go b/internal/scanner/walk_test.go index ba1c5ca..e5d0209 100644 --- a/internal/scanner/walk_test.go +++ b/internal/scanner/walk_test.go @@ -4,6 +4,7 @@ import ( "context" "os" "path/filepath" + "slices" "testing" "github.com/ohing504/devclean/internal/model" @@ -194,7 +195,7 @@ func TestScanWithProgress_MixedPartition(t *testing.T) { var labels []string reg := NewRegistry() - // The stub is listed first, but the walk batch must still run first. + // The stub is listed first, but the walk batch's results still come first. results, err := reg.ScanWithProgress(context.Background(), root, []Scanner{stub, alpha, beta}, func(eco string, _ int) { if eco != "" { @@ -221,14 +222,10 @@ func TestScanWithProgress_MixedPartition(t *testing.T) { t.Errorf("batch results misattributed: got %s, %s", results[0].Ecosystem, results[1].Ecosystem) } - // One "projects" label for the whole batch, then the stub by name. - wantLabels := []string{"projects", "stub"} - if len(labels) != len(wantLabels) { - t.Fatalf("expected labels %v, got %v", wantLabels, labels) - } - for i, want := range wantLabels { - if labels[i] != want { - t.Errorf("label[%d]: got %s, want %s", i, labels[i], want) - } + // One "projects" label for the whole batch and the stub by name. The two + // lanes run concurrently, so the labels may arrive in either order. + slices.Sort(labels) + if want := []string{"projects", "stub"}; !slices.Equal(labels, want) { + t.Errorf("labels = %v, want %v in any order", labels, want) } } From 34113ccf56e1bb812717bb61276f8617ed4091bd Mon Sep 17 00:00:00 2001 From: Youngsup Oh Date: Wed, 28 Oct 2026 22:25:21 +0900 Subject: [PATCH 2/8] refactor: bound all sizing walks by one process-wide limit and simplify lane code - sizePending workers now wait for a slot of the same 8-slot limit the sub-walks take, so both lanes' pools and the sub-walks together run at most 8 sizing walks instead of up to 24. - measure classifies a file before taking its lock once, instead of three lock sites and derived flags. - ScanWithProgress keeps the first lane error with sync.Once instead of filtering context.Canceled afterwards, and names its progress counters. Co-Authored-By: Claude Opus 5.5 --- docs/ARCHITECTURE.md | 2 +- internal/scanner/scanner.go | 179 ++++++++++++++++++-------------- internal/scanner/sizing_test.go | 8 +- internal/scanner/walk.go | 9 +- 4 files changed, 114 insertions(+), 84 deletions(-) diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index ed8c2dc..6c60f65 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -40,7 +40,7 @@ Three invariants keep the figures honest: - **Hugging Face shared blobs** referenced by several models go to `BlobShares` so `DedupedTotal` counts them once. - **Shared traversal**: sizing walks with `fstree`, so it counts what finding reports and deletion removes. -Neither scanner family sizes inline. Each collects its artifacts first — the walk during its single pass, stat scanners via `stat` — then sizes them through a bounded worker pool (`sizePending`, `min(NumCPU, 8)`) so the tree-walk I/O overlaps. Inside one artifact, `measure` hands a subdirectory to its own goroutine while one of 8 process-wide slots is free, so a lone large artifact (a pnpm store) is not walked by one goroutine; the sub-walks share one hard-link and clone tally, so the totals do not depend on the split. +Neither scanner family sizes inline. Each collects its artifacts first — the walk during its single pass, stat scanners via `stat` — then sizes them through a worker pool (`sizePending`) so the tree-walk I/O overlaps. At most 8 sizing walks run at once process-wide, across the pools of both lanes: a worker waits for a slot before measuring an artifact, and inside one artifact `measure` hands a subdirectory to its own goroutine whenever a slot is free, so a lone large artifact (a pnpm store) is not walked by one goroutine. The sub-walks share one hard-link and clone tally, so the totals do not depend on the split. ### Shared traversal (`fstree`) diff --git a/internal/scanner/scanner.go b/internal/scanner/scanner.go index 5319458..2a5f345 100644 --- a/internal/scanner/scanner.go +++ b/internal/scanner/scanner.go @@ -2,7 +2,6 @@ package scanner import ( "context" - "errors" "io/fs" "path/filepath" "sync" @@ -121,61 +120,71 @@ func (r *Registry) ScanWithProgress(ctx context.Context, root string, scanners [ walkTables, rest := partitionScanners(scanners) - // found[0] counts the walk lane, found[1] the other scanners. var mu sync.Mutex - var found [2]int - progress := func(lane int, eco string, n int) { - if onProgress == nil { - return + var walkFound, restFound int + report := func(eco string) { + if onProgress != nil { + onProgress(eco, walkFound+restFound) } + } + walkCtx := WithProgress(ctx, func(n int) { mu.Lock() defer mu.Unlock() - found[lane] = n - onProgress(eco, found[0]+found[1]) + walkFound = n + report("") + }) + restCtx := func(eco string, base int) context.Context { + mu.Lock() + restFound = base + report(eco) + mu.Unlock() + return WithProgress(ctx, func(n int) { + mu.Lock() + defer mu.Unlock() + restFound = base + n + report("") + }) } - laneCtx := func(lane int, done func() int) context.Context { - return WithProgress(ctx, func(n int) { progress(lane, "", done()+n) }) + + // The first lane to fail cancels the other, whose context.Canceled would + // otherwise hide the cause. + var firstErr error + var once sync.Once + fail := func(err error) { + once.Do(func() { + firstErr = err + cancel() + }) } var walkResults, restResults []model.ScanResult - var walkErr, restErr error var wg sync.WaitGroup if len(walkTables) > 0 { wg.Go(func() { - progress(0, "projects", 0) - walkResults, walkErr = runWalk(laneCtx(0, func() int { return 0 }), root, walkTables) - if walkErr != nil { - cancel() + mu.Lock() + report("projects") + mu.Unlock() + results, err := runWalk(walkCtx, root, walkTables) + if err != nil { + fail(err) + return } + walkResults = results }) } wg.Go(func() { - var all []model.ScanResult for _, s := range rest { - progress(1, s.Name(), len(all)) - results, err := s.Scan(laneCtx(1, func() int { return len(all) }), root) + results, err := s.Scan(restCtx(s.Name(), len(restResults)), root) if err != nil { - restErr = err - cancel() + fail(err) return } - all = append(all, results...) + restResults = append(restResults, results...) } - restResults = all }) wg.Wait() - // A lane cancelled by the other lane's failure reports context.Canceled; - // return the error that caused it. - for _, e := range []error{walkErr, restErr} { - if e != nil && !errors.Is(e, context.Canceled) { - return nil, e - } - } - if walkErr != nil { - return nil, walkErr - } - if restErr != nil { - return nil, restErr + if firstErr != nil { + return nil, firstErr } return append(walkResults, restResults...), nil } @@ -221,16 +230,27 @@ func Measure(path string) SizeStat { return measure(context.Background(), path, nil, "") } -// subWalkTokens bounds the extra goroutines measure starts to walk -// subdirectories of one artifact in parallel. Every measure in the process -// shares them, so once the other artifacts are sized a lone large one (a pnpm -// store) gets them all instead of being walked by one goroutine. -var subWalkTokens = make(chan struct{}, sizeWorkerCap) +// sizeTokens bounds the tree walks sizing runs at once, process-wide: a +// sizePending worker waits for one before measuring an artifact, and measure +// takes a free one, without waiting, to walk a subdirectory in its own +// goroutine. Once the other artifacts are sized, a lone large one (a pnpm +// store) gets every slot instead of being walked by one goroutine. +var sizeTokens = make(chan struct{}, sizeWorkerCap) + +// fileKind is how measure counts a regular file's blocks. +type fileKind int + +const ( + plainFile fileKind = iota // st_blocks as is + linkedFile // hard-linked: once per inode + clonedFile // pure APFS clone: once per clone group + extentFile // may share blocks: by physical extent +) // measure is Measure that stops once ctx is done. A non-nil clones makes it // clone-aware (see clone.go), netting out blocks shared with the files under // source, if given. A subdirectory is walked in its own goroutine whenever a -// subWalkTokens slot is free; the totals do not depend on how the tree is split. +// sizeTokens slot is free; the totals do not depend on how the tree is split. func measure(ctx context.Context, path string, clones *cloneSources, source string) SizeStat { var mu sync.Mutex // guards everything below, shared by the sub-walks var st SizeStat @@ -241,40 +261,47 @@ func measure(ctx context.Context, path string, clones *cloneSources, source stri var rootDev uint64 add := func(dir, name string, info fs.FileInfo) { sys, ok := info.Sys().(*syscall.Stat_t) - if !ok { - if !info.IsDir() && info.Mode()&fs.ModeSymlink == 0 { - mu.Lock() - st.Apparent += info.Size() // non-unix fallback: apparent only - mu.Unlock() - } + if info.Mode()&fs.ModeSymlink != 0 || (!ok && info.IsDir()) { return } - blocks := int64(sys.Blocks) * 512 - if info.IsDir() { + if !ok { mu.Lock() - st.Disk += blocks + st.Apparent += info.Size() // non-unix fallback: apparent only mu.Unlock() return } - if info.Mode()&fs.ModeSymlink != 0 { - return - } - // Read clone attributes and extents before taking the lock: they are + blocks := int64(sys.Blocks) * 512 + // Classify before taking the lock: clone attributes and extents are // syscalls, and the lock would serialize them across sub-walks. + kind := plainFile var full string var ci cloneInfo - var ciOK, xsOK bool var xs []extent - if sys.Nlink <= 1 && clones != nil && blocks > 0 { + switch { + case info.IsDir(): + case sys.Nlink > 1: + kind = linkedFile + case clones != nil && blocks > 0: full = filepath.Join(dir, name) + var ciOK bool ci, ciOK = fileCloneInfo(full) - if !ciOK || (ci.refcnt <= 1 && ci.mayShare) { - xs, xsOK = fileExtents(full, info.Size()) + if ciOK && ci.refcnt > 1 { + kind = clonedFile + } else if !ciOK || ci.mayShare { + if x, ok := fileExtents(full, info.Size()); ok { + kind, xs = extentFile, x + } } } + mu.Lock() defer mu.Unlock() - if sys.Nlink > 1 { + if info.IsDir() { + st.Disk += blocks // dirs add their own blocks to Disk, not Apparent + return + } + switch kind { + case linkedFile: // Count a multiply-linked inode once per artifact for both apparent // and disk (matches `du`/`du -A` within-call dedup), and record it // so DedupedTotal can net it out across artifacts. @@ -287,23 +314,20 @@ func measure(ctx context.Context, path string, clones *cloneSources, source stri st.Links = make(map[model.InodeKey]int64) } st.Links[key] = blocks - } else if full != "" { - switch { - case ciOK && ci.refcnt > 1: - g := groups[ci.id] - if g == nil { - if groups == nil { - groups = make(map[uint64]*cloneGroup) - } - g = &cloneGroup{CloneShare: model.CloneShare{Refcnt: ci.refcnt, Blocks: blocks}, path: full, size: info.Size()} - groups[ci.id] = g + case clonedFile: + g := groups[ci.id] + if g == nil { + if groups == nil { + groups = make(map[uint64]*cloneGroup) } - g.Seen++ - groupBlocks += blocks - case xsOK: - extents = append(extents, xs...) - extentBlocks += blocks + g = &cloneGroup{CloneShare: model.CloneShare{Refcnt: ci.refcnt, Blocks: blocks}, path: full, size: info.Size()} + groups[ci.id] = g } + g.Seen++ + groupBlocks += blocks + case extentFile: + extents = append(extents, xs...) + extentBlocks += blocks } st.Apparent += info.Size() st.Disk += blocks @@ -313,18 +337,17 @@ func measure(ctx context.Context, path string, clones *cloneSources, source stri walk = func(root string) { w := fstree.Walker{ Enter: func(dir string, info fs.FileInfo) bool { - if dir != root && info.IsDir() { + if dir != root { select { - case subWalkTokens <- struct{}{}: + case sizeTokens <- struct{}{}: wg.Go(func() { - defer func() { <-subWalkTokens }() + defer func() { <-sizeTokens }() walk(dir) }) return false // the sub-walk counts dir itself default: } - } - if sys, ok := info.Sys().(*syscall.Stat_t); ok && dir == path { + } else if sys, ok := info.Sys().(*syscall.Stat_t); ok && root == path { rootDev = uint64(sys.Dev) //nolint:unconvert // Dev is int32 on darwin, uint64 on linux } add(dir, "", info) diff --git a/internal/scanner/sizing_test.go b/internal/scanner/sizing_test.go index 2587c99..ef03949 100644 --- a/internal/scanner/sizing_test.go +++ b/internal/scanner/sizing_test.go @@ -284,12 +284,12 @@ func TestMeasureSplitMatchesSerial(t *testing.T) { } } - for range cap(subWalkTokens) { - subWalkTokens <- struct{}{} + for range cap(sizeTokens) { + sizeTokens <- struct{}{} } serial := Measure(dir) - for range cap(subWalkTokens) { - <-subWalkTokens + for range cap(sizeTokens) { + <-sizeTokens } split := Measure(dir) diff --git a/internal/scanner/walk.go b/internal/scanner/walk.go index 7ac60e1..286b492 100644 --- a/internal/scanner/walk.go +++ b/internal/scanner/walk.go @@ -311,7 +311,8 @@ func isPnpmStoreVersionDir(name string) bool { } // sizeWorkerCap bounds concurrent sizing walks so a large scan does not -// traverse every artifact tree at once. +// traverse every artifact tree at once. sizeTokens enforces it across the +// pools of all scanners and the sub-walks inside one artifact. const sizeWorkerCap = 8 // sizePending fills in the size fields of every result by running @@ -348,7 +349,13 @@ func sizePendingWorkers(ctx context.Context, results []model.ScanResult, workers if r.CloneAware && cloneSizingSupported { clones = sources } + select { + case sizeTokens <- struct{}{}: + case <-ctx.Done(): + continue + } st := measure(ctx, r.Path, clones, r.CloneSource) + <-sizeTokens r.Size = st.Reclaim r.ApparentSize = st.Apparent r.Links = st.Links From 6f3991b23d060b239c71179018e8a31e58635bed Mon Sep 17 00:00:00 2001 From: Youngsup Oh Date: Wed, 28 Oct 2026 23:03:17 +0900 Subject: [PATCH 3/8] fix: name the scanners still running in the scan spinner With the walk and the stat scanners running together, the spinner kept the name of the last stat scanner that started while the project walk went on. Progress now names every lane still running. Co-Authored-By: Claude Opus 5.5 --- internal/scanner/scanner.go | 40 +++++++++++++++++++++++------------ internal/scanner/walk.go | 3 ++- internal/scanner/walk_test.go | 17 +++++++++------ 3 files changed, 39 insertions(+), 21 deletions(-) diff --git a/internal/scanner/scanner.go b/internal/scanner/scanner.go index 2a5f345..1f3df48 100644 --- a/internal/scanner/scanner.go +++ b/internal/scanner/scanner.go @@ -4,6 +4,7 @@ import ( "context" "io/fs" "path/filepath" + "strings" "sync" "syscall" "time" @@ -120,29 +121,39 @@ func (r *Registry) ScanWithProgress(ctx context.Context, root string, scanners [ walkTables, rest := partitionScanners(scanners) + // Progress names the scanners still running in each lane, so the label + // does not stay on a finished stat scanner while the walk goes on. var mu sync.Mutex var walkFound, restFound int - report := func(eco string) { - if onProgress != nil { - onProgress(eco, walkFound+restFound) + var walkName, restName string + report := func(label bool) { + if onProgress == nil { + return + } + eco := "" + if label { + eco = strings.Trim(walkName+", "+restName, ", ") } + onProgress(eco, walkFound+restFound) + } + setName := func(name *string, to string) { + mu.Lock() + defer mu.Unlock() + *name = to + report(walkName != "" || restName != "") } walkCtx := WithProgress(ctx, func(n int) { mu.Lock() defer mu.Unlock() walkFound = n - report("") + report(false) }) - restCtx := func(eco string, base int) context.Context { - mu.Lock() - restFound = base - report(eco) - mu.Unlock() + restCtx := func(base int) context.Context { return WithProgress(ctx, func(n int) { mu.Lock() defer mu.Unlock() restFound = base + n - report("") + report(false) }) } @@ -161,9 +172,8 @@ func (r *Registry) ScanWithProgress(ctx context.Context, root string, scanners [ var wg sync.WaitGroup if len(walkTables) > 0 { wg.Go(func() { - mu.Lock() - report("projects") - mu.Unlock() + setName(&walkName, "projects") + defer setName(&walkName, "") results, err := runWalk(walkCtx, root, walkTables) if err != nil { fail(err) @@ -173,8 +183,10 @@ func (r *Registry) ScanWithProgress(ctx context.Context, root string, scanners [ }) } wg.Go(func() { + defer setName(&restName, "") for _, s := range rest { - results, err := s.Scan(restCtx(s.Name(), len(restResults)), root) + setName(&restName, s.Name()) + results, err := s.Scan(restCtx(len(restResults)), root) if err != nil { fail(err) return diff --git a/internal/scanner/walk.go b/internal/scanner/walk.go index 286b492..7fa097a 100644 --- a/internal/scanner/walk.go +++ b/internal/scanner/walk.go @@ -326,7 +326,8 @@ func sizePending(ctx context.Context, results []model.ScanResult) error { } // sizePendingWorkers is sizePending with an explicit pool size, split out so -// the worker count can be swept in benchmarks. +// the worker count can be swept in benchmarks. Above sizeWorkerCap the extra +// workers only wait: sizeTokens still allows that many measures at once. func sizePendingWorkers(ctx context.Context, results []model.ScanResult, workers int) error { if len(results) == 0 { return nil diff --git a/internal/scanner/walk_test.go b/internal/scanner/walk_test.go index e5d0209..945ec05 100644 --- a/internal/scanner/walk_test.go +++ b/internal/scanner/walk_test.go @@ -4,7 +4,7 @@ import ( "context" "os" "path/filepath" - "slices" + "strings" "testing" "github.com/ohing504/devclean/internal/model" @@ -222,10 +222,15 @@ func TestScanWithProgress_MixedPartition(t *testing.T) { t.Errorf("batch results misattributed: got %s, %s", results[0].Ecosystem, results[1].Ecosystem) } - // One "projects" label for the whole batch and the stub by name. The two - // lanes run concurrently, so the labels may arrive in either order. - slices.Sort(labels) - if want := []string{"projects", "stub"}; !slices.Equal(labels, want) { - t.Errorf("labels = %v, want %v in any order", labels, want) + // Labels name the running lanes: "projects" for the whole batch and the + // stub by name, alone or together since the lanes run concurrently. + named := map[string]bool{} + for _, l := range labels { + for name := range strings.SplitSeq(l, ", ") { + named[name] = true + } + } + if len(named) != 2 || !named["projects"] || !named["stub"] { + t.Errorf("labels = %v, want only projects and stub, each at least once", labels) } } From aa29779f203db9b7eee166087a86c49708f88646 Mon Sep 17 00:00:00 2001 From: Youngsup Oh Date: Wed, 28 Oct 2026 23:52:46 +0900 Subject: [PATCH 4/8] perf: walk project subtrees in parallel The project walk read every directory under the scan root with one goroutine. A directory it would descend into is now handed to its own goroutine, with a copy of the ancestors' project contexts, while one of 8 walk slots is free. Results are still sorted by (table order, path). Co-Authored-By: Claude Opus 5.5 --- docs/ARCHITECTURE.md | 2 +- internal/scanner/walk.go | 188 ++++++++++++++---------- internal/scanner/walk_traversal_test.go | 43 ++++++ 3 files changed, 155 insertions(+), 78 deletions(-) diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index 6c60f65..23020cc 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -60,7 +60,7 @@ A `walkEcosystem` table declares how one ecosystem participates in the shared wa | any-depth name | `__pycache__`, `.venv` | matches the directory name anywhere under the nearest project root | | any-depth suffix | `*.egg-info` | matches a directory-name suffix anywhere under the nearest project root | -Per directory, the engine matches artifact rules against the **nearest** enclosing project root of each active ecosystem (table order, first match wins), emits the artifact and skips its subtree on a match, and otherwise reads the directory once to detect new project roots (pushed onto a recursion-scoped context stack) before descending into its child directories. Artifact matching runs **before** the hidden-directory check so compound rules ending in a hidden segment (`android/.gradle`) still match; unmatched hidden directories below the scan root are descended into only when an active ecosystem lists the name as an artifact. +Per directory, the engine matches artifact rules against the **nearest** enclosing project root of each active ecosystem (table order, first match wins), emits the artifact and skips its subtree on a match, and otherwise reads the directory once to detect new project roots (pushed onto a recursion-scoped context stack) before descending into its child directories. A child directory it would descend into is handed to its own goroutine, with a copy of the context stack, while one of 8 walk slots is free (separate from the sizing slots, so the walk is not starved while the stat scanners size); results are sorted by (table order, path) at the end, so they do not depend on the split. Artifact matching runs **before** the hidden-directory check so compound rules ending in a hidden segment (`android/.gradle`) still match; unmatched hidden directories below the scan root are descended into only when an active ecosystem lists the name as an artifact. Tables can also: diff --git a/internal/scanner/walk.go b/internal/scanner/walk.go index 7fa097a..2d44d28 100644 --- a/internal/scanner/walk.go +++ b/internal/scanner/walk.go @@ -154,98 +154,127 @@ func runWalk(ctx context.Context, root string, tables []walkEcosystem) ([]model. } } + var mu sync.Mutex // guards results var results []model.ScanResult - var stack []projectContext numTables := len(tables) + var wg sync.WaitGroup - // fstree does the traversal; the engine only decides what a directory is. - walker := fstree.Walker{ - FollowRoot: true, - // Artifact match first, before the hidden-dir check, against the - // ancestor contexts (dir's own context is pushed only if we descend). - // Size is filled in afterwards by sizePending. - Enter: func(dir string, info fs.FileInfo) bool { - name := filepath.Base(dir) - if rule, tableIdx, projRoot, ok := matchArtifact(stack, dir, name, numTables); ok { - result := model.ScanResult{ - Path: dir, - Ecosystem: tables[tableIdx].Eco, - Category: rule.Category, - LastMod: info.ModTime(), - Safety: rule.Safety, + // walk walks the tree under top with stack as the ancestors' project + // contexts. A directory it would descend into is handed to its own walk, + // with a copy of the stack, whenever a walkTokens slot is free; results + // are sorted below, so the order does not depend on the split. + var walk func(top string, stack []projectContext) + walk = func(top string, stack []projectContext) { + // fstree does the traversal; the engine only decides what a directory is. + walker := fstree.Walker{ + FollowRoot: top == root, + // Artifact match first, before the hidden-dir check, against the + // ancestor contexts (dir's own context is pushed only if we descend). + // Size is filled in afterwards by sizePending. + Enter: func(dir string, info fs.FileInfo) bool { + name := filepath.Base(dir) + if rule, tableIdx, projRoot, ok := matchArtifact(stack, dir, name, numTables); ok { + result := model.ScanResult{ + Path: dir, + Ecosystem: tables[tableIdx].Eco, + Category: rule.Category, + LastMod: info.ModTime(), + Safety: rule.Safety, + } + if tables[tableIdx].SetProjectRoot { + result.ProjectRoot = projRoot + } + if rule.Describe != nil { + n := rule.Describe(dir) + result.Recommendation = n.Recommendation + result.CloneAware, result.CloneSource = n.CloneAware, n.CloneSource + } + mu.Lock() + results = append(results, result) + ReportProgress(ctx, len(results)) + mu.Unlock() + return false // matched artifact: do not descend } - if tables[tableIdx].SetProjectRoot { - result.ProjectRoot = projRoot + if dir == top { + return true // checked by the walk that handed it over, or the scan root } - if rule.Describe != nil { - n := rule.Describe(dir) - result.Recommendation = n.Recommendation - result.CloneAware, result.CloneSource = n.CloneAware, n.CloneSource + // After artifact matching, so a project's own node_modules is + // still reported. + if installedPackageDirNames[name] { + return false } - results = append(results, result) - ReportProgress(ctx, len(results)) - return false // matched artifact: do not descend - } - // After artifact matching, so a project's own node_modules is - // still reported. - if installedPackageDirNames[name] { - return false - } - // The root is always entered: the hidden-dir rule applies below it. - return dir == root || !strings.HasPrefix(name, ".") || hiddenDescend[name] - }, - Entries: func(dir string, entries []fs.DirEntry) bool { - names := make(map[string]bool, len(entries)) - for _, e := range entries { - names[e.Name()] = true - } - - // Prune before establishing any context or descending, so nothing - // inside is ever a deletion target. - if isInstalledPackageTree(dir, names) { - return false - } - for i := range tables { - if p := tables[i].PruneRoot; p != nil && p(dir, names) { + // The hidden-dir rule applies below the root. + if strings.HasPrefix(name, ".") && !hiddenDescend[name] { return false } - } + select { + case walkTokens <- struct{}{}: + sub := slices.Clone(stack) + wg.Go(func() { + defer func() { <-walkTokens }() + walk(dir, sub) + }) + return false // the handed-over walk enters dir itself + default: + return true + } + }, + Entries: func(dir string, entries []fs.DirEntry) bool { + names := make(map[string]bool, len(entries)) + for _, e := range entries { + names[e.Name()] = true + } - // Marker check: establish the project contexts rooted at this directory. - for i := range tables { - t := &tables[i] - rooted := false - for _, m := range t.Markers { - if names[m] { - rooted = true - break - } + // Prune before establishing any context or descending, so nothing + // inside is ever a deletion target. + if isInstalledPackageTree(dir, names) { + return false } - if !rooted { - continue + for i := range tables { + if p := tables[i].PruneRoot; p != nil && p(dir, names) { + return false + } } - rules := t.Rules - if t.ExtraRules != nil { - if extra := t.ExtraRules(dir, names); len(extra) > 0 { - combined := make([]artifactRule, 0, len(t.Rules)+len(extra)) - combined = append(combined, t.Rules...) - combined = append(combined, extra...) - rules = combined + // Marker check: establish the project contexts rooted at this directory. + for i := range tables { + t := &tables[i] + rooted := false + for _, m := range t.Markers { + if names[m] { + rooted = true + break + } + } + if !rooted { + continue + } + + rules := t.Rules + if t.ExtraRules != nil { + if extra := t.ExtraRules(dir, names); len(extra) > 0 { + combined := make([]artifactRule, 0, len(t.Rules)+len(extra)) + combined = append(combined, t.Rules...) + combined = append(combined, extra...) + rules = combined + } } + stack = append(stack, projectContext{root: dir, tableIdx: i, rules: rules}) } - stack = append(stack, projectContext{root: dir, tableIdx: i, rules: rules}) - } - return true - }, - // The contexts dir pushed are the ones on top rooted at dir. - Leave: func(dir string) { - for len(stack) > 0 && stack[len(stack)-1].root == dir { - stack = stack[:len(stack)-1] - } - }, + return true + }, + // The contexts dir pushed are the ones on top rooted at dir. + Leave: func(dir string) { + for len(stack) > 0 && stack[len(stack)-1].root == dir { + stack = stack[:len(stack)-1] + } + }, + } + _, _ = walker.Walk(ctx, top) } - if _, err := walker.Walk(ctx, root); err != nil { + walk(root, nil) + wg.Wait() + if err := ctx.Err(); err != nil { return nil, err } @@ -268,6 +297,11 @@ func runWalk(ctx context.Context, root string, tables []walkEcosystem) ([]model. return results, nil } +// walkTokens bounds the extra goroutines runWalk starts to walk subtrees in +// parallel. It is separate from sizeTokens so the walk is not starved while +// the stat scanners size their caches alongside it. +var walkTokens = make(chan struct{}, sizeWorkerCap) + // installedPackageDirNames hold only installed packages when the walk reaches // them without matching an artifact rule (a global lib/node_modules, a tool's // bundled dependencies, a Python environment's site-packages). Enter prunes diff --git a/internal/scanner/walk_traversal_test.go b/internal/scanner/walk_traversal_test.go index b5be58e..97064bd 100644 --- a/internal/scanner/walk_traversal_test.go +++ b/internal/scanner/walk_traversal_test.go @@ -163,3 +163,46 @@ func TestWalkScan_HiddenNamedRootIsScanned(t *testing.T) { t.Errorf("expected only %s, got %+v", cache, results) } } + +// TestWalkScan_SplitMatchesSerial scans a tree of nested projects twice: with +// every walk slot taken (one goroutine walks it all) and with the slots free +// (subtrees are walked in parallel, each with a copy of its ancestors' +// project contexts). Both must report the same artifacts, attributed to the +// same project roots. +func TestWalkScan_SplitMatchesSerial(t *testing.T) { + root := t.TempDir() + for _, p := range []string{ + "mono/package.json", "mono/node_modules/x", + "mono/packages/a/package.json", "mono/packages/a/node_modules/x", "mono/packages/a/dist/x", + "mono/packages/b/src/__pycache__/x", "mono/packages/b/pyproject.toml", + "mono/packages/b/.venv/pyvenv.cfg", "mono/packages/b/sub/lib/__pycache__/x", + "rs/Cargo.toml", "rs/target/x", "rs/crates/c/Cargo.toml", "rs/crates/c/target/x", + "plain/deep/er/__pycache__/x", ".hidden/package.json", ".hidden/node_modules/x", + } { + touch(t, filepath.Join(root, filepath.FromSlash(p))) + } + scan := func() []string { + results, err := runWalk(context.Background(), root, walkEcosystemTable) + if err != nil { + t.Fatal(err) + } + var out []string + for _, r := range results { + out = append(out, r.Path+" "+string(r.Ecosystem)+" "+r.ProjectRoot) + } + return out + } + + for range cap(walkTokens) { + walkTokens <- struct{}{} + } + serial := scan() + for range cap(walkTokens) { + <-walkTokens + } + split := scan() + + if len(serial) == 0 || !slices.Equal(serial, split) { + t.Errorf("split:\n%v\nserial:\n%v", split, serial) + } +} From 391b01ee1f14e6b5a904a2bb881938025572403f Mon Sep 17 00:00:00 2001 From: Youngsup Oh Date: Thu, 29 Oct 2026 22:26:04 +0900 Subject: [PATCH 5/8] refactor: share the slot hand-off and drop the pool-size benchmarks Co-Authored-By: Claude Opus 5.5 --- internal/scanner/scanner.go | 36 ++++++---------- internal/scanner/sizing_test.go | 24 ++++++++--- internal/scanner/walk.go | 57 +++++++++++-------------- internal/scanner/walk_bench_test.go | 18 ++------ internal/scanner/walk_traversal_test.go | 9 +--- 5 files changed, 62 insertions(+), 82 deletions(-) diff --git a/internal/scanner/scanner.go b/internal/scanner/scanner.go index 13edb45..c1af366 100644 --- a/internal/scanner/scanner.go +++ b/internal/scanner/scanner.go @@ -126,34 +126,29 @@ func (r *Registry) ScanWithProgress(ctx context.Context, root string, scanners [ var mu sync.Mutex var walkFound, restFound int var walkName, restName string - report := func(label bool) { - if onProgress == nil { - return - } - eco := "" - if label { - eco = strings.Trim(walkName+", "+restName, ", ") + report := func(eco string) { + if onProgress != nil { + onProgress(eco, walkFound+restFound) } - onProgress(eco, walkFound+restFound) } setName := func(name *string, to string) { mu.Lock() defer mu.Unlock() *name = to - report(walkName != "" || restName != "") + report(strings.Trim(walkName+", "+restName, ", ")) } walkCtx := WithProgress(ctx, func(n int) { mu.Lock() defer mu.Unlock() walkFound = n - report(false) + report("") }) restCtx := func(base int) context.Context { return WithProgress(ctx, func(n int) { mu.Lock() defer mu.Unlock() restFound = base + n - report(false) + report("") }) } @@ -283,6 +278,12 @@ func measure(ctx context.Context, path string, clones *cloneSources, source stri return } blocks := int64(sys.Blocks) * 512 + if info.IsDir() { + mu.Lock() + st.Disk += blocks // dirs add their own blocks to Disk, not Apparent + mu.Unlock() + return + } // Classify before taking the lock: clone attributes and extents are // syscalls, and the lock would serialize them across sub-walks. kind := plainFile @@ -290,7 +291,6 @@ func measure(ctx context.Context, path string, clones *cloneSources, source stri var ci cloneInfo var xs []extent switch { - case info.IsDir(): case sys.Nlink > 1: kind = linkedFile case clones != nil && blocks > 0: @@ -308,10 +308,6 @@ func measure(ctx context.Context, path string, clones *cloneSources, source stri mu.Lock() defer mu.Unlock() - if info.IsDir() { - st.Disk += blocks // dirs add their own blocks to Disk, not Apparent - return - } switch kind { case linkedFile: // Count a multiply-linked inode once per artifact for both apparent @@ -350,14 +346,8 @@ func measure(ctx context.Context, path string, clones *cloneSources, source stri w := fstree.Walker{ Enter: func(dir string, info fs.FileInfo) bool { if dir != root { - select { - case sizeTokens <- struct{}{}: - wg.Go(func() { - defer func() { <-sizeTokens }() - walk(dir) - }) + if tryGo(sizeTokens, &wg, func() { walk(dir) }) { return false // the sub-walk counts dir itself - default: } } else if sys, ok := info.Sys().(*syscall.Stat_t); ok && root == path { rootDev = uint64(sys.Dev) //nolint:unconvert // Dev is int32 on darwin, uint64 on linux diff --git a/internal/scanner/sizing_test.go b/internal/scanner/sizing_test.go index ef03949..d16ad02 100644 --- a/internal/scanner/sizing_test.go +++ b/internal/scanner/sizing_test.go @@ -284,13 +284,8 @@ func TestMeasureSplitMatchesSerial(t *testing.T) { } } - for range cap(sizeTokens) { - sizeTokens <- struct{}{} - } - serial := Measure(dir) - for range cap(sizeTokens) { - <-sizeTokens - } + var serial SizeStat + withSlotsTaken(t, sizeTokens, func() { serial = Measure(dir) }) split := Measure(dir) if serial.Apparent != split.Apparent || serial.Disk != split.Disk || len(serial.Links) != len(split.Links) { @@ -301,3 +296,18 @@ func TestMeasureSplitMatchesSerial(t *testing.T) { t.Errorf("Links = %v, want exactly 1 shared inode", split.Links) } } + +// withSlotsTaken runs f with every slot in slots taken, so no work is handed +// to another goroutine, and frees them even if f fails the test. +func withSlotsTaken(t *testing.T, slots chan struct{}, f func()) { + t.Helper() + for range cap(slots) { + slots <- struct{}{} + } + defer func() { + for range cap(slots) { + <-slots + } + }() + f() +} diff --git a/internal/scanner/walk.go b/internal/scanner/walk.go index 2d44d28..80cc320 100644 --- a/internal/scanner/walk.go +++ b/internal/scanner/walk.go @@ -172,6 +172,9 @@ func runWalk(ctx context.Context, root string, tables []walkEcosystem) ([]model. // ancestor contexts (dir's own context is pushed only if we descend). // Size is filled in afterwards by sizePending. Enter: func(dir string, info fs.FileInfo) bool { + if dir == top && top != root { + return true // matched and checked by the walk that handed it over + } name := filepath.Base(dir) if rule, tableIdx, projRoot, ok := matchArtifact(stack, dir, name, numTables); ok { result := model.ScanResult{ @@ -195,29 +198,19 @@ func runWalk(ctx context.Context, root string, tables []walkEcosystem) ([]model. mu.Unlock() return false // matched artifact: do not descend } - if dir == top { - return true // checked by the walk that handed it over, or the scan root + if dir == root { + return true // the hidden-dir rule applies below the root } // After artifact matching, so a project's own node_modules is // still reported. if installedPackageDirNames[name] { return false } - // The hidden-dir rule applies below the root. if strings.HasPrefix(name, ".") && !hiddenDescend[name] { return false } - select { - case walkTokens <- struct{}{}: - sub := slices.Clone(stack) - wg.Go(func() { - defer func() { <-walkTokens }() - walk(dir, sub) - }) - return false // the handed-over walk enters dir itself - default: - return true - } + sub := slices.Clone(stack) + return !tryGo(walkTokens, &wg, func() { walk(dir, sub) }) // a handed-over walk enters dir itself }, Entries: func(dir string, entries []fs.DirEntry) bool { names := make(map[string]bool, len(entries)) @@ -302,6 +295,21 @@ func runWalk(ctx context.Context, root string, tables []walkEcosystem) ([]model. // the stat scanners size their caches alongside it. var walkTokens = make(chan struct{}, sizeWorkerCap) +// tryGo runs f in its own goroutine on wg if a slot in slots is free, and +// frees the slot when f returns. It never waits for a slot. +func tryGo(slots chan struct{}, wg *sync.WaitGroup, f func()) bool { + select { + case slots <- struct{}{}: + wg.Go(func() { + defer func() { <-slots }() + f() + }) + return true + default: + return false + } +} + // installedPackageDirNames hold only installed packages when the walk reaches // them without matching an artifact rule (a global lib/node_modules, a tool's // bundled dependencies, a Python environment's site-packages). Enter prunes @@ -350,28 +358,15 @@ func isPnpmStoreVersionDir(name string) bool { const sizeWorkerCap = 8 // sizePending fills in the size fields of every result by running -// Measure concurrently across a bounded worker pool. The walk finds artifacts -// serially but defers their sizing (one in-process tree walk each) to here so -// the traversals and their I/O overlap. Returns ctx.Err() if the scan is +// Measure concurrently across a bounded worker pool. The walk defers sizing +// (one in-process tree walk each) to here so the traversals and their I/O +// overlap. Returns ctx.Err() if the scan is // cancelled mid-sizing. func sizePending(ctx context.Context, results []model.ScanResult) error { - workers := min(runtime.NumCPU(), sizeWorkerCap) - return sizePendingWorkers(ctx, results, workers) -} - -// sizePendingWorkers is sizePending with an explicit pool size, split out so -// the worker count can be swept in benchmarks. Above sizeWorkerCap the extra -// workers only wait: sizeTokens still allows that many measures at once. -func sizePendingWorkers(ctx context.Context, results []model.ScanResult, workers int) error { if len(results) == 0 { return nil } - if workers > len(results) { - workers = len(results) - } - if workers < 1 { - workers = 1 - } + workers := max(1, min(runtime.NumCPU(), sizeWorkerCap, len(results))) sources := newCloneSources() idx := make(chan int) diff --git a/internal/scanner/walk_bench_test.go b/internal/scanner/walk_bench_test.go index 2c0b83d..376a014 100644 --- a/internal/scanner/walk_bench_test.go +++ b/internal/scanner/walk_bench_test.go @@ -35,30 +35,20 @@ func buildArtifacts(b *testing.B, n, dirsPer, filesPer, fileSize int) []model.Sc return results } -func benchSize(b *testing.B, workers, n, dirsPer, filesPer, fileSize int) { +func benchSize(b *testing.B, n, dirsPer, filesPer, fileSize int) { template := buildArtifacts(b, n, dirsPer, filesPer, fileSize) ctx := context.Background() b.ResetTimer() for range b.N { results := make([]model.ScanResult, len(template)) copy(results, template) - if workers == 0 { // serial baseline - for i := range results { - results[i].Size = DirSize(results[i].Path) - } - } else { - _ = sizePendingWorkers(ctx, results, workers) - } + _ = sizePending(ctx, results) } } // 40 medium artifacts (20 dirs x 50 files each) — models a real home scan with -// many node_modules/target dirs. Serial vs parallel at several pool sizes. -func BenchmarkSizeSerial(b *testing.B) { benchSize(b, 0, 40, 20, 50, 512) } -func BenchmarkSizeParallel1(b *testing.B) { benchSize(b, 1, 40, 20, 50, 512) } -func BenchmarkSizeParallel4(b *testing.B) { benchSize(b, 4, 40, 20, 50, 512) } -func BenchmarkSizeParallel8(b *testing.B) { benchSize(b, 8, 40, 20, 50, 512) } -func BenchmarkSizeParallel16(b *testing.B) { benchSize(b, 16, 40, 20, 50, 512) } +// many node_modules/target dirs. +func BenchmarkSize(b *testing.B) { benchSize(b, 40, 20, 50, 512) } // buildTraversalTree materializes a wide, artifact-free directory tree — the // regime that dominates a real scan (thousands of source dirs to walk before diff --git a/internal/scanner/walk_traversal_test.go b/internal/scanner/walk_traversal_test.go index 97064bd..e5224ce 100644 --- a/internal/scanner/walk_traversal_test.go +++ b/internal/scanner/walk_traversal_test.go @@ -193,13 +193,8 @@ func TestWalkScan_SplitMatchesSerial(t *testing.T) { return out } - for range cap(walkTokens) { - walkTokens <- struct{}{} - } - serial := scan() - for range cap(walkTokens) { - <-walkTokens - } + var serial []string + withSlotsTaken(t, walkTokens, func() { serial = scan() }) split := scan() if len(serial) == 0 || !slices.Equal(serial, split) { From f6f85c057966c71103012d1140d8e6d327811f67 Mon Sep 17 00:00:00 2001 From: Youngsup Oh Date: Thu, 29 Oct 2026 22:40:04 +0900 Subject: [PATCH 6/8] refactor: walk subdirectories in parallel inside fstree Co-Authored-By: Claude Opus 5.5 --- docs/ARCHITECTURE.md | 5 +- internal/fstree/fstree.go | 93 ++++++----- internal/fstree/fstree_test.go | 96 ++++++++---- internal/scanner/clone.go | 6 +- internal/scanner/scanner.go | 49 +++--- internal/scanner/walk.go | 196 ++++++++++-------------- internal/scanner/walk_traversal_test.go | 14 +- 7 files changed, 243 insertions(+), 216 deletions(-) diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index 7676a88..5e829f1 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -39,13 +39,14 @@ Three invariants keep the figures honest: - **APFS clones** in a `CloneAware` artifact count once; clone groups split across artifacts go to `CloneShares` so `DedupedTotal` counts them once. - **Hugging Face shared blobs** referenced by several models go to `BlobShares` so `DedupedTotal` counts them once. -Neither scanner family sizes inline. Each collects its artifacts first — the walk during its single pass, stat scanners via `stat` — then sizes them through a worker pool (`sizePending`) so the tree-walk I/O overlaps. At most 8 sizing walks run at once process-wide, across the pools of both lanes: a worker waits for a slot before measuring an artifact, and inside one artifact `measure` hands a subdirectory to its own goroutine whenever a slot is free, so a lone large artifact (a pnpm store) is not walked by one goroutine. The sub-walks share one hard-link and clone tally, so the totals do not depend on the split. +Neither scanner family sizes inline. Each collects its artifacts first — the walk during its single pass, stat scanners via `stat` — then sizes them through a worker pool (`sizePending`) so the tree-walk I/O overlaps. At most 8 sizing walks run at once process-wide, across the pools of both lanes: a worker waits for a slot before measuring an artifact, and `measure` passes the same slots to `fstree`, so a lone large artifact (a pnpm store) is not walked by one goroutine. Its goroutines share one hard-link and clone tally, so the totals do not depend on the split. ### Shared traversal (`fstree`) Finding, sizing and the pre-delete check walk through one package, so they cover the same files: - **Read each directory once.** +- **Hand state down, not across**: a directory passes what its subdirectories need (the project walk's context stack) to them, so with `Slots` set subdirectories can be walked in other goroutines; hooks then run concurrently. - **Never follow symlinks.** Only the root is read through, and only for `--path` — [decision](decisions/symlink-no-follow.md) - **Stay on the root's filesystem** (like `du -x`); a directory on another device, of unknown device, or unreadable is returned as skipped — [decision](decisions/one-filesystem.md) - **Never read a dataless directory** (cloud placeholder whose content is not on disk); it is returned as skipped — [decision](decisions/skip-dataless.md) @@ -60,7 +61,7 @@ A `walkEcosystem` table declares how one ecosystem participates in the shared wa | any-depth name | `__pycache__`, `.venv` | matches the directory name anywhere under the nearest project root | | any-depth suffix | `*.egg-info` | matches a directory-name suffix anywhere under the nearest project root | -Per directory, the engine matches artifact rules against the **nearest** enclosing project root of each active ecosystem (table order, first match wins), emits the artifact and skips its subtree on a match, and otherwise reads the directory once to detect new project roots (pushed onto a recursion-scoped context stack) before descending into its child directories. A child directory it would descend into is handed to its own goroutine, with a copy of the context stack, while one of 8 walk slots is free (separate from the sizing slots, so the walk is not starved while the stat scanners size); results are sorted by (table order, path) at the end, so they do not depend on the split. Artifact matching runs **before** the hidden-directory check so compound rules ending in a hidden segment (`android/.gradle`) still match; unmatched hidden directories below the scan root are descended into only when an active ecosystem lists the name as an artifact. +Per directory, the engine matches artifact rules against the **nearest** enclosing project root of each active ecosystem (table order, first match wins), emits the artifact and skips its subtree on a match, and otherwise reads the directory once to detect new project roots (added to the context stack it hands its subdirectories) before descending into its child directories. The walk runs on 8 walk slots of its own (separate from the sizing slots, so the walk is not starved while the stat scanners size); results are sorted by (table order, path) at the end, so they do not depend on the split. Artifact matching runs **before** the hidden-directory check so compound rules ending in a hidden segment (`android/.gradle`) still match; unmatched hidden directories below the scan root are descended into only when an active ecosystem lists the name as an artifact. Tables can also: diff --git a/internal/fstree/fstree.go b/internal/fstree/fstree.go index d57ce51..205c130 100644 --- a/internal/fstree/fstree.go +++ b/internal/fstree/fstree.go @@ -12,7 +12,9 @@ import ( "io/fs" "os" "path/filepath" + "slices" "strings" + "sync" "syscall" ) @@ -33,28 +35,30 @@ var deviceOf = func(_ string, info fs.FileInfo) (uint64, bool) { // cloud provider. var dataless = isDataless -// Walker walks a tree depth-first, reading each directory once. Every hook is -// optional. -type Walker struct { +// Walker walks a tree depth-first, reading each directory once. S is what a +// directory hands its subdirectories (struct{} for nothing); the root gets the +// zero S. Every hook is optional. +type Walker[S any] struct { // FollowRoot reads through a symlinked root, for a user-given location // such as --path. Leave it off when the root is the item to delete: // deleting a symlink removes only the link. FollowRoot bool + // Slots, when set, bounds extra goroutines: a subdirectory is walked in + // its own goroutine while a slot is free. Hooks then run concurrently. + Slots chan struct{} // Enter is called before a directory is read (and for a non-directory - // root, which is not read). Returning false skips it. - Enter func(dir string, info fs.FileInfo) bool - // Entries is called once a directory is read. Returning false skips its - // subdirectories and Leave. - Entries func(dir string, entries []fs.DirEntry) bool - // Leave is called after a read directory's subtree is walked. - Leave func(dir string) + // root, which is not read), with its parent's S. Returning false skips it. + Enter func(dir string, info fs.FileInfo, parent S) bool + // Entries is called once a directory is read, with its parent's S, and + // returns the S for its subdirectories. Returning false skips them. + Entries func(dir string, entries []fs.DirEntry, parent S) (S, bool) } // Walk walks the tree under root and returns the directories it could not -// check: on another filesystem, of unknown filesystem, dataless, or +// check, sorted: on another filesystem, of unknown filesystem, dataless, or // unreadable. A root that does not exist is an empty tree. The only error is // ctx's. -func (w Walker) Walk(ctx context.Context, root string) ([]string, error) { +func (w Walker[S]) Walk(ctx context.Context, root string) ([]string, error) { statRoot := os.Lstat if w.FollowRoot { statRoot = os.Stat @@ -68,25 +72,36 @@ func (w Walker) Walk(ctx context.Context, root string) ([]string, error) { } rootDev, rootDevOK := deviceOf(root, info) + var mu sync.Mutex // guards skipped var skipped []string - var visit func(dir string, info fs.FileInfo) error - visit = func(dir string, info fs.FileInfo) error { - if err := ctx.Err(); err != nil { - return err + skip := func(dir string) { + mu.Lock() + skipped = append(skipped, dir) + mu.Unlock() + } + var wg sync.WaitGroup + var visit func(dir string, info fs.FileInfo, parent S) + visit = func(dir string, info fs.FileInfo, parent S) { + if ctx.Err() != nil { + return } - if w.Enter != nil && !w.Enter(dir, info) { - return nil + if w.Enter != nil && !w.Enter(dir, info, parent) { + return } if !info.IsDir() { - return nil + return } entries, err := readDir(dir) if err != nil { - skipped = append(skipped, dir) - return nil + skip(dir) + return } - if w.Entries != nil && !w.Entries(dir, entries) { - return nil + state := parent + if w.Entries != nil { + var ok bool + if state, ok = w.Entries(dir, entries, parent); !ok { + return + } } for _, e := range entries { if !e.IsDir() { // false for symlinks: ReadDir does not follow them @@ -97,30 +112,32 @@ func (w Walker) Walk(ctx context.Context, root string) ([]string, error) { if errors.Is(err, fs.ErrNotExist) { continue } - if err != nil { - skipped = append(skipped, child) - continue - } - if dataless(childInfo) { - skipped = append(skipped, child) + if err != nil || dataless(childInfo) { + skip(child) continue } if rootDevOK { if dev, ok := deviceOf(child, childInfo); !ok || dev != rootDev { - skipped = append(skipped, child) + skip(child) continue } } - if err := visit(child, childInfo); err != nil { - return err + select { + case w.Slots <- struct{}{}: // never ready when Slots is nil + wg.Go(func() { + defer func() { <-w.Slots }() + visit(child, childInfo, state) + }) + default: + visit(child, childInfo, state) } } - if w.Leave != nil { - w.Leave(dir) - } - return nil } - return skipped, visit(root, info) + var zero S + visit(root, info, zero) + wg.Wait() + slices.Sort(skipped) + return skipped, ctx.Err() } // UncheckedError reports a tree with directories Walk could not check. @@ -138,7 +155,7 @@ func (e *UncheckedError) Error() string { // directories Walk could not check. A recursive delete does not stop at // them, so it would remove files that were never sized or shown. func CheckOneFilesystem(ctx context.Context, path string) error { - skipped, err := Walker{}.Walk(ctx, path) + skipped, err := Walker[struct{}]{}.Walk(ctx, path) if err != nil { return err } diff --git a/internal/fstree/fstree_test.go b/internal/fstree/fstree_test.go index 0f86cf8..5d70cce 100644 --- a/internal/fstree/fstree_test.go +++ b/internal/fstree/fstree_test.go @@ -4,10 +4,12 @@ import ( "context" "errors" "io/fs" + "maps" "os" "path/filepath" "runtime" "slices" + "sync" "testing" ) @@ -32,12 +34,18 @@ func fakeDevices(t *testing.T, devs map[string]uint64) { t.Cleanup(func() { deviceOf = orig }) } +// plain is a Walker that hands nothing to subdirectories. +type plain = Walker[struct{}] + // entered walks root and returns every directory Enter saw, in order. -func entered(t *testing.T, w Walker, root string) ([]string, []string) { +func entered(t *testing.T, w plain, root string) ([]string, []string) { t.Helper() + var mu sync.Mutex var got []string - w.Enter = func(dir string, _ fs.FileInfo) bool { + w.Enter = func(dir string, _ fs.FileInfo, _ struct{}) bool { + mu.Lock() got = append(got, dir) + mu.Unlock() return true } skipped, err := w.Walk(context.Background(), root) @@ -54,7 +62,7 @@ func TestWalk_DoesNotEnterOtherFilesystem(t *testing.T) { mkdirAll(t, filepath.Join(root, "local")) fakeDevices(t, map[string]uint64{mounted: 2}) - got, skipped := entered(t, Walker{}, root) + got, skipped := entered(t, plain{}, root) if want := []string{root, filepath.Join(root, "local")}; !slices.Equal(got, want) { t.Errorf("entered = %v, want %v", got, want) @@ -70,7 +78,7 @@ func TestWalk_DoesNotEnterDirWithoutDevice(t *testing.T) { mkdirAll(t, filepath.Join(unknown, "inner")) fakeDevices(t, map[string]uint64{unknown: 0}) - got, skipped := entered(t, Walker{}, root) + got, skipped := entered(t, plain{}, root) if want := []string{root}; !slices.Equal(got, want) { t.Errorf("entered = %v, want %v", got, want) @@ -95,7 +103,7 @@ func TestWalk_DoesNotReadDatalessDir(t *testing.T) { mkdirAll(t, filepath.Join(root, "local")) fakeDataless(t, "cloud") - got, skipped := entered(t, Walker{}, root) + got, skipped := entered(t, plain{}, root) if want := []string{root, filepath.Join(root, "local")}; !slices.Equal(got, want) { t.Errorf("entered = %v, want %v", got, want) @@ -110,7 +118,7 @@ func TestWalk_DatalessRootIsUnchecked(t *testing.T) { mkdirAll(t, filepath.Join(root, "inner")) fakeDataless(t, "cloud") - got, skipped := entered(t, Walker{}, root) + got, skipped := entered(t, plain{}, root) if len(got) != 0 { t.Errorf("entered = %v, want none", got) @@ -133,7 +141,7 @@ func TestWalk_ReadsEachDirOnce(t *testing.T) { } t.Cleanup(func() { readDir = orig }) - entered(t, Walker{Entries: func(string, []fs.DirEntry) bool { return true }}, root) + entered(t, plain{Entries: func(string, []fs.DirEntry, struct{}) (struct{}, bool) { return struct{}{}, true }}, root) if len(counts) != 4 { t.Errorf("read %d dirs, want 4: %v", len(counts), counts) @@ -160,7 +168,7 @@ func TestWalk_DoesNotFollowSymlinkedDir(t *testing.T) { t.Fatal(err) } - got, _ := entered(t, Walker{}, root) + got, _ := entered(t, plain{}, root) if want := []string{root}; !slices.Equal(got, want) { t.Errorf("entered = %v, want %v", got, want) @@ -179,12 +187,12 @@ func TestWalk_SymlinkedRoot(t *testing.T) { t.Fatal(err) } - got, _ := entered(t, Walker{FollowRoot: true}, link) + got, _ := entered(t, plain{FollowRoot: true}, link) if want := []string{link, filepath.Join(link, "inner")}; !slices.Equal(got, want) { t.Errorf("FollowRoot: entered = %v, want %v", got, want) } - got, _ = entered(t, Walker{}, link) + got, _ = entered(t, plain{}, link) if want := []string{link}; !slices.Equal(got, want) { t.Errorf("no FollowRoot: entered = %v, want %v", got, want) } @@ -219,7 +227,7 @@ func TestWalk_SymlinkedRootOnOtherFilesystem(t *testing.T) { t.Fatal(err) } - got, skipped := entered(t, Walker{FollowRoot: true}, link) + got, skipped := entered(t, plain{FollowRoot: true}, link) if want := []string{link, filepath.Join(link, "inner")}; !slices.Equal(got, want) { t.Errorf("entered = %v, want %v (skipped %v)", got, want, skipped) @@ -232,7 +240,7 @@ func TestWalk_EnterFalseSkipsSubtree(t *testing.T) { mkdirAll(t, filepath.Join(skip, "inner")) var got []string - w := Walker{Enter: func(dir string, _ fs.FileInfo) bool { + w := plain{Enter: func(dir string, _ fs.FileInfo, _ struct{}) bool { got = append(got, dir) return dir != skip }} @@ -244,29 +252,63 @@ func TestWalk_EnterFalseSkipsSubtree(t *testing.T) { } } -func TestWalk_LeaveAfterChildren(t *testing.T) { +func TestWalk_EntriesStateReachesOnlySubdirs(t *testing.T) { root := t.TempDir() - child := filepath.Join(root, "child") - mkdirAll(t, child) - - var events []string - w := Walker{ - Enter: func(dir string, _ fs.FileInfo) bool { events = append(events, "enter "+dir); return true }, - Leave: func(dir string) { events = append(events, "leave "+dir) }, + mkdirAll(t, filepath.Join(root, "a", "inner")) + mkdirAll(t, filepath.Join(root, "b")) + + got := make(map[string]string) + w := Walker[string]{ + Enter: func(dir string, _ fs.FileInfo, parent string) bool { + got[dir] = parent + return true + }, + Entries: func(dir string, _ []fs.DirEntry, parent string) (string, bool) { + return parent + "/" + filepath.Base(dir), true + }, + } + if _, err := w.Walk(context.Background(), filepath.Join(root, "a")); err != nil { + t.Fatal(err) } - if _, err := w.Walk(context.Background(), root); err != nil { + if _, err := w.Walk(context.Background(), filepath.Join(root, "b")); err != nil { t.Fatal(err) } - want := []string{"enter " + root, "enter " + child, "leave " + child, "leave " + root} - if !slices.Equal(events, want) { - t.Errorf("events = %v, want %v", events, want) + want := map[string]string{ + filepath.Join(root, "a"): "", + filepath.Join(root, "a", "inner"): "/a", + filepath.Join(root, "b"): "", + } + if !maps.Equal(got, want) { + t.Errorf("parent states = %v, want %v", got, want) + } +} + +// TestWalk_SlotsMatchSerial: walking subdirectories in other goroutines +// enters and skips the same directories, each once. +func TestWalk_SlotsMatchSerial(t *testing.T) { + root := t.TempDir() + for _, d := range []string{"a/b/c", "a/d", "e/f", "g", "mnt/x"} { + mkdirAll(t, filepath.Join(root, d)) + } + fakeDevices(t, map[string]uint64{filepath.Join(root, "mnt"): 2}) + + serial, serialSkipped := entered(t, plain{}, root) + parallel, parallelSkipped := entered(t, plain{Slots: make(chan struct{}, 4)}, root) + + slices.Sort(serial) + slices.Sort(parallel) + if !slices.Equal(parallel, serial) { + t.Errorf("entered = %v, want %v", parallel, serial) + } + if !slices.Equal(parallelSkipped, serialSkipped) { + t.Errorf("skipped = %v, want %v", parallelSkipped, serialSkipped) } } func TestWalk_Cancelled(t *testing.T) { ctx, cancel := context.WithCancel(context.Background()) cancel() - if _, err := (Walker{}).Walk(ctx, t.TempDir()); !errors.Is(err, context.Canceled) { + if _, err := (plain{}).Walk(ctx, t.TempDir()); !errors.Is(err, context.Canceled) { t.Errorf("err = %v, want context.Canceled", err) } } @@ -305,7 +347,7 @@ func TestWalk_ReportsUncheckableDir(t *testing.T) { } t.Cleanup(func() { _ = os.Chmod(root, 0o755) }) - _, skipped := entered(t, Walker{}, root) + _, skipped := entered(t, plain{}, root) if want := []string{sub}; !slices.Equal(skipped, want) { t.Errorf("skipped = %v, want %v", skipped, want) @@ -326,7 +368,7 @@ func TestWalk_ReportsUnreadableDir(t *testing.T) { } t.Cleanup(func() { _ = os.Chmod(locked, 0o755) }) - _, skipped := entered(t, Walker{}, root) + _, skipped := entered(t, plain{}, root) if want := []string{locked}; !slices.Equal(skipped, want) { t.Errorf("skipped = %v, want %v", skipped, want) diff --git a/internal/scanner/clone.go b/internal/scanner/clone.go index 3b7e0a2..91af295 100644 --- a/internal/scanner/clone.go +++ b/internal/scanner/clone.go @@ -121,9 +121,9 @@ func (c *cloneSources) get(ctx context.Context, path string) (extents []extent, return } var xs []extent - w := fstree.Walker{ + w := fstree.Walker[struct{}]{ FollowRoot: true, - Entries: func(dir string, entries []fs.DirEntry) bool { + Entries: func(dir string, entries []fs.DirEntry, _ struct{}) (struct{}, bool) { for _, e := range entries { if !e.Type().IsRegular() { continue @@ -134,7 +134,7 @@ func (c *cloneSources) get(ctx context.Context, path string) (extents []extent, } } } - return true + return struct{}{}, true }, } if _, err := w.Walk(ctx, path); err != nil { diff --git a/internal/scanner/scanner.go b/internal/scanner/scanner.go index c1af366..23bd1c5 100644 --- a/internal/scanner/scanner.go +++ b/internal/scanner/scanner.go @@ -259,7 +259,7 @@ const ( // source, if given. A subdirectory is walked in its own goroutine whenever a // sizeTokens slot is free; the totals do not depend on how the tree is split. func measure(ctx context.Context, path string, clones *cloneSources, source string) SizeStat { - var mu sync.Mutex // guards everything below, shared by the sub-walks + var mu sync.Mutex // guards everything below, shared by the walk's goroutines var st SizeStat seen := make(map[model.InodeKey]struct{}) var extents []extent @@ -340,37 +340,28 @@ func measure(ctx context.Context, path string, clones *cloneSources, source stri st.Apparent += info.Size() st.Disk += blocks } - var wg sync.WaitGroup - var walk func(root string) - walk = func(root string) { - w := fstree.Walker{ - Enter: func(dir string, info fs.FileInfo) bool { - if dir != root { - if tryGo(sizeTokens, &wg, func() { walk(dir) }) { - return false // the sub-walk counts dir itself - } - } else if sys, ok := info.Sys().(*syscall.Stat_t); ok && root == path { - rootDev = uint64(sys.Dev) //nolint:unconvert // Dev is int32 on darwin, uint64 on linux + w := fstree.Walker[struct{}]{ + Slots: sizeTokens, + Enter: func(dir string, info fs.FileInfo, _ struct{}) bool { + if sys, ok := info.Sys().(*syscall.Stat_t); ok && dir == path { + rootDev = uint64(sys.Dev) //nolint:unconvert // Dev is int32 on darwin, uint64 on linux + } + add(dir, "", info) + return true + }, + Entries: func(dir string, entries []fs.DirEntry, _ struct{}) (struct{}, bool) { + for _, e := range entries { + if e.IsDir() { + continue // counted on Enter, if on this filesystem } - add(dir, "", info) - return true - }, - Entries: func(dir string, entries []fs.DirEntry) bool { - for _, e := range entries { - if e.IsDir() { - continue // counted on Enter, if on this filesystem - } - if info, err := e.Info(); err == nil { - add(dir, e.Name(), info) - } + if info, err := e.Info(); err == nil { + add(dir, e.Name(), info) } - return true - }, - } - _, _ = w.Walk(ctx, root) + } + return struct{}{}, true + }, } - walk(path) - wg.Wait() + _, _ = w.Walk(ctx, path) st.Reclaim = st.Disk if clones == nil { return st diff --git a/internal/scanner/walk.go b/internal/scanner/walk.go index 80cc320..002fb76 100644 --- a/internal/scanner/walk.go +++ b/internal/scanner/walk.go @@ -157,117 +157,100 @@ func runWalk(ctx context.Context, root string, tables []walkEcosystem) ([]model. var mu sync.Mutex // guards results var results []model.ScanResult numTables := len(tables) - var wg sync.WaitGroup - // walk walks the tree under top with stack as the ancestors' project - // contexts. A directory it would descend into is handed to its own walk, - // with a copy of the stack, whenever a walkTokens slot is free; results - // are sorted below, so the order does not depend on the split. - var walk func(top string, stack []projectContext) - walk = func(top string, stack []projectContext) { - // fstree does the traversal; the engine only decides what a directory is. - walker := fstree.Walker{ - FollowRoot: top == root, - // Artifact match first, before the hidden-dir check, against the - // ancestor contexts (dir's own context is pushed only if we descend). - // Size is filled in afterwards by sizePending. - Enter: func(dir string, info fs.FileInfo) bool { - if dir == top && top != root { - return true // matched and checked by the walk that handed it over - } - name := filepath.Base(dir) - if rule, tableIdx, projRoot, ok := matchArtifact(stack, dir, name, numTables); ok { - result := model.ScanResult{ - Path: dir, - Ecosystem: tables[tableIdx].Eco, - Category: rule.Category, - LastMod: info.ModTime(), - Safety: rule.Safety, - } - if tables[tableIdx].SetProjectRoot { - result.ProjectRoot = projRoot - } - if rule.Describe != nil { - n := rule.Describe(dir) - result.Recommendation = n.Recommendation - result.CloneAware, result.CloneSource = n.CloneAware, n.CloneSource - } - mu.Lock() - results = append(results, result) - ReportProgress(ctx, len(results)) - mu.Unlock() - return false // matched artifact: do not descend - } - if dir == root { - return true // the hidden-dir rule applies below the root + // fstree does the traversal, in parallel while walkTokens has a free slot; + // the engine only decides what a directory is. Each directory hands its + // subdirectories the ancestors' project contexts plus its own. Results are + // sorted below, so their order does not depend on the split. + walker := fstree.Walker[[]projectContext]{ + FollowRoot: true, + Slots: walkTokens, + // Artifact match first, before the hidden-dir check, against the + // ancestor contexts (dir's own context is added only if we descend). + // Size is filled in afterwards by sizePending. + Enter: func(dir string, info fs.FileInfo, stack []projectContext) bool { + name := filepath.Base(dir) + if rule, tableIdx, projRoot, ok := matchArtifact(stack, dir, name, numTables); ok { + result := model.ScanResult{ + Path: dir, + Ecosystem: tables[tableIdx].Eco, + Category: rule.Category, + LastMod: info.ModTime(), + Safety: rule.Safety, } - // After artifact matching, so a project's own node_modules is - // still reported. - if installedPackageDirNames[name] { - return false + if tables[tableIdx].SetProjectRoot { + result.ProjectRoot = projRoot } - if strings.HasPrefix(name, ".") && !hiddenDescend[name] { - return false - } - sub := slices.Clone(stack) - return !tryGo(walkTokens, &wg, func() { walk(dir, sub) }) // a handed-over walk enters dir itself - }, - Entries: func(dir string, entries []fs.DirEntry) bool { - names := make(map[string]bool, len(entries)) - for _, e := range entries { - names[e.Name()] = true + if rule.Describe != nil { + n := rule.Describe(dir) + result.Recommendation = n.Recommendation + result.CloneAware, result.CloneSource = n.CloneAware, n.CloneSource } + mu.Lock() + results = append(results, result) + ReportProgress(ctx, len(results)) + mu.Unlock() + return false // matched artifact: do not descend + } + if dir == root { + return true // the hidden-dir rule applies below the root + } + // After artifact matching, so a project's own node_modules is + // still reported. + if installedPackageDirNames[name] { + return false + } + return !strings.HasPrefix(name, ".") || hiddenDescend[name] + }, + Entries: func(dir string, entries []fs.DirEntry, stack []projectContext) ([]projectContext, bool) { + names := make(map[string]bool, len(entries)) + for _, e := range entries { + names[e.Name()] = true + } - // Prune before establishing any context or descending, so nothing - // inside is ever a deletion target. - if isInstalledPackageTree(dir, names) { - return false + // Prune before establishing any context or descending, so nothing + // inside is ever a deletion target. + if isInstalledPackageTree(dir, names) { + return nil, false + } + for i := range tables { + if p := tables[i].PruneRoot; p != nil && p(dir, names) { + return nil, false } - for i := range tables { - if p := tables[i].PruneRoot; p != nil && p(dir, names) { - return false + } + // Clip so siblings walked in other goroutines never append + // into one shared array. + stack = slices.Clip(stack) + + // Marker check: establish the project contexts rooted at this directory. + for i := range tables { + t := &tables[i] + rooted := false + for _, m := range t.Markers { + if names[m] { + rooted = true + break } } + if !rooted { + continue + } - // Marker check: establish the project contexts rooted at this directory. - for i := range tables { - t := &tables[i] - rooted := false - for _, m := range t.Markers { - if names[m] { - rooted = true - break - } - } - if !rooted { - continue - } - - rules := t.Rules - if t.ExtraRules != nil { - if extra := t.ExtraRules(dir, names); len(extra) > 0 { - combined := make([]artifactRule, 0, len(t.Rules)+len(extra)) - combined = append(combined, t.Rules...) - combined = append(combined, extra...) - rules = combined - } + rules := t.Rules + if t.ExtraRules != nil { + if extra := t.ExtraRules(dir, names); len(extra) > 0 { + combined := make([]artifactRule, 0, len(t.Rules)+len(extra)) + combined = append(combined, t.Rules...) + combined = append(combined, extra...) + rules = combined } - stack = append(stack, projectContext{root: dir, tableIdx: i, rules: rules}) - } - return true - }, - // The contexts dir pushed are the ones on top rooted at dir. - Leave: func(dir string) { - for len(stack) > 0 && stack[len(stack)-1].root == dir { - stack = stack[:len(stack)-1] } - }, - } - _, _ = walker.Walk(ctx, top) + stack = append(stack, projectContext{root: dir, tableIdx: i, rules: rules}) + } + return stack, true + }, } - walk(root, nil) - wg.Wait() - if err := ctx.Err(); err != nil { + if _, err := walker.Walk(ctx, root); err != nil { return nil, err } @@ -295,21 +278,6 @@ func runWalk(ctx context.Context, root string, tables []walkEcosystem) ([]model. // the stat scanners size their caches alongside it. var walkTokens = make(chan struct{}, sizeWorkerCap) -// tryGo runs f in its own goroutine on wg if a slot in slots is free, and -// frees the slot when f returns. It never waits for a slot. -func tryGo(slots chan struct{}, wg *sync.WaitGroup, f func()) bool { - select { - case slots <- struct{}{}: - wg.Go(func() { - defer func() { <-slots }() - f() - }) - return true - default: - return false - } -} - // installedPackageDirNames hold only installed packages when the walk reaches // them without matching an artifact rule (a global lib/node_modules, a tool's // bundled dependencies, a Python environment's site-packages). Enter prunes diff --git a/internal/scanner/walk_traversal_test.go b/internal/scanner/walk_traversal_test.go index e5224ce..95fdca8 100644 --- a/internal/scanner/walk_traversal_test.go +++ b/internal/scanner/walk_traversal_test.go @@ -6,6 +6,7 @@ import ( "path/filepath" "runtime" "slices" + "strconv" "testing" "github.com/ohing504/devclean/internal/model" @@ -166,9 +167,10 @@ func TestWalkScan_HiddenNamedRootIsScanned(t *testing.T) { // TestWalkScan_SplitMatchesSerial scans a tree of nested projects twice: with // every walk slot taken (one goroutine walks it all) and with the slots free -// (subtrees are walked in parallel, each with a copy of its ancestors' -// project contexts). Both must report the same artifacts, attributed to the -// same project roots. +// (subtrees are walked in parallel, each with its ancestors' project +// contexts). Both must report the same artifacts, attributed to the same +// project roots. Sibling projects under a three-deep stack add their contexts +// at the same depth, so they must not share the stack's spare capacity. func TestWalkScan_SplitMatchesSerial(t *testing.T) { root := t.TempDir() for _, p := range []string{ @@ -178,9 +180,15 @@ func TestWalkScan_SplitMatchesSerial(t *testing.T) { "mono/packages/b/.venv/pyvenv.cfg", "mono/packages/b/sub/lib/__pycache__/x", "rs/Cargo.toml", "rs/target/x", "rs/crates/c/Cargo.toml", "rs/crates/c/target/x", "plain/deep/er/__pycache__/x", ".hidden/package.json", ".hidden/node_modules/x", + "n1/package.json", "n1/n2/package.json", "n1/n2/n3/package.json", } { touch(t, filepath.Join(root, filepath.FromSlash(p))) } + for i := range 8 { + sib := filepath.Join(root, "n1", "n2", "n3", "s"+strconv.Itoa(i)) + touch(t, filepath.Join(sib, "package.json")) + touch(t, filepath.Join(sib, "node_modules", "x")) + } scan := func() []string { results, err := runWalk(context.Background(), root, walkEcosystemTable) if err != nil { From 99feb8a6d91cd16fab8a84af92871a23c24086f7 Mon Sep 17 00:00:00 2001 From: Youngsup Oh Date: Fri, 30 Oct 2026 00:07:57 +0900 Subject: [PATCH 7/8] refactor: tidy the concurrent scan - cancel the other lane with context.WithCancelCause instead of a hand-rolled first error - share one progress callback between the lanes - bound sizing only by its slots and give the walk its own slot count - state that Walker hooks must not modify a shared parent state Co-Authored-By: Claude Opus 5.5 --- internal/fstree/fstree.go | 4 ++- internal/fstree/fstree_test.go | 13 +++++---- internal/scanner/scanner.go | 47 +++++++++++---------------------- internal/scanner/sizing_test.go | 3 ++- internal/scanner/walk.go | 12 +++++---- 5 files changed, 34 insertions(+), 45 deletions(-) diff --git a/internal/fstree/fstree.go b/internal/fstree/fstree.go index 205c130..d5358d3 100644 --- a/internal/fstree/fstree.go +++ b/internal/fstree/fstree.go @@ -50,7 +50,9 @@ type Walker[S any] struct { // root, which is not read), with its parent's S. Returning false skips it. Enter func(dir string, info fs.FileInfo, parent S) bool // Entries is called once a directory is read, with its parent's S, and - // returns the S for its subdirectories. Returning false skips them. + // returns the S for its subdirectories. Returning false skips them. With + // Slots the subdirectories run concurrently on the same S, so neither hook + // may modify a parent S in place. Entries func(dir string, entries []fs.DirEntry, parent S) (S, bool) } diff --git a/internal/fstree/fstree_test.go b/internal/fstree/fstree_test.go index 5d70cce..5309db0 100644 --- a/internal/fstree/fstree_test.go +++ b/internal/fstree/fstree_test.go @@ -267,16 +267,15 @@ func TestWalk_EntriesStateReachesOnlySubdirs(t *testing.T) { return parent + "/" + filepath.Base(dir), true }, } - if _, err := w.Walk(context.Background(), filepath.Join(root, "a")); err != nil { - t.Fatal(err) - } - if _, err := w.Walk(context.Background(), filepath.Join(root, "b")); err != nil { + if _, err := w.Walk(context.Background(), root); err != nil { t.Fatal(err) } + r := "/" + filepath.Base(root) want := map[string]string{ - filepath.Join(root, "a"): "", - filepath.Join(root, "a", "inner"): "/a", - filepath.Join(root, "b"): "", + root: "", + filepath.Join(root, "a"): r, + filepath.Join(root, "a", "inner"): r + "/a", + filepath.Join(root, "b"): r, } if !maps.Equal(got, want) { t.Errorf("parent states = %v, want %v", got, want) diff --git a/internal/scanner/scanner.go b/internal/scanner/scanner.go index 23bd1c5..d28fba7 100644 --- a/internal/scanner/scanner.go +++ b/internal/scanner/scanner.go @@ -116,8 +116,10 @@ func (r *Registry) ScanWithProgress(ctx context.Context, root string, scanners [ if err != nil { return nil, err } - ctx, cancel := context.WithCancel(ctx) - defer cancel() + // The first lane to fail cancels the other with its error as the cause, + // which the other's context.Canceled would otherwise hide. + ctx, cancel := context.WithCancelCause(ctx) + defer cancel(nil) walkTables, rest := partitionScanners(scanners) @@ -137,61 +139,44 @@ func (r *Registry) ScanWithProgress(ctx context.Context, root string, scanners [ *name = to report(strings.Trim(walkName+", "+restName, ", ")) } - walkCtx := WithProgress(ctx, func(n int) { - mu.Lock() - defer mu.Unlock() - walkFound = n - report("") - }) - restCtx := func(base int) context.Context { + progress := func(found *int, base int) context.Context { return WithProgress(ctx, func(n int) { mu.Lock() defer mu.Unlock() - restFound = base + n + *found = base + n report("") }) } - // The first lane to fail cancels the other, whose context.Canceled would - // otherwise hide the cause. - var firstErr error - var once sync.Once - fail := func(err error) { - once.Do(func() { - firstErr = err - cancel() - }) - } - var walkResults, restResults []model.ScanResult + var walkErr, restErr error var wg sync.WaitGroup if len(walkTables) > 0 { wg.Go(func() { setName(&walkName, "projects") defer setName(&walkName, "") - results, err := runWalk(walkCtx, root, walkTables) - if err != nil { - fail(err) - return + walkResults, walkErr = runWalk(progress(&walkFound, 0), root, walkTables) + if walkErr != nil { + cancel(walkErr) } - walkResults = results }) } wg.Go(func() { defer setName(&restName, "") for _, s := range rest { setName(&restName, s.Name()) - results, err := s.Scan(restCtx(len(restResults)), root) - if err != nil { - fail(err) + var results []model.ScanResult + results, restErr = s.Scan(progress(&restFound, len(restResults)), root) + if restErr != nil { + cancel(restErr) return } restResults = append(restResults, results...) } }) wg.Wait() - if firstErr != nil { - return nil, firstErr + if walkErr != nil || restErr != nil { + return nil, context.Cause(ctx) } return append(walkResults, restResults...), nil } diff --git a/internal/scanner/sizing_test.go b/internal/scanner/sizing_test.go index d16ad02..088fec6 100644 --- a/internal/scanner/sizing_test.go +++ b/internal/scanner/sizing_test.go @@ -4,6 +4,7 @@ import ( "context" "os" "path/filepath" + "strconv" "testing" "github.com/ohing504/devclean/internal/model" @@ -272,7 +273,7 @@ func TestMeasureSplitMatchesSerial(t *testing.T) { t.Fatal(err) } for i := range 32 { - sub := filepath.Join(dir, "d", string(rune('a'+i%26))+string(rune('0'+i/26)), "x") + sub := filepath.Join(dir, "d", "d"+strconv.Itoa(i), "x") if err := os.MkdirAll(sub, 0o755); err != nil { t.Fatal(err) } diff --git a/internal/scanner/walk.go b/internal/scanner/walk.go index 002fb76..fc881dc 100644 --- a/internal/scanner/walk.go +++ b/internal/scanner/walk.go @@ -4,7 +4,6 @@ import ( "context" "io/fs" "path/filepath" - "runtime" "slices" "sort" "strings" @@ -276,7 +275,10 @@ func runWalk(ctx context.Context, root string, tables []walkEcosystem) ([]model. // walkTokens bounds the extra goroutines runWalk starts to walk subtrees in // parallel. It is separate from sizeTokens so the walk is not starved while // the stat scanners size their caches alongside it. -var walkTokens = make(chan struct{}, sizeWorkerCap) +var walkTokens = make(chan struct{}, walkSlots) + +// walkSlots is the number of walkTokens. +const walkSlots = 8 // installedPackageDirNames hold only installed packages when the walk reaches // them without matching an artifact rule (a global lib/node_modules, a tool's @@ -328,13 +330,13 @@ const sizeWorkerCap = 8 // sizePending fills in the size fields of every result by running // Measure concurrently across a bounded worker pool. The walk defers sizing // (one in-process tree walk each) to here so the traversals and their I/O -// overlap. Returns ctx.Err() if the scan is -// cancelled mid-sizing. +// overlap. Returns ctx.Err() if the scan is cancelled mid-sizing. func sizePending(ctx context.Context, results []model.ScanResult) error { if len(results) == 0 { return nil } - workers := max(1, min(runtime.NumCPU(), sizeWorkerCap, len(results))) + // sizeTokens bounds the concurrent measures; the workers only hand them out. + workers := min(sizeWorkerCap, len(results)) sources := newCloneSources() idx := make(chan int) From e531c065a9598e2ee2b7579ef7d0254dfa147811 Mon Sep 17 00:00:00 2001 From: Youngsup Oh Date: Fri, 30 Oct 2026 00:21:47 +0900 Subject: [PATCH 8/8] fix: keep pruning a scan root named like an installed-package tree Co-Authored-By: Claude Opus 5.5 --- internal/scanner/scanner.go | 6 ++++-- internal/scanner/walk.go | 9 ++++----- internal/scanner/walk_traversal_test.go | 17 +++++++++++++++++ 3 files changed, 25 insertions(+), 7 deletions(-) diff --git a/internal/scanner/scanner.go b/internal/scanner/scanner.go index d28fba7..0165551 100644 --- a/internal/scanner/scanner.go +++ b/internal/scanner/scanner.go @@ -77,12 +77,14 @@ func (r *Registry) ForEcosystems(ecos []model.Ecosystem) []Scanner { return filtered } -// ScanAll runs all registered scanners sequentially and collects results. +// ScanAll runs all registered scanners (see ScanWithProgress) and collects results. func (r *Registry) ScanAll(ctx context.Context, root string) ([]model.ScanResult, error) { return r.ScanWith(ctx, root, r.scanners) } -// ProgressFunc is called during scanning with the current ecosystem name and total items found so far. +// ProgressFunc is called during scanning with the names of the scanners +// running, joined with ", " ("" keeps the previous label), and the total items +// found so far. type ProgressFunc func(ecosystem string, totalFound int) type progressKey struct{} diff --git a/internal/scanner/walk.go b/internal/scanner/walk.go index fc881dc..fba467b 100644 --- a/internal/scanner/walk.go +++ b/internal/scanner/walk.go @@ -191,15 +191,14 @@ func runWalk(ctx context.Context, root string, tables []walkEcosystem) ([]model. mu.Unlock() return false // matched artifact: do not descend } - if dir == root { - return true // the hidden-dir rule applies below the root - } // After artifact matching, so a project's own node_modules is - // still reported. + // still reported. Also for the root: a scan of an installed tree + // reports nothing inside it. if installedPackageDirNames[name] { return false } - return !strings.HasPrefix(name, ".") || hiddenDescend[name] + // The root is always entered: the hidden-dir rule applies below it. + return dir == root || !strings.HasPrefix(name, ".") || hiddenDescend[name] }, Entries: func(dir string, entries []fs.DirEntry, stack []projectContext) ([]projectContext, bool) { names := make(map[string]bool, len(entries)) diff --git a/internal/scanner/walk_traversal_test.go b/internal/scanner/walk_traversal_test.go index 95fdca8..43a16c8 100644 --- a/internal/scanner/walk_traversal_test.go +++ b/internal/scanner/walk_traversal_test.go @@ -165,6 +165,23 @@ func TestWalkScan_HiddenNamedRootIsScanned(t *testing.T) { } } +// TestWalkScan_InstalledPackageRootIsPruned pins that a scan root named like +// an installed-package tree is not read: a package's own node_modules inside +// it is not a deletion target. +func TestWalkScan_InstalledPackageRootIsPruned(t *testing.T) { + root := filepath.Join(t.TempDir(), "node_modules") + touch(t, filepath.Join(root, "foo", "package.json")) + touch(t, filepath.Join(root, "foo", "node_modules", "bar", "index.js")) + + results, err := WalkScan(context.Background(), root, model.EcoNode) + if err != nil { + t.Fatal(err) + } + if len(results) != 0 { + t.Errorf("got %v, want no results", paths(results)) + } +} + // TestWalkScan_SplitMatchesSerial scans a tree of nested projects twice: with // every walk slot taken (one goroutine walks it all) and with the slots free // (subtrees are walked in parallel, each with its ancestors' project