Conversation
There was a problem hiding this comment.
Pull request overview
This PR addresses an over-allocation bug in multitude::Arena::reset by ensuring reset only affects local-chunk state (instead of also retiring/clearing shared-chunk state), and it updates allocation routing helpers and regression coverage accordingly.
Changes:
- Adjust
Arena::resetto stop detaching/emptying the current shared chunk (preventing “one new shared chunk per reset cycle” growth patterns). - Consolidate oversized-allocation threshold checks into a single
is_oversizedhelper and update call sites. - Expand regression tests (including loom scenarios) and add new benches to measure/track the affected workload patterns.
Reviewed changes
Copilot reviewed 20 out of 20 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| crates/multitude/src/arena/mod.rs | Changes reset behavior; unifies oversized routing helper documentation and usage. |
| crates/multitude/src/vec/mod.rs | Updates vec growth path to use the unified is_oversized. |
| crates/multitude/src/arena/alloc_value.rs | Updates oversized routing checks to is_oversized. |
| crates/multitude/src/arena/alloc_utf16.rs | Updates oversized routing checks to is_oversized. |
| crates/multitude/src/arena/alloc_unsized.rs | Updates oversized routing checks to is_oversized. |
| crates/multitude/src/arena/alloc_str.rs | Updates oversized routing checks to is_oversized. |
| crates/multitude/src/arena/alloc_slice_ref.rs | Updates oversized routing checks to is_oversized. |
| crates/multitude/src/arena/alloc_slice_box.rs | Updates oversized routing checks to is_oversized. |
| crates/multitude/src/arena/alloc_slice_arc.rs | Updates oversized routing checks to is_oversized. |
| crates/multitude/src/arena/alloc_prefixed.rs | Updates oversized routing checks to is_oversized. |
| crates/multitude/src/allocator_impl.rs | Updates allocator impl oversized routing to is_oversized. |
| crates/multitude/tests/arena.rs | Adds regression tests validating shared-chunk stability and nested-Arc validity across reset cycles. |
| crates/multitude/tests/alloc_ref.rs | Refines wasted-tail invariants to reflect local-vs-shared reset semantics. |
| crates/multitude/tests/loom.rs | Renames/retargets loom tests to reflect reset-vs-drop semantics around shared chunks. |
| crates/multitude/tests/mutant_kills_post_fix.rs | Updates post-fix mutant test commentary for the renamed oversized helper. |
| crates/multitude/tests/zst_uninit_arc_fix.rs | Updates regression comment to the unified is_oversized naming. |
| crates/multitude/Cargo.toml | Adds dev-dep + bench targets for the new arc-array benchmarks. |
| crates/multitude/benches/criterion_arc_array.rs | Introduces a criterion benchmark intended to compare global vs arena-backed arc-array builds. |
| crates/multitude/benches/gungraun_arc_array/main.rs | Adds a gungraun benchmark entrypoint for instruction-precise arc-array builds. |
| crates/multitude/benches/gungraun_arc_array/linux.rs | Implements the Linux-only gungraun arc-array benchmark bodies and setup. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #508 +/- ##
=======================================
Coverage 100.0% 100.0%
=======================================
Files 345 345
Lines 26248 26159 -89
=======================================
- Hits 26248 26159 -89 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
9f5e9d7 to
3e9320e
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 21 out of 22 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (1)
crates/multitude/src/arena/alloc_utf16.rs:189
- This loop reserves
totalbytes withelem_align, but routes oversized allocations and refills based ontotalalone. Becausetry_allocmay need up toelem_align - 1bytes of alignment padding at the front of the reservation, usingtotalforis_oversized/refill_sharedcan miss the max-normal boundary and lead to repeated refills. Using arefill_hint = total + elem_alignfor routing/refill (while still reservingtotal) avoids thetotalvstotal + alignmismatch.
if self.is_oversized(total) {
return self.alloc_oversized_shared_with(total, |mutator, chunk_ptr| {
let (base, _chunk_unused) = mutator
.try_alloc_with_chunk(total, elem_align)
.expect("dedicated oversized chunk sized to fit utf-16 payload");
|
I tried to rebase my example: on top of this change and I was hoping the amount of allocations will drop significantly, but that was not the case. |
- Arcs are now reference counted individually in order to avoid circular reference problem leading to memory leaks. This increases the amount of space used by the arc's payload by 4 bytes, and makes it so arcs are eagerly disposed (like boxes were) which is a semantic improvement. - Arena::reset was confused and leading to memory leaks of shared chunks
Arcs are now reference counted individually in order to avoid
circular reference problem leading to memory leaks. This increases
the amount of space used by the arc's payload by 4 bytes, and makes
it so arcs are eagerly disposed (like boxes were) which is a
semantic improvement.
Arena::reset was confused and leading to memory leaks of shared chunks