From 3ac31a5983648b407e2b83664de746b5769f1e25 Mon Sep 17 00:00:00 2001 From: Francis De Brabandere Date: Tue, 8 Sep 2026 08:24:38 +0200 Subject: [PATCH 1/2] perf: allocate a mini chain's sectors for a whole write at once A write allocates all the mini sectors it needs first, writes their MiniFAT entries together and grows the mini stream once, then writes through consecutive mini sectors of a regular sector in one call. --- src/internal/directory.rs | 15 -- src/internal/minialloc.rs | 283 ++++++++++++++++++++++++++++---------- src/internal/minichain.rs | 84 ++++++++--- tests/mini_streams.rs | 62 +++++++++ 4 files changed, 334 insertions(+), 110 deletions(-) diff --git a/src/internal/directory.rs b/src/internal/directory.rs index daedfde..6f625b3 100644 --- a/src/internal/directory.rs +++ b/src/internal/directory.rs @@ -420,21 +420,6 @@ impl Directory { self.allocator.seek_within_sector(sector_id, offset_within_sector) } - pub fn seek_within_subsector( - &mut self, - sector_id: u32, - subsector_index_within_sector: u32, - subsector_len: usize, - offset_within_subsector: u64, - ) -> io::Result> { - self.allocator.seek_within_subsector( - sector_id, - subsector_index_within_sector, - subsector_len, - offset_within_subsector, - ) - } - pub fn seek_within_header( &mut self, offset_within_header: u64, diff --git a/src/internal/minialloc.rs b/src/internal/minialloc.rs index 6e3fb90..0d232ec 100644 --- a/src/internal/minialloc.rs +++ b/src/internal/minialloc.rs @@ -90,6 +90,10 @@ impl MiniAllocator { self.directory.into_inner() } + pub fn sector_len(&self) -> usize { + self.directory.sector_len() + } + pub fn stream_id_for_name_chain(&self, names: &[&str]) -> Option { self.directory.stream_id_for_name_chain(names) } @@ -209,6 +213,10 @@ impl MiniAllocator { ) } + /// Seeks to `offset_within_mini_sector` bytes into mini sector + /// `mini_sector`. The returned sector runs to the end of the regular + /// sector the mini sector lives in, so consecutive mini sectors that + /// share a regular sector can be read or written in one go. pub fn seek_within_mini_sector( &mut self, mini_sector: u32, @@ -228,11 +236,10 @@ impl MiniAllocator { .ok_or_else(|| { io::Error::new(io::ErrorKind::InvalidData, "invalid sector id") })?; - self.directory.seek_within_subsector( + self.directory.seek_within_sector( sector_id, - mini_sector_within_sector, - consts::MINI_SECTOR_LEN, - offset_within_mini_sector, + mini_sector_within_sector as u64 * consts::MINI_SECTOR_LEN as u64 + + offset_within_mini_sector, ) } } @@ -283,47 +290,75 @@ impl MiniAllocator { self.directory.with_dir_entry_mut(stream_id, func) } - /// Allocates a new mini chain with one sector, and returns the starting - /// sector number. - pub fn begin_mini_chain(&mut self) -> io::Result { - self.allocate_mini_sector(consts::END_OF_CHAIN) - } - - /// Given the starting mini sector (or any internal mini sector) of a mini - /// chain, extends the end of that chain by one mini sector and returns the - /// new mini sector number, updating the MiniFAT as necessary. - pub fn extend_mini_chain( + /// Adds `count` mini sectors to the end of the mini chain whose last + /// mini sector is `last_mini_sector` (or starts a new mini chain, if + /// that is `END_OF_CHAIN`), and returns their IDs in chain order. + /// + /// Doing this for a whole write at once, rather than a mini sector at a + /// time, lets the MiniFAT entries of consecutive mini sectors be written + /// together, and the mini stream (and so the root directory entry) be + /// grown once. + pub fn extend_mini_chain_by( &mut self, - start_mini_sector: u32, - ) -> io::Result { - debug_assert_ne!(start_mini_sector, consts::END_OF_CHAIN); - let mut last_mini_sector = start_mini_sector; - loop { - let next = self.minifat[last_mini_sector as usize]; - if next == consts::END_OF_CHAIN { - break; + last_mini_sector: u32, + count: usize, + ) -> io::Result> { + debug_assert!( + last_mini_sector == consts::END_OF_CHAIN + || self.minifat[last_mini_sector as usize] + == consts::END_OF_CHAIN + ); + let mut mini_sectors = Vec::with_capacity(count); + let mut appended = 0; + for _ in 0..count { + mini_sectors.push(self.take_mini_sector_id(&mut appended)?); + } + // Link the new mini sectors up, writing the MiniFAT entries of each + // run of consecutive IDs in one go. + let mut start = 0; + while start < mini_sectors.len() { + let mut end = start + 1; + while end < mini_sectors.len() + && mini_sectors[end] == mini_sectors[end - 1] + 1 + { + end += 1; } - last_mini_sector = next; + let values: Vec = (start..end) + .map(|i| { + mini_sectors + .get(i + 1) + .copied() + .unwrap_or(consts::END_OF_CHAIN) + }) + .collect(); + self.set_minifat_run(mini_sectors[start], &values)?; + start = end; } - let new_mini_sector = - self.allocate_mini_sector(consts::END_OF_CHAIN)?; - self.set_minifat(last_mini_sector, new_mini_sector)?; - Ok(new_mini_sector) + if last_mini_sector != consts::END_OF_CHAIN && count > 0 { + self.set_minifat(last_mini_sector, mini_sectors[0])?; + } + if appended > 0 { + self.append_mini_sectors(appended)?; + } + Ok(mini_sectors) } - /// Allocates a new entry in the MiniFAT, sets its value to `value`, and - /// returns the new mini sector number. - fn allocate_mini_sector(&mut self, value: u32) -> io::Result { - // If there's an existing free mini sector, use that. + /// Picks the ID for a new mini sector: a free one if there is one, + /// otherwise the one past the end of the mini stream (adding a MiniFAT + /// sector first if the MiniFAT is full), counting it in `appended`. + /// Its MiniFAT entry is set to `END_OF_CHAIN` in memory only; the + /// caller writes the entry, and grows the mini stream by the appended + /// mini sectors. + fn take_mini_sector_id( + &mut self, + appended: &mut usize, + ) -> io::Result { while let Some(free_idx) = self.free_mini_sectors.pop() { if self.minifat[free_idx as usize] == consts::FREE_SECTOR { - self.set_minifat(free_idx, value)?; + self.minifat[free_idx as usize] = consts::END_OF_CHAIN; return Ok(free_idx); } } - // Otherwise, we need a new mini sector; if there's not room in the - // MiniFAT to add it, then first we need to allocate a new MiniFAT - // sector. let minifat_entries_per_sector = self.directory.sector_len() / 4; if self.minifat_start_sector == consts::END_OF_CHAIN { debug_assert!(self.minifat.is_empty()); @@ -345,23 +380,23 @@ impl MiniAllocator { let mut header = self.directory.seek_within_header(64)?; header.write_le_u32(num_minifat_sectors)?; } - // Add a new mini sector to the end of the mini stream and return it. let new_mini_sector = self.minifat.len() as u32; - self.set_minifat(new_mini_sector, value)?; - self.append_mini_sector()?; + self.minifat.push(consts::END_OF_CHAIN); + *appended += 1; Ok(new_mini_sector) } - /// Adds a new mini sector to the end of the mini stream. - fn append_mini_sector(&mut self) -> io::Result<()> { + /// Adds `count` mini sectors to the end of the mini stream, adding + /// regular sectors to its chain as needed. + fn append_mini_sectors(&mut self, count: usize) -> io::Result<()> { let mini_stream_start_sector = self.directory.root_dir_entry().start_sector; let mini_stream_len = self.directory.root_dir_entry().stream_len; debug_assert_eq!(mini_stream_len % consts::MINI_SECTOR_LEN as u64, 0); - let sector_len = self.directory.sector_len(); + let sector_len = self.directory.sector_len() as u64; + let new_mini_stream_len = + mini_stream_len + (count * consts::MINI_SECTOR_LEN) as u64; - // If the mini stream doesn't have room for new mini sector, add - // another regular sector to its chain. let new_start_sector = if mini_stream_start_sector == consts::END_OF_CHAIN { @@ -370,23 +405,25 @@ impl MiniAllocator { self.mini_stream_sectors = Some(vec![start_sector]); start_sector } else { - if mini_stream_len % sector_len as u64 == 0 { - // Extending from the chain's last sector avoids walking - // it from the start; `extend_chain` accepts any sector - // of the chain. - let last_sector = *self.mini_stream_sectors()?.last().unwrap(); - let new_sector = self - .directory - .extend_chain(last_sector, SectorInit::Zero)?; - self.mini_stream_sectors.as_mut().unwrap().push(new_sector); - } mini_stream_start_sector }; + // If the mini stream doesn't have room for the new mini sectors, add + // regular sectors to its chain. + while (self.mini_stream_sectors()?.len() as u64) * sector_len + < new_mini_stream_len + { + // Extending from the chain's last sector avoids walking it from + // the start; `extend_chain` accepts any sector of the chain. + let last_sector = *self.mini_stream_sectors()?.last().unwrap(); + let new_sector = + self.directory.extend_chain(last_sector, SectorInit::Zero)?; + self.mini_stream_sectors.as_mut().unwrap().push(new_sector); + } // Update length of mini stream in root directory entry. self.directory.with_root_dir_entry_mut(|dir_entry| { dir_entry.start_sector = new_start_sector; - dir_entry.stream_len += consts::MINI_SECTOR_LEN as u64; + dir_entry.stream_len = new_mini_stream_len; }) } @@ -453,27 +490,50 @@ impl MiniAllocator { /// underlying file. The `index` must be <= `self.minifat.len()`. fn set_minifat(&mut self, index: u32, value: u32) -> io::Result<()> { debug_assert!(index as usize <= self.minifat.len()); - let offset = (index as u64) * size_of::() as u64; - let sector_len = self.directory.sector_len() as u64; - let sector_index = (offset / sector_len) as usize; - let offset_within_sector = offset % sector_len; - let sector_id = - self.minifat_sectors()?.get(sector_index).copied().ok_or_else( - || { + if (index as usize) == self.minifat.len() { + self.minifat.push(value); + } + self.set_minifat_run(index, &[value]) + } + + /// Sets `self.minifat[index..index + values.len()] = values`, and also + /// writes that change to the underlying file, one write per MiniFAT + /// sector touched. The entries must already exist. + fn set_minifat_run( + &mut self, + index: u32, + values: &[u32], + ) -> io::Result<()> { + let entries_per_sector = + self.directory.sector_len() / size_of::(); + let mut done = 0; + while done < values.len() { + let minifat_index = index as usize + done; + let index_within_sector = minifat_index % entries_per_sector; + let count = (entries_per_sector - index_within_sector) + .min(values.len() - done); + let mut bytes = Vec::with_capacity(count * size_of::()); + for &value in &values[done..done + count] { + bytes.extend_from_slice(&value.to_le_bytes()); + } + let sector_id = self + .minifat_sectors()? + .get(minifat_index / entries_per_sector) + .copied() + .ok_or_else(|| { io::Error::new( io::ErrorKind::InvalidData, "MiniFAT sector missing", ) - }, + })?; + let mut sector = self.directory.seek_within_sector( + sector_id, + (index_within_sector * size_of::()) as u64, )?; - let mut sector = self - .directory - .seek_within_sector(sector_id, offset_within_sector)?; - sector.write_le_u32(value)?; - if (index as usize) == self.minifat.len() { - self.minifat.push(value); - } else { - self.minifat[index as usize] = value; + sector.write_all(&bytes)?; + self.minifat[minifat_index..minifat_index + count] + .copy_from_slice(&values[done..done + count]); + done += count; } Ok(()) } @@ -491,8 +551,8 @@ mod tests { use std::io::Cursor; use crate::internal::{ - consts, Allocator, DirEntry, Directory, ObjType, Sectors, Timestamp, - Validation, Version, + consts, Allocator, DirEntry, Directory, ObjType, SectorInit, Sectors, + Timestamp, Validation, Version, }; use super::MiniAllocator; @@ -510,7 +570,15 @@ mod tests { let version = Version::V3; let num_sectors = 4; // FAT, Directory, MiniFAT, and mini chain let data_len = (1 + num_sectors) * version.sector_len(); - let cursor = Cursor::new(vec![0; data_len]); + let mut data = vec![0; data_len]; + // The MiniFAT lives in sector 2; write it out so that what is on + // disk matches what is in memory. + let minifat_offset = 3 * version.sector_len(); + for (i, &entry) in minifat.iter().enumerate() { + data[minifat_offset + 4 * i..minifat_offset + 4 * i + 4] + .copy_from_slice(&entry.to_le_bytes()); + } + let cursor = Cursor::new(data); let sectors = Sectors::new(version, data_len as u64, cursor); let mut fat = vec![consts::END_OF_CHAIN; num_sectors]; fat[0] = consts::FAT_SECTOR; @@ -530,6 +598,75 @@ mod tests { MiniAllocator::new(directory, minifat, 2, validation).unwrap() } + fn mini_chain( + minialloc: &MiniAllocator>>, + start: u32, + ) -> Vec { + let mut ids = vec![]; + let mut id = start; + while id != consts::END_OF_CHAIN { + ids.push(id); + id = minialloc.next_mini_sector(id).unwrap(); + } + ids + } + + /// Reads the MiniFAT back from the file, as a reader would. + fn minifat_on_disk( + minialloc: &mut MiniAllocator>>, + ) -> Vec { + use crate::ReadLeNumber; + let start = minialloc.minifat_start_sector; + let mut chain = minialloc.open_chain(start, SectorInit::Fat).unwrap(); + let mut minifat = Vec::new(); + for _ in 0..(chain.len() / 4) { + minifat.push(chain.read_le_u32().unwrap()); + } + minifat.truncate(minialloc.minifat.len()); + minifat + } + + #[test] + fn extending_a_mini_chain_links_and_grows_the_mini_stream() { + let mut minialloc = make_minialloc(vec![consts::END_OF_CHAIN]); + assert_eq!(minialloc.root_dir_entry().stream_len, 64); + // Ten mini sectors after the existing one: the MiniFAT entries are + // written, and the mini stream grows to hold them (a V3 sector + // holds 8 mini sectors). + let ids = minialloc.extend_mini_chain_by(0, 10).unwrap(); + assert_eq!(ids, (1..=10).collect::>()); + assert_eq!(mini_chain(&minialloc, 0), (0..=10).collect::>()); + assert_eq!(minifat_on_disk(&mut minialloc), minialloc.minifat); + assert_eq!(minialloc.root_dir_entry().stream_len, 11 * 64); + assert_eq!(minialloc.mini_stream_sectors().unwrap().len(), 2); + // A new chain, then freeing it and extending again reuses its mini + // sectors, last freed first. + let other = + minialloc.extend_mini_chain_by(consts::END_OF_CHAIN, 3).unwrap(); + assert_eq!(other, vec![11, 12, 13]); + minialloc.free_mini_chain(11).unwrap(); + // Freed mini sectors at the end of the mini stream are dropped from + // it again. + assert_eq!(minialloc.root_dir_entry().stream_len, 11 * 64); + let more = minialloc.extend_mini_chain_by(10, 5).unwrap(); + assert_eq!(more, vec![11, 12, 13, 14, 15]); + assert_eq!(mini_chain(&minialloc, 0), (0..=15).collect::>()); + assert_eq!(minifat_on_disk(&mut minialloc), minialloc.minifat); + assert_eq!(minialloc.root_dir_entry().stream_len, 16 * 64); + } + + #[test] + fn extending_across_minifat_sectors() { + let mut minialloc = make_minialloc(vec![consts::END_OF_CHAIN]); + // A V3 MiniFAT sector holds 128 entries. + let ids = + minialloc.extend_mini_chain_by(consts::END_OF_CHAIN, 300).unwrap(); + assert_eq!(mini_chain(&minialloc, ids[0]), ids); + assert_eq!(minialloc.minifat_sectors().unwrap().len(), 3); + assert_eq!(minifat_on_disk(&mut minialloc), minialloc.minifat); + assert_eq!(minialloc.root_dir_entry().stream_len, 301 * 64); + } + #[test] #[should_panic( expected = "Malformed MiniFAT (MiniFAT has 3 entries, but root stream \ diff --git a/src/internal/minichain.rs b/src/internal/minichain.rs index 61f6c09..27613e8 100644 --- a/src/internal/minichain.rs +++ b/src/internal/minichain.rs @@ -38,9 +38,46 @@ impl<'a, F> MiniChain<'a, F> { pub fn len(&self) -> u64 { (consts::MINI_SECTOR_LEN as u64) * (self.sector_ids.len() as u64) } + + /// How many mini sectors from `index` on are consecutive within one + /// regular sector (and so can be read or written in one go), looking + /// no further than needed to cover `wanted` bytes from + /// `offset_within_sector` into the first. + fn run_len( + &self, + index: usize, + offset_within_sector: u64, + wanted: usize, + ) -> usize { + let mini_sector_len = consts::MINI_SECTOR_LEN as u64; + let per_sector = + (self.minialloc.sector_len() / consts::MINI_SECTOR_LEN) as u32; + let first = self.sector_ids[index]; + let mut run = 1; + while index + run < self.sector_ids.len() + && self.sector_ids[index + run] + == self.sector_ids[index + run - 1] + 1 + && self.sector_ids[index + run] / per_sector == first / per_sector + && run as u64 * mini_sector_len - offset_within_sector + < wanted as u64 + { + run += 1; + } + run + } } impl<'a, F: Read + Write + Seek> MiniChain<'a, F> { + /// Adds `count` mini sectors to the end of the chain. + fn extend_by(&mut self, count: usize) -> io::Result<()> { + let last_sector_id = + self.sector_ids.last().copied().unwrap_or(consts::END_OF_CHAIN); + let new_sector_ids = + self.minialloc.extend_mini_chain_by(last_sector_id, count)?; + self.sector_ids.extend(new_sector_ids); + Ok(()) + } + /// Resizes the chain to the minimum number of sectors large enough to old /// `new_len` bytes, allocating or freeing sectors as needed. pub fn set_len(&mut self, new_len: u64) -> io::Result<()> { @@ -60,15 +97,7 @@ impl<'a, F: Read + Write + Seek> MiniChain<'a, F> { } self.zero_tail(new_len)?; } else { - for _ in self.sector_ids.len()..new_num_sectors { - let new_sector_id = - if let Some(&last_sector_id) = self.sector_ids.last() { - self.minialloc.extend_mini_chain(last_sector_id)? - } else { - self.minialloc.begin_mini_chain()? - }; - self.sector_ids.push(new_sector_id); - } + self.extend_by(new_num_sectors - self.sector_ids.len())?; } Ok(()) } @@ -130,6 +159,12 @@ impl<'a, F: Read + Seek> Read for MiniChain<'a, F> { debug_assert!(current_sector_index < self.sector_ids.len()); let current_sector_id = self.sector_ids[current_sector_index]; let offset_within_sector = self.offset_from_start % sector_len; + // Read through as many consecutive mini sectors as the buffer + // covers. + let run = + self.run_len(current_sector_index, offset_within_sector, max_len); + let run_len = run as u64 * sector_len - offset_within_sector; + let max_len = max_len.min(run_len as usize); let mut sector = self.minialloc.seek_within_mini_sector( current_sector_id, offset_within_sector, @@ -146,31 +181,36 @@ impl<'a, F: Read + Write + Seek> Write for MiniChain<'a, F> { if buf.is_empty() { return Ok(0); } - let mut total_len = self.len(); + let total_len = self.len(); + debug_assert!(self.offset_from_start <= total_len); let sector_len = consts::MINI_SECTOR_LEN as u64; - if self.offset_from_start == total_len { - let new_sector_id = - if let Some(&last_sector_id) = self.sector_ids.last() { - self.minialloc.extend_mini_chain(last_sector_id)? - } else { - self.minialloc.begin_mini_chain()? - }; - self.sector_ids.push(new_sector_id); - total_len += sector_len; - debug_assert_eq!(total_len, self.len()); + // Make room for the whole buffer at once. + let end = self.offset_from_start + buf.len() as u64; + if end > total_len { + let count = (end - total_len).div_ceil(sector_len) as usize; + self.extend_by(count)?; } let current_sector_index = (self.offset_from_start / sector_len) as usize; debug_assert!(current_sector_index < self.sector_ids.len()); let current_sector_id = self.sector_ids[current_sector_index]; let offset_within_sector = self.offset_from_start % sector_len; + // Write through as many consecutive mini sectors as the buffer + // covers. + let run = self.run_len( + current_sector_index, + offset_within_sector, + buf.len(), + ); + let run_len = run as u64 * sector_len - offset_within_sector; + let max_len = buf.len().min(run_len as usize); let mut sector = self.minialloc.seek_within_mini_sector( current_sector_id, offset_within_sector, )?; - let bytes_written = sector.write(buf)?; + let bytes_written = sector.write(&buf[..max_len])?; self.offset_from_start += bytes_written as u64; - debug_assert!(self.offset_from_start <= total_len); + debug_assert!(self.offset_from_start <= self.len()); Ok(bytes_written) } diff --git a/tests/mini_streams.rs b/tests/mini_streams.rs index cf951ee..3658159 100644 --- a/tests/mini_streams.rs +++ b/tests/mini_streams.rs @@ -98,3 +98,65 @@ fn a_stream_crossing_the_mini_cutoff_keeps_its_neighbours_intact() { comp.open_stream("/big").unwrap().read_to_end(&mut data).unwrap(); assert_eq!(data, vec![7u8; 50]); } + +/// A mini stream written in one go lands in one run of mini sectors, one +/// appended in pieces crosses mini sector boundaries mid-write, and one +/// created after others were removed reuses their scattered mini sectors; +/// all of them read back intact, also from a strictly reopened file. +#[test] +fn mini_streams_written_whole_in_pieces_and_into_reused_sectors() { + let mut comp = CompoundFile::create(Cursor::new(Vec::new())).unwrap(); + let whole: Vec = (0..3000).map(|i| (i % 253) as u8).collect(); + comp.create_stream("/whole").unwrap().write_all(&whole).unwrap(); + + let mut pieces = Vec::new(); + { + let mut stream = comp.create_stream("/pieces").unwrap(); + for (i, len) in + [10usize, 60, 100, 1, 63, 64, 65, 500].iter().enumerate() + { + let piece = vec![i as u8 + 1; *len]; + stream.write_all(&piece).unwrap(); + pieces.extend_from_slice(&piece); + } + } + + write_streams(&mut comp, 20, 200); + for i in (0..20).step_by(2) { + comp.remove_stream(format!("/s{i}")).unwrap(); + } + let reused: Vec = (0..2500).map(|i| (i % 7) as u8).collect(); + comp.create_stream("/reused").unwrap().write_all(&reused).unwrap(); + + let check = |comp: &mut CompoundFile>>| { + let mut read = |path: &str| { + let mut data = Vec::new(); + comp.open_stream(path).unwrap().read_to_end(&mut data).unwrap(); + data + }; + assert_eq!(read("/whole"), whole); + assert_eq!(read("/pieces"), pieces); + assert_eq!(read("/reused"), reused); + for i in (1..20).step_by(2) { + assert_stream(comp, i, 200); + } + }; + check(&mut comp); + let bytes = comp.into_inner().into_inner(); + let mut comp = CompoundFile::open_strict(Cursor::new(bytes)).unwrap(); + check(&mut comp); +} + +/// Enough mini sectors to need more than one MiniFAT sector (1024 entries +/// per V4 sector) still link up and reopen strictly. +#[test] +fn mini_streams_beyond_one_minifat_sector() { + let mut comp = CompoundFile::create(Cursor::new(Vec::new())).unwrap(); + // 40 streams of 4000 bytes take 63 mini sectors each: 2520 entries. + write_streams(&mut comp, 40, 4000); + let bytes = comp.into_inner().into_inner(); + let mut comp = CompoundFile::open_strict(Cursor::new(bytes)).unwrap(); + for i in 0..40 { + assert_stream(&mut comp, i, 4000); + } +} From df01b038ce6b9e756374d1fac871fb503be533c1 Mon Sep 17 00:00:00 2001 From: Francis De Brabandere Date: Wed, 9 Sep 2026 08:55:25 +0200 Subject: [PATCH 2/2] test: release the chain before reading the MiniFAT length --- src/internal/minialloc.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/internal/minialloc.rs b/src/internal/minialloc.rs index 0d232ec..52533e3 100644 --- a/src/internal/minialloc.rs +++ b/src/internal/minialloc.rs @@ -622,6 +622,7 @@ mod tests { for _ in 0..(chain.len() / 4) { minifat.push(chain.read_le_u32().unwrap()); } + drop(chain); minifat.truncate(minialloc.minifat.len()); minifat }