Repository navigation
perf(core): share frizbee hot-loop instantiations to cut binary size ~17% - #872
Conversation
|
Understand this PR’s impact Explore downstream dependencies and potential security impact with Blast Radius. No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Advanced Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (2)
Included review availability: Your plan provides up to 10 included reviews per hour; 9 remain after this review. 📝 WalkthroughWalkthroughThe change upgrades Changesneo_frizbee migration
Priority: ⬇️ Low Estimated code review effort: 2 (Simple) | ~12 minutes Change: Refactor 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Docstring CoverageExplanation Docstring coverage is 76.92% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 13 functions across 4 files. (1 skipped: 1 unsupported.)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Confidence Score: 4/5The PR appears safe to merge, though the new full-index allocation on every unconstrained search warrants performance measurement for large repositories. Borrowing Files Needing Attention: crates/fff-core/src/score.rs
|
| Filename | Overview |
|---|---|
| crates/fff-core/src/score.rs | Unifies matcher input types to reduce binary size, with a non-blocking concern about the new per-search O(n) reference-vector allocation. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart LR
A[FileItems::All] --> B[Collect Vec of FileItem references]
C[FileItems::Filtered] --> D[Existing reference slice]
B --> E[Shared frizbee FileItem-reference instantiation]
D --> E
F[Page path Strings] --> G[Create str views]
G --> H[Shared frizbee str instantiation]
Reviews (1): Last reviewed commit: "perf(core): share frizbee hot-loop insta..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| let all_refs: Vec<&FileItem>; | ||
| let candidates: &[&FileItem] = match working_files { | ||
| FileItems::All(files) => { | ||
| all_refs = files.iter().collect(); |
There was a problem hiding this comment.
The common unconstrained-search path now builds a fresh Vec<&FileItem> containing every indexed file before each match. The previous implementation passed the existing file slice directly. For large indexes, repeated searches therefore add O(n) work and temporary memory use before matching starts. This is non-blocking, but the hot-path cost should be benchmarked and, if material, avoided with reusable storage or a design that does not rebuild the reference vector.
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
- rebased fork on upstream frizbee 0.13.0; drops the scalar SIMD fallbacks that can never be selected on aarch64 (~3 MB of inlined dead dispatch code) and adds `SortStrategy::Unsorted`, which skips the parallel k-merge fff never needed - API migration: `Config.sort` is now an enum, match indices are `Vec<u32>`, and the free `match_list*` functions became `Matcher::new(..).match_list*(..)` | `libfff_nvim.dylib` (release, aarch64) | before | after | |---|---|---| | file size | 8.19 MB | 6.76 MB | | `__text` | 5.85 MB | 4.45 MB | Search latency is unchanged within noise (+1 to +3% on fff's fuzzy_search bench, identical results).
- `neo_frizbee` 0.13.1 adds `match_range_parallel_resolved(len, resolve, threads)`,
resolving items by index instead of through a `&[T]` slice
- drops the `Vec<&FileItem>` built for the full list and the per-part `subset`
vectors for files and dirs; each pass now addresses survivors through the
previous round's matches directly
```rust
neo_frizbee::match_range_parallel_resolved(
part,
survivors.len(),
&|index, buf: &mut [*const u8; MAX_PATH_CHUNKS]| {
let file = working_files.index(survivors[index as usize].index as usize);
resolve_file_chunks(file, arena, buf)
},
&part_options,
max_threads,
);
```
3f3e9f3 to
af2a2c3
Compare
1 participant
Why
libfff_nvim.sois ~15MB. Almost 70% of its code isneo_frizbee: the matcher hot loop is monomorphized per call site × 8 SIMD backends × 10 typo/unicode variants. Three of the six call sites existed only because fff passed distinct haystack types (FileItem,&FileItem,String) for what is the same loop.What
match_fuzzy_partsalways feeds frizbee&[&FileItem]. The unconstrained "all files" path now collects a slice of references (one pointer per file) instead of carrying its ownFileIteminstantiation.fuzzy_match_byte_offsets_for_pagematches on&strinstead ofString, sharing the instantiation already used by fuzzy grep.Result (
--profile ci, x86_64 linux)libfff_nvim.so.textNo behavior change.
make lint-rustandmake test-rustpass.Follow-ups (frizbee side)
Remaining frizbee code is still ~7MB. Unicode variants are 3.3× the ASCII ones, and the scalar fallback is the single largest backend (2.5MB) while only reachable on x86_64 without SSE4.1. Those belong in the frizbee repo.
Summary by CodeRabbit