From 6dc9a90fecb3aeff2fa075859d036d9a966e71ea Mon Sep 17 00:00:00 2001 From: Ming-Hung Tsai Date: Thu, 31 Mar 2016 23:08:14 +0800 Subject: [PATCH] [counting_visitor] fix unnecessary value visiting 1. Do not inherit btree_damage_visitor to avoid unnecessary value visiting. (reverts commit b22495997a8bf427741225ef86edf267ee746d7c) 2. Use btree_node_checker to do node checking --- .../data-structures/btree_counter.h | 92 ++++++++++++------- .../data-structures/btree_damage_visitor.h | 5 +- 2 files changed, 63 insertions(+), 34 deletions(-) diff --git a/persistent-data/data-structures/btree_counter.h b/persistent-data/data-structures/btree_counter.h index 6e52c29..1b8c7a2 100644 --- a/persistent-data/data-structures/btree_counter.h +++ b/persistent-data/data-structures/btree_counter.h @@ -2,44 +2,36 @@ #define PERSISTENT_DATA_DATA_STRUCTURES_BTREE_COUNTER_H #include "persistent-data/data-structures/btree.h" -#include "persistent-data/data-structures/btree_base_visitor.h" -#include "persistent-data/data-structures/btree_damage_visitor.h" +#include "persistent-data/data-structures/btree_node_checker.h" #include "persistent-data/block_counter.h" //---------------------------------------------------------------- namespace persistent_data { namespace btree_count_detail { - template - class counting_visitor : public btree_damage_visitor { - typedef btree_damage_visitor BtreeDamageVisitor; + template + class counting_visitor : public btree::visitor { public: typedef btree tree; - counting_visitor(ValueVisitor &value_visitor, - DamageVisitor &damage_visitor, - block_counter &bc, - ValueCounter &vc) - : BtreeDamageVisitor(value_visitor, damage_visitor, false), - bc_(bc), + counting_visitor(block_counter &bc, ValueCounter &vc) + : bc_(bc), vc_(vc) { } virtual bool visit_internal(node_location const &l, typename tree::internal_node const &n) { - return BtreeDamageVisitor::visit_internal(l, n) ? - visit_node(n) : false; + return check_internal(l, n) ? visit_node(n) : false; } virtual bool visit_internal_leaf(node_location const &l, typename tree::internal_node const &n) { - return BtreeDamageVisitor::visit_internal_leaf(l, n) ? - visit_node(n) : false; + return check_leaf(l, n) ? visit_node(n) : false; } virtual bool visit_leaf(node_location const &l, typename tree::leaf_node const &n) { - if (BtreeDamageVisitor::visit_leaf(l, n) && visit_node(n)) { + if (check_leaf(l, n) && visit_node(n)) { unsigned nr = n.get_nr_entries(); for (unsigned i = 0; i < nr; i++) { @@ -55,7 +47,57 @@ namespace persistent_data { return false; } + typedef typename btree::visitor::error_outcome error_outcome; + + error_outcome error_accessing_node(node_location const &l, block_address b, + std::string const &what) { + return btree::visitor::EXCEPTION_HANDLED; + } + private: + bool check_internal(node_location const &l, + btree_detail::node_ref const &n) { + if (!checker_.check_block_nr(n) || + !checker_.check_value_size(n) || + !checker_.check_max_entries(n) || + !checker_.check_nr_entries(n, l.is_sub_root()) || + !checker_.check_ordered_keys(n) || + !checker_.check_parent_key(n, l.is_sub_root() ? boost::optional() : l.key)) + return false; + + if (l.is_sub_root()) + new_root(l.level()); + + return true; + } + + template + bool check_leaf(node_location const &l, + btree_detail::node_ref const &n) { + if (!checker_.check_block_nr(n) || + !checker_.check_value_size(n) || + !checker_.check_max_entries(n) || + !checker_.check_nr_entries(n, l.is_sub_root()) || + !checker_.check_ordered_keys(n) || + !checker_.check_parent_key(n, l.is_sub_root() ? boost::optional() : l.key)) + return false; + + if (l.is_sub_root()) + new_root(l.level()); + + bool r = checker_.check_leaf_key(n, last_leaf_key_[l.level()]); + if (r && n.get_nr_entries() > 0) + last_leaf_key_[l.level()] = n.key_at(n.get_nr_entries() - 1); + + return r; + } + + void new_root(unsigned level) { + // we're starting a new subtree, so should + // reset the last_leaf value. + last_leaf_key_[level] = boost::optional(); + } + template bool visit_node(Node const &n) { block_address b = n.get_location(); @@ -66,6 +108,8 @@ namespace persistent_data { block_counter &bc_; ValueCounter &vc_; + btree_node_checker checker_; + boost::optional last_leaf_key_[Levels]; }; } @@ -94,21 +138,7 @@ namespace persistent_data { // is not corrupt. template void count_btree_blocks(btree const &tree, block_counter &bc, ValueCounter &vc) { - typedef noop_value_visitor NoopValueVisitor; - NoopValueVisitor noop_vv; - noop_damage_visitor noop_dv; - btree_count_detail::counting_visitor v(noop_vv, noop_dv, bc, vc); - tree.visit_depth_first(v); - } - - template - void count_btree_blocks(btree const &tree, block_counter &bc) { - typedef noop_value_visitor NoopValueVisitor; - NoopValueVisitor noop_vv; - noop_damage_visitor noop_dv; - typedef noop_value_counter NoopValueCounter; - NoopValueCounter vc; - btree_count_detail::counting_visitor v(noop_vv, noop_dv, bc, vc); + btree_count_detail::counting_visitor v(bc, vc); tree.visit_depth_first(v); } } diff --git a/persistent-data/data-structures/btree_damage_visitor.h b/persistent-data/data-structures/btree_damage_visitor.h index 8be623b..5a96d5d 100644 --- a/persistent-data/data-structures/btree_damage_visitor.h +++ b/persistent-data/data-structures/btree_damage_visitor.h @@ -158,9 +158,8 @@ namespace persistent_data { typedef boost::optional maybe_run64; btree_damage_visitor(ValueVisitor &value_visitor, - DamageVisitor &damage_visitor, - bool avoid_repeated_visits = true) - : avoid_repeated_visits_(avoid_repeated_visits), + DamageVisitor &damage_visitor) + : avoid_repeated_visits_(true), value_visitor_(value_visitor), damage_visitor_(damage_visitor) { }