fix(efficiency): count add-then-delete bytes as waste - #100
Merged
Conversation
A file added in one layer and deleted (whiteout or opaque whiteout) in a later layer was contributing zero to WastedBytes and never appeared in WastedFiles. Deleting a file in a later layer only records a whiteout; it never reclaims the earlier layer's bytes, which remain stored in the image and are transferred on every pull. Treating those bytes as "cleaned up" is correct for a live filesystem but wrong for an immutable layered image, and it let the classic download-build-rm bloat pattern pass the efficiency and wasted-bytes gates undetected. pathRuns now records why each run ended: a run closed by a deletion charges all of its occurrences as waste (nothing survives, everything shipped), while a run still live at the top of the stack keeps its last occurrence as before. The score formula, CI rules, thresholds, exit codes, and JSON shape are unchanged; affected images simply report an honest, higher waste figure.
occurrenceCount was accumulated over all runs unconditionally, so a reinstalled file's live single-occurrence run inflated LayerCount even though that copy is the surviving keeper and contributes no waste. Fix: only accumulate occurrenceCount for runs that actually contribute waste — the single-occurrence live run continue now skips the count loop too, so the reinstall copy is not counted. Also removes the stale pre-Round-8 comment paragraph from TestEfficiency_InstallCleanReinstall_FirstCopyWasted that contradicted the test's own assertion, and adds a LayerCount assertion to pin the corrected behaviour.
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.
Summary
Efficiency analysis now counts a file that is added in one layer and deleted in a later layer as wasted space. Previously such files contributed nothing to the waste total and never appeared in the wasted-files list, which understated waste and overstated the efficiency score for one of the most common image-bloat patterns.
Why
Container image layers are immutable and additive. When a later layer "deletes" a file, it only records a whiteout marker — the original bytes remain physically stored in the earlier layer and are transferred on every pull. They are never reclaimed. Treating a deleted file as "cleaned up" is correct for a live filesystem but wrong for a layered image.
This is the textbook "download a big archive, build, then delete it" pattern: the delete makes it look clean, but the bytes ship forever. Because the CI evaluator reads the same numbers, an image over its real byte budget could pass the
lowest-efficiencyandhighest-wasted-bytesgates.What changed
image/efficiency.go): a path's occurrences are grouped into runs separated by deletions. A run that ends in a deletion now charges all of its occurrences as waste — nothing survives into the final image, yet every copy shipped. A run still live at the top of the stack keeps its last occurrence as before (that copy is the one present in the image). This handles add then delete then re-add correctly: the first run is fully charged, the re-install is a fresh live run and is not penalised.wastedBytesdefinition and note that a score may be lower than a tool that counts only files duplicated across layers.Compatibility
The score formula (
1 - wastedBytes / (liveBytes + wastedBytes)), CI rules, thresholds, exit codes, and JSON schema are all unchanged. Affected images simply report an honest, higher waste figure and a correspondingly lower score.Verification
Verified against a purpose-built image (alpine base, write a 1 MiB file, delete it, write and overwrite a 512 KiB file) exported both with and without BuildKit, and cross-checked byte-for-byte against the raw layer tars:
The corrected total (1,048,576 + 524,288) matches the physical bytes stored in the layers.