From 881b1532b5bdc89357c3b095e50bcbca6baf0a47 Mon Sep 17 00:00:00 2001 From: Eric Coissac Date: Tue, 25 Aug 2026 18:17:32 +0200 Subject: [PATCH] refactor: consolidate mmap file operations into shared helper functions Extracts file creation, opening, and memory mapping logic into a new `mmap_file` module. This replaces repetitive unsafe operations across persistent data structures with standardized helper functions, ensuring consistent header validation, error handling, and data layout while keeping public APIs unchanged. --- src/obicompactvec/src/bitvec.rs | 52 +++------------------- src/obicompactvec/src/fixedintvec.rs | 25 +++-------- src/obicompactvec/src/lib.rs | 1 + src/obicompactvec/src/mmap_file.rs | 66 ++++++++++++++++++++++++++++ src/obicompactvec/src/rankselect.rs | 26 +++-------- 5 files changed, 85 insertions(+), 85 deletions(-) create mode 100644 src/obicompactvec/src/mmap_file.rs diff --git a/src/obicompactvec/src/bitvec.rs b/src/obicompactvec/src/bitvec.rs index a9737811..3cde1910 100644 --- a/src/obicompactvec/src/bitvec.rs +++ b/src/obicompactvec/src/bitvec.rs @@ -1,9 +1,10 @@ -use std::fs::{self, File, OpenOptions}; +use std::fs::{self, OpenOptions}; use std::io::{self, Seek, SeekFrom, Write as _}; use std::path::{Path, PathBuf}; use memmap2::{Mmap, MmapMut}; +use crate::mmap_file::{create_mmap_file, open_mmap_file}; use crate::reader::PersistentCompactIntVec; use crate::views::{BitSliceIter, BitSliceView, IntSliceView}; @@ -33,17 +34,7 @@ pub struct PersistentBitVec { impl PersistentBitVec { pub fn open(path: &Path) -> io::Result { - let mmap = unsafe { Mmap::map(&File::open(path)?)? }; - if mmap.len() < HEADER_SIZE { - return Err(io::Error::new( - io::ErrorKind::InvalidData, - "PBIV file too short", - )); - } - if &mmap[0..4] != &MAGIC { - return Err(io::Error::new(io::ErrorKind::InvalidData, "bad PBIV magic")); - } - let n = u64::from_le_bytes(mmap[8..16].try_into().unwrap()) as usize; + let (mmap, n) = open_mmap_file(path, MAGIC, HEADER_SIZE)?; Ok(Self { mmap, n, @@ -184,18 +175,7 @@ pub struct PersistentBitVecBuilder { impl PersistentBitVecBuilder { pub fn new(n: usize, path: &Path) -> io::Result { let file_size = HEADER_SIZE + n_bytes_for_words(n); - let mut file = OpenOptions::new() - .read(true) - .write(true) - .create(true) - .truncate(true) - .open(path)?; - file.write_all(&MAGIC)?; - file.write_all(&[0u8; 4])?; - file.write_all(&(n as u64).to_le_bytes())?; - file.seek(SeekFrom::Start(0))?; - file.set_len(file_size as u64)?; - let mmap = unsafe { MmapMut::map_mut(&file)? }; + let mmap = create_mmap_file(path, MAGIC, [0u8; 4], n, 0, file_size)?; Ok(Self { mmap, n, @@ -205,16 +185,7 @@ impl PersistentBitVecBuilder { pub fn from_raw_bytes(bytes: &[u8], n: usize, path: &Path) -> io::Result { let file_size = HEADER_SIZE + n_bytes_for_words(n); - let file = OpenOptions::new() - .read(true) - .write(true) - .create(true) - .truncate(true) - .open(path)?; - file.set_len(file_size as u64)?; - let mut mmap = unsafe { MmapMut::map_mut(&file)? }; - mmap[0..4].copy_from_slice(&MAGIC); - mmap[8..16].copy_from_slice(&(n as u64).to_le_bytes()); + let mut mmap = create_mmap_file(path, MAGIC, [0u8; 4], n, 0, file_size)?; mmap[HEADER_SIZE..HEADER_SIZE + bytes.len()].copy_from_slice(bytes); Ok(Self { mmap, @@ -276,18 +247,7 @@ impl PersistentBitVecBuilder { ) -> io::Result { let n = source.len(); let file_size = HEADER_SIZE + n_bytes_for_words(n); - let mut file = OpenOptions::new() - .read(true) - .write(true) - .create(true) - .truncate(true) - .open(path)?; - file.write_all(&MAGIC)?; - file.write_all(&[0u8; 4])?; - file.write_all(&(n as u64).to_le_bytes())?; - file.seek(SeekFrom::Start(0))?; - file.set_len(file_size as u64)?; - let mut mmap = unsafe { MmapMut::map_mut(&file)? }; + let mut mmap = create_mmap_file(path, MAGIC, [0u8; 4], n, 0, file_size)?; { let nw = n_words(n); let ptr = mmap[HEADER_SIZE..].as_mut_ptr() as *mut u64; diff --git a/src/obicompactvec/src/fixedintvec.rs b/src/obicompactvec/src/fixedintvec.rs index e92fb694..4645d82c 100644 --- a/src/obicompactvec/src/fixedintvec.rs +++ b/src/obicompactvec/src/fixedintvec.rs @@ -10,12 +10,13 @@ //! values roughly uniform over a known range (e.g. genome indices, or //! `dict_id`s) — every value costs the same, `width` bits, by construction. -use std::fs::{File, OpenOptions}; -use std::io::{self, Seek, SeekFrom, Write as _}; +use std::io; use std::path::{Path, PathBuf}; use memmap2::{Mmap, MmapMut}; +use crate::mmap_file::{create_mmap_file, open_mmap_file}; + const MAGIC: [u8; 4] = *b"PFIV"; // Header: magic(4) + width(1) + _pad(3) + n(8) = 16 bytes. @@ -58,15 +59,8 @@ pub struct PersistentFixedIntVec { impl PersistentFixedIntVec { pub fn open(path: &Path) -> io::Result { - let mmap = unsafe { Mmap::map(&File::open(path)?)? }; - if mmap.len() < HEADER_SIZE { - return Err(io::Error::new(io::ErrorKind::InvalidData, "PFIV file too short")); - } - if mmap[0..4] != MAGIC { - return Err(io::Error::new(io::ErrorKind::InvalidData, "bad PFIV magic")); - } + let (mmap, n) = open_mmap_file(path, MAGIC, HEADER_SIZE)?; let width = mmap[4] as u32; - let n = u64::from_le_bytes(mmap[8..16].try_into().unwrap()) as usize; Ok(Self { mmap, n, width, path: path.to_path_buf() }) } @@ -144,15 +138,8 @@ impl PersistentFixedIntVecBuilder { pub fn new(n: usize, width: u32, path: &Path) -> io::Result { assert!(width <= 64, "width must be 0..=64, got {width}"); let file_size = HEADER_SIZE + n_words_for(n, width) * 8; - let mut file = OpenOptions::new() - .read(true).write(true).create(true).truncate(true) - .open(path)?; - file.write_all(&MAGIC)?; - file.write_all(&[width as u8, 0, 0, 0])?; - file.write_all(&(n as u64).to_le_bytes())?; - file.seek(SeekFrom::Start(0))?; - file.set_len(file_size as u64)?; - let mmap = unsafe { MmapMut::map_mut(&file)? }; + let pad = [width as u8, 0, 0, 0]; + let mmap = create_mmap_file(path, MAGIC, pad, n, 0, file_size)?; Ok(Self { mmap, n, width, path: path.to_path_buf() }) } diff --git a/src/obicompactvec/src/lib.rs b/src/obicompactvec/src/lib.rs index caa08ef4..84cf95e8 100644 --- a/src/obicompactvec/src/lib.rs +++ b/src/obicompactvec/src/lib.rs @@ -9,6 +9,7 @@ mod rankselect; mod intmatrix; mod layer_meta; mod meta; +mod mmap_file; mod reader; mod storage_kind; mod tempbitvec; diff --git a/src/obicompactvec/src/mmap_file.rs b/src/obicompactvec/src/mmap_file.rs new file mode 100644 index 00000000..02c56265 --- /dev/null +++ b/src/obicompactvec/src/mmap_file.rs @@ -0,0 +1,66 @@ +//! Shared header/mmap boilerplate for this crate's `Persistent*` formats — +//! every one of them starts with `magic(4) + pad(4) + n(8)`, is created by +//! writing that header then extending the file to its final size, and is +//! read back by mapping it and validating the header. `n` at bytes `[8, +//! 16)` is a crate-wide convention, not particular to any one format. +//! +//! Deliberately narrow: only the byte-identical create/open dance is +//! extracted here. Each format keeps its own bit-level layout (word +//! indexing, any trailing index section like `PersistentRankSelectBitVec`'s +//! `block_ranks`) and its own higher-level API — those genuinely differ +//! between formats and aren't forced into a shared shape. + +use std::fs::{File, OpenOptions}; +use std::io::{self, Seek, SeekFrom, Write as _}; +use std::path::Path; + +use memmap2::{Mmap, MmapMut}; + +/// Creates (or truncates) `path`, writes the standard header — `magic` + +/// `pad` (4 bytes right after `magic`; usually all-zero, but +/// `PersistentFixedIntVec` repurposes byte 4 to store its `width`) + +/// `n as u64`, followed by `extra_header_zeros` more zero bytes for a +/// caller-defined trailing header field (e.g. +/// `PersistentRankSelectBitVec`'s `total_ones`, patched in by the caller +/// after construction) — extends the file to `total_size` bytes, and maps +/// it read-write. +pub(crate) fn create_mmap_file( + path: &Path, + magic: [u8; 4], + pad: [u8; 4], + n: usize, + extra_header_zeros: usize, + total_size: usize, +) -> io::Result { + let mut file = OpenOptions::new() + .read(true) + .write(true) + .create(true) + .truncate(true) + .open(path)?; + file.write_all(&magic)?; + file.write_all(&pad)?; + file.write_all(&(n as u64).to_le_bytes())?; + if extra_header_zeros > 0 { + file.write_all(&vec![0u8; extra_header_zeros])?; + } + file.seek(SeekFrom::Start(0))?; + file.set_len(total_size as u64)?; + unsafe { MmapMut::map_mut(&file) } +} + +/// Opens `path` read-only, validates it is at least `header_size` bytes +/// and starts with `magic`, and returns the mapping plus `n` (read from +/// bytes `[8, 16)`). +pub(crate) fn open_mmap_file(path: &Path, magic: [u8; 4], header_size: usize) -> io::Result<(Mmap, usize)> { + let mmap = unsafe { Mmap::map(&File::open(path)?)? }; + let tag = String::from_utf8_lossy(&magic).into_owned(); + if mmap.len() < header_size { + return Err(io::Error::new(io::ErrorKind::InvalidData, format!("{tag} file too short"))); + } + if mmap[0..4] != magic { + return Err(io::Error::new(io::ErrorKind::InvalidData, format!("bad {tag} magic"))); + } + let n = u64::from_le_bytes(mmap[8..16].try_into().unwrap()) as usize; + Ok((mmap, n)) +} diff --git a/src/obicompactvec/src/rankselect.rs b/src/obicompactvec/src/rankselect.rs index 704fca54..a3514ef1 100644 --- a/src/obicompactvec/src/rankselect.rs +++ b/src/obicompactvec/src/rankselect.rs @@ -15,13 +15,14 @@ //! `common_traits::SelectInWord` locates the exact bit within the winning //! word. -use std::fs::{File, OpenOptions}; -use std::io::{self, Seek, SeekFrom, Write as _}; +use std::io; use std::path::{Path, PathBuf}; use common_traits::SelectInWord; use memmap2::{Mmap, MmapMut}; +use crate::mmap_file::{create_mmap_file, open_mmap_file}; + const MAGIC: [u8; 4] = *b"PRSB"; /// Words per rank-sample block — 8 words = 512 bits, one cache line's @@ -62,14 +63,7 @@ pub struct PersistentRankSelectBitVec { impl PersistentRankSelectBitVec { pub fn open(path: &Path) -> io::Result { - let mmap = unsafe { Mmap::map(&File::open(path)?)? }; - if mmap.len() < HEADER_SIZE { - return Err(io::Error::new(io::ErrorKind::InvalidData, "PRSB file too short")); - } - if mmap[0..4] != MAGIC { - return Err(io::Error::new(io::ErrorKind::InvalidData, "bad PRSB magic")); - } - let n = u64::from_le_bytes(mmap[8..16].try_into().unwrap()) as usize; + let (mmap, n) = open_mmap_file(path, MAGIC, HEADER_SIZE)?; let total_ones = u64::from_le_bytes(mmap[16..24].try_into().unwrap()); Ok(Self { mmap, n, total_ones, path: path.to_path_buf() }) } @@ -212,16 +206,8 @@ pub struct PersistentRankSelectBitVecBuilder { impl PersistentRankSelectBitVecBuilder { pub fn new(n: usize, path: &Path) -> io::Result { let file_size = HEADER_SIZE + bits_bytes(n) + blocks_bytes(n); - let mut file = OpenOptions::new() - .read(true).write(true).create(true).truncate(true) - .open(path)?; - file.write_all(&MAGIC)?; - file.write_all(&[0u8; 4])?; - file.write_all(&(n as u64).to_le_bytes())?; - file.write_all(&0u64.to_le_bytes())?; // total_ones, patched in close() - file.seek(SeekFrom::Start(0))?; - file.set_len(file_size as u64)?; - let mmap = unsafe { MmapMut::map_mut(&file)? }; + // 8 extra header zero bytes: `total_ones`, patched in close(). + let mmap = create_mmap_file(path, MAGIC, [0u8; 4], n, 8, file_size)?; Ok(Self { mmap, n, path: path.to_path_buf() }) }