find: stream -files0-from input instead of reading it all into memory - #831
Open
MsfPablo wants to merge 1 commit into
Open
find: stream -files0-from input instead of reading it all into memory#831MsfPablo wants to merge 1 commit into
MsfPablo wants to merge 1 commit into
Conversation
-files0-from read the whole input into memory with read_to_end before splitting on NUL, so an endless source such as /dev/urandom or /dev/zero exhausted memory and produced no output. Read the starting points incrementally with read_until(0) and walk each one as it is parsed. Fixes uutils#779
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #831 +/- ##
==========================================
- Coverage 91.93% 91.92% -0.01%
==========================================
Files 35 35
Lines 7251 7255 +4
Branches 378 378
==========================================
+ Hits 6666 6669 +3
- Misses 443 444 +1
Partials 142 142 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
Commit 57ed3a4 has test result changes: bfs testsuite: |
Merging this PR will improve performance by 10.05%
|
| Benchmark | BASE |
HEAD |
Efficiency | |
|---|---|---|---|---|
| ⚡ | type_f |
20.5 ms | 18.7 ms | +10.05% |
Tip
Curious why this is faster? Comment @codspeedbot explain why this is faster on this PR, or directly use the CodSpeed MCP with your agent.
Comparing MsfPablo:files0-stream (57ed3a4) with main (5aa8184)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #779.
Problem
parse_files0_argsread the entire-files0-fromsource withread_to_endbefore splitting it on NUL, sofind -files0-from /dev/urandom(or/dev/zero, or any very large list) grew the buffer without bound and never walked a single starting point.Fix
The starting points are now produced lazily by a
Files0Pathsiterator built onBufRead::read_until(0, ..), anddo_findwalks each one as soon as it is read. The source is still opened during argument parsing, socannot open ... for readingis still reported before any traversal starts.Behaviour is otherwise unchanged: a trailing NUL still terminates the last name rather than producing an empty one, zero-length names still warn
invalid zero-length file nameand are skipped, and invalid UTF-8 is still an error — the difference is that it now surfaces when that entry is reached, after the earlier valid entries have been walked, which is closer to the GNU behaviour described in the issue.Measured on this branch (macOS,
/usr/bin/time -l):I also checked
src/xargs: it already reads its input incrementally and shares no code with this path.Test
Bounded memory is awkward to assert in the test suite, so the new test covers the observable consequence of streaming:
files0_streams_before_invalid_utf8feeds./test_data/simple\0\xff\0and asserts the valid starting point is printed before the UTF-8 error. That fails on the previous slurp-everything implementation.Verification
cargo test— 226 + 60 + 27 + 25 + 15 + 6 passed, 0 failedcargo fmt --check— cleancargo clippy --all-targets -- -D warnings— cleanDisclosure: this change was prepared with AI assistance (Claude). I reviewed the diff and ran the checks above myself.