Skip to content

refactor: simplify buffer ownership and unused bindings - #64

Open
tisonkun wants to merge 4 commits into
mainfrom
codex/simplify-buffer-ownership
Open

tisonkun wants to merge 4 commits into
mainfrom
codex/simplify-buffer-ownership

Conversation

@tisonkun

@tisonkun tisonkun commented Sep 21, 2026 •

Copy link
Copy Markdown
Contributor

Summary

BufferLease always owns an allocation, but previously modeled it as optional and manually coordinated deallocation with separate fixed/transient ownership cases. Store the allocation directly and share one reservation guard with append staging. Field drop order frees the allocation before returning its budget; allocation failure releases the reservation automatically.

Before After
Option<AlignedBuffer>, BufferOwner, and a custom lease destructor An owned AlignedBuffer followed by an optional MemoryReservation
Separate accounting paths for staging and read buffers MemoryReservation / ManagedMemory::reserve for aggregate and individual charges
Test-only preparation and growth APIs Tests exercise production initialization and capacity interfaces
Underscore-prefixed ownership fields and required trait parameters Ordinary names with narrowly scoped #[expect(dead_code)] or #[expect(unused_variables)] where needed
Unused mutation-result bindings and benchmark display-name parameters Removed, including the obsolete argument-count lint suppression

Local guards and resources retained until scope exit keep conventional underscore-prefixed bindings without extra lint attributes. CONTRIBUTING.md records the distinction between fields and local lifetime bindings. Guard lifetimes, field drop order, public APIs, and on-disk formats are preserved.

Review cache2/src/managed_memory.rs first. Existing coverage exercises memory limits, retained values, I/O completion, quarantined buffers, cancellation, and poisoned locks; updated buffer tests cover initialized prefixes, reuse, zeroed fixed buffers, and rejected out-of-bounds access.

Validation: cargo x check, cargo x test (including extended tests), cargo x lint, and cargo check --target x86_64-unknown-linux-gnu --workspace --all-targets --all-features passed.

Store aligned allocations directly in BufferLease and reuse MemoryReservation for individual buffers and aggregate staging charges. Field drop order frees the allocation before releasing its charge; allocation errors release the reservation automatically.

Remove the optional allocation, BufferOwner enum, custom lease destructor, and redundant deallocation state while preserving alignment and memory-limit behavior.

Validation: managed-memory and append-staging unit tests passed.
Remove test-only preparation and growth methods from buffer ownership types. Initialize engine test buffers through the same target and completion interfaces as production, and use prepared ranges for fixed staging buffers.

Replace growth-only coverage with initialized-prefix, reuse, zero-initialization, and out-of-bounds access checks.

Validation: cargo x check, cargo x test including extended tests, cargo x lint, and Linux all-targets workspace compilation with all features passed.
Give retained ownership fields, scope guards, and required trait parameters ordinary names with narrowly scoped expect attributes. Preserve drop order and guard lifetimes, including write-lock poisoning tests, and rename actively used fields without suppressions.

Remove unused return-value bindings and obsolete benchmark parameters. Document the naming convention and the distinction between retained guards and discarded values in CONTRIBUTING.md.

Validation: cargo x check, cargo x test including extended tests, cargo x lint, and Linux all-targets workspace compilation with all features passed.
@tisonkun tisonkun changed the title refactor: simplify aligned buffer ownership and accounting refactor: simplify buffer ownership and unused bindings Sep 21, 2026
Use underscore-prefixed local bindings for guards and resources retained
until scope exit, removing redundant lint expectations without changing
drop timing. Keep ordinary field names and precise dead-code expectations
for unread ownership fields.

Document this distinction in CONTRIBUTING.md. Unused trait parameters and
unnecessary bindings retain their existing cleanup.

Validation: cargo x check, cargo x test (including extended tests), cargo x
lint, and Linux workspace compilation with all targets and features passed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant