Repository navigation
detect_binary_per_byte reads every changed file to EOF (no early exit, no size cap on watcher path) #895
Description
Which fff frontend?
Node SDK (@ff-labs/fff-node). The code is in fff-core, so it affects every frontend.
Has logs
Stack samples are below. I didn't enable the fff log file for this.
Description
FileItem::detect_binary_per_byte (crates/fff-core/src/types.rs, unchanged on main at 89c1927) reads the whole file to EOF. It has two problems:
- No early exit. When a chunk contains a NUL byte, it calls
set_binary(true)and keeps reading until EOF. One NUL byte is enough to decide. - No size cap on the watcher path. The bulk scan (
bigram_filter.rs) skips files aboveMAX_FFFILE_SIZE. Buthandle_create_or_modifyandadd_new_filecalldetect_binary_per_byteon every created or modified file of any size. So each write to a large text file re-reads the whole file: think of an append-only log, a JSONL trace or a large generated file under the root.
In practice this is the main CPU cost when the root is a large folder that changes a lot. T3 Code embeds fff for its workspace index. With a ~1.65M-file non-git parent folder of active repos (macOS, Apple M4), sample of the host process showed:
fff-watcher-own (busy in ~50% of samples):
FilePicker::handle_file_modify
-> FileItem::detect_binary_per_byte -> open() 1308
-> FileItem::detect_binary_per_byte -> read() 1252
fff-scan (busy in 100% of samples):
ScanJob::run -> bigram_filter::sniff_binary_for_non_indexable
-> FileItem::detect_binary_per_byte -> read() 4229
-> FileItem::detect_binary_per_byte -> open() 1549
I ran a standalone fff-node harness on that folder with T3's options (disableContentIndexing: true, disableMmapCache: true). Upgrading 0.9.4 → 0.11.0 brought the process from 80% to 47% of a core, thanks to the #751 rescan throttle, which is great. In both versions, most of the remaining CPU is this sniff, not the scans: 7 scans of about 2–4 s each took 191 s of CPU in 0.9.4.
Suggested fix (a few lines):
Ok(n) => {
if detect_binary_content(&chunk[..n]) {
self.set_binary(true);
break;
}
}Also cap the bytes read: either skip files above MAX_FFFILE_SIZE on the watcher path, as the scan already does, or read only the first N KB as git does (it checks the first 8000 bytes). The second option changes classification for text files that have a NUL byte late in the file. A cap on the watcher path alone keeps today's results for anything the scan already classifies.
Downstream report: pingdotgg/t3code#14536
Activity
dmtrKovalenko commentedon Oct 3, 2026
I agree largely, I've been researching the way this actually works in the linux file command for example and there is unfortunately no other way to detect if file is not a binary if you are scanning every single byte till the end.
But I agree this is proably too much I will warp my head around it how to make it faster
dmtrKovalenko commentedon Oct 3, 2026
Yes I think
MAX_FFFILE_SIZEfix is the correct safeguard @gustav-fff go fix itgustav-fff commentedon Oct 3, 2026
[triage-bot] LINKED PR: #14543 (open) — pingdotgg/t3code#14543
Only linked PR is downstream in
pingdotgg/t3code, not in this repo. It bumps@ff-labs/fff-node0.9.4 -> 0.11.0 to pick up the watcher rescan throttle from #751, re-targets the vendored patch and fixes the Windows native-load probe path. It does not touchFileItem::detect_binary_per_byteand its own body says so explicitly: the sniff cost reported here stays unfixed.So: no upstream PR exists for this issue.
crates/fff-core/src/types.rsstill reads to EOF with no early exit and no size cap on the watcher path. @dmtrKovalenko greenlit theMAX_FFFILE_SIZEsafeguard above - that work is unclaimed, not covered by #14543.Honk-Honk 🪿
dmtrKovalenko commentedon Oct 3, 2026
@gustav-fff I mean in the detect binary path!
gustav-fff commentedon Oct 3, 2026
[triage-bot] DIRECTED: fix landed in #899
Guard is now inside
FileItem::detect_binary_per_byte(crates/fff-core/src/types.rs:563), not at call sites:size > MAX_FFFILE_SIZE-> return beforeopen(). Covers watcher modify (file_picker.rs:1694) and add (file_picker.rs:1741); scan path already skipped these, so its results don't change.breakafter first chunk containing NUL.Downstream consumers (
prefilter.rs:78,bigram_filter.rs:1123,file_picker.rs:2154,:2228) all check size anyway, so >10 MiB files stayingis_binary=falsechanges nothing. The scan already left them that way.Regression test
detect_binary_tests::skips_files_above_max_sizefails without the guard:cargo test -p fff-searchgreen.Honk-Honk 🪿