From 368e1a461e41a78568b276f9a013ad8516cfe90e Mon Sep 17 00:00:00 2001 From: hashmap Date: Thu, 1 Nov 2018 12:34:47 +0100 Subject: [PATCH] Tiny store crate cleanup (#1899) --- chain/src/txhashset/txhashset.rs | 2 +- store/src/leaf_set.rs | 27 ++++++++------------ store/src/lib.rs | 18 +++++++++++--- store/src/lmdb.rs | 8 +++--- store/src/pmmr.rs | 42 ++++++++++++++++---------------- store/src/prune_list.rs | 14 ++++------- store/src/types.rs | 30 +++++++++++------------ store/tests/utxo_set_perf.rs | 2 +- 8 files changed, 72 insertions(+), 71 deletions(-) diff --git a/chain/src/txhashset/txhashset.rs b/chain/src/txhashset/txhashset.rs index e0472df99..698239b55 100644 --- a/chain/src/txhashset/txhashset.rs +++ b/chain/src/txhashset/txhashset.rs @@ -63,7 +63,7 @@ impl HashOnlyMMRHandle { fn new(root_dir: &str, sub_dir: &str, file_name: &str) -> Result { let path = Path::new(root_dir).join(sub_dir).join(file_name); fs::create_dir_all(path.clone())?; - let backend = HashOnlyMMRBackend::new(path.to_str().unwrap().to_string())?; + let backend = HashOnlyMMRBackend::new(path.to_str().unwrap())?; let last_pos = backend.unpruned_size()?; Ok(HashOnlyMMRHandle { backend, last_pos }) } diff --git a/store/src/leaf_set.rs b/store/src/leaf_set.rs index c9221a191..b08d6aa60 100644 --- a/store/src/leaf_set.rs +++ b/store/src/leaf_set.rs @@ -23,10 +23,10 @@ use core::core::hash::Hashed; use core::core::pmmr; use core::core::BlockHeader; use prune_list::PruneList; -use save_via_temp_file; +use {read_bitmap, save_via_temp_file}; use std::fs::File; -use std::io::{self, BufWriter, Read, Write}; +use std::io::{self, BufWriter, Write}; /// Compact (roaring) bitmap representing the set of positions of /// leaves that are currently unpruned in the MMR. @@ -39,26 +39,23 @@ pub struct LeafSet { impl LeafSet { /// Open the remove log file. /// The content of the file will be read in memory for fast checking. - pub fn open(path: String) -> io::Result { + pub fn open(path: &str) -> io::Result { let file_path = Path::new(&path); let bitmap = if file_path.exists() { - let mut bitmap_file = File::open(path.clone())?; - let mut buffer = vec![]; - bitmap_file.read_to_end(&mut buffer)?; - Bitmap::deserialize(&buffer) + read_bitmap(file_path)? } else { Bitmap::create() }; Ok(LeafSet { - path: path.clone(), - bitmap: bitmap.clone(), + path: path.to_string(), bitmap_bak: bitmap.clone(), + bitmap, }) } /// Copies a snapshot of the utxo file into the primary utxo file. - pub fn copy_snapshot(path: String, cp_path: String) -> io::Result<()> { + pub fn copy_snapshot(path: &str, cp_path: &str) -> io::Result<()> { let cp_file_path = Path::new(&cp_path); if !cp_file_path.exists() { @@ -66,17 +63,13 @@ impl LeafSet { return Ok(()); } - let mut bitmap_file = File::open(cp_path.clone())?; - let mut buffer = vec![]; - bitmap_file.read_to_end(&mut buffer)?; - let bitmap = Bitmap::deserialize(&buffer); - + let bitmap = read_bitmap(cp_file_path)?; debug!("leaf_set: copying rewound file {} to {}", cp_path, path); let mut leaf_set = LeafSet { - path: path.clone(), - bitmap: bitmap.clone(), + path: path.to_string(), bitmap_bak: bitmap.clone(), + bitmap, }; leaf_set.flush()?; diff --git a/store/src/lib.rs b/store/src/lib.rs index 881a1b152..58026dc2d 100644 --- a/store/src/lib.rs +++ b/store/src/lib.rs @@ -32,7 +32,6 @@ extern crate log; extern crate failure; #[macro_use] extern crate failure_derive; - #[macro_use] extern crate grin_core as core; extern crate grin_util as util; @@ -43,7 +42,7 @@ pub mod pmmr; pub mod prune_list; pub mod types; -const SEP: u8 = ':' as u8; +const SEP: u8 = b':'; use byteorder::{BigEndian, WriteBytesExt}; @@ -68,7 +67,7 @@ pub fn to_key_u64(prefix: u8, k: &mut Vec, val: u64) -> Vec { res } /// Build a db key from a prefix and a numeric identifier. -pub fn u64_to_key<'a>(prefix: u8, val: u64) -> Vec { +pub fn u64_to_key(prefix: u8, val: u64) -> Vec { let mut u64_vec = vec![]; u64_vec.write_u64::(val).unwrap(); u64_vec.insert(0, SEP); @@ -111,3 +110,16 @@ where Ok(()) } + +use croaring::Bitmap; +use std::io::{self, Read}; +use std::path::Path; +/// Read Bitmap from a file +pub fn read_bitmap>(file_path: P) -> io::Result { + use std::fs::File; + let mut bitmap_file = File::open(file_path)?; + let f_md = bitmap_file.metadata()?; + let mut buffer = Vec::with_capacity(f_md.len() as usize); + bitmap_file.read_to_end(&mut buffer)?; + Ok(Bitmap::deserialize(&buffer)) +} diff --git a/store/src/lmdb.rs b/store/src/lmdb.rs index 8c26ae304..ce47f1c9a 100644 --- a/store/src/lmdb.rs +++ b/store/src/lmdb.rs @@ -148,11 +148,11 @@ impl Store { /// Produces an iterator of `Readable` types moving forward from the /// provided key. pub fn iter(&self, from: &[u8]) -> Result, Error> { - let txn = Arc::new(lmdb::ReadTransaction::new(self.env.clone())?); - let cursor = Arc::new(txn.cursor(self.db.clone()).unwrap()); + let tx = Arc::new(lmdb::ReadTransaction::new(self.env.clone())?); + let cursor = Arc::new(tx.cursor(self.db.clone()).unwrap()); Ok(SerIterator { - tx: txn, - cursor: cursor, + tx, + cursor, seek: false, prefix: from.to_vec(), _marker: marker::PhantomData, diff --git a/store/src/pmmr.rs b/store/src/pmmr.rs index 2946092fe..31a2772e1 100644 --- a/store/src/pmmr.rs +++ b/store/src/pmmr.rs @@ -25,10 +25,10 @@ use leaf_set::LeafSet; use prune_list::PruneList; use types::{prune_noop, AppendOnlyFile, HashFile}; -const PMMR_HASH_FILE: &'static str = "pmmr_hash.bin"; -const PMMR_DATA_FILE: &'static str = "pmmr_data.bin"; -const PMMR_LEAF_FILE: &'static str = "pmmr_leaf.bin"; -const PMMR_PRUN_FILE: &'static str = "pmmr_prun.bin"; +const PMMR_HASH_FILE: &str = "pmmr_hash.bin"; +const PMMR_DATA_FILE: &str = "pmmr_data.bin"; +const PMMR_LEAF_FILE: &str = "pmmr_leaf.bin"; +const PMMR_PRUN_FILE: &str = "pmmr_prun.bin"; /// The list of PMMR_Files for internal purposes pub const PMMR_FILES: [&str; 4] = [ @@ -77,7 +77,7 @@ where self.leaf_set.add(position); } self.data_file.append(&mut ser::ser_vec(&data).unwrap()); - for ref h in hashes { + for h in &hashes { self.hash_file.append(&mut ser::ser_vec(h).unwrap()); } Ok(()) @@ -105,7 +105,7 @@ where "Corrupted storage, could not read an entry from hash store: {:?}", e ); - return None; + None } } } @@ -128,7 +128,7 @@ where "Corrupted storage, could not read an entry from data store: {:?}", e ); - return None; + None } } } @@ -220,8 +220,8 @@ where prunable: bool, header: Option<&BlockHeader>, ) -> io::Result> { - let hash_file = AppendOnlyFile::open(format!("{}/{}", data_dir, PMMR_HASH_FILE))?; - let data_file = AppendOnlyFile::open(format!("{}/{}", data_dir, PMMR_DATA_FILE))?; + let hash_file = AppendOnlyFile::open(&format!("{}/{}", data_dir, PMMR_HASH_FILE))?; + let data_file = AppendOnlyFile::open(&format!("{}/{}", data_dir, PMMR_DATA_FILE))?; let leaf_set_path = format!("{}/{}", data_dir, PMMR_LEAF_FILE); @@ -229,11 +229,11 @@ where // place so we use it. if let Some(header) = header { let leaf_snapshot_path = format!("{}/{}.{}", data_dir, PMMR_LEAF_FILE, header.hash()); - LeafSet::copy_snapshot(leaf_set_path.clone(), leaf_snapshot_path.clone())?; + LeafSet::copy_snapshot(&leaf_set_path, &leaf_snapshot_path)?; } - let leaf_set = LeafSet::open(leaf_set_path.clone())?; - let prune_list = PruneList::open(format!("{}/{}", data_dir, PMMR_PRUN_FILE))?; + let leaf_set = LeafSet::open(&leaf_set_path)?; + let prune_list = PruneList::open(&format!("{}/{}", data_dir, PMMR_PRUN_FILE))?; Ok(PMMRBackend { data_dir, @@ -357,7 +357,7 @@ where self.hash_file.save_prune( tmp_prune_file_hash.clone(), - off_to_rm, + &off_to_rm, record_len, &prune_noop, )?; @@ -381,7 +381,7 @@ where self.data_file.save_prune( tmp_prune_file_data.clone(), - off_to_rm, + &off_to_rm, record_len, prune_cb, )?; @@ -400,14 +400,14 @@ where tmp_prune_file_hash.clone(), format!("{}/{}", self.data_dir, PMMR_HASH_FILE), )?; - self.hash_file = AppendOnlyFile::open(format!("{}/{}", self.data_dir, PMMR_HASH_FILE))?; + self.hash_file = AppendOnlyFile::open(&format!("{}/{}", self.data_dir, PMMR_HASH_FILE))?; // 5. Rename the compact copy of the data file and reopen it. fs::rename( tmp_prune_file_data.clone(), format!("{}/{}", self.data_dir, PMMR_DATA_FILE), )?; - self.data_file = AppendOnlyFile::open(format!("{}/{}", self.data_dir, PMMR_DATA_FILE))?; + self.data_file = AppendOnlyFile::open(&format!("{}/{}", self.data_dir, PMMR_DATA_FILE))?; // 6. Write the leaf_set to disk. // Optimize the bitmap storage in the process. @@ -445,7 +445,7 @@ where } } } - (leaf_pos_to_rm, removed_excl_roots(expanded)) + (leaf_pos_to_rm, removed_excl_roots(&expanded)) } } @@ -457,7 +457,7 @@ pub struct HashOnlyMMRBackend { impl HashOnlyBackend for HashOnlyMMRBackend { fn append(&mut self, hashes: Vec) -> Result<(), String> { - for ref h in hashes { + for h in &hashes { self.hash_file .append(h) .map_err(|e| format!("Failed to append to backend, {:?}", e))?; @@ -480,8 +480,8 @@ impl HashOnlyBackend for HashOnlyMMRBackend { impl HashOnlyMMRBackend { /// Instantiates a new PMMR backend. /// Use the provided dir to store its files. - pub fn new(data_dir: String) -> io::Result { - let hash_file = HashFile::open(format!("{}/{}", data_dir, PMMR_HASH_FILE))?; + pub fn new(data_dir: &str) -> io::Result { + let hash_file = HashFile::open(&format!("{}/{}", data_dir, PMMR_HASH_FILE))?; Ok(HashOnlyMMRBackend { hash_file }) } @@ -510,7 +510,7 @@ impl HashOnlyMMRBackend { /// Filter remove list to exclude roots. /// We want to keep roots around so we have hashes for Merkle proofs. -fn removed_excl_roots(removed: Bitmap) -> Bitmap { +fn removed_excl_roots(removed: &Bitmap) -> Bitmap { removed .iter() .filter(|pos| { diff --git a/store/src/prune_list.rs b/store/src/prune_list.rs index 443a35d01..624160f94 100644 --- a/store/src/prune_list.rs +++ b/store/src/prune_list.rs @@ -21,14 +21,13 @@ //! must be shifted the appropriate amount when reading from the hash and data //! files. -use std::fs::File; -use std::io::{self, BufWriter, Read, Write}; +use std::io::{self, BufWriter, Write}; use std::path::Path; use croaring::Bitmap; use core::core::pmmr::{bintree_postorder_height, family, path}; -use save_via_temp_file; +use {read_bitmap, save_via_temp_file}; /// Maintains a list of previously pruned nodes in PMMR, compacting the list as /// parents get pruned and allowing checking whether a leaf is pruned. Given @@ -64,19 +63,16 @@ impl PruneList { } /// Open an existing prune_list or create a new one. - pub fn open(path: String) -> io::Result { + pub fn open(path: &str) -> io::Result { let file_path = Path::new(&path); let bitmap = if file_path.exists() { - let mut bitmap_file = File::open(path.clone())?; - let mut buffer = vec![]; - bitmap_file.read_to_end(&mut buffer)?; - Bitmap::deserialize(&buffer) + read_bitmap(&file_path)? } else { Bitmap::create() }; let mut prune_list = PruneList { - path: Some(path.clone()), + path: Some(path.to_string()), bitmap, pruned_cache: Bitmap::create(), shift_cache: vec![], diff --git a/store/src/types.rs b/store/src/types.rs index 97b5da487..985b0dcea 100644 --- a/store/src/types.rs +++ b/store/src/types.rs @@ -38,7 +38,7 @@ pub struct HashFile { impl HashFile { /// Open (or create) a hash file at the provided path on disk. - pub fn open(path: String) -> io::Result { + pub fn open(path: &str) -> io::Result { let file = AppendOnlyFile::open(path)?; Ok(HashFile { file }) } @@ -67,7 +67,7 @@ impl HashFile { "Corrupted storage, could not read an entry from hash file: {:?}", e ); - return None; + None } } } @@ -113,15 +113,15 @@ pub struct AppendOnlyFile { impl AppendOnlyFile { /// Open a file (existing or not) as append-only, backed by a mmap. - pub fn open(path: String) -> io::Result { + pub fn open(path: &str) -> io::Result { let file = OpenOptions::new() .read(true) .append(true) .create(true) - .open(path.clone())?; + .open(&path)?; let mut aof = AppendOnlyFile { - path: path.clone(), - file: file, + file, + path: path.to_string(), mmap: None, buffer_start: 0, buffer: vec![], @@ -216,7 +216,7 @@ impl AppendOnlyFile { let buffer_offset = offset - self.buffer_start; return self.read_from_buffer(buffer_offset, length); } - if let None = self.mmap { + if self.mmap.is_none() { return vec![]; } let mmap = self.mmap.as_ref().unwrap(); @@ -255,7 +255,7 @@ impl AppendOnlyFile { pub fn save_prune( &self, target: String, - prune_offs: Vec, + prune_offs: &[u64], prune_len: u64, prune_cb: T, ) -> io::Result<()> @@ -263,11 +263,11 @@ impl AppendOnlyFile { T: Fn(&[u8]), { if prune_offs.is_empty() { - fs::copy(self.path.clone(), target.clone())?; + fs::copy(&self.path, &target)?; Ok(()) } else { - let mut reader = File::open(self.path.clone())?; - let mut writer = BufWriter::new(File::create(target.clone())?); + let mut reader = File::open(&self.path)?; + let mut writer = BufWriter::new(File::create(&target)?); // align the buffer on prune_len to avoid misalignments let mut buf = vec![0; (prune_len * 256) as usize]; @@ -299,7 +299,7 @@ impl AppendOnlyFile { break; } } - writer.write_all(&mut buf[buf_start..(len as usize)])?; + writer.write_all(&buf[buf_start..(len as usize)])?; read += len; } } @@ -322,14 +322,14 @@ impl AppendOnlyFile { } /// Read an ordered vector of scalars from a file. -pub fn read_ordered_vec(path: String, elmt_len: usize) -> io::Result> +pub fn read_ordered_vec(path: &str, elmt_len: usize) -> io::Result> where T: ser::Readable + cmp::Ord, { let file_path = Path::new(&path); let mut ovec = Vec::with_capacity(1000); if file_path.exists() { - let mut file = BufReader::with_capacity(elmt_len * 1000, File::open(path.clone())?); + let mut file = BufReader::with_capacity(elmt_len * 1000, File::open(&path)?); loop { // need a block to end mutable borrow before consume let buf_len = { @@ -360,7 +360,7 @@ where } /// Writes an ordered vector to a file -pub fn write_vec(path: String, v: &Vec) -> io::Result<()> +pub fn write_vec(path: &str, v: &Vec) -> io::Result<()> where T: ser::Writeable, { diff --git a/store/tests/utxo_set_perf.rs b/store/tests/utxo_set_perf.rs index 2b0b1b578..d6fcfaf54 100644 --- a/store/tests/utxo_set_perf.rs +++ b/store/tests/utxo_set_perf.rs @@ -101,7 +101,7 @@ fn setup(test_name: &str) -> (LeafSet, String) { let _ = env_logger::init(); let data_dir = format!("./target/{}-{}", test_name, Utc::now().timestamp()); fs::create_dir_all(data_dir.clone()).unwrap(); - let leaf_set = LeafSet::open(format!("{}/{}", data_dir, "utxo.bin")).unwrap(); + let leaf_set = LeafSet::open(&format!("{}/{}", data_dir, "utxo.bin")).unwrap(); (leaf_set, data_dir) }