From 7657a6d5b30dd181350acf19681847d9c8f5d694 Mon Sep 17 00:00:00 2001 From: Colin McCabe Date: Tue, 28 Sep 2010 19:00:28 -0700 Subject: [PATCH] interval_set: hide data members This change makes interval_set::m and interval_set::_size private data members in interval_set, instead of public. This change also creates a non-const iterator. Using this iterator, users can modify the length of an interval. So now, all users can use the iterators rather than interacting with the class internals directly. --- src/include/interval_set.h | 93 ++++++++++++++++++++++++++++++++++---- src/osd/ReplicatedPG.cc | 86 +++++++++++++++++++---------------- src/osd/osd_types.h | 2 +- 3 files changed, 131 insertions(+), 50 deletions(-) diff --git a/src/include/interval_set.h b/src/include/interval_set.h index 4617a72441594..fa824e5ffbcdd 100644 --- a/src/include/interval_set.h +++ b/src/include/interval_set.h @@ -34,9 +34,65 @@ using namespace std; template class interval_set { public: - map m; // map start -> len - int64_t _size; - + class iterator : public std::iterator + { + public: + explicit iterator(typename std::map::iterator iter) + : _iter(iter) + { } + + // For the copy constructor and assignment operator, the compiler-generated functions, which + // perform simple bitwise copying, should be fine. + + bool operator==(const iterator& rhs) const { + return (_iter == rhs._iter); + } + + bool operator!=(const iterator& rhs) const { + return (_iter != rhs._iter); + } + + // Dereference this iterator to get a pair. + pair < T, T > &operator*() { + return *_iter; + } + + // Return the interval start. + T get_start() const { + return _iter->first; + } + + // Return the interval length. + T get_len() const { + return _iter->second; + } + + // Set the interval length. + void set_len(T len) { + _iter->second = len; + } + + // Preincrement + iterator &operator++() + { + ++_iter; + return *this; + } + + // Postincrement + iterator operator++(int) + { + iterator prev(_iter); + ++_iter; + return prev; + } + + friend class interval_set::const_iterator; + + protected: + typename map::iterator _iter; + }; + class const_iterator : public std::iterator { public: @@ -44,6 +100,10 @@ class interval_set { : _iter(iter) { } + const_iterator(const iterator &i) + : _iter(i._iter) + { } + // For the copy constructor and assignment operator, the compiler-generated functions, which // perform simple bitwise copying, should be fine. @@ -56,17 +116,17 @@ class interval_set { } // Dereference this iterator to get a pair. - pair < T, T > &operator*() { + pair < T, T > operator*() const { return *_iter; } // Return the interval start. - const T& get_start() const { + T get_start() const { return _iter->first; } // Return the interval length. - const T& get_len() const { + T get_len() const { return _iter->second; } @@ -91,13 +151,24 @@ class interval_set { interval_set() : _size(0) {} - typename interval_set::const_iterator begin() const + int num_intervals() const { + return m.size(); + } + + typename interval_set::iterator begin() { + return typename interval_set::iterator(m.begin()); + } + + typename interval_set::iterator end() { + return typename interval_set::iterator(m.end()); + } + + typename interval_set::const_iterator begin() const { return typename interval_set::const_iterator(m.begin()); } - typename interval_set::const_iterator end() const - { + typename interval_set::const_iterator end() const { return typename interval_set::const_iterator(m.end()); } @@ -434,6 +505,10 @@ class interval_set { } } +private: + // data + int64_t _size; + map m; // map start -> len }; diff --git a/src/osd/ReplicatedPG.cc b/src/osd/ReplicatedPG.cc index 343b8a17501a2..542005f8d7f55 100644 --- a/src/osd/ReplicatedPG.cc +++ b/src/osd/ReplicatedPG.cc @@ -1650,9 +1650,9 @@ void ReplicatedPG::make_writeable(OpContext *ctx) void ReplicatedPG::add_interval_usage(interval_set& s, pg_stat_t& stats) { - for (map::iterator p = s.m.begin(); p != s.m.end(); p++) { - stats.num_bytes += p->second; - stats.num_kb += SHIFT_ROUND_UP(p->first+p->second, 10) - (p->first >> 10); + for (interval_set::const_iterator p = s.begin(); p != s.end(); ++p) { + stats.num_bytes += p.get_len(); + stats.num_kb += SHIFT_ROUND_UP(p.get_start() + p.get_len(), 10) - (p.get_start() >> 10); } } @@ -2894,15 +2894,16 @@ void ReplicatedPG::send_push_op(const sobject_t& soid, int peer, bufferlist bl; map attrset; - for (map::iterator p = data_subset.m.begin(); - p != data_subset.m.end(); - p++) { + for (interval_set::iterator p = data_subset.begin(); + p != data_subset.end(); + ++p) { bufferlist bit; - osd->store->read(coll_t::build_pg_coll(info.pgid), soid, p->first, p->second, bit); - if (p->second != bit.length()) { - dout(10) << " extent " << p->first << "~" << p->second - << " is actually " << p->first << "~" << bit.length() << dendl; - p->second = bit.length(); + osd->store->read(coll_t::build_pg_coll(info.pgid), + soid, p.get_start(), p.get_len(), bit); + if (p.get_len() != bit.length()) { + dout(10) << " extent " << p.get_start() << "~" << p.get_len() + << " is actually " << p.get_start() << "~" << bit.length() << dendl; + p.set_len(bit.length()); } bl.claim_append(bit); } @@ -3178,23 +3179,23 @@ void ReplicatedPG::sub_op_push(MOSDSubOp *op) bufferlist result; int off = 0; - for (map::iterator p = data_subset.m.begin(); - p != data_subset.m.end(); - p++) { + for (interval_set::const_iterator p = data_subset.begin(); + p != data_subset.end(); + ++p) { interval_set x; - x.insert(p->first, p->second); + x.insert(p.get_start(), p.get_len()); x.intersection_of(data_needed); - dout(20) << " data_subset object extent " << p->first << "~" << p->second << " need " << x << dendl; + dout(20) << " data_subset object extent " << p.get_start() << "~" << p.get_len() << " need " << x << dendl; if (!x.empty()) { - uint64_t first = x.m.begin()->first; - uint64_t len = x.m.begin()->second; + uint64_t first = x.begin().get_start(); + uint64_t len = x.begin().get_len(); bufferlist sub; - int boff = off + (first - p->first); + int boff = off + (first - p.get_start()); dout(20) << " keeping buffer extent " << boff << "~" << len << dendl; sub.substr_of(data, boff, len); result.claim_append(sub); } - off += p->second; + off += p.get_len(); } data.claim(result); dout(20) << " new data len is " << data.length() << dendl; @@ -3255,14 +3256,14 @@ void ReplicatedPG::sub_op_push(MOSDSubOp *op) // write data uint64_t boff = 0; - for (map::iterator p = data_subset.m.begin(); - p != data_subset.m.end(); - p++) { + for (interval_set::const_iterator p = data_subset.begin(); + p != data_subset.end(); + ++p) { bufferlist bit; - bit.substr_of(data, boff, p->second); - dout(15) << " write " << p->first << "~" << p->second << dendl; - t->write(target, soid, p->first, p->second, bit); - boff += p->second; + bit.substr_of(data, boff, p.get_len()); + dout(15) << " write " << p.get_start() << "~" << p.get_len() << dendl; + t->write(target, soid, p.get_start(), p.get_len(), bit); + boff += p.get_len(); } if (complete) { @@ -3273,15 +3274,20 @@ void ReplicatedPG::sub_op_push(MOSDSubOp *op) } // clone bits - for (map >::iterator p = clone_subsets.begin(); + for (map >::const_iterator p = clone_subsets.begin(); p != clone_subsets.end(); - p++) - for (map::iterator q = p->second.m.begin(); - q != p->second.m.end(); - q++) { - dout(15) << " clone_range " << p->first << " " << q->first << "~" << q->second << dendl; - t->clone_range(coll_t::build_pg_coll(info.pgid), p->first, soid, q->first, q->second); + ++p) + { + for (interval_set::const_iterator q = p->second.begin(); + q != p->second.end(); + ++q) + { + dout(15) << " clone_range " << p->first << " " + << q.get_start() << "~" << q.get_len() << dendl; + t->clone_range(coll_t::build_pg_coll(info.pgid), p->first, soid, + q.get_start(), q.get_len()); } + } if (data_subset.empty()) t->touch(coll_t::build_pg_coll(info.pgid), soid); @@ -3877,12 +3883,12 @@ int ReplicatedPG::_scrub(ScrubMap& scrubmap, int& errors, int& fixed) // subtract off any clone overlap for (map >::iterator q = snapset.clone_overlap.begin(); q != snapset.clone_overlap.end(); - q++) { - for (map::iterator r = q->second.m.begin(); - r != q->second.m.end(); - r++) { - stat.num_bytes -= r->second; - stat.num_kb -= SHIFT_ROUND_UP(r->first+r->second, 10) - (r->first >> 10); + ++q) { + for (interval_set::const_iterator r = q->second.begin(); + r != q->second.end(); + ++r) { + stat.num_bytes -= r.get_len(); + stat.num_kb -= SHIFT_ROUND_UP(r.get_start()+r.get_len(), 10) - (r.get_start() >> 10); } } } diff --git a/src/osd/osd_types.h b/src/osd/osd_types.h index 4eedb91bd82e3..744abd1f9d7cb 100644 --- a/src/osd/osd_types.h +++ b/src/osd/osd_types.h @@ -798,7 +798,7 @@ struct pg_pool_t { __u8 struct_v = CEPH_PG_POOL_VERSION; ::encode(struct_v, bl); v.num_snaps = snaps.size(); - v.num_removed_snap_intervals = removed_snaps.m.size(); + v.num_removed_snap_intervals = removed_snaps.num_intervals(); ::encode(v, bl); ::encode_nohead(snaps, bl); removed_snaps.encode_nohead(bl); -- 2.39.5