diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index 6f35d05..5e829f1 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`) @@ -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 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 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. 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..d5358d3 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,32 @@ 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. 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) } // 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 +74,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 +114,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 +157,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..5309db0 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,62 @@ 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(), root); 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) + r := "/" + filepath.Base(root) + want := map[string]string{ + 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) + } +} + +// 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 +346,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 +367,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 14ec725..0165551 100644 --- a/internal/scanner/scanner.go +++ b/internal/scanner/scanner.go @@ -4,6 +4,8 @@ import ( "context" "io/fs" "path/filepath" + "strings" + "sync" "syscall" "time" @@ -75,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{} @@ -104,47 +108,79 @@ 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) - }) - } + // 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) - if len(walkTables) > 0 { + // 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 + var walkName, restName string + report := func(eco string) { if onProgress != nil { - onProgress("projects", len(all)) + onProgress(eco, walkFound+restFound) } - results, err := runWalk(scanCtx, root, walkTables) - if err != nil { - return nil, err - } - all = append(all, results...) + } + setName := func(name *string, to string) { + mu.Lock() + defer mu.Unlock() + *name = to + report(strings.Trim(walkName+", "+restName, ", ")) + } + progress := func(found *int, base int) context.Context { + return WithProgress(ctx, func(n int) { + mu.Lock() + defer mu.Unlock() + *found = base + n + report("") + }) } - for _, s := range rest { - if onProgress != nil { - onProgress(s.Name(), len(all)) - } - results, err := s.Scan(scanCtx, root) - if err != nil { - return nil, err + 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, "") + walkResults, walkErr = runWalk(progress(&walkFound, 0), root, walkTables) + if walkErr != nil { + cancel(walkErr) + } + }) + } + wg.Go(func() { + defer setName(&restName, "") + for _, s := range rest { + setName(&restName, s.Name()) + var results []model.ScanResult + results, restErr = s.Scan(progress(&restFound, len(restResults)), root) + if restErr != nil { + cancel(restErr) + return + } + restResults = append(restResults, results...) } - all = append(all, results...) + }) + wg.Wait() + if walkErr != nil || restErr != nil { + return nil, context.Cause(ctx) } - return all, nil + return append(walkResults, restResults...), nil } // partitionScanners splits scanners into walk-engine tables (batched into a @@ -188,10 +224,29 @@ func Measure(path string) SizeStat { return measure(context.Background(), path, nil, "") } +// 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. +// 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 walk's goroutines var st SizeStat seen := make(map[model.InodeKey]struct{}) var extents []extent @@ -200,21 +255,48 @@ 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 info.Mode()&fs.ModeSymlink != 0 || (!ok && info.IsDir()) { + return + } if !ok { - if !info.IsDir() && info.Mode()&fs.ModeSymlink == 0 { - st.Apparent += info.Size() // non-unix fallback: apparent only - } + mu.Lock() + st.Apparent += info.Size() // non-unix fallback: apparent only + mu.Unlock() return } blocks := int64(sys.Blocks) * 512 if info.IsDir() { - st.Disk += blocks + mu.Lock() + st.Disk += blocks // dirs add their own blocks to Disk, not Apparent + mu.Unlock() return } - if info.Mode()&fs.ModeSymlink != 0 { - return + // 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 xs []extent + switch { + 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 { + kind = clonedFile + } else if !ciOK || ci.mayShare { + if x, ok := fileExtents(full, info); ok { + kind, xs = extentFile, x + } + } } - if sys.Nlink > 1 { + + mu.Lock() + defer mu.Unlock() + 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. @@ -227,40 +309,34 @@ 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) - switch { - case ok && 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, info: info} - groups[ci.id] = g - } - g.Seen++ - groupBlocks += blocks - case !ok || ci.mayShare: - if xs, ok := fileExtents(full, info); ok { - extents = append(extents, xs...) - extentBlocks += blocks + case clonedFile: + 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, info: info} + groups[ci.id] = g } + g.Seen++ + groupBlocks += blocks + case extentFile: + extents = append(extents, xs...) + extentBlocks += blocks } st.Apparent += info.Size() st.Disk += blocks } - w := fstree.Walker{ - Enter: func(dir string, info fs.FileInfo) bool { + 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) bool { + 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 @@ -269,7 +345,7 @@ func measure(ctx context.Context, path string, clones *cloneSources, source stri add(dir, e.Name(), info) } } - return true + return struct{}{}, true }, } _, _ = w.Walk(ctx, path) 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..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" @@ -260,3 +261,54 @@ 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", "d"+strconv.Itoa(i), "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) + } + } + + 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) { + 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) + } +} + +// 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 7ac60e1..fba467b 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" @@ -154,17 +153,21 @@ 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) - // fstree does the traversal; the engine only decides what a directory is. - walker := fstree.Walker{ + // 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 pushed only if we descend). + // 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) bool { + 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{ @@ -182,19 +185,22 @@ func runWalk(ctx context.Context, root string, tables []walkEcosystem) ([]model. 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 } // 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 } // 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 { + 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 @@ -203,13 +209,16 @@ func runWalk(ctx context.Context, root string, tables []walkEcosystem) ([]model. // Prune before establishing any context or descending, so nothing // inside is ever a deletion target. if isInstalledPackageTree(dir, names) { - return false + return nil, false } for i := range tables { if p := tables[i].PruneRoot; p != nil && p(dir, names) { - return false + return nil, 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 { @@ -236,13 +245,7 @@ func runWalk(ctx context.Context, root string, tables []walkEcosystem) ([]model. } 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 stack, true }, } if _, err := walker.Walk(ctx, root); err != nil { @@ -268,6 +271,14 @@ 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{}, 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 // bundled dependencies, a Python environment's site-packages). Enter prunes @@ -311,31 +322,20 @@ 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 -// 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 -// cancelled mid-sizing. +// 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. -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 - } + // sizeTokens bounds the concurrent measures; the workers only hand them out. + workers := min(sizeWorkerCap, len(results)) sources := newCloneSources() idx := make(chan int) @@ -348,7 +348,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 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_test.go b/internal/scanner/walk_test.go index ba1c5ca..945ec05 100644 --- a/internal/scanner/walk_test.go +++ b/internal/scanner/walk_test.go @@ -4,6 +4,7 @@ import ( "context" "os" "path/filepath" + "strings" "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,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, 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) + // 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) + } } diff --git a/internal/scanner/walk_traversal_test.go b/internal/scanner/walk_traversal_test.go index b5be58e..43a16c8 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" @@ -163,3 +164,65 @@ func TestWalkScan_HiddenNamedRootIsScanned(t *testing.T) { t.Errorf("expected only %s, got %+v", cache, results) } } + +// 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 +// 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{ + "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", + "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 { + t.Fatal(err) + } + var out []string + for _, r := range results { + out = append(out, r.Path+" "+string(r.Ecosystem)+" "+r.ProjectRoot) + } + return out + } + + var serial []string + withSlotsTaken(t, walkTokens, func() { serial = scan() }) + split := scan() + + if len(serial) == 0 || !slices.Equal(serial, split) { + t.Errorf("split:\n%v\nserial:\n%v", split, serial) + } +}