Merge pull request #46 from triblespace/codex/evaluate-multiple-mutable-bytearena-buffers

Refactor ByteArena into ByteArea with SectionWriter

Author: somethingelseentirely

CHANGELOG.md

@@ -1,15 +1,19 @@
 # Changelog
 
 ## Unreleased
-- added `ByteArena` for staged file writes with `Buffer::finish()` to return `Bytes`
-- `ByteArena::write` now accepts a zerocopy type instead of an alignment constant
-- `ByteArena` reuses previous pages so allocations align only to the element type
-- `Buffer::finish` converts the writable mapping to read-only instead of remapping
-- documented all fields in `ByteArena` and `Buffer`
-- documented ByteArena usage under advanced usage with proper heading
-- added `ByteArena::persist` to rename the temporary file
-- removed the old `ByteBuffer` type in favor of `ByteArena`
-- added tests covering `ByteArena` writes, typed buffers and persistence
+- added `ByteArea` for staged file writes with `Section::freeze()` to return `Bytes`
+- `SectionWriter::reserve` now accepts a zerocopy type instead of an alignment constant
+- `ByteArea` reuses previous pages so allocations align only to the element type
+- `Section::freeze` converts the writable mapping to read-only instead of remapping
+- simplified `ByteArea` by introducing `SectionWriter` for mutable access without
+  interior mutability
+- tie `Section` lifetime to `ByteArea` to prevent dangling sections
+- allow multiple `ByteArea` sections at once with non-overlapping byte ranges
+- documented all fields in `ByteArea`, `SectionWriter` and `Section`
+- documented ByteArea usage under advanced usage with proper heading
+- added `ByteArea::persist` to rename the temporary file
+- removed the old `ByteBuffer` type in favor of `ByteArea`
+- added tests covering `ByteArea` sections, typed reserves and persistence
 - added test verifying alignment padding between differently aligned writes
 - split Kani verification into `verify.sh` and streamline `preflight.sh`
 - clarify that `verify.sh` runs on a dedicated system and document avoiding async code
@@ -34,6 +38,7 @@
 - compile-time assertion that `ALIGN` is a power of two
 - added `reserve_total` to `ByteBuffer` for reserving absolute capacity
 - fixed potential UB in `Bytes::try_unwrap_owner` for custom owners
+- renamed `ByteArea::writer` to `sections` for clarity
 - prevent dangling `data` by dropping references before unwrapping the owner
 - refined `Bytes::try_unwrap_owner` to cast the data slice to a pointer only
   when the owner type matches

README.md

@@ -101,26 +101,29 @@ fn read_header(file: &std::fs::File) -> std::io::Result<anybytes::view::View<Hea
 To map only a portion of a file use the unsafe helper
 `Bytes::map_file_region(file, offset, len)`.
 
-### Byte Arena
+### Byte Area
 
-Use `ByteArena` to incrementally build immutable bytes on disk:
+Use `ByteArea` to incrementally build immutable bytes on disk:
 
 ```rust
-use anybytes::arena::ByteArena;
+use anybytes::area::ByteArea;
 
-let mut arena = ByteArena::new().unwrap();
-let mut buffer = arena.write::<u8>(4).unwrap();
-buffer.copy_from_slice(b"test");
-let bytes = buffer.finish().unwrap();
+let mut area = ByteArea::new().unwrap();
+let mut sections = area.sections();
+let mut section = sections.reserve::<u8>(4).unwrap();
+section.copy_from_slice(b"test");
+let bytes = section.freeze().unwrap();
 assert_eq!(bytes.as_ref(), b"test".as_ref());
-let all = arena.finish().unwrap();
+drop(sections);
+let all = area.freeze().unwrap();
 assert_eq!(all.as_ref(), b"test".as_ref());
 ```
 
-Call `arena.persist(path)` to keep the temporary file instead of mapping it.
+Call `area.persist(path)` to keep the temporary file instead of mapping it.
 
-The arena only aligns allocations to the element type and may share pages
-between adjacent buffers to minimize wasted space.
+The area only aligns allocations to the element type and may share pages
+between adjacent sections to minimize wasted space. Multiple sections may be
+active simultaneously; their byte ranges do not overlap.
 
 ## Features
 

src/arena.rs src/area.rssrc/arena.rs renamed to src/area.rs

@@ -6,12 +6,12 @@
  * LICENSE file in the root directory of this source tree.
  */
 
-//! Temporary byte arena backed by a file.
+//! Temporary byte area backed by a file.
 //!
-//! The arena allows staged writing through [`ByteArena::write`]. Each
-//! call returns a mutable [`Buffer`] bound to the arena so only one
-//! writer can exist at a time. Finalizing the buffer via
-//! [`Buffer::finish`] remaps the written range as immutable and
+//! The area offers staged writing through a [`SectionWriter`]. Each call to
+//! [`SectionWriter::reserve`] returns a mutable [`Section`] tied to the area's
+//! lifetime. Multiple sections may coexist; their byte ranges do not overlap.
+//! Freezing a section via [`Section::freeze`] remaps its range as immutable and
 //! returns [`Bytes`].
 
 use std::io::{self, Seek, SeekFrom};
@@ -31,93 +31,98 @@ fn align_up(val: usize, align: usize) -> usize {
     (val + align - 1) & !(align - 1)
 }
 
-/// Arena managing a temporary file.
+/// Area managing a temporary file.
 #[derive(Debug)]
-pub struct ByteArena {
-    /// Temporary file backing the arena.
+pub struct ByteArea {
+    /// Temporary file backing the area.
     file: NamedTempFile,
     /// Current length of initialized data in bytes.
     len: usize,
 }
 
-impl ByteArena {
-    /// Create a new empty arena.
+impl ByteArea {
+    /// Create a new empty area.
     pub fn new() -> io::Result<Self> {
         let file = NamedTempFile::new()?;
         Ok(Self { file, len: 0 })
     }
 
-    /// Start a new write of `elems` elements of type `T`.
-    pub fn write<'a, T>(&'a mut self, elems: usize) -> io::Result<Buffer<'a, T>>
+    /// Obtain a handle for reserving sections.
+    pub fn sections(&mut self) -> SectionWriter<'_> {
+        SectionWriter { area: self }
+    }
+
+    /// Freeze the area and return immutable bytes for the entire file.
+    pub fn freeze(self) -> io::Result<Bytes> {
+        let file = self.file.into_file();
+        let mmap = unsafe { memmap2::MmapOptions::new().map(&file)? };
+        Ok(Bytes::from_source(mmap))
+    }
+
+    /// Persist the temporary area file to `path` and return the underlying [`File`].
+    pub fn persist<P: AsRef<std::path::Path>>(self, path: P) -> io::Result<std::fs::File> {
+        self.file.persist(path).map_err(Into::into)
+    }
+}
+
+/// RAII guard giving temporary exclusive write access.
+#[derive(Debug)]
+pub struct SectionWriter<'area> {
+    area: &'area mut ByteArea,
+}
+
+impl<'area> SectionWriter<'area> {
+    /// Reserve a new section inside the area.
+    pub fn reserve<T>(&mut self, elems: usize) -> io::Result<Section<'area, T>>
     where
         T: FromBytes + Immutable,
     {
         let page = page_size::get();
         let align = core::mem::align_of::<T>();
         let len_bytes = core::mem::size_of::<T>() * elems;
-        let start = align_up(self.len, align);
+        let start = align_up(self.area.len, align);
         let end = start + len_bytes;
-        self.file.as_file_mut().set_len(end as u64)?;
-        // Ensure subsequent mappings see the extended size.
-        self.file.as_file_mut().seek(SeekFrom::Start(end as u64))?;
 
-        // Map must start on a page boundary; round `start` down while
-        // keeping track of how far into the mapping the buffer begins.
         let aligned_offset = start & !(page - 1);
         let offset = start - aligned_offset;
         let map_len = end - aligned_offset;
 
+        let file = &mut self.area.file;
+        file.as_file_mut().set_len(end as u64)?;
+        // Ensure subsequent mappings see the extended size.
+        file.as_file_mut().seek(SeekFrom::Start(end as u64))?;
         let mmap = unsafe {
             memmap2::MmapOptions::new()
                 .offset(aligned_offset as u64)
                 .len(map_len)
-                .map_mut(self.file.as_file())?
+                .map_mut(file.as_file())?
         };
-        Ok(Buffer {
-            arena: self,
+
+        self.area.len = end;
+
+        Ok(Section {
             mmap,
-            start,
             offset,
             elems,
             _marker: PhantomData,
         })
     }
-
-    fn update_len(&mut self, end: usize) {
-        self.len = end;
-    }
-
-    /// Finalize the arena and return immutable bytes for the entire file.
-    pub fn finish(self) -> io::Result<Bytes> {
-        let file = self.file.into_file();
-        let mmap = unsafe { memmap2::MmapOptions::new().map(&file)? };
-        Ok(Bytes::from_source(mmap))
-    }
-
-    /// Persist the temporary arena file to `path` and return the underlying [`File`].
-    pub fn persist<P: AsRef<std::path::Path>>(self, path: P) -> io::Result<std::fs::File> {
-        self.file.persist(path).map_err(Into::into)
-    }
 }
 
-/// Mutable buffer for writing into a [`ByteArena`].
+/// Mutable section reserved from a [`ByteArea`].
 #[derive(Debug)]
-pub struct Buffer<'a, T> {
-    /// Arena that owns the underlying file.
-    arena: &'a mut ByteArena,
+pub struct Section<'arena, T> {
     /// Writable mapping for the current allocation.
     mmap: memmap2::MmapMut,
-    /// Start position of this buffer within the arena file in bytes.
-    start: usize,
     /// Offset from the beginning of `mmap` to the start of the buffer.
     offset: usize,
     /// Number of elements in the buffer.
     elems: usize,
-    /// Marker to tie the buffer to element type `T`.
-    _marker: PhantomData<T>,
+    /// Marker tying the section to the area and element type.
+    _marker: PhantomData<(&'arena ByteArea, *mut T)>,
 }
 
-impl<'a, T> Buffer<'a, T>
+impl<'arena, T> Section<'arena, T>
 where
     T: FromBytes + Immutable,
 {
@@ -129,21 +134,19 @@ where
         }
     }
 
-    /// Finalize the buffer and return immutable [`Bytes`].
-    pub fn finish(self) -> io::Result<Bytes> {
+    /// Freeze the section and return immutable [`Bytes`].
+    pub fn freeze(self) -> io::Result<Bytes> {
         self.mmap.flush()?;
         let len_bytes = self.elems * core::mem::size_of::<T>();
         let offset = self.offset;
-        let arena = self.arena;
         // Convert the writable mapping into a read-only view instead of
         // unmapping and remapping the region.
         let map = self.mmap.make_read_only()?;
-        arena.update_len(self.start + len_bytes);
         Ok(Bytes::from_source(map).slice(offset..offset + len_bytes))
     }
 }
 
-impl<'a, T> core::ops::Deref for Buffer<'a, T>
+impl<'arena, T> core::ops::Deref for Section<'arena, T>
 where
     T: FromBytes + Immutable,
 {
@@ -157,7 +160,7 @@ where
     }
 }
 
-impl<'a, T> core::ops::DerefMut for Buffer<'a, T>
+impl<'arena, T> core::ops::DerefMut for Section<'arena, T>
 where
     T: FromBytes + Immutable,
 {
@@ -169,7 +172,7 @@ where
     }
 }
 
-impl<'a, T> AsRef<[T]> for Buffer<'a, T>
+impl<'arena, T> AsRef<[T]> for Section<'arena, T>
 where
     T: FromBytes + Immutable,
 {
@@ -178,7 +181,7 @@ where
     }
 }
 
-impl<'a, T> AsMut<[T]> for Buffer<'a, T>
+impl<'arena, T> AsMut<[T]> for Section<'arena, T>
 where
     T: FromBytes + Immutable,
 {

src/lib.rs

@@ -10,7 +10,7 @@
 #![warn(missing_docs)]
 
 #[cfg(all(feature = "mmap", feature = "zerocopy"))]
-pub mod arena;
+pub mod area;
 /// Core byte container types and traits.
 pub mod bytes;
 mod sources;
@@ -31,7 +31,7 @@ pub mod winnow;
 mod tests;
 
 #[cfg(all(feature = "mmap", feature = "zerocopy"))]
-pub use crate::arena::{Buffer, ByteArena};
+pub use crate::area::{ByteArea, Section, SectionWriter};
 pub use crate::bytes::ByteOwner;
 pub use crate::bytes::ByteSource;
 pub use crate::bytes::Bytes;