Port #402: ranged fiemap queries in the dedupe phase - #13
Merged
Conversation
Ports markfasheh#405 by Tobias Klausmann. With --skip-zeroes, process_blocks() ran is_block_zeroed() on buffer->buf + buffer->dl_offset - always block 0 - instead of block #i, so it skipped either every block or none depending only on whether the first block happened to be zero. Check the block actually being hashed (matching the offset already passed to process_block() just below). Adds an integration test that scans a two-block file with block hashing and --skip-zeroes and asserts which block survives, catching the bug in both directions (leading and trailing zero block). Co-authored-by: Tobias Klausmann <klausman@schwarzvogel.de> Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Ports markfasheh#402 by ElXreno. During dedupe, fiemap_count_shared() and fiemap_scan_extent() called do_fiemap(), which fetches the file's ENTIRE extent map even though they only need the extent(s) at one offset. On large/fragmented files that enumerates every extent per call and dominates the dedupe phase. Add do_fiemap_range(), which sets fm_start/fm_length to map only the requested byte range, and use it in both callers. fiemap_scan_extent() now reads fm_extents[0] directly (the range starts at e_loff, so the first returned extent is the target) instead of walking the whole map. Measured on a 1395-extent duplicated file: dedupe 3.6s -> 1.3s (~2.7x). Correctness unchanged; 39 integration tests pass. Reviewed via /simplify: merged the duplicate fiemap_count_extents helper into one range-aware version, and documented that do_fiemap_range() returns NULL for an empty range as well as on error (which is why fiemap_count_shared() now treats NULL as "0 shared", matching the old whole-file behaviour for holes). Co-authored-by: ElXreno <elxreno@gmail.com> Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
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
1 participant
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.
Ports markfasheh/duperemove#402 by ElXreno (@ElXreno).
During dedupe,
fiemap_count_shared()andfiemap_scan_extent()calleddo_fiemap(), which fetches the file's entire extent map even though they only need the extent(s) at one offset. On large/fragmented files that enumerates every extent on every call and dominates the dedupe phase.Adds
do_fiemap_range()(maps only[start, start+length)) and uses it in both callers;fiemap_scan_extent()now readsfm_extents[0]directly since the range starts at the target offset.Measured on a 1395-extent duplicated file: dedupe 3.6s → 1.3s (~2.7×). Upstream reports far larger wins on 200 GB+/200K-extent images. Correctness unchanged; 39 integration tests pass (the existing
@requires_reflinkdedupe tests exercise both patched functions).Reviewed via
/simplify:fiemap_count_extentshelper into one range-aware version;do_fiemap_range()returnsNULLfor an empty range (hole) as well as on error — which is exactly whyfiemap_count_shared()now treatsNULLas "0 shared", preserving the old whole-file behaviour for holes.(Noted follow-up, not done here:
fiemap_scan_extent()only reads the first extent, so it could skip the count ioctl entirely with a fixed size-1 query — a further ~2× syscall cut on that path.)🤖 Generated with Claude Code