From e6b4a63d74ffb38ab8634411bcbdd0e457e7597f Mon Sep 17 00:00:00 2001 From: Ed Guloien Date: Wed, 3 Jun 2026 19:49:08 -0400 Subject: [PATCH] comment cleanup --- src/main.rs | 91 +++++++++++++++++++++++++++++++++++------------------ 1 file changed, 60 insertions(+), 31 deletions(-) diff --git a/src/main.rs b/src/main.rs index e3435f7..c857b64 100644 --- a/src/main.rs +++ b/src/main.rs @@ -23,23 +23,26 @@ impl Range { #[derive(Debug)] pub struct Blob { - // todo: consider fallible collections to detect OOM on append - // space complexity O(n + m) // where n is number of bytes - // and m is number of ranges + // where m is number of ranges + // (m should usually be smaller than n, unless reads are very sparse) // ------------------------- - // Selected VecDeque>> for fast push/pop/access - // pop_front may re-allocate, but better than vector most of time - // also since we use box, we are only copying pointers to the arrays, not each u8 + // Selected VecDeque>> for + // fast pop_front and random access, both are O(1) + // push_back is usually O(1), but may re-allocate and copy O(n) + // also since we use box, we are only copying pointers to the arrays, not each u8_array // ------------------------- - // also tried using LinkedList, but slow reading when ranges were inside nodes + // also tried using LinkedList, but slow reading O(n^2) when ranges were inside nodes // and indexing into LinkedList nodes doesn't seem to jive well with Rust's ownership model // especially when considering multi-threaded access // ------------------------- // Vec>> also worth considering, but instead of pop // using None slots to maintain indexes, if arrays are likely to be sparse // this would probably be preferrable + // ------------------------- + // note: consider fallible collections to detect OOM on append if needed + // this code will panic if we run out of memory arrays: VecDeque>, // complexity variable n dropped: usize, ranges: Vec, // valid, global // complexity variable m @@ -64,10 +67,10 @@ impl Blob { ) } - // appending complexity time, worst: O(n + a + m) + // appending complexity time, worst: O(n + a + r) // where n is bytes written - // and a is arrays added - // and m is ranges added, usually 0 or 1, but reallocation could copy (m) + // where a is arrays added + // where r is ranges added, usually 0 or 1, but reallocation could copy (r) fn append(&mut self, input: &[u8]) -> usize { if input.is_empty() { return 0; @@ -80,16 +83,18 @@ impl Blob { let within = global_offset % ARRAY_SIZE; let physical = global_offset / ARRAY_SIZE - self.dropped; if physical == self.arrays.len() { - // O(1) to push_back + // O(1) for single push_back + // O(a) for all arrays that are necessary for the write self.arrays.push_back(Box::new([0u8; ARRAY_SIZE])); } - // write to array - worst: O(n) + // write to array - O(n) let take = (ARRAY_SIZE - within).min(input.len() - written); let array = &mut self.arrays[physical]; array[within..within + take].copy_from_slice(&input[written..written + take]); written += take; } - // update the ranges vector - O(1) + // update the ranges vector - usually O(1) + // but could push could re-allocate and copy: O(r) let new_range = Range::new(self.end_index, self.end_index + input.len()); match self.ranges.last_mut() { Some(last) if last.end == new_range.start => last.end = new_range.end, @@ -100,10 +105,10 @@ impl Blob { written } - // read time complexity, worst: O(n + m + d) + // read time complexity, worst: O(n + r + d) // where n is bytes read - // and m is ranges added. Usually O(1), but could be O(m) on re-allocation copy - // and d is arrays dropped + // where r is ranges checked for already-read bytes + // where d is arrays dropped fn read(&mut self, start: usize, len: usize) -> Result, BlobError> { if len == 0 { return Ok(Vec::new()); @@ -113,6 +118,8 @@ impl Blob { return Err(BlobError::InvalidRange); } + // check if range is valid - O(r) + // todo: we should be able to early exit here once we've gone too far let read_range = Range::new(start, start + len); let Some(idx) = self.ranges.iter().position(|r| r.contains(&read_range)) else { return Err(BlobError::BytesAlreadyRead); @@ -128,7 +135,7 @@ impl Blob { copied += take; } - // update valid ranges - worst case: O(m), usual case O(1) + // update valid ranges - usually: O(1), but could re-allocate O(r) let vr = self.ranges[idx]; let mut remainder = Vec::new(); if vr.start < read_range.start { @@ -139,17 +146,17 @@ impl Blob { } self.ranges.splice(idx..idx + 1, remainder); - // reclaim arrays below the lowest unread byte - // this will keep memory footprint relatively small - // at low cost O(1) or worst on re-allocation: O(number_of_arrays) - // compared to a Vec O(n) - // could also use a Vec>, but this would leave None slots + // reclaim arrays below the lowest unread byte - O(d) + // this will keep memory footprint relatively small unless reads are sparse + // using Vec>> would handle sparse data better for more stable addressing, + // but it leaves None slots that cannot be reclaimed as effectively let min_unread = self .ranges .first() .map(|r| r.start) .unwrap_or(self.end_index); while self.dropped < min_unread / ARRAY_SIZE { + // note: if data is particularly sensitive we should consider zeroing the array on drop self.arrays.pop_front(); self.dropped += 1; } @@ -167,18 +174,35 @@ enum BlobError { } struct BlobManager { - // fine if we assume: - // * IDs are sequentially generated - // * and IDs can be re-used - blobs: RwLock>>>>, + // blob indexing is left up to the caller, but + // vector is fine if we assume IDs are sequentially generated + // fast access: O(1) + // usually fast push_back: O(1), unless re-allocation: O(number_of_blobs) + // space complexity: O(number_of_blobs * blob_size) + // ------------------------ // Also note RwLock (create) starvation might be a concern if caller is // append/read heavy due to platform-defined fairness - // todo: better approach? // RwLock benefits shrink as create traffic becomes heavy + // Could address with sharding the collection into n collection shards + // and determine which shard to use with id % n + // Each shard would have it's own RwLock + // ------------------------ + // If blob creation is arbitrary by ID then + // RwLock>>> would be a better fit. + // There wouldn't be a bunch of empty slots so we save on memory + // but we would pay a bit in access for hash computation and collision handling + // average insert: O(1), but can be long if there are many collisions O(n) + // average access: O(1), but can be long if there are many collisions O(n) + // ------------------------ + // If blob IDs are created more arbitrarily + // it would probably good to provide an API that quickly pops unused IDs + // from a collection (probably VecDeque) so the user doesn't need to poll + // the create function on Error + // This would make adding blob deletion easier which could get us more + // memory space when using big numbers of blobs + blobs: RwLock>>>>, } -// todo: blob indexing is left up to the caller maybe add ID generator -// for cross-thread synchronization or change approach impl BlobManager { pub fn new() -> Self { Self { @@ -191,7 +215,11 @@ impl BlobManager { if let Some(Some(_)) = blobs.get(id) { return Err(BlobError::BlobExists); } else if id >= blobs.len() { - // todo: better strategy possible? + // note on resizing tradeoff: we assumed above that IDs are sequentially generated + // so it might be better to extend the vector by a larger amount all at once (say 2*id) + // if we are more interested in paying that cost upfront. + // though moving to the cost upfront would make even less sense if IDs are added arbitrarily. + // leaving as is for now blobs.resize_with(id + 1, || None); } blobs[id] = Some(Arc::new(Mutex::new(Blob::new()))); @@ -219,7 +247,8 @@ impl BlobManager { } }; let read = blob.lock().unwrap().read(start, len)?; // propagate error to caller - // todo: if read clears a blob entirely should the blob be removed? + // note: if read clears a blob entirely we maintain the blob for eternal blobs + // though could free up some memory if we delete when we don't need anymore Ok(read) } }