From af1c869ba8e648619c5fda1bb34f669ace62e7c2 Mon Sep 17 00:00:00 2001 From: Lars Wirzenius Date: Fri, 10 Dec 2021 13:17:58 +0200 Subject: refactor a lot Sponsored-by: author --- src/bin/obnam-benchmark.rs | 10 ++- src/data.rs | 95 ---------------------------- src/lib.rs | 3 +- src/result.rs | 9 ++- src/state.rs | 119 ----------------------------------- src/step.rs | 2 +- src/suite.rs | 153 +++++++++++++++++++++++++++++++++++++++++++++ 7 files changed, 166 insertions(+), 225 deletions(-) delete mode 100644 src/data.rs delete mode 100644 src/state.rs create mode 100644 src/suite.rs diff --git a/src/bin/obnam-benchmark.rs b/src/bin/obnam-benchmark.rs index 76ab620..85ef3f9 100644 --- a/src/bin/obnam-benchmark.rs +++ b/src/bin/obnam-benchmark.rs @@ -1,7 +1,7 @@ use log::{debug, error, info}; use obnam_benchmark::result::Result; use obnam_benchmark::specification::Specification; -use obnam_benchmark::state::State; +use obnam_benchmark::suite::Suite; use std::fs::File; use std::path::PathBuf; use std::process::exit; @@ -61,12 +61,10 @@ impl Run { fn run(&self) -> anyhow::Result<()> { info!("running benchmarks from {}", self.spec.display()); let spec = Specification::from_file(&self.spec)?; - let mut state = State::new()?; + let mut suite = Suite::new()?; let mut result = Result::default(); - for step in spec.steps() { - if let Some(m) = state.execute(&step)? { - result.push(m); - } + for step in spec.steps().iter() { + result.push(suite.execute(step)?); } debug!("writing results to {}", self.output.display()); let output = File::create(&self.output)?; diff --git a/src/data.rs b/src/data.rs deleted file mode 100644 index 2069110..0000000 --- a/src/data.rs +++ /dev/null @@ -1,95 +0,0 @@ -use crate::specification::{Create, FileCount}; -use log::debug; -use std::fs::File; -use std::path::{Path, PathBuf}; -use tempfile::{tempdir_in, TempDir}; -use walkdir::WalkDir; - -/// Test data management for Obnam benchmarks. -/// -/// Each backup in a benchmark backs up some test data created for -/// that backup. That data is later cleaned up. Is all handled by this -/// structure. -#[derive(Debug)] -pub struct Data { - live: TempDir, -} - -/// Possible errors from managing data. -#[derive(Debug, thiserror::Error)] -pub enum DataError { - /// Directory creation failed. - #[error("Failed to create directory {0}: {1}")] - CreateDirectory(PathBuf, std::io::Error), - - /// File creation failed. - #[error("Failed to create file {0}: {1}")] - CreateFile(PathBuf, std::io::Error), - - /// File counting failed. - #[error("Failed to count files in {0}: {1}")] - CountFiles(PathBuf, walkdir::Error), - - /// Failed to create a temporary directory. - #[error(transparent)] - CreateTemp(#[from] std::io::Error), -} - -impl Data { - pub(crate) fn new(tempdir: &Path) -> Result { - let live = tempdir_in(&tempdir)?; - debug!("created temporary directory {}", live.path().display()); - Ok(Self { live }) - } - - pub(crate) fn create(&self, create: &Create) -> Result<(), DataError> { - debug!( - "creating {} files in {}", - create.files, - self.live.path().display() - ); - for i in 0..create.files { - let filename = self.live.path().join(format!("{}", i)); - debug!("creating {}", filename.display()); - File::create(&filename).map_err(|err| DataError::CreateFile(filename, err))?; - } - Ok(()) - } - - pub(crate) fn rename(&self, count: &FileCount) -> Result<(), DataError> { - debug!("renaming {} files", count.files); - Ok(()) - } - - pub(crate) fn delete(&self, count: &FileCount) -> Result<(), DataError> { - debug!("deleting {}", count.files); - Ok(()) - } - - pub(crate) fn file_count(&self) -> Result { - debug!("counting files in {}", self.live.path().display()); - let mut n = 0; - for entry in WalkDir::new(&self.live).into_iter() { - if let Err(err) = entry { - return Err(DataError::CountFiles(self.live.path().to_path_buf(), err)); - } - n += 1; - } - debug!("found {} files in {}", n, self.live.path().display()); - Ok(n) - } - - pub(crate) fn file_size(&self) -> Result { - debug!( - "counting combined slze of files in {}", - self.live.path().display() - ); - let n = 0; - debug!( - "found {} bytes of data in {}", - n, - self.live.path().display() - ); - Ok(n) - } -} diff --git a/src/lib.rs b/src/lib.rs index 0f05dd8..22d7184 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -28,8 +28,7 @@ //! This crate only collects data from a set of benchmarks. It does //! not analyze the data. The data can be stored for later analysis. -pub mod data; pub mod result; pub mod specification; -pub mod state; pub mod step; +pub mod suite; diff --git a/src/result.rs b/src/result.rs index b9ea0d8..cf6c0ac 100644 --- a/src/result.rs +++ b/src/result.rs @@ -21,8 +21,13 @@ pub enum Measurement { #[derive(Debug, Clone, Copy, Serialize)] pub enum Operation { - Backup(usize), - Restore(usize), + Start, + Stop, + Create, + Rename, + Delete, + Backup, + Restore, } impl Result { diff --git a/src/state.rs b/src/state.rs deleted file mode 100644 index d70af11..0000000 --- a/src/state.rs +++ /dev/null @@ -1,119 +0,0 @@ -use crate::data::{Data, DataError}; -use crate::result::{Measurement, OpMeasurements, Operation}; -use crate::step::Step; -use log::{debug, info}; -use std::path::Path; -use std::time::Instant; -use tempfile::{tempdir, TempDir}; - -/// Runtime state for running Obnam benchmarks. -pub struct State { - current_benchmark: Option, - tempdir: TempDir, -} - -/// Possible errors from changing benchmark state. -#[derive(Debug, thiserror::Error)] -pub enum StateError { - /// A step is executed in the wrong order. - #[error("Internal error: step expected inside a benchmark")] - OutOfStep, - - /// Failed to create a temporary directory. - #[error(transparent)] - CreateTemp(#[from] std::io::Error), - - /// Error managing test data. - #[error(transparent)] - Data(#[from] DataError), -} - -impl State { - pub fn new() -> Result { - Ok(Self { - current_benchmark: None, - tempdir: tempdir()?, - }) - } - - fn current(&self) -> Result<&CurrentBenchmark, StateError> { - if let Some(x) = &self.current_benchmark { - Ok(x) - } else { - Err(StateError::OutOfStep) - } - } - - pub fn execute(&mut self, step: &Step) -> Result, StateError> { - debug!("executing step {:?}", step); - let now = Instant::now(); - let om = match step { - Step::Start(name) => { - info!("starting benchmark {}", name); - self.current_benchmark = Some(CurrentBenchmark::new(name, self.tempdir.path())?); - None - } - Step::Stop(name) => { - info!("ending benchmark {}", name); - self.current_benchmark = None; - None - } - Step::Create(x) => { - info!("creating {} test data files", x.files); - self.current()?.data.create(x)?; - None - } - Step::Rename(x) => { - info!("renaming {} test data files", x.files); - self.current()?.data.rename(x)?; - None - } - Step::Delete(x) => { - info!("deleting {} test data files", x.files); - self.current()?.data.delete(x)?; - None - } - Step::Backup(x) => Some(backup(*x, self.current()?)?), - Step::Restore(x) => Some(restore(*x, self.current()?)?), - }; - - let t = std::time::Duration::from_millis(10); - std::thread::sleep(t); - - if let Some(mut om) = om { - let ms = now.elapsed().as_millis(); - debug!("step duration was {} ms", ms); - om.push(Measurement::DurationMs(ms)); - Ok(Some(om)) - } else { - Ok(None) - } - } -} - -fn backup(i: usize, current: &CurrentBenchmark) -> Result { - info!("backing up generation number {}", i); - let mut om = OpMeasurements::new(¤t.name, Operation::Backup(i)); - om.push(Measurement::TotalFiles(current.data.file_count()?)); - om.push(Measurement::TotalData(current.data.file_size()?)); - Ok(om) -} - -fn restore(i: usize, current: &CurrentBenchmark) -> Result { - info!("restoring generation number {}", i); - Ok(OpMeasurements::new(¤t.name, Operation::Restore(i))) -} - -struct CurrentBenchmark { - name: String, - data: Data, -} - -impl CurrentBenchmark { - fn new(name: &str, tempdir: &Path) -> Result { - Ok(Self { - name: name.to_string(), - data: Data::new(tempdir)?, - }) - } -} diff --git a/src/step.rs b/src/step.rs index dee07d3..fa7abd6 100644 --- a/src/step.rs +++ b/src/step.rs @@ -1,6 +1,6 @@ use crate::specification::{Change, Create, FileCount}; -/// A step in the execution of a benchmark. +/// A step specification in the execution of a benchmark. #[derive(Debug)] pub enum Step { /// Start a benchmark. diff --git a/src/suite.rs b/src/suite.rs new file mode 100644 index 0000000..4e35723 --- /dev/null +++ b/src/suite.rs @@ -0,0 +1,153 @@ +use crate::result::{Measurement, OpMeasurements, Operation}; +use crate::specification::{Create, FileCount}; +use crate::step::Step; +use log::{debug, info}; +use std::fs::File; +use std::path::PathBuf; +use std::time::Instant; +use tempfile::{tempdir, TempDir}; + +/// A running benchmark suite. +/// +/// This manages temporary data created for the benchmarks, and +/// executes individual steps in the suite. +pub struct Suite { + benchmark: Option, +} + +/// Possible errors from running a benchmark suite. +#[derive(Debug, thiserror::Error)] +pub enum SuiteError { + /// Error creating a temporary directory. + #[error(transparent)] + TempDir(#[from] std::io::Error), + + /// File creation failed. + #[error("Failed to create file {0}: {1}")] + CreateFile(PathBuf, std::io::Error), +} + +impl Suite { + pub fn new() -> Result { + Ok(Self { benchmark: None }) + } + + /// Execute one step in the benchmark suite. + /// + /// Return a measurement of the step. + pub fn execute(&mut self, step: &Step) -> Result { + debug!("executing step {:?}", step); + let time = Instant::now(); + let mut om = match step { + Step::Start(name) => { + assert!(self.benchmark.is_none()); + let mut benchmark = Benchmark::new(name)?; + let om = benchmark.start()?; + self.benchmark = Some(benchmark); + om + } + Step::Stop(name) => { + assert!(self.benchmark.is_some()); + assert_eq!(name, self.benchmark.as_ref().unwrap().name()); + let om = self.benchmark.as_mut().unwrap().stop()?; + self.benchmark = None; + om + } + Step::Create(x) => { + assert!(self.benchmark.is_some()); + self.benchmark.as_mut().unwrap().create(x)? + } + Step::Rename(x) => { + assert!(self.benchmark.is_some()); + self.benchmark.as_mut().unwrap().rename(x)? + } + Step::Delete(x) => { + assert!(self.benchmark.is_some()); + self.benchmark.as_mut().unwrap().delete(x)? + } + Step::Backup(x) => { + assert!(self.benchmark.is_some()); + self.benchmark.as_mut().unwrap().backup(*x)? + } + Step::Restore(x) => { + assert!(self.benchmark.is_some()); + self.benchmark.as_mut().unwrap().restore(*x)? + } + }; + + let t = std::time::Duration::from_millis(10); + std::thread::sleep(t); + + let ms = time.elapsed().as_millis(); + debug!("step duration was {} ms", ms); + om.push(Measurement::DurationMs(ms)); + Ok(om) + } +} + +struct Benchmark { + name: String, + tempdir: Option, +} + +impl Benchmark { + fn new(name: &str) -> Result { + Ok(Self { + name: name.to_string(), + tempdir: Some(tempdir()?), + }) + } + + fn name(&self) -> &str { + &self.name + } + + fn tempdir(&self) -> &TempDir { + self.tempdir.as_ref().unwrap() + } + + fn start(&mut self) -> Result { + info!("starting benchmark {}", self.name()); + Ok(OpMeasurements::new(self.name(), Operation::Start)) + } + + fn stop(&mut self) -> Result { + info!("ending benchmark {}", self.name); + self.tempdir.take().unwrap().close()?; + Ok(OpMeasurements::new(self.name(), Operation::Stop)) + } + + fn create(&mut self, create: &Create) -> Result { + info!("creating {} test data files", create.files); + let tempdir = self.tempdir().path(); + debug!("creating {} files in {}", create.files, tempdir.display()); + + for i in 0..create.files { + let filename = tempdir.join(format!("{}", i)); + debug!("creating {}", filename.display()); + File::create(&filename).map_err(|err| SuiteError::CreateFile(filename, err))?; + } + + Ok(OpMeasurements::new(self.name(), Operation::Create)) + } + + fn rename(&mut self, count: &FileCount) -> Result { + info!("renaming {} test data files", count.files); + Ok(OpMeasurements::new(self.name(), Operation::Rename)) + } + + fn delete(&mut self, count: &FileCount) -> Result { + info!("deleting {} test data files", count.files); + Ok(OpMeasurements::new(self.name(), Operation::Delete)) + } + + fn backup(&mut self, n: usize) -> Result { + info!("making backup {} in benchmark {}", n, self.name()); + Ok(OpMeasurements::new(self.name(), Operation::Backup)) + } + + fn restore(&mut self, n: usize) -> Result { + info!("restoring backup {} in benchmark {}", n, self.name()); + Ok(OpMeasurements::new(self.name(), Operation::Restore)) + } +} -- cgit v1.2.1