file_scan: skip all-hole blocks in sparse files (#87) - #89
Conversation
A large sparse file (the classic `truncate -s 1T`, but also image files with big holes) was scanned in full: FIEMAP does not report holes, so get_extent() returned NULL/the next extent and the existing skip only covered UNWRITTEN/INLINE extents. Every 1 MiB chunk of a hole was still pread (zeroes) and, worse, folded into the whole-file checksum, so even --skip-zeroes didn't help. A 1 TiB hole took ~16 min ETA. Skip whole blocks that are entirely holes, working in blocks aligned to the file start so block boundaries - and thus block hashes - are unchanged from a plain read (a block that only partially overlaps a hole is still read, its hole bytes coming back as zeroes, and hashed as before). fill_buffer() caps each read at the next all-hole block; hole_run_at() skips the aligned all-hole run in the scan loop. Instead of hashing the hole's zeroes into the file checksum, fold a cheap (offset, length) descriptor so two files with identical data but different sparse layout still get distinct digests. allocate_hashes() now sizes the block array from mapped bytes, not filesize, so a huge sparse file no longer pre-allocates hundreds of MB of unused records. Measured on a 50 GiB fully-sparse file: 20s -> 2s wall, 15.9 -> 7.7 MB RSS. Adds sparse regression tests: a large hole-only file records no extents yet is digested, and different hole layouts over identical data produce different digests. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Follow-up cleanups on the sparse hole-skip: - get_extent(): trust the resume cursor whenever the *previous* extent ends at or before loff, not only when the pointed extent starts at/before loff. After skipping a hole the scan resumes at a block-floored offset that sits in the hole just before the next extent, which failed the old test and rescanned the extent list from 0 - O(extents^2) on a fragmented sparse file. The new test is strictly more permissive and answer-preserving. - Extract block_is_hole() so hole_run_at() and next_hole_block() share one predicate instead of the same test written in opposite polarity. - assert() the read_cap contract in fill_buffer() (off's block holds data, so the unsigned read clamp can't underflow), and note that UNWRITTEN/INLINE extents deliberately keep the older faked-zeroes checksum path. No behaviour change; adds a get_extent unit case for the hole-resume path. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
| * If the block at ctxt->off is entirely a hole, return the length of the | ||
| * block-aligned run of all-hole blocks starting there; otherwise 0. | ||
| */ | ||
| static size_t hole_run_at(struct scan_ctxt *ctxt) |
There was a problem hiding this comment.
Minor nit, perhaps this function should be called hole_run_length because it returns the length of the hole run at the context offset. The name suggests that it returns the position of a hole run.
| if (mapped > ctxt->filesize) | ||
| mapped = ctxt->filesize; | ||
|
|
||
| hashes->blocks_count = mapped / blocksize + 1; |
There was a problem hiding this comment.
Does this under-count the number of blocks? Extents are block aligned, but that's not necessarily, and unlikely to be the same as the blocksize (by default 128K whereas filesystem blocksize is commonly 4K). So extents will likely not be aligned on an oans block boundary. This means that there could be oans blocks with data outside (before and after) the extent. Assuming 4k fs blocksize, if you have an extent that starts at an offset such that fe_logical % blocksize == blocksize - 4K and the end of the extent is at an offset such that (fe_logical + fe_length) % blocksize == 4K, then fe_length % blocksize == 8K, but you'll need 2 extra block hashes. And that's just for this one extent, more such extents will need even more block hashes more over the sum(extent length) / blocksize +1. If I'm correct, I'd suggest doing something like the following in the for loop instead:
loff_t bstart = ctxt->fiemap->fm_extents[i].fe_logical - (ctxt->fiemap->fm_extents[i].fe_logical % blocksize);
loff_t bend = ctxt->fiemap->fm_extents[i].fe_logical + ctxt->fiemap->fm_extents[i].fe_length + ((ctxt->fiemap->fm_extents[i].fe_logica + ctxt->fiemap->fm_extents[i].fe_length)l % blocksize ? 1 : 0);
mapped += (bend - bstart) / blocksize;
Note that there are cases where potential over counting is happening too, but only by 1 because of the unconditional + 1.
| ctxt.off += hole; | ||
| tprogress->file_scanned_bytes += hole; | ||
| tprogress->total_scanned_bytes += hole; | ||
| continue; |
There was a problem hiding this comment.
This looks correct, but using the name hole is a tad confusing. I would suggest hole_length, which also aligns better with the comment description.
…review) Follow-up to the sparse-hole-skip work in #89, addressing crass's review comments on that PR. allocate_hashes() sized the block array from sum(fe_length)/blocksize + 1. FIEMAP extents are aligned to the filesystem block size (~4K), not to oans blocksize (128K default), so an extent can start and end mid-block and a partially-overlapped block is still read and hashed. The old estimate could therefore under-count by up to ~2 blocks per mapped extent, tripping the one-at-a-time realloc growth in add_block_hash() on fragmented files. Round each extent out to block boundaries before summing so the estimate never under-counts; adjacent extents sharing a block can over-count, but erring high only wastes a little memory once, so it is the safe bias for a preallocation. Also rename hole_run_at() -> hole_run_length() and the local `hole` -> `hole_length`: both name a byte length, not a position. No behavioural change to hashing or dedupe; verify.sh green (90 tests + valgrind smoke). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
Thanks again @crass — all three addressed in #94:
Appreciate the precise review. |
…review) (#94) Follow-up to the sparse-hole-skip work in #89, addressing crass's review comments on that PR. allocate_hashes() sized the block array from sum(fe_length)/blocksize + 1. FIEMAP extents are aligned to the filesystem block size (~4K), not to oans blocksize (128K default), so an extent can start and end mid-block and a partially-overlapped block is still read and hashed. The old estimate could therefore under-count by up to ~2 blocks per mapped extent, tripping the one-at-a-time realloc growth in add_block_hash() on fragmented files. Round each extent out to block boundaries before summing so the estimate never under-counts; adjacent extents sharing a block can over-count, but erring high only wastes a little memory once, so it is the safe bias for a preallocation. Also rename hole_run_at() -> hole_run_length() and the local `hole` -> `hole_length`: both name a byte length, not a position. No behavioural change to hashing or dedupe; verify.sh green (90 tests + valgrind smoke). Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
Closes #87.
Problem
A large sparse file was scanned in full. FIEMAP does not report holes, so
get_extent()returned NULL / the next extent, and the existing skip only coveredUNWRITTEN/INLINEextents (FIEMAP_SKIP_FLAGS). Every 1 MiB chunk of a hole was stillpread(zeroes) and — the part--skip-zeroescouldn't help with — folded into the whole-file checksum.truncate -s 1T; oans test.large.fileshowed an ETA of ~16 min.Approach
Skip whole blocks that are entirely holes, working in blocks aligned to the file start so block boundaries — and therefore block hashes — stay identical to a plain read. A block that only partially overlaps a hole is still read (its hole bytes come back as zeroes) and hashed as before, so dedupe of sparse data is unaffected.
hole_run_at()skips the block-aligned run of all-hole blocks in the scan loop.next_hole_block()caps eachfill_buffer()read at the next all-hole block (sub-block hole tails are still read as zeroes to keep alignment).(offset, length)descriptor — so two files with identical data but different sparse layout still get distinct digests (addresses the collision the issue raised). PreallocatedUNWRITTEN/INLINEextents deliberately keep the older faked-zeroes path.allocate_hashes()sizes the block array from mapped bytes, notfilesize, so a huge sparse file no longer pre-allocates hundreds of MB of unused block records.get_extent()now trusts its resume cursor whenever the previous extent ends at/before the query offset, keeping the post-hole resume O(1) (avoids an O(extents²) rescan on fragmented sparse files).Measurements
The 1 TiB case goes from ~7 min to instant.
Tests
test_large_hole_only_file— a big hole-only file records no extents yet is digested.test_interior_hole_layout_distinct_digest— same data, different hole layout → different digest.get_extentunit case for the hole-resume cursor path.🤖 Generated with Claude Code