split: do not overflow on -n l/K when K is larger than the byte count - #13903
split: do not overflow on -n l/K when K is larger than the byte count#13903AlejandroCoronadoN wants to merge 1 commit into
Conversation
Merging this PR will degrade performance by 7.83%
|
| Mode | Benchmark | BASE |
HEAD |
Efficiency | |
|---|---|---|---|---|---|
| ❌ | Simulation | df_with_path |
573.7 µs | 704.9 µs | -18.62% |
| ⚡ | Simulation | du_summarize_balanced_tree[(5, 4, 10)] |
16.8 ms | 16.1 ms | +4.38% |
Tip
Investigate this regression by commenting @codspeedbot fix this regression on this PR, or directly use the CodSpeed MCP with your agent.
Comparing AlejandroCoronadoN:fix-split-n-lk-overflow (eed3fdb) with main (822aa83)
Footnotes
-
318 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports. ↩
| num_bytes_written += num_line_bytes; | ||
| let mut skipped = -1; | ||
| while num_bytes_should_be_written <= num_bytes_written { | ||
| // Stop once the last chunk is reached. When there are more chunks than |
There was a problem hiding this comment.
I don't think we need 5 five comment lines
split -n l/K with more chunks than bytes panicked with an integer overflow: the trailing empty chunks never advance the loop, so it spins until chunk_number and skipped overflow. Bound the loop by the chunk count so the extra chunks are written as empty files, matching GNU.
eed3fdb to
f4b23f4
Compare
|
Shortened the comment to two lines. Thanks! |
Problem
When there are more chunks than bytes,
chunk_size_base = num_bytes / num_chunksis 0 and the trailing empty chunks add 0 to
num_bytes_should_be_written, so theinner
while num_bytes_should_be_written <= num_bytes_writtenloop never advancesand spins until
chunk_number/skippedoverflow.Fix
Add
chunk_number < num_chunksto the loop condition so it stops at the lastchunk. There are only
num_chunkschunks, so this never truncates real output.Verification
Compared against GNU
splitfor-n l/5onab,-n l/3ona,-n l/2onempty input,
-n l/3on a 10-byte line, and multi-line inputs: the file set andcontents match exactly (
xaagets the data, the rest are empty). Added aregression test; the full
test_splitsuite (129 tests) passes andcargo fmt/
cargo clippyare clean.