From 572015c8d5fc7563dbdac66b6b848e2b8a191e31 Mon Sep 17 00:00:00 2001 From: Paul Khuong Date: Sun, 5 Sep 2021 17:00:26 -0400 Subject: [PATCH] plain, sharded: stop creating directory structure eagerly Now that we don't assume the cache directory structure after creation, we don't have to spend time creating that structure in cache constructors. This saves time and I/O for high shard count, and makes the constructors infallible. TESTED=updated tests. --- src/plain.rs | 23 ++++++++--------------- src/sharded.rs | 32 ++++++++------------------------ 2 files changed, 16 insertions(+), 39 deletions(-) diff --git a/src/plain.rs b/src/plain.rs index 1fbdf2b..005c37c 100644 --- a/src/plain.rs +++ b/src/plain.rs @@ -61,22 +61,15 @@ impl CacheDir for PlainCache { impl PlainCache { /// Returns a new cache for approximately `capacity` files in /// `base_dir`. - /// - /// # Errors - /// - /// Returns `Err` if `$base_dir/.temp` does not exist and we fail - /// to create it. - pub fn new(base_dir: PathBuf, capacity: usize) -> Result { + pub fn new(base_dir: PathBuf, capacity: usize) -> PlainCache { let mut temp_dir = base_dir; temp_dir.push(TEMP_SUBDIR); - std::fs::create_dir_all(&temp_dir)?; - - Ok(PlainCache { + PlainCache { temp_dir, trigger: PeriodicTrigger::new((capacity / MAINTENANCE_SCALE) as u64), capacity, - }) + } } /// Returns a read-only file for `name` in the cache directory if @@ -142,7 +135,7 @@ fn smoke_test() { // Make sure the garbage file is old enough to be deleted. std::thread::sleep(std::time::Duration::from_secs_f64(2.5)); - let cache = PlainCache::new(temp.path("."), 10).expect("::new must succeed"); + let cache = PlainCache::new(temp.path("."), 10); for i in 0..20 { let name = format!("{}", i); @@ -188,7 +181,7 @@ fn test_set() { use test_dir::{DirBuilder, TestDir}; let temp = TestDir::temp(); - let cache = PlainCache::new(temp.path("."), 1).expect("::new must succeed"); + let cache = PlainCache::new(temp.path("."), 1); { let tmp = NamedTempFile::new_in(cache.temp_dir().expect("temp_dir must succeed")) @@ -241,7 +234,7 @@ fn test_put() { use test_dir::{DirBuilder, TestDir}; let temp = TestDir::temp(); - let cache = PlainCache::new(temp.path("."), 1).expect("::new must succeed"); + let cache = PlainCache::new(temp.path("."), 1); { let tmp = NamedTempFile::new_in(cache.temp_dir().expect("temp_dir must succeed")) @@ -293,7 +286,7 @@ fn test_touch() { use test_dir::{DirBuilder, TestDir}; let temp = TestDir::temp(); - let cache = PlainCache::new(temp.path("."), 5).expect("::new must succeed"); + let cache = PlainCache::new(temp.path("."), 5); for i in 0..15 { let name = format!("{}", i); @@ -331,7 +324,7 @@ fn test_recent_temp_file() { // The garbage file must exist. assert!(std::fs::metadata(temp.path(&format!("{}/garbage", TEMP_SUBDIR))).is_ok()); - let cache = PlainCache::new(temp.path("."), 1).expect("::new must succeed"); + let cache = PlainCache::new(temp.path("."), 1); for i in 0..2 { let tmp = NamedTempFile::new_in(cache.temp_dir().expect("temp_dir must succeed")) diff --git a/src/sharded.rs b/src/sharded.rs index 803ef27..f9259cf 100644 --- a/src/sharded.rs +++ b/src/sharded.rs @@ -136,16 +136,11 @@ impl CacheDir for Shard { impl ShardedCache { /// Returns a new cache for approximately `total_capacity` files, /// stores in `num_shards` subdirectories of `base_dir`. - /// - /// # Errors - /// - /// Returns `Err` if we fail to create any of the cache - /// subdirectories and their `.temp` sub-subdirectories. pub fn new( - mut base_dir: PathBuf, + base_dir: PathBuf, mut num_shards: usize, mut total_capacity: usize, - ) -> Result { + ) -> ShardedCache { // We assume at least two shards. if num_shards < 2 { num_shards = 2; @@ -155,17 +150,6 @@ impl ShardedCache { total_capacity = num_shards; } - // Create the directory structure eagerly. - for i in 0..num_shards { - base_dir.push(&format_id(i)); - base_dir.push(TEMP_SUBDIR); - - std::fs::create_dir_all(&base_dir)?; - - base_dir.pop(); - base_dir.pop(); - } - let mut load_estimates = Vec::with_capacity(num_shards); load_estimates.resize_with(num_shards, || AtomicU8::new(0)); let shard_capacity = @@ -173,13 +157,13 @@ impl ShardedCache { let trigger = PeriodicTrigger::new(shard_capacity.min(total_capacity / MAINTENANCE_SCALE) as u64); - Ok(ShardedCache { + ShardedCache { load_estimates: load_estimates.into_boxed_slice().into(), base_dir, trigger, num_shards, shard_capacity, - }) + } } /// Returns a random shard id. @@ -413,7 +397,7 @@ fn smoke_test() { const PAYLOAD_MULTIPLIER: usize = 113; let temp = TestDir::temp(); - let cache = ShardedCache::new(temp.path("."), 3, 9).expect("::new must succeed"); + let cache = ShardedCache::new(temp.path("."), 3, 9); for i in 0..200 { let name = format!("{}", i); @@ -466,7 +450,7 @@ fn test_set() { use test_dir::{DirBuilder, TestDir}; let temp = TestDir::temp(); - let cache = ShardedCache::new(temp.path("."), 0, 0).expect("::new must succeed"); + let cache = ShardedCache::new(temp.path("."), 0, 0); { let tmp = NamedTempFile::new_in(cache.temp_dir(None).expect("temp_dir must succeed")) @@ -519,7 +503,7 @@ fn test_put() { use test_dir::{DirBuilder, TestDir}; let temp = TestDir::temp(); - let cache = ShardedCache::new(temp.path("."), 0, 0).expect("::new must succeed"); + let cache = ShardedCache::new(temp.path("."), 0, 0); { let tmp = NamedTempFile::new_in(cache.temp_dir(None).expect("temp_dir must succeed")) @@ -565,7 +549,7 @@ fn test_touch() { const PAYLOAD_MULTIPLIER: usize = 113; let temp = TestDir::temp(); - let cache = ShardedCache::new(temp.path("."), 2, 600).expect("::new must succeed"); + let cache = ShardedCache::new(temp.path("."), 2, 600); for i in 0..2000 { // After the first write, we should find our file.