Skip to content

Commit d1cd426

Browse files
sandersaaresCopilotCopilot
authored
feat(bytesbuf)!: make MemoryShared thread-aware (#539)
## Summary Makes the `bytesbuf` `MemoryShared` abstraction thread-aware, closing a design gap where our thread-mobile (`Send`) memory providers were expected to be thread-aware but the abstraction did not require or express it. A provider now owns the decision of how and when to be thread-aware, and any adapter around it simply forwards relocation rather than imposing its own strategy. ## Changes - `MemoryShared` now has `ThreadAware` as a supertrait, so the abstraction requires and expresses thread-awareness. This makes it possible to relocate erased/abstract memory providers correctly across threads. - `MemoryShared` gains an object-safe `clone_boxed` method, blanket-provided for any `Clone` provider, so a type-erased provider can be duplicated without knowing its concrete type. Every `MemoryShared` provider is therefore required to be `Clone`. - Redesigned `CallbackMemory` into a single `CallbackMemory<D>` provider that pairs thread-aware `data: D` with a bare `reserve_fn: fn(&D, usize) -> BytesBuf`. Because the reserve function is a bare function pointer it cannot capture anything, so all reservation state must live in `data`, which is `ThreadAware` and is relocated together with the provider. `CallbackMemory::new(data, reserve_fn)` constructs one; `data` is typically the wrapped inner memory provider, but can be any thread-aware value (e.g. a tuple pairing the provider with configuration). - Reworked `OpaqueMemory` to own a `Box<dyn MemoryShared>`, forwarding both cloning (via `clone_boxed`) and `ThreadAware` relocation to the wrapped provider. It imposes no thread-awareness strategy of its own, leaving that decision entirely to the wrapped provider (e.g. `GlobalPool`, which already manages per-core state internally), and each clone is an independent provider. - Gave the test providers `TransparentMemory` and `FixedBlockMemory` no-op `ThreadAware` impls, since they hold no thread-affine mutable state. - Downstream: `http_extensions` drops its now-unnecessary `Unaware` wrapper around `OpaqueMemory`; `bytesbuf_io` test providers migrate to the new `CallbackMemory` constructor. ## Breaking changes - `MemoryShared` gained `ThreadAware` as a supertrait and a new `clone_boxed` method, so implementors must now also be `ThreadAware` (and `Clone`, which the blanket impl requires). - `CallbackMemory`'s constructor changed from taking a capturing `Fn(usize) -> BytesBuf` closure to taking thread-aware `data` plus a bare `fn(&D, usize) -> BytesBuf`. State that a closure would previously have captured must now be passed as `data`. ## Validation - `just format`, `just readme`, and `just spellcheck` all clean. - Tests pass for `bytesbuf`, `bytesbuf_io`, and `http_extensions` (unit + doctests), including new tests covering `CallbackMemory`/`OpaqueMemory` relocation forwarding, non-`Debug` callback data, and the no-op `relocate` on the test providers. - Clippy `-D warnings` clean; all remaining dependents build. - Mutation testing of the changed logic (`cargo mutants --in-diff`) leaves no surviving mutants. --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
1 parent f993094 commit d1cd426

18 files changed

Lines changed: 452 additions & 219 deletions

File tree

.spelling

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -631,3 +631,4 @@ ASTs
631631
DAGs
632632
representable
633633
rc
634+
destructure

crates/bytesbuf/Cargo.toml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,7 @@ new_zealand = { workspace = true }
4646
nm = { workspace = true }
4747
num-traits = { workspace = true }
4848
smallvec = { workspace = true, features = ["const_new", "union"] }
49-
thread_aware = { workspace = true }
49+
thread_aware = { workspace = true, features = ["derive"] }
5050

5151
[dev-dependencies]
5252
alloc_tracker = { workspace = true }

crates/bytesbuf/README.md

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -311,13 +311,12 @@ const UDP_CONNECTION_OPTIMAL_MEMORY_CONFIGURATION: MemoryConfiguration = MemoryC
311311

312312
impl HasMemory for UdpConnection {
313313
fn memory(&self) -> impl MemoryShared {
314-
CallbackMemory::new({
315-
// Cloning is cheap, as it is a service that shares resources between clones.
316-
let io_context = self.io_context.clone();
314+
// The I/O memory provider carries the thread-affine state, which is relocated when the
315+
// returned provider is moved between threads via a thread-aware runtime mechanism.
316+
let io_memory = self.io_context.io_memory();
317317

318-
move |min_len| {
319-
io_context.reserve_io_memory(min_len, UDP_CONNECTION_OPTIMAL_MEMORY_CONFIGURATION)
320-
}
318+
CallbackMemory::new(io_memory, |io_memory, min_len| {
319+
io_memory.reserve_with_config(min_len, &UDP_CONNECTION_OPTIMAL_MEMORY_CONFIGURATION)
321320
})
322321
}
323322
}
@@ -471,7 +470,7 @@ See the `mem::testing` module for details (requires `test-util` Cargo feature).
471470
This crate was developed as part of <a href="../..">The Oxidizer Project</a>. Browse this crate's <a href="https://github.com/microsoft/oxidizer/tree/main/crates/bytesbuf">source code</a>.
472471
</sub>
473472

474-
[__cargo_doc2readme_dependencies_info]: ggGmYW0CYXZlMC43LjJhdIQbLiTyV0MU86EbZU15e0PmecoboQ9jo59bnAEbyDXw04U13GlhYvRhcoQb4gvD4zw9iycbirtUutkkzqcbSlRn3SgH6bsbjAbxNhZA3wBhZIGCaGJ5dGVzYnVmZTAuNS42
473+
[__cargo_doc2readme_dependencies_info]: ggGmYW0CYXZlMC43LjJhdIQbLiTyV0MU86EbZU15e0PmecoboQ9jo59bnAEbyDXw04U13GlhYvRhcoQbJ1n5emfG9rYb9IJBtfUIpXYbaB5j18j1RxEbDiXhMXybg_xhZIGCaGJ5dGVzYnVmZTAuNS42
475474
[__link0]: https://docs.rs/bytesbuf/0.5.6/bytesbuf/?search=BytesBuf
476475
[__link1]: https://docs.rs/bytesbuf/0.5.6/bytesbuf/?search=BytesView
477476
[__link10]: https://docs.rs/bytesbuf/0.5.6/bytesbuf/?search=BytesView

crates/bytesbuf/examples/bb_has_memory_optimizing.rs

Lines changed: 36 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
88
use bytesbuf::mem::{CallbackMemory, HasMemory, Memory, MemoryShared};
99
use bytesbuf::{BytesBuf, BytesView};
10+
use thread_aware::ThreadAware;
1011

1112
fn main() {
1213
// In a real application, the I/O context would be provided by the framework.
@@ -66,11 +67,14 @@ const UDP_CONNECTION_OPTIMAL_MEMORY_CONFIGURATION: MemoryConfiguration = MemoryC
6667

6768
impl HasMemory for UdpConnection {
6869
fn memory(&self) -> impl MemoryShared {
69-
CallbackMemory::new({
70-
// Cloning is cheap, as it is a service that shares resources between clones.
71-
let io_context = self.io_context.clone();
72-
73-
move |min_len| io_context.reserve_io_memory(min_len, UDP_CONNECTION_OPTIMAL_MEMORY_CONFIGURATION)
70+
// The I/O memory provider carries the thread-affine state, which is relocated when the
71+
// returned provider is moved between threads via a thread-aware runtime mechanism.
72+
let io_memory = self.io_context.io_memory();
73+
74+
CallbackMemory::new(io_memory, |io_memory, min_len| {
75+
// Apply the connection-specific configuration when reserving from the (relocated)
76+
// I/O memory provider.
77+
io_memory.reserve_with_config(min_len, &UDP_CONNECTION_OPTIMAL_MEMORY_CONFIGURATION)
7478
})
7579
}
7680
}
@@ -88,14 +92,40 @@ impl IoContext {
8892
Self {}
8993
}
9094

95+
/// Returns the thread-affine I/O memory provider that reservations are drawn from.
9196
#[expect(clippy::unused_self, reason = "for example realism")]
92-
pub(crate) fn reserve_io_memory(&self, min_len: usize, _memory_configuration: MemoryConfiguration) -> BytesBuf {
97+
pub(crate) fn io_memory(&self) -> IoMemory {
98+
IoMemory
99+
}
100+
}
101+
102+
/// The thread-affine I/O memory provider. In a real application this would carry per-thread I/O
103+
/// resources; here it is a thin wrapper for illustration.
104+
#[derive(Clone, Debug, ThreadAware)]
105+
struct IoMemory;
106+
107+
impl IoMemory {
108+
#[expect(clippy::unused_self, reason = "for example realism")]
109+
fn reserve_with_config(&self, min_len: usize, _memory_configuration: &MemoryConfiguration) -> BytesBuf {
93110
// This is a wrong way to implement this! Only to make the example compile.
94111
let memory = bytesbuf::mem::testing::TransparentMemory::new();
95112
memory.reserve(min_len)
96113
}
97114
}
98115

116+
impl Memory for IoMemory {
117+
fn reserve(&self, min_bytes: usize) -> BytesBuf {
118+
self.reserve_with_config(
119+
min_bytes,
120+
&MemoryConfiguration {
121+
requires_page_alignment: false,
122+
zero_memory_on_release: false,
123+
requires_registered_memory: false,
124+
},
125+
)
126+
}
127+
}
128+
99129
#[expect(dead_code, reason = "just an example, fields unused")]
100130
struct MemoryConfiguration {
101131
requires_page_alignment: bool,

crates/bytesbuf/examples/bb_optimal_path.rs

Lines changed: 27 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ use std::num::NonZero;
1717

1818
use bytesbuf::mem::{BlockSize, CallbackMemory, GlobalPool, HasMemory, MemoryShared};
1919
use bytesbuf::{BytesBuf, BytesView};
20+
use thread_aware::ThreadAware;
2021

2122
fn main() {
2223
// In real-world code, both of these would be provided by the application framework.
@@ -117,11 +118,12 @@ const CONNECTION_OPTIMAL_MEMORY_CONFIGURATION: MemoryConfiguration = MemoryConfi
117118

118119
impl HasMemory for Connection {
119120
fn memory(&self) -> impl MemoryShared {
120-
CallbackMemory::new({
121-
// Cloning is cheap, as it is a service that shares resources between clones.
122-
let io_context = self.io_context.clone();
121+
// The I/O memory provider carries the thread-affine I/O resources, which are relocated when
122+
// the returned provider is moved between threads via a thread-aware runtime mechanism.
123+
let io_memory = self.io_context.io_memory();
123124

124-
move |min_len| io_context.reserve_io_memory(min_len, CONNECTION_OPTIMAL_MEMORY_CONFIGURATION)
125+
CallbackMemory::new(io_memory, |io_memory, min_len| {
126+
io_memory.reserve_with_config(min_len, CONNECTION_OPTIMAL_MEMORY_CONFIGURATION)
125127
})
126128
}
127129
}
@@ -139,8 +141,21 @@ impl IoContext {
139141
Self {}
140142
}
141143

144+
/// Returns the thread-affine I/O memory provider that reservations are drawn from.
142145
#[expect(clippy::unused_self, reason = "for example realism")]
143-
pub(crate) fn reserve_io_memory(&self, min_len: usize, memory_configuration: MemoryConfiguration) -> BytesBuf {
146+
pub(crate) fn io_memory(&self) -> IoMemory {
147+
IoMemory
148+
}
149+
}
150+
151+
/// The thread-affine I/O memory provider. In a real application this would carry per-thread I/O
152+
/// resources; here it holds no state and just performs the allocation.
153+
#[derive(Clone, Debug, ThreadAware)]
154+
struct IoMemory;
155+
156+
impl IoMemory {
157+
#[expect(clippy::unused_self, reason = "for example realism")]
158+
fn reserve_with_config(&self, min_len: usize, memory_configuration: MemoryConfiguration) -> BytesBuf {
144159
let min_len: BlockSize = min_len
145160
.try_into()
146161
.expect("this example is limited to max allocation size of BlockSize, just to keep it simple");
@@ -155,7 +170,13 @@ impl IoContext {
155170
}
156171
}
157172

158-
#[derive(Debug)]
173+
impl bytesbuf::mem::Memory for IoMemory {
174+
fn reserve(&self, min_bytes: usize) -> BytesBuf {
175+
self.reserve_with_config(min_bytes, CONNECTION_OPTIMAL_MEMORY_CONFIGURATION)
176+
}
177+
}
178+
179+
#[derive(Clone, Copy, Debug)]
159180
#[expect(dead_code, reason = "unused fields just for example realism")]
160181
struct MemoryConfiguration {
161182
requires_page_alignment: bool,

crates/bytesbuf/src/lib.rs

Lines changed: 18 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -339,29 +339,35 @@
339339
//!
340340
//! impl HasMemory for UdpConnection {
341341
//! fn memory(&self) -> impl MemoryShared {
342-
//! CallbackMemory::new({
343-
//! // Cloning is cheap, as it is a service that shares resources between clones.
344-
//! let io_context = self.io_context.clone();
342+
//! // The I/O memory provider carries the thread-affine state, which is relocated when the
343+
//! // returned provider is moved between threads via a thread-aware runtime mechanism.
344+
//! let io_memory = self.io_context.io_memory();
345345
//!
346-
//! move |min_len| {
347-
//! io_context.reserve_io_memory(min_len, UDP_CONNECTION_OPTIMAL_MEMORY_CONFIGURATION)
348-
//! }
346+
//! CallbackMemory::new(io_memory, |io_memory, min_len| {
347+
//! io_memory.reserve_with_config(min_len, &UDP_CONNECTION_OPTIMAL_MEMORY_CONFIGURATION)
349348
//! })
350349
//! }
351350
//! }
352351
//!
353352
//! # use bytesbuf::BytesBuf;
353+
//! # use bytesbuf::mem::Memory;
354+
//! # use thread_aware::ThreadAware;
354355
//! # #[derive(Clone, Debug)]
355356
//! # struct IoContext;
356357
//! # impl IoContext {
357-
//! # pub fn reserve_io_memory(
358-
//! # &self,
359-
//! # min_len: usize,
360-
//! # _memory_configuration: MemoryConfiguration,
361-
//! # ) -> BytesBuf {
362-
//! # todo!()
358+
//! # pub fn io_memory(&self) -> IoMemory { IoMemory }
359+
//! # }
360+
//! # #[derive(Clone, Debug, ThreadAware)]
361+
//! # struct IoMemory;
362+
//! # impl IoMemory {
363+
//! # fn reserve_with_config(&self, min_len: usize, _configuration: &MemoryConfiguration) -> BytesBuf {
364+
//! # bytesbuf::mem::GlobalPool::new().reserve(min_len)
363365
//! # }
364366
//! # }
367+
//! # impl Memory for IoMemory {
368+
//! # fn reserve(&self, min_bytes: usize) -> BytesBuf { self.reserve_with_config(min_bytes, &UDP_CONNECTION_OPTIMAL_MEMORY_CONFIGURATION) }
369+
//! # }
370+
//! # #[derive(Clone, Copy)]
365371
//! # struct MemoryConfiguration { requires_page_alignment: bool, zero_memory_on_release: bool, requires_registered_memory: bool }
366372
//! ```
367373
//!

0 commit comments

Comments
 (0)