From a7c96c0e1ec04886c8f68018404cfdd5633ffc10 Mon Sep 17 00:00:00 2001 From: Joe Thornber Date: Tue, 26 Aug 2014 11:14:49 +0100 Subject: [PATCH] [everything] Fix circular shared pointer references. We had a cycle from transaction_manager <-> space_map, and also from the ref_counters back up to the tm. This prevented objects being destroyed when various programs exited. From now on we'll try and only use a shared ptr if ownership is implied. Otherwise a reference will be used (eg, for up pointers). --- caching/cache_check.cc | 6 +- caching/hint_array.cc | 12 +- caching/hint_array.h | 5 +- caching/metadata.cc | 14 +- era/era_check.cc | 4 +- era/metadata.cc | 10 +- era/restore_emitter.cc | 2 +- era/writeset_tree.cc | 2 +- persistent-data/data-structures/array.h | 22 +- persistent-data/data-structures/bitset.cc | 8 +- persistent-data/data-structures/bitset.h | 6 +- .../data-structures/bloom_filter.cc | 4 +- .../data-structures/bloom_filter.h | 7 +- persistent-data/data-structures/btree.h | 14 +- persistent-data/data-structures/btree.tcc | 28 +- persistent-data/space-maps/disk.cc | 46 +- persistent-data/space-maps/disk.h | 8 +- thin-provisioning/metadata.cc | 48 +-- thin-provisioning/metadata_dumper.cc | 2 +- thin-provisioning/restore_emitter.cc | 2 +- thin-provisioning/thin_check.cc | 6 +- thin-provisioning/thin_delta.cc | 6 +- thin-provisioning/thin_pool.cc | 4 +- thin-provisioning/thin_rmap.cc | 2 +- unit-tests/array_t.cc | 16 +- unit-tests/bitset_t.cc | 52 ++- unit-tests/bloom_filter_t.cc | 4 +- unit-tests/btree_counter_t.cc | 4 +- unit-tests/btree_damage_visitor_t.cc | 4 +- unit-tests/btree_t.cc | 41 +- unit-tests/space_map_t.cc | 408 +++++++++--------- 31 files changed, 391 insertions(+), 406 deletions(-) diff --git a/caching/cache_check.cc b/caching/cache_check.cc index d5ebc19..593ba4e 100644 --- a/caching/cache_check.cc +++ b/caching/cache_check.cc @@ -237,7 +237,7 @@ namespace { out << "examining mapping array" << end_message(); { nested_output::nest _ = out.push(); - mapping_array ma(tm, mapping_array::ref_counter(), sb.mapping_root, sb.cache_blocks); + mapping_array ma(*tm, mapping_array::ref_counter(), sb.mapping_root, sb.cache_blocks); check_mapping_array(ma, mapping_rep); } } @@ -250,7 +250,7 @@ namespace { out << "examining hint array" << end_message(); { nested_output::nest _ = out.push(); - hint_array ha(tm, sb.policy_hint_size, sb.hint_root, sb.cache_blocks); + hint_array ha(*tm, sb.policy_hint_size, sb.hint_root, sb.cache_blocks); ha.check(hint_rep); } } @@ -264,7 +264,7 @@ namespace { out << "examining discard bitset" << end_message(); { nested_output::nest _ = out.push(); - persistent_data::bitset discards(tm, sb.discard_root, sb.discard_nr_blocks); + persistent_data::bitset discards(*tm, sb.discard_root, sb.discard_nr_blocks); } } } diff --git a/caching/hint_array.cc b/caching/hint_array.cc index 037a879..cb9f2f1 100644 --- a/caching/hint_array.cc +++ b/caching/hint_array.cc @@ -38,7 +38,7 @@ namespace { xx(4); template - shared_ptr mk_array(transaction_manager::ptr tm) { + shared_ptr mk_array(transaction_manager &tm) { typedef hint_traits traits; typedef array ha; @@ -47,7 +47,7 @@ namespace { return r; } - shared_ptr mk_array(transaction_manager::ptr tm, uint32_t width) { + shared_ptr mk_array(transaction_manager &tm, uint32_t width) { switch (width) { #define xx(n) case n: return mk_array(tm) @@ -76,7 +76,7 @@ namespace { //-------------------------------- template - shared_ptr mk_array(transaction_manager::ptr tm, block_address root, unsigned nr_entries) { + shared_ptr mk_array(transaction_manager &tm, block_address root, unsigned nr_entries) { typedef hint_traits traits; typedef array ha; @@ -85,7 +85,7 @@ namespace { return r; } - shared_ptr mk_array(transaction_manager::ptr tm, uint32_t width, block_address root, unsigned nr_entries) { + shared_ptr mk_array(transaction_manager &tm, uint32_t width, block_address root, unsigned nr_entries) { switch (width) { #define xx(n) case n: return mk_array(tm, root, nr_entries) all_widths @@ -230,13 +230,13 @@ missing_hints::visit(damage_visitor &v) const //---------------------------------------------------------------- -hint_array::hint_array(tm_ptr tm, unsigned width) +hint_array::hint_array(transaction_manager &tm, unsigned width) : width_(check_width(width)), impl_(mk_array(tm, width)) { } -hint_array::hint_array(hint_array::tm_ptr tm, unsigned width, +hint_array::hint_array(transaction_manager &tm, unsigned width, block_address root, unsigned nr_entries) : width_(check_width(width)), impl_(mk_array(tm, width, root, nr_entries)) diff --git a/caching/hint_array.h b/caching/hint_array.h index 6e8121b..45430cc 100644 --- a/caching/hint_array.h +++ b/caching/hint_array.h @@ -56,10 +56,9 @@ namespace caching { class hint_array { public: typedef boost::shared_ptr ptr; - typedef persistent_data::transaction_manager::ptr tm_ptr; - hint_array(tm_ptr tm, unsigned width); - hint_array(tm_ptr tm, unsigned width, block_address root, unsigned nr_entries); + hint_array(transaction_manager &tm, unsigned width); + hint_array(transaction_manager &tm, unsigned width, block_address root, unsigned nr_entries); unsigned get_nr_entries() const; diff --git a/caching/metadata.cc b/caching/metadata.cc index 0a246f9..2368a3f 100644 --- a/caching/metadata.cc +++ b/caching/metadata.cc @@ -61,7 +61,7 @@ metadata::setup_hint_array(size_t width) { if (width > 0) hints_ = hint_array::ptr( - new hint_array(tm_, width)); + new hint_array(*tm_, width)); } void @@ -70,16 +70,16 @@ metadata::create_metadata(block_manager<>::ptr bm) tm_ = open_tm(bm); space_map::ptr core = tm_->get_sm(); - metadata_sm_ = create_metadata_sm(tm_, tm_->get_bm()->get_nr_blocks()); + metadata_sm_ = create_metadata_sm(*tm_, tm_->get_bm()->get_nr_blocks()); copy_space_maps(metadata_sm_, core); tm_->set_sm(metadata_sm_); - mappings_ = mapping_array::ptr(new mapping_array(tm_, mapping_array::ref_counter())); + mappings_ = mapping_array::ptr(new mapping_array(*tm_, mapping_array::ref_counter())); // We can't instantiate the hint array yet, since we don't know the // hint width. - discard_bits_ = persistent_data::bitset::ptr(new persistent_data::bitset(tm_)); + discard_bits_ = persistent_data::bitset::ptr(new persistent_data::bitset(*tm_)); } void @@ -89,19 +89,19 @@ metadata::open_metadata(block_manager<>::ptr bm) sb_ = read_superblock(tm_->get_bm()); mappings_ = mapping_array::ptr( - new mapping_array(tm_, + new mapping_array(*tm_, mapping_array::ref_counter(), sb_.mapping_root, sb_.cache_blocks)); if (sb_.hint_root) hints_ = hint_array::ptr( - new hint_array(tm_, sb_.policy_hint_size, + new hint_array(*tm_, sb_.policy_hint_size, sb_.hint_root, sb_.cache_blocks)); if (sb_.discard_root) discard_bits_ = persistent_data::bitset::ptr( - new persistent_data::bitset(tm_, sb_.discard_root, sb_.discard_nr_blocks)); + new persistent_data::bitset(*tm_, sb_.discard_root, sb_.discard_nr_blocks)); } void diff --git a/era/era_check.cc b/era/era_check.cc index c4008c2..25c4d20 100644 --- a/era/era_check.cc +++ b/era/era_check.cc @@ -217,14 +217,14 @@ namespace { writeset_tree_reporter wt_rep(out); { era_detail_traits::ref_counter rc(tm); - writeset_tree wt(tm, sb.writeset_tree_root, rc); + writeset_tree wt(*tm, sb.writeset_tree_root, rc); check_writeset_tree(tm, wt, wt_rep); } era_array_reporter ea_rep(out); { uint32_traits::ref_counter rc; - era_array ea(tm, rc, sb.era_array_root, sb.nr_blocks); + era_array ea(*tm, rc, sb.era_array_root, sb.nr_blocks); check_era_array(ea, sb.current_era, ea_rep); } diff --git a/era/metadata.cc b/era/metadata.cc index 2b2d072..dd4fe7a 100644 --- a/era/metadata.cc +++ b/era/metadata.cc @@ -51,12 +51,12 @@ metadata::create_metadata(block_manager<>::ptr bm) tm_ = open_tm(bm); space_map::ptr core = tm_->get_sm(); - metadata_sm_ = create_metadata_sm(tm_, tm_->get_bm()->get_nr_blocks()); + metadata_sm_ = create_metadata_sm(*tm_, tm_->get_bm()->get_nr_blocks()); copy_space_maps(metadata_sm_, core); tm_->set_sm(metadata_sm_); - writeset_tree_ = writeset_tree::ptr(new writeset_tree(tm_, era_detail_traits::ref_counter(tm_))); - era_array_ = era_array::ptr(new era_array(tm_, + writeset_tree_ = writeset_tree::ptr(new writeset_tree(*tm_, era_detail_traits::ref_counter(tm_))); + era_array_ = era_array::ptr(new era_array(*tm_, uint32_traits::ref_counter())); } @@ -66,11 +66,11 @@ metadata::open_metadata(block_manager<>::ptr bm, block_address loc) tm_ = open_tm(bm); sb_ = read_superblock(tm_->get_bm(), loc); - writeset_tree_ = writeset_tree::ptr(new writeset_tree(tm_, + writeset_tree_ = writeset_tree::ptr(new writeset_tree(*tm_, sb_.writeset_tree_root, era_detail_traits::ref_counter(tm_))); - era_array_ = era_array::ptr(new era_array(tm_, + era_array_ = era_array::ptr(new era_array(*tm_, uint32_traits::ref_counter(), sb_.era_array_root, sb_.nr_blocks)); diff --git a/era/restore_emitter.cc b/era/restore_emitter.cc index c3465c0..a5e714e 100644 --- a/era/restore_emitter.cc +++ b/era/restore_emitter.cc @@ -52,7 +52,7 @@ namespace { in_writeset_ = true; era_ = era; - bits_.reset(new bitset(md_.tm_)); + bits_.reset(new bitset(*md_.tm_)); bits_->grow(nr_bits, false); } diff --git a/era/writeset_tree.cc b/era/writeset_tree.cc index 4e2c478..62f5fc3 100644 --- a/era/writeset_tree.cc +++ b/era/writeset_tree.cc @@ -54,7 +54,7 @@ namespace { void visit(btree_path const &path, era_detail const &era) { era_ = path[0]; - persistent_data::bitset bs(tm_, era.writeset_root, era.nr_bits); + persistent_data::bitset bs(*tm_, era.writeset_root, era.nr_bits); writeset_v_.writeset_begin(era_, era.nr_bits); bs.walk_bitset(*this); writeset_v_.writeset_end(); diff --git a/persistent-data/data-structures/array.h b/persistent-data/data-structures/array.h index cf8ad4e..c03daaa 100644 --- a/persistent-data/data-structures/array.h +++ b/persistent-data/data-structures/array.h @@ -172,7 +172,7 @@ namespace persistent_data { unsigned visit_array_block(ValueVisitor &vv, btree_path const &p, typename block_traits::value_type const &v) const { - rblock rb(tm_->read_lock(v, validator_), rc_); + rblock rb(tm_.read_lock(v, validator_), rc_); for (uint32_t i = 0; i < rb.nr_entries(); i++) vv.visit(p[0] * rb.max_entries() + i, rb.get(i)); @@ -207,8 +207,6 @@ namespace persistent_data { unsigned entries_per_block_; }; - typedef typename persistent_data::transaction_manager::ptr tm_ptr; - typedef block_manager<>::write_ref write_ref; typedef block_manager<>::read_ref read_ref; @@ -219,23 +217,23 @@ namespace persistent_data { typedef typename ValueTraits::value_type value_type; typedef typename ValueTraits::ref_counter ref_counter; - array(tm_ptr tm, ref_counter rc) + array(transaction_manager &tm, ref_counter rc) : tm_(tm), entries_per_block_(rblock::calc_max_entries()), nr_entries_(0), - block_rc_(tm->get_sm(), *this), + block_rc_(tm.get_sm(), *this), block_tree_(tm, block_rc_), rc_(rc), validator_(new array_detail::array_block_validator) { } - array(tm_ptr tm, ref_counter rc, + array(transaction_manager &tm, ref_counter rc, block_address root, unsigned nr_entries) : tm_(tm), entries_per_block_(rblock::calc_max_entries()), nr_entries_(nr_entries), - block_rc_(tm->get_sm(), *this), + block_rc_(tm.get_sm(), *this), block_tree_(tm, root, block_rc_), rc_(rc), validator_(new array_detail::array_block_validator) { @@ -393,7 +391,7 @@ namespace persistent_data { wblock new_ablock(unsigned ablock_index) { uint64_t key[1] = {ablock_index}; - write_ref b = tm_->new_block(validator_); + write_ref b = tm_.new_block(validator_); block_address location = b.get_location(); wblock wb(b, rc_); @@ -404,13 +402,13 @@ namespace persistent_data { rblock get_ablock(unsigned ablock_index) const { block_address addr = lookup_block_address(ablock_index); - return rblock(tm_->read_lock(addr, validator_), rc_); + return rblock(tm_.read_lock(addr, validator_), rc_); } wblock shadow_ablock(unsigned ablock_index) { uint64_t key[1] = {ablock_index}; block_address addr = lookup_block_address(ablock_index); - std::pair p = tm_->shadow(addr, validator_); + std::pair p = tm_.shadow(addr, validator_); wblock wb = wblock(p.first, rc_); if (p.second) @@ -422,11 +420,11 @@ namespace persistent_data { } void dec_ablock_entries(block_address addr) { - rblock b(tm_->read_lock(addr, validator_), rc_); + rblock b(tm_.read_lock(addr, validator_), rc_); b.dec_all_entries(); } - tm_ptr tm_; + transaction_manager &tm_; unsigned entries_per_block_; unsigned nr_entries_; block_ref_counter block_rc_; diff --git a/persistent-data/data-structures/bitset.cc b/persistent-data/data-structures/bitset.cc index e49e19f..98805fa 100644 --- a/persistent-data/data-structures/bitset.cc +++ b/persistent-data/data-structures/bitset.cc @@ -33,12 +33,12 @@ namespace persistent_data { typedef boost::shared_ptr ptr; typedef persistent_data::transaction_manager::ptr tm_ptr; - bitset_impl(tm_ptr tm) + bitset_impl(transaction_manager &tm) : nr_bits_(0), array_(tm, rc_) { } - bitset_impl(tm_ptr tm, block_address root, unsigned nr_bits) + bitset_impl(transaction_manager &tm, block_address root, unsigned nr_bits) : nr_bits_(nr_bits), array_(tm, rc_, root, nr_bits / BITS_PER_ULL) { } @@ -203,12 +203,12 @@ namespace persistent_data { //---------------------------------------------------------------- -persistent_data::bitset::bitset(tm_ptr tm) +persistent_data::bitset::bitset(transaction_manager &tm) : impl_(new bitset_impl(tm)) { } -persistent_data::bitset::bitset(tm_ptr tm, block_address root, unsigned nr_bits) +persistent_data::bitset::bitset(transaction_manager &tm, block_address root, unsigned nr_bits) : impl_(new bitset_impl(tm, root, nr_bits)) { } diff --git a/persistent-data/data-structures/bitset.h b/persistent-data/data-structures/bitset.h index 70688aa..3b69cb9 100644 --- a/persistent-data/data-structures/bitset.h +++ b/persistent-data/data-structures/bitset.h @@ -49,10 +49,10 @@ namespace persistent_data { class bitset { public: typedef boost::shared_ptr ptr; - typedef persistent_data::transaction_manager::ptr tm_ptr; - bitset(tm_ptr tm); - bitset(tm_ptr tm, block_address root, unsigned nr_bits); + bitset(transaction_manager &tm); + bitset(transaction_manager &tm, + block_address root, unsigned nr_bits); block_address get_root() const; unsigned get_nr_bits() const; void grow(unsigned new_nr_bits, bool default_value); diff --git a/persistent-data/data-structures/bloom_filter.cc b/persistent-data/data-structures/bloom_filter.cc index 3ca6ffb..a250835 100644 --- a/persistent-data/data-structures/bloom_filter.cc +++ b/persistent-data/data-structures/bloom_filter.cc @@ -34,7 +34,7 @@ namespace { //---------------------------------------------------------------- -bloom_filter::bloom_filter(tm_ptr tm, +bloom_filter::bloom_filter(transaction_manager &tm, unsigned nr_bits, unsigned nr_probes) : tm_(tm), bits_(tm), @@ -45,7 +45,7 @@ bloom_filter::bloom_filter(tm_ptr tm, bits_.grow(nr_bits, false); } -bloom_filter::bloom_filter(tm_ptr tm, block_address root, +bloom_filter::bloom_filter(transaction_manager &tm, block_address root, unsigned nr_bits, unsigned nr_probes) : tm_(tm), bits_(tm, root, nr_bits), diff --git a/persistent-data/data-structures/bloom_filter.h b/persistent-data/data-structures/bloom_filter.h index da91088..6407878 100644 --- a/persistent-data/data-structures/bloom_filter.h +++ b/persistent-data/data-structures/bloom_filter.h @@ -12,13 +12,12 @@ namespace persistent_data { class bloom_filter { public: typedef boost::shared_ptr ptr; - typedef persistent_data::transaction_manager::ptr tm_ptr; // nr_bits must be a power of two - bloom_filter(tm_ptr tm, + bloom_filter(transaction_manager &tm, unsigned nr_bits, unsigned nr_probes); - bloom_filter(tm_ptr tm, block_address root, + bloom_filter(transaction_manager &tm, block_address root, unsigned nr_bits_power, unsigned nr_probes); block_address get_root() const; @@ -34,7 +33,7 @@ namespace persistent_data { void fill_probes(block_address b, vector &probes) const; - tm_ptr tm_; + transaction_manager &tm_; persistent_data::bitset bits_; unsigned nr_probes_; uint64_t mask_; diff --git a/persistent-data/data-structures/btree.h b/persistent-data/data-structures/btree.h index 9c9c1a2..f4130c7 100644 --- a/persistent-data/data-structures/btree.h +++ b/persistent-data/data-structures/btree.h @@ -198,7 +198,7 @@ namespace persistent_data { class ro_spine : private boost::noncopyable { public: - ro_spine(transaction_manager::ptr tm, + ro_spine(transaction_manager &tm, bcache::validator::ptr v) : tm_(tm), validator_(v) { @@ -212,7 +212,7 @@ namespace persistent_data { } private: - transaction_manager::ptr tm_; + transaction_manager &tm_; bcache::validator::ptr validator_; std::list::read_ref> spine_; }; @@ -223,7 +223,7 @@ namespace persistent_data { typedef transaction_manager::write_ref write_ref; typedef boost::optional maybe_block; - shadow_spine(transaction_manager::ptr tm, + shadow_spine(transaction_manager &tm, bcache::validator::ptr v) : tm_(tm), @@ -276,7 +276,7 @@ namespace persistent_data { } private: - transaction_manager::ptr tm_; + transaction_manager &tm_; bcache::validator::ptr validator_; std::list::write_ref> spine_; maybe_block root_; @@ -335,10 +335,10 @@ namespace persistent_data { typedef typename btree_detail::node_ref leaf_node; typedef typename btree_detail::node_ref internal_node; - btree(typename persistent_data::transaction_manager::ptr tm, + btree(transaction_manager &tm, typename ValueTraits::ref_counter rc); - btree(typename transaction_manager::ptr tm, + btree(transaction_manager &tm, block_address root, typename ValueTraits::ref_counter rc); @@ -434,7 +434,7 @@ namespace persistent_data { void inc_children(btree_detail::shadow_spine &spine, RefCounter &leaf_rc); - typename persistent_data::transaction_manager::ptr tm_; + transaction_manager &tm_; bool destroy_; block_address root_; block_ref_counter internal_rc_; diff --git a/persistent-data/data-structures/btree.tcc b/persistent-data/data-structures/btree.tcc index 9034aca..ef03013 100644 --- a/persistent-data/data-structures/btree.tcc +++ b/persistent-data/data-structures/btree.tcc @@ -64,7 +64,7 @@ namespace persistent_data { inline void ro_spine::step(block_address b) { - spine_.push_back(tm_->read_lock(b, validator_)); + spine_.push_back(tm_.read_lock(b, validator_)); if (spine_.size() > 2) spine_.pop_front(); } @@ -72,11 +72,11 @@ namespace persistent_data { inline bool shadow_spine::step(block_address b) { - pair p = tm_->shadow(b, validator_); + pair p = tm_.shadow(b, validator_); try { step(p.first); } catch (...) { - tm_->get_sm()->dec(p.first.get_location()); + tm_.get_sm()->dec(p.first.get_location()); throw; } return p.second; @@ -392,17 +392,17 @@ namespace persistent_data { template btree:: - btree(typename transaction_manager::ptr tm, + btree(transaction_manager &tm, typename ValueTraits::ref_counter rc) : tm_(tm), destroy_(false), - internal_rc_(tm->get_sm()), + internal_rc_(tm.get_sm()), rc_(rc), validator_(new btree_node_validator) { using namespace btree_detail; - write_ref root = tm_->new_block(validator_); + write_ref root = tm_.new_block(validator_); if (Levels > 1) { internal_node n = to_node(root); @@ -424,13 +424,13 @@ namespace persistent_data { template btree:: - btree(typename transaction_manager::ptr tm, + btree(transaction_manager &tm, block_address root, typename ValueTraits::ref_counter rc) : tm_(tm), destroy_(false), root_(root), - internal_rc_(tm->get_sm()), + internal_rc_(tm.get_sm()), rc_(rc), validator_(new btree_node_validator) { @@ -559,7 +559,7 @@ namespace persistent_data { typename btree::ptr btree::clone() const { - tm_->get_sm()->inc(root_); + tm_.get_sm()->inc(root_); return ptr(new btree(tm_, root_, rc_)); } @@ -635,13 +635,13 @@ namespace persistent_data { node_type type; unsigned nr_left, nr_right; - write_ref left = tm_->new_block(validator_); + write_ref left = tm_.new_block(validator_); node_ref l = to_node(left); l.set_nr_entries(0); l.set_max_entries(); l.set_value_size(sizeof(typename ValueTraits::disk_type)); - write_ref right = tm_->new_block(validator_); + write_ref right = tm_.new_block(validator_); node_ref r = to_node(right); r.set_nr_entries(0); r.set_max_entries(); @@ -695,7 +695,7 @@ namespace persistent_data { node_ref l = spine.template get_node(); block_address left = spine.get_block(); - write_ref right = tm_->new_block(validator_); + write_ref right = tm_.new_block(validator_); node_ref r = to_node(right); unsigned nr_left = l.get_nr_entries() / 2; @@ -822,14 +822,14 @@ namespace persistent_data { { using namespace btree_detail; - read_ref blk = tm_->read_lock(b, validator_); + read_ref blk = tm_.read_lock(b, validator_); internal_node o = to_node(blk); // FIXME: use a switch statement if (o.get_type() == INTERNAL) { if (v.visit_internal(loc, o)) { for (unsigned i = 0; i < o.get_nr_entries(); i++) - tm_->prefetch(o.value_at(i)); + tm_.prefetch(o.value_at(i)); for (unsigned i = 0; i < o.get_nr_entries(); i++) { node_location loc2(loc); diff --git a/persistent-data/space-maps/disk.cc b/persistent-data/space-maps/disk.cc index a1ec321..c9ca9ea 100644 --- a/persistent-data/space-maps/disk.cc +++ b/persistent-data/space-maps/disk.cc @@ -98,7 +98,7 @@ namespace { typedef transaction_manager::read_ref read_ref; typedef transaction_manager::write_ref write_ref; - bitmap(transaction_manager::ptr tm, + bitmap(transaction_manager &tm, index_entry const &ie, bcache::validator::ptr v) : tm_(tm), @@ -107,7 +107,7 @@ namespace { } ref_t lookup(unsigned b) const { - read_ref rr = tm_->read_lock(ie_.blocknr_, validator_); + read_ref rr = tm_.read_lock(ie_.blocknr_, validator_); void const *bits = bitmap_data(rr); ref_t b1 = test_bit_le(bits, b * 2); ref_t b2 = test_bit_le(bits, b * 2 + 1); @@ -117,7 +117,7 @@ namespace { } void insert(unsigned b, ref_t n) { - write_ref wr = tm_->shadow(ie_.blocknr_, validator_).first; + write_ref wr = tm_.shadow(ie_.blocknr_, validator_).first; void *bits = bitmap_data(wr); bool was_free = !test_bit_le(bits, b * 2) && !test_bit_le(bits, b * 2 + 1); if (n == 1 || n == 3) @@ -158,7 +158,7 @@ namespace { } void iterate(block_address offset, block_address hi, space_map::iterator &it) const { - read_ref rr = tm_->read_lock(ie_.blocknr_, validator_); + read_ref rr = tm_.read_lock(ie_.blocknr_, validator_); void const *bits = bitmap_data(rr); for (unsigned b = 0; b < hi; b++) { @@ -181,7 +181,7 @@ namespace { return h + 1; } - transaction_manager::ptr tm_; + transaction_manager &tm_; bcache::validator::ptr validator_; index_entry ie_; @@ -241,7 +241,7 @@ namespace { typedef transaction_manager::write_ref write_ref; sm_disk(index_store::ptr indexes, - transaction_manager::ptr tm) + transaction_manager &tm) : tm_(tm), bitmap_validator_(new bitmap_block_validator), indexes_(indexes), @@ -251,7 +251,7 @@ namespace { } sm_disk(index_store::ptr indexes, - transaction_manager::ptr tm, + transaction_manager &tm, sm_root const &root) : tm_(tm), bitmap_validator_(new bitmap_block_validator), @@ -354,7 +354,7 @@ namespace { indexes_->resize(bitmap_count); for (block_address i = old_bitmap_count; i < bitmap_count; i++) { - write_ref wr = tm_->new_block(bitmap_validator_); + write_ref wr = tm_.new_block(bitmap_validator_); index_entry ie; ie.blocknr_ = wr.get_location(); @@ -437,7 +437,7 @@ namespace { } protected: - transaction_manager::ptr get_tm() const { + transaction_manager &get_tm() const { return tm_; } @@ -501,7 +501,7 @@ namespace { ref_counts_.remove(key); } - transaction_manager::ptr tm_; + transaction_manager &tm_; bcache::validator::ptr bitmap_validator_; index_store::ptr indexes_; block_address nr_blocks_; @@ -544,12 +544,12 @@ namespace { public: typedef boost::shared_ptr ptr; - btree_index_store(transaction_manager::ptr tm) + btree_index_store(transaction_manager &tm) : tm_(tm), bitmaps_(tm, index_entry_traits::ref_counter()) { } - btree_index_store(transaction_manager::ptr tm, + btree_index_store(transaction_manager &tm, block_address root) : tm_(tm), bitmaps_(tm, root, index_entry_traits::ref_counter()) { @@ -592,7 +592,7 @@ namespace { } private: - transaction_manager::ptr tm_; + transaction_manager &tm_; btree<1, index_entry_traits> bitmaps_; }; @@ -600,13 +600,13 @@ namespace { public: typedef boost::shared_ptr ptr; - metadata_index_store(transaction_manager::ptr tm) + metadata_index_store(transaction_manager &tm) : tm_(tm) { - block_manager<>::write_ref wr = tm_->new_block(index_validator()); + block_manager<>::write_ref wr = tm_.new_block(index_validator()); bitmap_root_ = wr.get_location(); } - metadata_index_store(transaction_manager::ptr tm, block_address root, block_address nr_indexes) + metadata_index_store(transaction_manager &tm, block_address root, block_address nr_indexes) : tm_(tm), bitmap_root_(root) { resize(nr_indexes); @@ -627,7 +627,7 @@ namespace { virtual void commit_ies() { std::pair::write_ref, bool> p = - tm_->shadow(bitmap_root_, index_validator()); + tm_.shadow(bitmap_root_, index_validator()); bitmap_root_ = p.first.get_location(); metadata_index *mdi = reinterpret_cast(p.first.data()); @@ -661,14 +661,14 @@ namespace { private: void load_ies() { block_manager<>::read_ref rr = - tm_->read_lock(bitmap_root_, index_validator()); + tm_.read_lock(bitmap_root_, index_validator()); metadata_index const *mdi = reinterpret_cast(rr.data()); for (unsigned i = 0; i < entries_.size(); i++) index_entry_traits::unpack(*(mdi->index + i), entries_[i]); } - transaction_manager::ptr tm_; + transaction_manager &tm_; block_address bitmap_root_; std::vector entries_; }; @@ -677,7 +677,7 @@ namespace { //---------------------------------------------------------------- checked_space_map::ptr -persistent_data::create_disk_sm(transaction_manager::ptr tm, +persistent_data::create_disk_sm(transaction_manager &tm, block_address nr_blocks) { index_store::ptr store(new btree_index_store(tm)); @@ -688,7 +688,7 @@ persistent_data::create_disk_sm(transaction_manager::ptr tm, } checked_space_map::ptr -persistent_data::open_disk_sm(transaction_manager::ptr tm, void *root) +persistent_data::open_disk_sm(transaction_manager &tm, void *root) { sm_root_disk d; sm_root v; @@ -700,7 +700,7 @@ persistent_data::open_disk_sm(transaction_manager::ptr tm, void *root) } checked_space_map::ptr -persistent_data::create_metadata_sm(transaction_manager::ptr tm, block_address nr_blocks) +persistent_data::create_metadata_sm(transaction_manager &tm, block_address nr_blocks) { index_store::ptr store(new metadata_index_store(tm)); checked_space_map::ptr sm(new sm_disk(store, tm)); @@ -711,7 +711,7 @@ persistent_data::create_metadata_sm(transaction_manager::ptr tm, block_address n } checked_space_map::ptr -persistent_data::open_metadata_sm(transaction_manager::ptr tm, void *root) +persistent_data::open_metadata_sm(transaction_manager &tm, void *root) { sm_root_disk d; sm_root v; diff --git a/persistent-data/space-maps/disk.h b/persistent-data/space-maps/disk.h index 5241419..0a69f04 100644 --- a/persistent-data/space-maps/disk.h +++ b/persistent-data/space-maps/disk.h @@ -26,16 +26,16 @@ namespace persistent_data { checked_space_map::ptr - create_disk_sm(transaction_manager::ptr tm, block_address nr_blocks); + create_disk_sm(transaction_manager &tm, block_address nr_blocks); checked_space_map::ptr - open_disk_sm(transaction_manager::ptr tm, void *root); + open_disk_sm(transaction_manager &tm, void *root); checked_space_map::ptr - create_metadata_sm(transaction_manager::ptr tm, block_address nr_blocks); + create_metadata_sm(transaction_manager &tm, block_address nr_blocks); checked_space_map::ptr - open_metadata_sm(transaction_manager::ptr tm, void *root); + open_metadata_sm(transaction_manager &tm, void *root); } //---------------------------------------------------------------- diff --git a/thin-provisioning/metadata.cc b/thin-provisioning/metadata.cc index 8725b17..098314c 100644 --- a/thin-provisioning/metadata.cc +++ b/thin-provisioning/metadata.cc @@ -71,36 +71,36 @@ metadata::metadata(std::string const &dev_path, open_type ot, if (sb_.version_ != 1) throw runtime_error("unknown metadata version"); - metadata_sm_ = open_metadata_sm(tm_, &sb_.metadata_space_map_root_); + metadata_sm_ = open_metadata_sm(*tm_, &sb_.metadata_space_map_root_); tm_->set_sm(metadata_sm_); - data_sm_ = open_disk_sm(tm_, static_cast(&sb_.data_space_map_root_)); + data_sm_ = open_disk_sm(*tm_, static_cast(&sb_.data_space_map_root_)); details_ = device_tree::ptr( - new device_tree(tm_, sb_.device_details_root_, + new device_tree(*tm_, sb_.device_details_root_, device_tree_detail::device_details_traits::ref_counter())); mappings_top_level_ = dev_tree::ptr( - new dev_tree(tm_, sb_.data_mapping_root_, + new dev_tree(*tm_, sb_.data_mapping_root_, mapping_tree_detail::mtree_ref_counter(tm_))); mappings_ = mapping_tree::ptr( - new mapping_tree(tm_, sb_.data_mapping_root_, + new mapping_tree(*tm_, sb_.data_mapping_root_, mapping_tree_detail::block_time_ref_counter(data_sm_))); break; case CREATE: tm_ = open_tm(open_bm(dev_path, block_manager<>::READ_WRITE)); space_map::ptr core = tm_->get_sm(); - metadata_sm_ = create_metadata_sm(tm_, tm_->get_bm()->get_nr_blocks()); + metadata_sm_ = create_metadata_sm(*tm_, tm_->get_bm()->get_nr_blocks()); copy_space_maps(metadata_sm_, core); tm_->set_sm(metadata_sm_); - data_sm_ = create_disk_sm(tm_, nr_data_blocks); - details_ = device_tree::ptr(new device_tree(tm_, device_tree_detail::device_details_traits::ref_counter())); - mappings_ = mapping_tree::ptr(new mapping_tree(tm_, + data_sm_ = create_disk_sm(*tm_, nr_data_blocks); + details_ = device_tree::ptr(new device_tree(*tm_, device_tree_detail::device_details_traits::ref_counter())); + mappings_ = mapping_tree::ptr(new mapping_tree(*tm_, mapping_tree_detail::block_time_ref_counter(data_sm_))); - mappings_top_level_ = dev_tree::ptr(new dev_tree(tm_, mappings_->get_root(), + mappings_top_level_ = dev_tree::ptr(new dev_tree(*tm_, mappings_->get_root(), mapping_tree_detail::mtree_ref_counter(tm_))); ::memset(&sb_, 0, sizeof(sb_)); @@ -125,11 +125,11 @@ metadata::metadata(std::string const &dev_path, block_address metadata_snap) //metadata_sm_ = open_metadata_sm(tm_, &sb_.metadata_space_map_root_); //tm_->set_sm(metadata_sm_); - data_sm_ = open_disk_sm(tm_, static_cast(&sb_.data_space_map_root_)); - details_ = device_tree::ptr(new device_tree(tm_, sb_.device_details_root_, device_tree_detail::device_details_traits::ref_counter())); - mappings_top_level_ = dev_tree::ptr(new dev_tree(tm_, sb_.data_mapping_root_, + data_sm_ = open_disk_sm(*tm_, static_cast(&sb_.data_space_map_root_)); + details_ = device_tree::ptr(new device_tree(*tm_, sb_.device_details_root_, device_tree_detail::device_details_traits::ref_counter())); + mappings_top_level_ = dev_tree::ptr(new dev_tree(*tm_, sb_.data_mapping_root_, mapping_tree_detail::mtree_ref_counter(tm_))); - mappings_ = mapping_tree::ptr(new mapping_tree(tm_, sb_.data_mapping_root_, + mappings_ = mapping_tree::ptr(new mapping_tree(*tm_, sb_.data_mapping_root_, mapping_tree_detail::block_time_ref_counter(data_sm_))); } @@ -146,29 +146,29 @@ metadata::metadata(block_manager<>::ptr bm, open_type ot, if (sb_.version_ != 1) throw runtime_error("unknown metadata version"); - metadata_sm_ = open_metadata_sm(tm_, &sb_.metadata_space_map_root_); + metadata_sm_ = open_metadata_sm(*tm_, &sb_.metadata_space_map_root_); tm_->set_sm(metadata_sm_); - data_sm_ = open_disk_sm(tm_, static_cast(&sb_.data_space_map_root_)); - details_ = device_tree::ptr(new device_tree(tm_, sb_.device_details_root_, device_tree_detail::device_details_traits::ref_counter())); - mappings_top_level_ = dev_tree::ptr(new dev_tree(tm_, sb_.data_mapping_root_, + data_sm_ = open_disk_sm(*tm_, static_cast(&sb_.data_space_map_root_)); + details_ = device_tree::ptr(new device_tree(*tm_, sb_.device_details_root_, device_tree_detail::device_details_traits::ref_counter())); + mappings_top_level_ = dev_tree::ptr(new dev_tree(*tm_, sb_.data_mapping_root_, mapping_tree_detail::mtree_ref_counter(tm_))); - mappings_ = mapping_tree::ptr(new mapping_tree(tm_, sb_.data_mapping_root_, + mappings_ = mapping_tree::ptr(new mapping_tree(*tm_, sb_.data_mapping_root_, mapping_tree_detail::block_time_ref_counter(data_sm_))); break; case CREATE: tm_ = open_tm(bm); space_map::ptr core = tm_->get_sm(); - metadata_sm_ = create_metadata_sm(tm_, tm_->get_bm()->get_nr_blocks()); + metadata_sm_ = create_metadata_sm(*tm_, tm_->get_bm()->get_nr_blocks()); copy_space_maps(metadata_sm_, core); tm_->set_sm(metadata_sm_); - data_sm_ = create_disk_sm(tm_, nr_data_blocks); - details_ = device_tree::ptr(new device_tree(tm_, device_tree_detail::device_details_traits::ref_counter())); - mappings_ = mapping_tree::ptr(new mapping_tree(tm_, + data_sm_ = create_disk_sm(*tm_, nr_data_blocks); + details_ = device_tree::ptr(new device_tree(*tm_, device_tree_detail::device_details_traits::ref_counter())); + mappings_ = mapping_tree::ptr(new mapping_tree(*tm_, mapping_tree_detail::block_time_ref_counter(data_sm_))); - mappings_top_level_ = dev_tree::ptr(new dev_tree(tm_, mappings_->get_root(), + mappings_top_level_ = dev_tree::ptr(new dev_tree(*tm_, mappings_->get_root(), mapping_tree_detail::mtree_ref_counter(tm_))); ::memset(&sb_, 0, sizeof(sb_)); diff --git a/thin-provisioning/metadata_dumper.cc b/thin-provisioning/metadata_dumper.cc index 0bd284e..db656ee 100644 --- a/thin-provisioning/metadata_dumper.cc +++ b/thin-provisioning/metadata_dumper.cc @@ -200,7 +200,7 @@ namespace { private: void emit_mappings(block_address subtree_root) { mapping_emitter me(e_); - single_mapping_tree tree(md_->tm_, subtree_root, + single_mapping_tree tree(*md_->tm_, subtree_root, mapping_tree_detail::block_time_ref_counter(md_->data_sm_)); walk_mapping_tree(tree, static_cast(me), *damage_policy_); } diff --git a/thin-provisioning/restore_emitter.cc b/thin-provisioning/restore_emitter.cc index fd1d4ab..5fae879 100644 --- a/thin-provisioning/restore_emitter.cc +++ b/thin-provisioning/restore_emitter.cc @@ -134,7 +134,7 @@ namespace { private: single_mapping_tree::ptr new_mapping_tree() { return single_mapping_tree::ptr( - new single_mapping_tree(md_->tm_, + new single_mapping_tree(*md_->tm_, mapping_tree_detail::block_time_ref_counter(md_->data_sm_))); } diff --git a/thin-provisioning/thin_check.cc b/thin-provisioning/thin_check.cc index e4cae83..9e90699 100644 --- a/thin-provisioning/thin_check.cc +++ b/thin-provisioning/thin_check.cc @@ -194,7 +194,7 @@ namespace { out << "examining devices tree" << end_message(); { nested_output::nest _ = out.push(); - device_tree dtree(tm, sb.device_details_root_, + device_tree dtree(*tm, sb.device_details_root_, device_tree_detail::device_details_traits::ref_counter()); check_device_tree(dtree, dev_rep); } @@ -204,7 +204,7 @@ namespace { out << "examining top level of mapping tree" << end_message(); { nested_output::nest _ = out.push(); - dev_tree dtree(tm, sb.data_mapping_root_, + dev_tree dtree(*tm, sb.data_mapping_root_, mapping_tree_detail::mtree_traits::ref_counter(tm)); check_mapping_tree(dtree, mapping_rep); } @@ -213,7 +213,7 @@ namespace { out << "examining mapping tree" << end_message(); { nested_output::nest _ = out.push(); - mapping_tree mtree(tm, sb.data_mapping_root_, + mapping_tree mtree(*tm, sb.data_mapping_root_, mapping_tree_detail::block_traits::ref_counter(tm->get_sm())); check_mapping_tree(mtree, mapping_rep); } diff --git a/thin-provisioning/thin_delta.cc b/thin-provisioning/thin_delta.cc index 59fefd3..21ecd01 100644 --- a/thin-provisioning/thin_delta.cc +++ b/thin-provisioning/thin_delta.cc @@ -450,7 +450,7 @@ namespace { superblock_detail::superblock sb = read_superblock(bm); - dev_tree dtree(tm, sb.data_mapping_root_, + dev_tree dtree(*tm, sb.data_mapping_root_, mapping_tree_detail::mtree_traits::ref_counter(tm)); dev_tree::key k = {*fs.snap1}; @@ -462,7 +462,7 @@ namespace { app.die(out.str()); } - single_mapping_tree snap1(tm, *snap1_root, mapping_tree_detail::block_traits::ref_counter(tm->get_sm())); + single_mapping_tree snap1(*tm, *snap1_root, mapping_tree_detail::block_traits::ref_counter(tm->get_sm())); k[0] = *fs.snap2; boost::optional snap2_root = dtree.lookup(k); @@ -473,7 +473,7 @@ namespace { app.die(out.str()); } - single_mapping_tree snap2(tm, *snap2_root, mapping_tree_detail::block_traits::ref_counter(tm->get_sm())); + single_mapping_tree snap2(*tm, *snap2_root, mapping_tree_detail::block_traits::ref_counter(tm->get_sm())); btree_visit_values(snap1, mr1, damage_v); btree_visit_values(snap2, mr2, damage_v); } diff --git a/thin-provisioning/thin_pool.cc b/thin-provisioning/thin_pool.cc index 23725d4..53940e3 100644 --- a/thin-provisioning/thin_pool.cc +++ b/thin-provisioning/thin_pool.cc @@ -122,7 +122,7 @@ thin_pool::create_thin(thin_dev_t dev) if (device_exists(dev)) throw std::runtime_error("Device already exists"); - single_mapping_tree::ptr new_tree(new single_mapping_tree(md_->tm_, + single_mapping_tree::ptr new_tree(new single_mapping_tree(*md_->tm_, mapping_tree_detail::block_time_ref_counter(md_->data_sm_))); md_->mappings_top_level_->insert(key, new_tree->get_root()); md_->mappings_->set_root(md_->mappings_top_level_->get_root()); // FIXME: ugly @@ -140,7 +140,7 @@ thin_pool::create_snap(thin_dev_t dev, thin_dev_t origin) if (!mtree_root) throw std::runtime_error("unknown origin"); - single_mapping_tree otree(md_->tm_, *mtree_root, + single_mapping_tree otree(*md_->tm_, *mtree_root, mapping_tree_detail::block_time_ref_counter(md_->data_sm_)); single_mapping_tree::ptr clone(otree.clone()); diff --git a/thin-provisioning/thin_rmap.cc b/thin-provisioning/thin_rmap.cc index 70c7b38..71180b3 100644 --- a/thin-provisioning/thin_rmap.cc +++ b/thin-provisioning/thin_rmap.cc @@ -75,7 +75,7 @@ namespace { transaction_manager::ptr tm = open_tm(bm); superblock_detail::superblock sb = read_superblock(bm); - mapping_tree mtree(tm, sb.data_mapping_root_, + mapping_tree mtree(*tm, sb.data_mapping_root_, mapping_tree_detail::block_traits::ref_counter(tm->get_sm())); btree_visit_values(mtree, rv, dv); diff --git a/unit-tests/array_t.cc b/unit-tests/array_t.cc index 5d0a733..74fac04 100644 --- a/unit-tests/array_t.cc +++ b/unit-tests/array_t.cc @@ -37,7 +37,9 @@ namespace { class ArrayTests : public Test { public: ArrayTests() - : tm_(create_tm()) { + : bm_(new block_manager<>("./test.data", NR_BLOCKS, 4, block_manager<>::READ_WRITE)), + sm_(new core_map(NR_BLOCKS)), + tm_(bm_, sm_) { } void @@ -76,15 +78,9 @@ namespace { array64::ptr a_; private: - static transaction_manager::ptr - create_tm() { - block_manager<>::ptr bm(new block_manager<>("./test.data", NR_BLOCKS, 4, block_manager<>::READ_WRITE)); - space_map::ptr sm(new core_map(NR_BLOCKS)); - transaction_manager::ptr tm(new transaction_manager(bm, sm)); - return tm; - } - - transaction_manager::ptr tm_; + block_manager<>::ptr bm_; + space_map::ptr sm_; + transaction_manager tm_; }; class value_visitor { diff --git a/unit-tests/bitset_t.cc b/unit-tests/bitset_t.cc index 20d1b1f..36634f6 100644 --- a/unit-tests/bitset_t.cc +++ b/unit-tests/bitset_t.cc @@ -32,34 +32,40 @@ using namespace testing; namespace { block_address const NR_BLOCKS = 102400; - transaction_manager::ptr - create_tm() { - block_manager<>::ptr bm(new block_manager<>("./test.data", NR_BLOCKS, 4, block_manager<>::READ_WRITE)); - space_map::ptr sm(new core_map(NR_BLOCKS)); - transaction_manager::ptr tm(new transaction_manager(bm, sm)); - return tm; - } + class BitsetTests : public Test { + public: + BitsetTests() + : bm_(new block_manager<>("./test.data", NR_BLOCKS, 4, block_manager<>::READ_WRITE)), + sm_(new core_map(NR_BLOCKS)), + tm_(bm_, sm_) { + } - bitset::ptr - create_bitset() { - return bitset::ptr(new bitset(create_tm())); - } + bitset::ptr + create_bitset() { + return bitset::ptr(new bitset(tm_)); + } - bitset::ptr - open_bitset(block_address root, unsigned count) { - return bitset::ptr(new bitset(create_tm(), root, count)); - } + bitset::ptr + open_bitset(block_address root, unsigned count) { + return bitset::ptr(new bitset(tm_, root, count)); + } + + private: + block_manager<>::ptr bm_; + space_map::ptr sm_; + transaction_manager tm_; + }; } //---------------------------------------------------------------- -TEST(BitsetTests, create_empty_bitset) +TEST_F(BitsetTests, create_empty_bitset) { bitset::ptr bs = create_bitset(); ASSERT_THROW(bs->get(0), runtime_error); } -TEST(BitsetTests, grow_default_false) +TEST_F(BitsetTests, grow_default_false) { unsigned const COUNT = 100000; @@ -70,7 +76,7 @@ TEST(BitsetTests, grow_default_false) ASSERT_FALSE(bs->get(i)); } -TEST(BitsetTests, grow_default_true) +TEST_F(BitsetTests, grow_default_true) { unsigned const COUNT = 100000; @@ -81,7 +87,7 @@ TEST(BitsetTests, grow_default_true) ASSERT_TRUE(bs->get(i)); } -TEST(BitsetTests, grow_throws_if_actualy_asked_to_shrink) +TEST_F(BitsetTests, grow_throws_if_actualy_asked_to_shrink) { unsigned const COUNT = 100000; @@ -90,7 +96,7 @@ TEST(BitsetTests, grow_throws_if_actualy_asked_to_shrink) ASSERT_THROW(bs->grow(COUNT / 2, false), runtime_error); } -TEST(BitsetTests, multiple_grow_calls) +TEST_F(BitsetTests, multiple_grow_calls) { unsigned const COUNT = 100000; unsigned const STEP = 37; @@ -121,7 +127,7 @@ TEST(BitsetTests, multiple_grow_calls) } } -TEST(BitsetTests, set_out_of_bounds_throws) +TEST_F(BitsetTests, set_out_of_bounds_throws) { unsigned const COUNT = 100000; bitset::ptr bs = create_bitset(); @@ -131,7 +137,7 @@ TEST(BitsetTests, set_out_of_bounds_throws) ASSERT_THROW(bs->set(COUNT, true), runtime_error); } -TEST(BitsetTests, set_works) +TEST_F(BitsetTests, set_works) { unsigned const COUNT = 100000; bitset::ptr bs = create_bitset(); @@ -144,7 +150,7 @@ TEST(BitsetTests, set_works) ASSERT_THAT(bs->get(i), Eq(i % 7 ? true : false)); } -TEST(BitsetTests, reopen_works) +TEST_F(BitsetTests, reopen_works) { unsigned const COUNT = 100000; block_address root; diff --git a/unit-tests/bloom_filter_t.cc b/unit-tests/bloom_filter_t.cc index 313355e..032eb48 100644 --- a/unit-tests/bloom_filter_t.cc +++ b/unit-tests/bloom_filter_t.cc @@ -31,7 +31,7 @@ namespace { BloomFilterTests() : bm_(create_bm(NR_BLOCKS)), sm_(setup_core_map()), - tm_(new transaction_manager(bm_, sm_)) { + tm_(bm_, sm_) { } set generate_random_blocks(unsigned count, @@ -73,7 +73,7 @@ namespace { with_temp_directory dir_; block_manager<>::ptr bm_; space_map::ptr sm_; - transaction_manager::ptr tm_; + transaction_manager tm_; boost::random::mt19937 rng_; }; diff --git a/unit-tests/btree_counter_t.cc b/unit-tests/btree_counter_t.cc index adbf523..dbc36f6 100644 --- a/unit-tests/btree_counter_t.cc +++ b/unit-tests/btree_counter_t.cc @@ -25,7 +25,7 @@ namespace { BTreeCounterTests() : bm_(create_bm(NR_BLOCKS)), sm_(setup_core_map()), - tm_(new transaction_manager(bm_, sm_)) { + tm_(bm_, sm_) { } void check_nr_metadata_blocks_is_ge(unsigned n) { @@ -38,7 +38,7 @@ namespace { with_temp_directory dir_; block_manager<>::ptr bm_; space_map::ptr sm_; - transaction_manager::ptr tm_; + transaction_manager tm_; uint64_traits::ref_counter rc_; btree<1, uint64_traits>::ptr tree_; diff --git a/unit-tests/btree_damage_visitor_t.cc b/unit-tests/btree_damage_visitor_t.cc index 201d8e3..f88ee18 100644 --- a/unit-tests/btree_damage_visitor_t.cc +++ b/unit-tests/btree_damage_visitor_t.cc @@ -281,7 +281,7 @@ namespace { DamageTests() : bm_(create_bm(NR_BLOCKS)), sm_(setup_core_map()), - tm_(new transaction_manager(bm_, sm_)) { + tm_(bm_, sm_) { } virtual ~DamageTests() {} @@ -315,7 +315,7 @@ namespace { with_temp_directory dir_; block_manager<>::ptr bm_; space_map::ptr sm_; - transaction_manager::ptr tm_; + transaction_manager tm_; thing_traits::ref_counter rc_; boost::optional layout_; diff --git a/unit-tests/btree_t.cc b/unit-tests/btree_t.cc index 0d68343..13a525e 100644 --- a/unit-tests/btree_t.cc +++ b/unit-tests/btree_t.cc @@ -31,21 +31,28 @@ using namespace testing; namespace { block_address const NR_BLOCKS = 102400; - transaction_manager::ptr - create_tm() { - block_manager<>::ptr bm(new block_manager<>("./test.data", NR_BLOCKS, 4, block_manager<>::READ_WRITE)); - space_map::ptr sm(new core_map(NR_BLOCKS)); - transaction_manager::ptr tm(new transaction_manager(bm, sm)); - return tm; - } + class BtreeTests : public Test { + public: + BtreeTests() + : bm_(new block_manager<>("./test.data", NR_BLOCKS, 4, block_manager<>::READ_WRITE)), + sm_(new core_map(NR_BLOCKS)), + tm_(bm_, sm_) { + } - btree<1, uint64_traits>::ptr - create_btree() { - uint64_traits::ref_counter rc; + btree<1, uint64_traits>::ptr + create_btree() { + uint64_traits::ref_counter rc; + + return btree<1, uint64_traits>::ptr( + new btree<1, uint64_traits>(tm_, rc)); + } + + private: + block_manager<>::ptr bm_; + space_map::ptr sm_; + transaction_manager tm_; + }; - return btree<1, uint64_traits>::ptr( - new btree<1, uint64_traits>(create_tm(), rc)); - } // Checks that a btree is well formed. // @@ -99,7 +106,7 @@ namespace { //---------------------------------------------------------------- -TEST(BtreeTests, empty_btree_contains_nothing) +TEST_F(BtreeTests, empty_btree_contains_nothing) { btree<1, uint64_traits>::ptr tree = create_btree(); check_constraints(tree); @@ -110,7 +117,7 @@ TEST(BtreeTests, empty_btree_contains_nothing) } } -TEST(BtreeTests, insert_works) +TEST_F(BtreeTests, insert_works) { unsigned const COUNT = 100000; @@ -129,7 +136,7 @@ TEST(BtreeTests, insert_works) check_constraints(tree); } -TEST(BtreeTests, insert_does_not_insert_imaginary_values) +TEST_F(BtreeTests, insert_does_not_insert_imaginary_values) { btree<1, uint64_traits>::ptr tree = create_btree(); uint64_t key[1] = {0}; @@ -156,7 +163,7 @@ TEST(BtreeTests, insert_does_not_insert_imaginary_values) check_constraints(tree); } -TEST(BtreeTests, clone) +TEST_F(BtreeTests, clone) { typedef btree<1, uint64_traits> tree64; diff --git a/unit-tests/space_map_t.cc b/unit-tests/space_map_t.cc index c87d6f4..0848909 100644 --- a/unit-tests/space_map_t.cc +++ b/unit-tests/space_map_t.cc @@ -33,278 +33,258 @@ namespace { block_address const SUPERBLOCK = 0; block_address const MAX_LOCKS = 8; - transaction_manager::ptr - create_tm() { - block_manager<>::ptr bm( - new block_manager<>("./test.data", NR_BLOCKS, MAX_LOCKS, block_manager<>::READ_WRITE)); - space_map::ptr sm(new core_map(NR_BLOCKS)); - transaction_manager::ptr tm( - new transaction_manager(bm, sm)); - return tm; - } - - struct sm_core_creator { - static space_map::ptr - create() { - return space_map::ptr(new persistent_data::core_map(NR_BLOCKS)); - } - }; - - struct sm_careful_alloc_creator { - static space_map::ptr - create() { - return create_careful_alloc_sm( - checked_space_map::ptr( - new core_map(NR_BLOCKS))); - } - }; - - struct sm_recursive_creator { - static checked_space_map::ptr - create() { - return create_recursive_sm( - checked_space_map::ptr( - new core_map(NR_BLOCKS))); - } - }; - - struct sm_disk_creator { - static persistent_space_map::ptr - create() { - transaction_manager::ptr tm = create_tm(); - return persistent_data::create_disk_sm(tm, NR_BLOCKS); + class SpaceMapTests : public Test { + public: + SpaceMapTests() + : bm_(new block_manager<>("./test.data", NR_BLOCKS, MAX_LOCKS, block_manager<>::READ_WRITE)), + sm_(new core_map(NR_BLOCKS)), + tm_(bm_, sm_) { } - static persistent_space_map::ptr - open(void *root) { - transaction_manager::ptr tm = create_tm(); - return persistent_data::open_disk_sm(tm, root); - } - }; + struct sm_core_creator { + static space_map::ptr + create(transaction_manager &tm) { + return space_map::ptr(new persistent_data::core_map(NR_BLOCKS)); + } + }; - struct sm_metadata_creator { - static persistent_space_map::ptr - create() { - transaction_manager::ptr tm = create_tm(); - return persistent_data::create_metadata_sm(tm, NR_BLOCKS); + struct sm_careful_alloc_creator { + static space_map::ptr + create(transaction_manager &tm) { + return create_careful_alloc_sm( + checked_space_map::ptr( + new core_map(NR_BLOCKS))); + } + }; + + struct sm_recursive_creator { + static checked_space_map::ptr + create(transaction_manager &tm) { + return create_recursive_sm( + checked_space_map::ptr( + new core_map(NR_BLOCKS))); + } + }; + + struct sm_disk_creator { + static persistent_space_map::ptr + create(transaction_manager &tm) { + return persistent_data::create_disk_sm(tm, NR_BLOCKS); + } + + static persistent_space_map::ptr + open(transaction_manager &tm, void *root) { + return persistent_data::open_disk_sm(tm, root); + } + }; + + struct sm_metadata_creator { + static persistent_space_map::ptr + create(transaction_manager &tm) { + return persistent_data::create_metadata_sm(tm, NR_BLOCKS); + } + + static persistent_space_map::ptr + open(transaction_manager &tm, void *root) { + return persistent_data::open_metadata_sm(tm, root); + } + }; + + //-------------------------------- + + void test_get_nr_blocks(space_map::ptr sm) { + ASSERT_THAT(sm->get_nr_blocks(), Eq(NR_BLOCKS)); } - static persistent_space_map::ptr - open(void *root) { - transaction_manager::ptr tm = create_tm(); - return persistent_data::open_metadata_sm(tm, root); + void test_get_nr_free(space_map::ptr sm) { + ASSERT_THAT(sm->get_nr_free(), Eq(NR_BLOCKS)); + + for (unsigned i = 0; i < NR_BLOCKS; i++) { + boost::optional mb = sm->new_block(); + ASSERT_TRUE(mb); + ASSERT_THAT(sm->get_nr_free(), Eq(NR_BLOCKS - i - 1)); + } + + for (unsigned i = 0; i < NR_BLOCKS; i++) { + sm->dec(i); + ASSERT_THAT(sm->get_nr_free(), Eq(i + 1)); + } } - }; - //-------------------------------- + void test_runs_out_of_space(space_map::ptr sm) { + boost::optional mb; - void test_get_nr_blocks(space_map::ptr sm) - { - ASSERT_THAT(sm->get_nr_blocks(), Eq(NR_BLOCKS)); - } + for (unsigned i = 0; i < NR_BLOCKS; i++) + mb = sm->new_block(); - void test_get_nr_free(space_map::ptr sm) - { - ASSERT_THAT(sm->get_nr_free(), Eq(NR_BLOCKS)); + mb = sm->new_block(); + ASSERT_FALSE(mb); + } - for (unsigned i = 0; i < NR_BLOCKS; i++) { + void test_inc_and_dec(space_map::ptr sm) { + block_address b = 63; + + for (unsigned i = 0; i < 50; i++) { + ASSERT_THAT(sm->get_count(b), Eq(i)); + sm->inc(b); + } + + for (unsigned i = 50; i > 0; i--) { + ASSERT_THAT(sm->get_count(b), Eq(i)); + sm->dec(b); + } + } + + void test_not_allocated_twice(space_map::ptr sm) { boost::optional mb = sm->new_block(); ASSERT_TRUE(mb); - ASSERT_THAT(sm->get_nr_free(), Eq(NR_BLOCKS - i - 1)); + + for (;;) { + boost::optional b = sm->new_block(); + if (!b) + break; + + if (b) + ASSERT_TRUE(*b != *mb); + } } - for (unsigned i = 0; i < NR_BLOCKS; i++) { - sm->dec(i); - ASSERT_THAT(sm->get_nr_free(), Eq(i + 1)); - } - } - - void test_runs_out_of_space(space_map::ptr sm) - { - boost::optional mb; - - for (unsigned i = 0; i < NR_BLOCKS; i++) - mb = sm->new_block(); - - mb = sm->new_block(); - ASSERT_FALSE(mb); - } - - void test_inc_and_dec(space_map::ptr sm) - { - block_address b = 63; - - for (unsigned i = 0; i < 50; i++) { - ASSERT_THAT(sm->get_count(b), Eq(i)); - sm->inc(b); + void test_set_count(space_map::ptr sm) { + sm->set_count(43, 5); + ASSERT_THAT(sm->get_count(43), Eq(5u)); } - for (unsigned i = 50; i > 0; i--) { - ASSERT_THAT(sm->get_count(b), Eq(i)); - sm->dec(b); - } - } + void test_set_affects_nr_allocated(space_map::ptr sm) { + for (unsigned i = 0; i < NR_BLOCKS; i++) { + sm->set_count(i, 1); + ASSERT_THAT(sm->get_nr_free(), Eq(NR_BLOCKS - i - 1)); + } - void test_not_allocated_twice(space_map::ptr sm) - { - boost::optional mb = sm->new_block(); - ASSERT_TRUE(mb); - - for (;;) { - boost::optional b = sm->new_block(); - if (!b) - break; - - if (b) - ASSERT_TRUE(*b != *mb); - } - } - - void test_set_count(space_map::ptr sm) - { - sm->set_count(43, 5); - ASSERT_THAT(sm->get_count(43), Eq(5u)); - } - - void test_set_affects_nr_allocated(space_map::ptr sm) - { - for (unsigned i = 0; i < NR_BLOCKS; i++) { - sm->set_count(i, 1); - ASSERT_THAT(sm->get_nr_free(), Eq(NR_BLOCKS - i - 1)); + for (unsigned i = 0; i < NR_BLOCKS; i++) { + sm->set_count(i, 0); + ASSERT_THAT(sm->get_nr_free(), Eq(i + 1)); + } } - for (unsigned i = 0; i < NR_BLOCKS; i++) { - sm->set_count(i, 0); - ASSERT_THAT(sm->get_nr_free(), Eq(i + 1)); - } - } - - // Ref counts below 3 gets stored as bitmaps, above 3 they go into - // a btree with uint32_t values. Worth checking this thoroughly, - // especially for the metadata format which may have complications - // due to recursion. - void test_high_ref_counts(space_map::ptr sm) - { - srand(1234); - for (unsigned i = 0; i < NR_BLOCKS; i++) - sm->set_count(i, rand() % 6789); - sm->commit(); - - for (unsigned i = 0; i < NR_BLOCKS; i++) { - sm->inc(i); - sm->inc(i); - if (i % 1000) - sm->commit(); - } - sm->commit(); - - srand(1234); - for (unsigned i = 0; i < NR_BLOCKS; i++) - ASSERT_THAT(sm->get_count(i), Eq((rand() % 6789u) + 2u)); - - for (unsigned i = 0; i < NR_BLOCKS; i++) - sm->dec(i); - - srand(1234); - for (unsigned i = 0; i < NR_BLOCKS; i++) - ASSERT_THAT(sm->get_count(i), Eq((rand() % 6789u) + 1u)); - } - - template - void test_sm_reopen() - { - unsigned char buffer[128]; - - { - persistent_space_map::ptr sm = SMCreator::create(); - for (unsigned i = 0, step = 1; i < NR_BLOCKS; i += step, step++) - sm->inc(i); + // Ref counts below 3 gets stored as bitmaps, above 3 they go into + // a btree with uint32_t values. Worth checking this thoroughly, + // especially for the metadata format which may have complications + // due to recursion. + void test_high_ref_counts(space_map::ptr sm) { + srand(1234); + for (unsigned i = 0; i < NR_BLOCKS; i++) + sm->set_count(i, rand() % 6789); sm->commit(); - ASSERT_THAT(sm->root_size(), Le(sizeof(buffer))); - sm->copy_root(buffer, sizeof(buffer)); + for (unsigned i = 0; i < NR_BLOCKS; i++) { + sm->inc(i); + sm->inc(i); + if (i % 1000) + sm->commit(); + } + sm->commit(); + + srand(1234); + for (unsigned i = 0; i < NR_BLOCKS; i++) + ASSERT_THAT(sm->get_count(i), Eq((rand() % 6789u) + 2u)); + + for (unsigned i = 0; i < NR_BLOCKS; i++) + sm->dec(i); + + srand(1234); + for (unsigned i = 0; i < NR_BLOCKS; i++) + ASSERT_THAT(sm->get_count(i), Eq((rand() % 6789u) + 1u)); } - { - persistent_space_map::ptr sm = SMCreator::open(buffer); + template + void test_sm_reopen() { + unsigned char buffer[128]; - for (unsigned i = 0, step = 1; i < NR_BLOCKS; i += step, step++) - ASSERT_THAT(sm->get_count(i), Eq(1u)); + { + persistent_space_map::ptr sm = SMCreator::create(tm_); + for (unsigned i = 0, step = 1; i < NR_BLOCKS; i += step, step++) + sm->inc(i); + sm->commit(); + + ASSERT_THAT(sm->root_size(), Le(sizeof(buffer))); + sm->copy_root(buffer, sizeof(buffer)); + } + + { + persistent_space_map::ptr sm = SMCreator::open(tm_, buffer); + + for (unsigned i = 0, step = 1; i < NR_BLOCKS; i += step, step++) + ASSERT_THAT(sm->get_count(i), Eq(1u)); + } } - } - typedef void (*sm_test)(space_map::ptr); - - template - void do_tests(sm_test (&tests)[NTests]) - { - for (unsigned t = 0; t < NTests; t++) { - space_map::ptr sm = SMCreator::create(); - tests[t](sm); + template + void do_tests() { + test_get_nr_blocks(SMCreator::create(tm_)); + test_get_nr_free(SMCreator::create(tm_)); + test_runs_out_of_space(SMCreator::create(tm_)); + test_inc_and_dec(SMCreator::create(tm_)); + test_not_allocated_twice(SMCreator::create(tm_)); + test_set_count(SMCreator::create(tm_)); + test_set_affects_nr_allocated(SMCreator::create(tm_)); + test_high_ref_counts(SMCreator::create(tm_)); } - } - sm_test space_map_tests[] = { - test_get_nr_blocks, - test_get_nr_free, - test_runs_out_of_space, - test_inc_and_dec, - test_not_allocated_twice, - test_set_count, - test_set_affects_nr_allocated, - test_high_ref_counts + void + copy_space_maps(space_map::ptr lhs, space_map::ptr rhs) { + for (block_address b = 0; b < rhs->get_nr_blocks(); b++) { + uint32_t count = rhs->get_count(b); + if (count > 0) + lhs->set_count(b, rhs->get_count(b)); + } + } + + block_manager<>::ptr bm_; + space_map::ptr sm_; + transaction_manager tm_; }; - - void - copy_space_maps(space_map::ptr lhs, space_map::ptr rhs) { - for (block_address b = 0; b < rhs->get_nr_blocks(); b++) { - uint32_t count = rhs->get_count(b); - if (count > 0) - lhs->set_count(b, rhs->get_count(b)); - } - } } //---------------------------------------------------------------- -TEST(SpaceMapTests, test_sm_core) +TEST_F(SpaceMapTests, test_sm_core) { - do_tests(space_map_tests); + do_tests(); } -TEST(SpaceMapTests, test_sm_careful_alloc) +TEST_F(SpaceMapTests, test_sm_careful_alloc) { - do_tests(space_map_tests); + do_tests(); } -TEST(SpaceMapTests, test_sm_recursive) +TEST_F(SpaceMapTests, test_sm_recursive) { - do_tests(space_map_tests); + do_tests(); } -TEST(SpaceMapTests, test_sm_disk) +TEST_F(SpaceMapTests, test_sm_disk) { - do_tests(space_map_tests); + do_tests(); test_sm_reopen(); } -TEST(SpaceMapTests, test_sm_metadata) +TEST_F(SpaceMapTests, test_sm_metadata) { - do_tests(space_map_tests); + do_tests(); test_sm_reopen(); } -TEST(SpaceMapTests, test_metadata_and_disk) +TEST_F(SpaceMapTests, test_metadata_and_disk) { block_manager<>::ptr bm( new block_manager<>("./test.data", NR_BLOCKS, MAX_LOCKS, block_manager<>::READ_WRITE)); space_map::ptr core_sm(new core_map(NR_BLOCKS)); transaction_manager::ptr tm(new transaction_manager(bm, core_sm)); - persistent_space_map::ptr metadata_sm = persistent_data::create_metadata_sm(tm, NR_BLOCKS); + persistent_space_map::ptr metadata_sm = persistent_data::create_metadata_sm(*tm, NR_BLOCKS); copy_space_maps(metadata_sm, core_sm); tm->set_sm(metadata_sm); - persistent_space_map::ptr data_sm_ = create_disk_sm(tm, NR_BLOCKS * 2); + persistent_space_map::ptr data_sm_ = create_disk_sm(*tm, NR_BLOCKS * 2); } //----------------------------------------------------------------