Skip to content

Fix quadratic marking, path resolution, and removal review - #5

Merged
Harold Hunt (huntharo) merged 1 commit into
mainfrom
fix/pwrdisk-mark-performance-20261029
Oct 29, 2026
Merged

Harold Hunt (huntharo) merged 1 commit into
mainfrom
fix/pwrdisk-mark-performance-20261029

Conversation

@huntharo

Copy link
Copy Markdown

Large removal queues cause repeated path searches, nested-target scans, mark-vector compactions, and per-row status searches during marking and rendering. This ports the performance fixes from tobi/disktree#105 into PwrDisk for release.

The branch contains one commit on PwrDisk main (93e27ad). It retains PwrDisk’s existing SSH path validation and CI portability fixes; their overlapping upstream test repairs are not reapplied. Earlier development and red/green snapshots are recorded in huntharo/disktree#9.

The timings below were collected against the original upstream main (6c8d4ce), before this port. They measure the same performance algorithms, not a fresh PwrDisk release benchmark.

A mark is one file or folder queued for removal. A folder containing a million files is still one mark. A queue of 1,024 independently marked paths is a large-queue stress case, not an established frequent manual workload. This PR improves the existing state/render paths; it does not add an MCP endpoint.

The cost is paid at different points:

  • Building a queue: repeated state-level additions, before drawing. Ancestor and descendant searches currently scan all prior marks.
  • Marking a parent: one marking action can compact the mark vector once per absorbed child.
  • Redrawing Explore or Review: marking/unmarking, selection, hover and opening Review can trigger rendering. The plan processes the queue again; Explore also resolves marked paths, and Review searches blocked/covered lists for each row.
  • Receiving a scan result or changing the metric: Marks::refresh resolves the marked paths again. This refresh is not part of every hover or marking action.

The previously reported 6.60 seconds measured four forced Explore draws/eight prepares, with a large queue already present. It was not one scan, one click, or the time to select 1,024 items. The input measurements below have a different, explicit denominator.

Already queued paths One Space input: main → combined One hover input: main → combined Open Review with C: main → combined
1 3.79 → 3.76 ms 3.94 → 3.87 ms 3.47 → 3.43 ms
8 5.14 → 5.01 ms 5.28 → 5.14 ms 5.82 → 5.74 ms
128 41.54 → 16.13 ms 40.85 → 16.26 ms 62.99 → 38.19 ms
1,024 1,584.73 → 96.49 ms 1,575.83 → 97.78 ms 1,786.18 → 300.71 ms

These are medians of three input repetitions in the real GPUI window harness on a fixed, real temporary tree: 1,025 sibling folders with one file each, a 1,400×900 window, and both panels visible. Space adds one more folder; hover/Review then run with that extra mark. Each timed input causes two or three renders in the harness and asserts zero mark refreshes/rescans. Main was replayed immediately before the combined implementation on macOS arm64/Rust 1.97.1 with the repository test profile. These are observed harness timings, not release-app per-click guarantees. Small queues were already fast; no substantial small-queue speedup is claimed.

The implementation changes six paths:

Path Resulting bound and behavior
Removal nesting Sort precomputed platform guard keys, then check the last retained ancestor: O(M log M) path comparisons plus M prefix checks. Output preserves its path ordering.
Mark refresh One lazy sibling index per resolution batch: expected O(S + M·D), where S is indexed sibling entries (S ≤ scanned nodes) and D is path depth.
Explore marked crumbs Share the resolved set across unchanged frames. Mark/tree/root/metric changes and refresh invalidate it; a weak tree identity avoids retaining an old scan.
Disjoint queue construction Ordered exact-path index plus ancestor/range queries: O(M·D·log M) index work for M additions, rather than O(M²) mark visits. Both file and folder queues are covered.
Parent absorption One stable compaction and indexed removal of K descendants: O(M + K log M), plus the ancestor lookup for that action.
Review row status Borrow covered/blocked indexes from the newly computed plan: expected O(B + C + L) status work for B blocked, C covered and L displayed rows (L ≤ 1,200).

These bounds have additional path/name-length costs. Exact mark membership changes from expected hash O(1) to ordered O(log M); no fields are added to scan nodes. Mark order, existing path/component/case comparisons, exact non-UTF-8 paths, first blocked reason, absorbed-count notices and cached-crumb invalidation are tested. Single-item unmarking still compacts the ordered vector; multiple separately invoked parent edits can each require a compaction.

The expansion has its own red/green evidence. The “before” below is pre-expansion #9, with the original path-resolution/removal-plan fixes already included, not a fresh comparison against main:

Added workload (1,024-item setting) Logical work before → combined Median wall before → combined
Queue all 1,024 files, no drawing 1,048,576 → 9,216 176.13 → 6.41 ms
One parent-marking action over 1,024 descendants and one neighbour 528,899 → 2,058 15.60 → 0.47 ms
Blocked Review, four draws/eight renders 524,800 → 3,072 per render 1,003.88 → 883.69 ms total
Covered Review, four draws/eight renders 525,824 → 3,074 per render 1,110.66 → 1,009.63 ms total

The blocked fixture queues paths outside the scanned root. The covered fixture supplies a raw overlapping parent/children queue, which normal manual marking would absorb. These fixtures plan and render only; they attempt no deletion.

Mutation units count visits/index requests, not internal B-tree comparisons; the logarithmic factor is included in the bounds above. Review units include index construction and row queries. The regular budgets grow 128→1,024 entries, require at most 16× work and enforce absolute caps. File insertion, folder insertion, parent absorption and both Review status cases fail before their fixes and pass afterward. Review also passes at 8,192 items, with 10,592 status units per render and the 1,200-row cap preserved. The tests drive actual state transitions or window rendering, so a faster helper alone cannot satisfy them.

Review’s total wall improvement is smaller than its status-lookup reduction: the screen still lays out up to 1,200 rows and evaluates guards/volume attribution for the queue. Plan computation remains per render, and the removal worker still rereads mounts/checks links immediately before deletion. Only display data is reused across renders; no removal authorization is cached. Future MCP bulk control should submit additions within one UI transaction to avoid a redraw per tool call.

Combined evidence, historical snapshots, raw samples, complexity details and reproduction commands. Earlier measurements are retained and labelled as historical stages. The deeper breadcrumb, mount-topology and unusual classification cases from the broader audit remain separate follow-ups. The PR has no dependency on the APFS feature branch.

Validation on this PwrDisk branch: cargo xtask lint and RUST_TEST_THREADS=1 cargo xtask test pass locally on macOS arm64: 106 app + 175 core + 4 scanner integration + 3 xtask tests (288 total), with eight opt-in tests ignored. All original and expanded regular performance budgets pass. The source contribution at dd9855b also passed all six upstream CI jobs: Linux, macOS, and Windows on both x86_64 and arm64 (run). PwrDisk’s own CI will validate this integration.

The rebase preserves the read-only SSH plan and measured mark bytes in file-count mode, with refresh coverage for both metrics. Only complete trees participate in path resolution and mark rendering; provisional previews remain separate. Removal safety checks still run immediately before deletion. Scans and destructive tests use private temporary fixtures.

Historical red/green snapshots and reproduction

The IDs below record pre-consolidation development snapshots from the fork PR; they are not separate commits in this upstream branch. The linked evidence explains how to fetch each historical snapshot.

  • 5a3c92e: original main algorithms plus probes; original refresh/window/removal-nesting budgets fail.
  • 1206fda: original path-resolution/removal-plan fixes.
  • 57d4efd: new queue/parent/Review budgets fail on pre-expansion Report APFS clone sharing and estimate reclaimable space #9.
  • e88b9e1: indexed marking and one-pass absorption.
  • b71718f: indexed Review status from each freshly computed plan.
  • 1c36fae: folder insertion budget, also demonstrated failing against the old algorithm.
  • d886865: preserve the original ancestor choice for raw path spellings with redundant separators.

To reproduce the expanded red stage in a disposable checkout:

git fetch https://github.com/huntharo/disktree.git \
  57d4efde372e7f08e0a2b91a906ff3b870abaddc
git switch --detach FETCH_HEAD
cargo test -p disktree-app budget -- --nocapture --test-threads=1

At the final head:

cargo xtask lint
cargo xtask test
cargo test --workspace budget -- --nocapture --test-threads=1
cargo test -p disktree-app marked_interaction_timings -- \
  --ignored --nocapture --test-threads=1

Batch marked-path resolution and cache display crumbs across unchanged frames. Bound removal nesting, index queue mutations, absorb descendants in one pass, and index Review status from each fresh plan.

Include deterministic state/window work budgets, path and invalidation regression coverage, and measured red/green evidence. Removal authorization remains freshly revalidated before deletion.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant