diff --git a/src/lib.rs b/src/lib.rs index cc3c7b878c005423a3cd6c7dbc44572e53273071..088d47bfb58af230b79b91b428e8e0e29a3dcd65 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -9,6 +9,8 @@ extern crate alloc; use core::sync::atomic::AtomicUsize; +// The alloc log grows by 1 block about every 21 generations +pub const ALLOC_GC_THRESHOLD: u64 = 1024; pub const BLOCK_SIZE: u64 = 4096; // A record is 4KiB << 5 = 128KiB pub const RECORD_LEVEL: usize = 5; diff --git a/src/tests.rs b/src/tests.rs index 2adf51de371149fa9902f5bfe3393c13f3441907..c911ab54e9942068af62e56e5adffc35a952deaa 100644 --- a/src/tests.rs +++ b/src/tests.rs @@ -1,4 +1,4 @@ -use crate::{unmount_path, DiskSparse, FileSystem, Node, TreePtr}; +use crate::{unmount_path, DiskSparse, FileSystem, Node, TreePtr, ALLOC_GC_THRESHOLD}; use std::path::Path; use std::process::Command; use std::sync::atomic::AtomicUsize; @@ -166,7 +166,11 @@ fn many_create_remove_should_not_increase_size() { let name = "test"; // Iterate over 255 times to prove deleted files don't retain space within the node tree - for i in 0..600 { + // Iterate to an ALLOC_GC_THRESHOLD boundary to ensure the allocator GC reclaims space + let start = fs.header.generation.to_ne(); + let end = start + ALLOC_GC_THRESHOLD; + let end = end - (end % ALLOC_GC_THRESHOLD) + 1 + ALLOC_GC_THRESHOLD; + for i in start..end { let _ = fs .tx(|tx| { tx.create_node( @@ -181,9 +185,6 @@ fn many_create_remove_should_not_increase_size() { .unwrap(); } - // Sync with squash to exclude the allocation log growth from the assertion - let _ = fs.tx(|tx| tx.sync(true)); - // Any value greater than 0 indicates a storage leak let diff = initially_free - fs.allocator().free(); assert_eq!(diff, 0); diff --git a/src/transaction.rs b/src/transaction.rs index d2473ab41084f5a52139f75513b9adf09a9ff3d5..59360a396e31071ce40bcc20ae4c533c87be5eb9 100644 --- a/src/transaction.rs +++ b/src/transaction.rs @@ -15,7 +15,7 @@ use syscall::error::{ use crate::{ AllocEntry, AllocList, Allocator, BlockAddr, BlockData, BlockLevel, BlockPtr, BlockTrait, DirEntry, DirList, Disk, FileSystem, Header, Node, NodeLevel, RecordRaw, TreeData, TreePtr, - ALLOC_LIST_ENTRIES, DIR_ENTRY_MAX_LENGTH, HEADER_RING, + ALLOC_GC_THRESHOLD, ALLOC_LIST_ENTRIES, DIR_ENTRY_MAX_LENGTH, HEADER_RING, }; pub struct Transaction<'a, D: Disk> { @@ -113,11 +113,14 @@ impl<'a, D: Disk> Transaction<'a, D> { /// This method does not write anything to disk, /// all writes are cached. /// - /// If `squash` is true, fully rebuild the allocator log - /// using the state of `self.allocator`. - fn sync_allocator(&mut self, squash: bool) -> Result<bool> { + /// To keep the allocator log from growing excessively, it will + /// periodically be fully rebuilt using the state of `self.allocator`. + /// This rebuild can be forced by setting `force_squash` to `true`. + fn sync_allocator(&mut self, force_squash: bool) -> Result<bool> { let mut prev_ptr = BlockPtr::default(); - if squash { + let should_gc = self.header.generation() % ALLOC_GC_THRESHOLD == 0 + && self.header.generation() >= ALLOC_GC_THRESHOLD; + if force_squash || should_gc { // Clear and rebuild alloc log self.allocator_log.clear(); let levels = self.allocator.levels(); @@ -204,11 +207,10 @@ impl<'a, D: Disk> Transaction<'a, D> { Ok(true) } - // TODO: change this function, provide another way to squash, only write header in commit /// Write all changes cached in this [`Transaction`] to disk. - pub fn sync(&mut self, squash: bool) -> Result<bool> { + pub fn sync(&mut self, force_squash: bool) -> Result<bool> { // Make sure alloc is synced - self.sync_allocator(squash)?; + self.sync_allocator(force_squash)?; // Write all items in write cache for (addr, raw) in self.write_cache.iter_mut() {