Record the remaining file buffer cleanup issue
This commit is contained in:
@@ -1,6 +1,51 @@
|
||||
# Implementation Review Remediation
|
||||
|
||||
Status: All recorded findings have been resolved in separate remediation units.
|
||||
Status: One computational-efficiency finding awaits remediation. Earlier
|
||||
findings below remain resolved.
|
||||
|
||||
## Computational Efficiency Review
|
||||
|
||||
Review baseline: `8a5c0de`, compared with planning commit `3fde1f6`, the
|
||||
[performance roadmap](performance.md), and [implementation plan](implementation.md).
|
||||
|
||||
### Finding 1 — Release A Stale File Reservation For An Empty Body (P3)
|
||||
|
||||
**Status: Open.**
|
||||
|
||||
Location: [file reader](../../internal/artifact/reader.go), lines 137–166.
|
||||
|
||||
The new file-size reservation is compacted only when the completed body is
|
||||
nonempty. If the opened file reports a positive size but yields no bytes
|
||||
(for example, because it is truncated before the read), the empty artifact
|
||||
retains the entire reservation. An offline diagnostic using the existing
|
||||
opened-file test seam reported a 16 MiB size and returned immediate EOF:
|
||||
the result had zero body bytes and size zero, but retained a 1 MiB backing
|
||||
array. The previous reader did not reserve that storage for an empty result.
|
||||
|
||||
This is a retained-memory edge case, not incorrect content, a permanent leak,
|
||||
or a problem with ordinary empty files whose size hint is zero. It leaves
|
||||
Stage 4's stale-hint cleanup incomplete.
|
||||
|
||||
Remediation: release the reserved backing storage when the completed body is
|
||||
empty, preserving existing empty-artifact semantics. Keep size and hash based
|
||||
on the actual body, and retain the existing EOF and cancellation behavior.
|
||||
No new input limit or public capacity guarantee is needed.
|
||||
|
||||
Verification: extend the stale-size cases to include an empty actual file
|
||||
with a positive reported size; check accepted content, metadata, and hash.
|
||||
Use a focused retained-memory diagnostic or benchmark to verify that the
|
||||
reservation is released, rather than making an exact capacity or allocation
|
||||
count a durable test requirement.
|
||||
|
||||
Also correct the accompanying wording in
|
||||
[Ordinary Artifacts](../internal/sources.md#ordinary-artifacts): the current
|
||||
predicate compacts when unused capacity exceeds the body length (total
|
||||
capacity exceeds twice the body length), not when unused capacity exceeds
|
||||
twice the body length.
|
||||
|
||||
Review verification: the complete offline race suite, repository-wide Go vet,
|
||||
and maintained consumer examples passed. No additional actionable architecture
|
||||
gaps or strengthened public contracts were identified in this review.
|
||||
|
||||
## Effective Requirement Inspection Review
|
||||
|
||||
|
||||
Reference in New Issue
Block a user