Skip to content

chore: unify filter mode & squeeze more perf#974

Merged
LoricAndre merged 6 commits intomasterfrom
claude/refactor-filter-options-ESpNQ
Feb 15, 2026
Merged

chore: unify filter mode & squeeze more perf#974
LoricAndre merged 6 commits intomasterfrom
claude/refactor-filter-options-ESpNQ

Conversation

@LoricAndre
Copy link
Contributor

@LoricAndre LoricAndre commented Feb 15, 2026

Benches

FZF

=== Results ===
Completed runs: 10 / 10
Average items matched: 2895782 / 10000000 (min: 2895782, max: 2895782)
Average time: 8.994s (min: 8.729s, max: 9.390s)
Average items/second: 1112446 (min: 1064963, max: 1145607)
Average peak memory usage: 1955 MB (min: 1849 MB, max: 2028 MB)
Average peak CPU usage: 1680.2% (min: 1653.0%, max: 1692.0%)

Current

=== Results ===
Completed runs: 10 / 10
Average items matched: 4874395 / 10000000 (min: 4874395, max: 4874395)
Average time: 5.813s (min: 5.516s, max: 6.917s)
Average items/second: 1730604 (min: 1445713, max: 1812908)
Average peak memory usage: 2102 MB (min: 1940 MB, max: 2174 MB)
Average peak CPU usage: 958.4% (min: 911.0%, max: 1010.0%)

Filter mode (which this PR make relevant)

hyperfine "cat ./bench_data.txt | FZF_DEFAULT_OPTS= fzf --filter test" "cat ./bench_data.txt | SKIM_DEFAULT_OPTIONS= sk --filter test" "cat ./bench_data.txt | SKIM_DEFAULT_OPTIONS= ./target/release/sk --filter test" "cat ./bench_data.txt | SKIM_DEFAULT_OPTIONS= ./target/release/sk --filter test --algo frizbee" -n 3
Benchmark 1: 3
  Time (mean ± σ):      3.742 s ±  0.092 s    [User: 12.369 s, System: 0.616 s]
  Range (min … max):    3.617 s …  3.933 s    10 runs

Benchmark 2: cat ./bench_data.txt | SKIM_DEFAULT_OPTIONS= sk --filter test
  Time (mean ± σ):     15.369 s ±  0.143 s    [User: 14.684 s, System: 3.611 s]
  Range (min … max):   15.219 s … 15.639 s    10 runs

Benchmark 3: cat ./bench_data.txt | SKIM_DEFAULT_OPTIONS= ./target/release/sk --filter test
  Time (mean ± σ):      6.296 s ±  0.062 s    [User: 24.548 s, System: 1.949 s]
  Range (min … max):    6.226 s …  6.427 s    10 runs

Benchmark 4: cat ./bench_data.txt | SKIM_DEFAULT_OPTIONS= ./target/release/sk --filter test --algo frizbee
  Time (mean ± σ):      8.241 s ±  0.069 s    [User: 43.829 s, System: 2.142 s]
  Range (min … max):    8.163 s …  8.413 s    10 runs

Summary
  3 ran
    1.68 ± 0.04 times faster than cat ./bench_data.txt | SKIM_DEFAULT_OPTIONS= ./target/release/sk --filter test
    2.20 ± 0.06 times faster than cat ./bench_data.txt | SKIM_DEFAULT_OPTIONS= ./target/release/sk --filter test --algo frizbee
    4.11 ± 0.11 times faster than cat ./bench_data.txt | SKIM_DEFAULT_OPTIONS= sk --filter test

Checklist

  • The title of my PR follows conventional commits
  • I have updated the documentation (README.md, comments, src/manpage.rs and/or src/options.rs if applicable)
  • I have added unit tests
  • I have added integration tests
  • I have linked all related issues or PRs

Description of the changes

Note: codecov runs on the PR on this repo, but feel free to ignore it.

claude and others added 6 commits February 15, 2026 17:35
Remove the standalone `filter` function from main.rs and integrate
filter mode into the main `run_with` pipeline. When `options.filter`
is set, `should_enter` now waits for all items to be processed and
returns false (skipping TUI), and `App::results` returns all matched
items. This unifies filter mode with the rest of the codebase so it
benefits from all other flags (sorting, tiebreaks, etc.).

https://claude.ai/code/session_01T7pa2RRX85MvBtnH7PWtmw
Three changes that eliminate the performance regression from routing
filter mode through run_with:

1. should_enter(): Wait for reader to finish BEFORE starting matcher,
   then run matcher exactly once. The old polling loop called
   restart_matcher() repeatedly, each time resetting the ItemPool taken
   counter and re-processing all items from scratch. With 1M items
   arriving in batches, items were matched multiple times.

2. matcher.run(): Remove unnecessary .enumerate() (index was discarded)
   and remove item.clone() — into_par_iter() yields owned values so the
   Arc can be moved directly into MatchedItem.

3. App::results(): In filter mode, drain items instead of cloning to
   avoid 271K MatchedItem clones + Arc allocations.

Benchmark (1M file paths, query "test", 10 runs):
  Old standalone filter: 5.306s ± 0.085s
  New unified filter:    4.932s ± 0.099s  (1.08x faster)

https://claude.ai/code/session_01T7pa2RRX85MvBtnH7PWtmw
When force=false, skip the item pool reset so take() only returns new
(untaken) items. The callback merges new sorted matches into the
existing sorted result list using an O(n+m) merge, instead of
replacing it. This fixes the root cause: previously every
restart_matcher call re-processed ALL items from scratch because
reset() set the taken counter to 0.

This benefits all polling callers (filter, select-1, exit-0, sync),
not just filter mode. Items arriving in batches are now each matched
exactly once, and matching overlaps with I/O since batches are
processed as they arrive.

When force=true (query changed), behavior is unchanged: full reset
and re-match.

Reverts the filter-specific workaround from the previous commit in
favor of this general fix; the original polling loop in should_enter
is now efficient.

Benchmark (1M file paths, query "test", 10 runs):
  Old standalone filter (master):  4.566s ± 0.055s
  Incremental restart_matcher:     3.654s ± 0.035s  (1.25x faster)

https://claude.ai/code/session_01T7pa2RRX85MvBtnH7PWtmw
Move the two-sorted-list merge from the restart_matcher closure into
MatchedItem::sorted_merge() for clarity and reusability. The method
merges two Vec<MatchedItem> lists that are already sorted by rank
into a single sorted Vec in O(n+m) time.

https://claude.ai/code/session_01T7pa2RRX85MvBtnH7PWtmw
When restart_matcher runs with force=false, the callback marks results
with a MergeStrategy so the render loop knows how to combine them with
existing items. Previously, render's .take() would drain processed_items
to None, and the next incremental callback would create a fresh batch —
causing the render to replace all items with just the latest batch,
losing previously matched items.

Now:
- Replace: full re-match (force=true), replaces item list entirely
- SortedMerge: incremental sorted results, merged into item_list.items
- Append: incremental unsorted results (--no-sort), appended

This fixes match count consistency in interactive mode. With 1M items
and query "test", match count is now 290,083 on every run (matching
fzf's consistency), vs wildly varying counts before (min 13, max 56,950).

https://claude.ai/code/session_01T7pa2RRX85MvBtnH7PWtmw
@codecov
Copy link

codecov bot commented Feb 15, 2026

Codecov Report

❌ Patch coverage is 47.72727% with 46 lines in your changes missing coverage. Please review.
✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
src/item.rs 20.00% 16 Missing ⚠️
src/lib.rs 15.78% 14 Missing and 2 partials ⚠️
src/tui/item_list.rs 47.05% 9 Missing ⚠️
src/tui/app.rs 84.37% 3 Missing and 2 partials ⚠️

📢 Thoughts on this report? Let us know!

@LoricAndre LoricAndre merged commit 4147ecc into master Feb 15, 2026
26 checks passed
@LoricAndre LoricAndre deleted the claude/refactor-filter-options-ESpNQ branch February 15, 2026 18:08
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.

2 participants