Skip to content

Commit

Permalink
plain, sharded: stop creating directory structure eagerly
Browse files Browse the repository at this point in the history 
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.
  • Loading branch information
@pkhuong
pkhuong committed Sep 6, 2021
1 parent 4b50760 commit 8280b48
Show file tree
Hide file tree
Showing 2 changed files with 16 additions and 39 deletions.
23 changes: 8 additions & 15 deletions src/plain.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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<PlainCache> {
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
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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"))
Expand Down Expand Up @@ -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"))
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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"))
Expand Down
32 changes: 8 additions & 24 deletions src/sharded.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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> {
) -> ShardedCache {
// We assume at least two shards.
if num_shards < 2 {
num_shards = 2;
Expand All @@ -155,31 +150,20 @@ 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 =
(total_capacity / num_shards) + ((total_capacity % num_shards) != 0) as usize;
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.
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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"))
Expand Down Expand Up @@ -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"))
Expand Down Expand Up @@ -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.
Expand Down

0 comments on commit 8280b48

Please sign in to comment.