From 7cf03ab15328126cc7ccdd1e914c79d42581dbf0 Mon Sep 17 00:00:00 2001 From: Radoslaw Zarzynski Date: Mon, 8 Oct 2018 02:25:51 +0200 Subject: [PATCH] common: switch bufferlist to use boost::intrusive for _buffers. Signed-off-by: Radoslaw Zarzynski --- src/common/buffer.cc | 38 ++++++++++--------- src/include/buffer.h | 56 +++++++++++++++++++++++----- src/test/rgw/test_rgw_compression.cc | 2 +- 3 files changed, 67 insertions(+), 29 deletions(-) diff --git a/src/common/buffer.cc b/src/common/buffer.cc index 40143964d47..5c4a43b74ad 100644 --- a/src/common/buffer.cc +++ b/src/common/buffer.cc @@ -1322,7 +1322,7 @@ using namespace ceph; void buffer::list::rebuild() { if (_len == 0) { - _buffers.clear(); + _buffers.clear_and_dispose(ptr_node::disposer()); return; } ptr nb; @@ -1341,9 +1341,9 @@ using namespace ceph; pos += node.length(); } _memcopy_count += pos; - _buffers.clear(); + _buffers.clear_and_dispose(ptr_node::disposer()); if (nb.length()) - _buffers.push_back(nb); + _buffers.push_back(ptr_node::create(nb)); invalidate_crc(); last_p = begin(); } @@ -1387,8 +1387,11 @@ using namespace ceph; << " not ok" << std::endl; */ offset += p->length(); - unaligned.push_back(*p); - p = _buffers.erase(p); + // no need to reallocate, relinking is enough thankfully to bi::list. + auto after = _buffers.erase(p); + unaligned._buffers.push_back(*p); + unaligned._len += p->length(); + p = after; } while (p != std::end(_buffers) && (!p->is_aligned(align_memory) || !p->is_n_align_sized(align_size) || @@ -1398,7 +1401,7 @@ using namespace ceph; unaligned.rebuild(nb); _memcopy_count += unaligned._len; } - _buffers.insert(p, unaligned._buffers.front()); + _buffers.insert(p, ptr_node::create(unaligned._buffers.front())); } last_p = begin(); @@ -1432,9 +1435,7 @@ using namespace ceph; _len += bl._len; if (!(flags & CLAIM_ALLOW_NONSHAREABLE)) bl.make_shareable(); - std::move(std::begin(bl._buffers), std::end(bl._buffers), - std::back_inserter(_buffers)); - bl._buffers.clear(); + _buffers.splice(std::end(_buffers), bl._buffers); bl._len = 0; bl.last_p = bl.begin(); } @@ -1562,7 +1563,7 @@ using namespace ceph; { _len += bl._len; for (const auto& node : bl._buffers) { - _buffers.push_back(node); + _buffers.push_back(ptr_node::create(node)); } } @@ -1598,10 +1599,10 @@ using namespace ceph; void buffer::list::prepend_zero(unsigned len) { - ptr bp(len); + auto& bp = ptr_node::create(len); bp.zero(false); _len += len; - _buffers.emplace_front(std::move(bp)); + _buffers.push_front(bp); } void buffer::list::append_zero(unsigned len) @@ -1613,9 +1614,10 @@ using namespace ceph; len -= need; } if (len) { - ptr bp = buffer::create_page_aligned(len); + auto& bp = ptr_node::create(buffer::create_page_aligned(len)); bp.zero(false); - append(std::move(bp)); + _len += bp.length(); + _buffers.push_back(bp); } } @@ -1687,7 +1689,7 @@ using namespace ceph; // partial? if (off + len < curbuf->length()) { //cout << "copying partial of " << *curbuf << std::endl; - _buffers.push_back( ptr( *curbuf, off, len ) ); + _buffers.push_back( ptr_node::create( *curbuf, off, len ) ); _len += len; break; } @@ -1695,7 +1697,7 @@ using namespace ceph; // through end //cout << "copying end (all?) of " << *curbuf << std::endl; unsigned howmuch = curbuf->length() - off; - _buffers.push_back( ptr( *curbuf, off, howmuch ) ); + _buffers.push_back( ptr_node::create( *curbuf, off, howmuch ) ); _len += howmuch; len -= howmuch; off = 0; @@ -1735,7 +1737,7 @@ using namespace ceph; // add a reference to the front bit // insert it before curbuf (which we'll hose) //cout << "keeping front " << off << " of " << *curbuf << std::endl; - _buffers.insert( curbuf, ptr( *curbuf, 0, off ) ); + _buffers.insert( curbuf, ptr_node::create( *curbuf, 0, off ) ); _len += off; } @@ -1758,7 +1760,7 @@ using namespace ceph; if (claim_by) claim_by->append( *curbuf, off, howmuch ); _len -= (*curbuf).length(); - curbuf = _buffers.erase( curbuf ); + curbuf = _buffers.erase_and_dispose( curbuf, ptr_node::disposer() ); len -= howmuch; off = 0; } diff --git a/src/include/buffer.h b/src/include/buffer.h index c4bd7d14298..d89ce2a2a26 100644 --- a/src/include/buffer.h +++ b/src/include/buffer.h @@ -50,6 +50,8 @@ #include #include +#include + #include "page.h" #include "crc32c.h" #include "buffer_fwd.h" @@ -379,13 +381,39 @@ namespace buffer CEPH_BUFFER_API { }; + class ptr_node : public ptr, public boost::intrusive::list_base_hook<> { + template + ptr_node(Args&&... args) : ptr(std::forward(args)...) { + } + + ptr_node(const ptr_node&) = default; + public: + + ~ptr_node() = default; + + template + static ptr_node& create(Args&&... args) { + return *new ptr_node(std::forward(args)...); + } + + struct cloner { + ptr_node* operator()(const ptr_node& clone_this) { + return new ptr_node(clone_this); + } + }; + struct disposer { + void operator()(ptr_node* const delete_this) { + delete delete_this; + } + }; + }; /* * list - the useful bit! */ class CEPH_BUFFER_API list { public: - typedef std::list buffers_t; + typedef boost::intrusive::list buffers_t; class iterator; private: @@ -703,20 +731,27 @@ namespace buffer CEPH_BUFFER_API { reserve(prealloc); } - list(const list& other) : _buffers(other._buffers), _len(other._len), + list(const list& other) : _len(other._len), _memcopy_count(other._memcopy_count), last_p(this) { + _buffers.clone_from(other._buffers, + ptr_node::cloner(), ptr_node::disposer()); make_shareable(); } list(list&& other) noexcept; + + ~list() { + _buffers.clear_and_dispose(ptr_node::disposer()); + } + list& operator= (const list& other) { if (this != &other) { - _buffers = other._buffers; + _buffers.clone_from(other._buffers, + ptr_node::cloner(), ptr_node::disposer()); _len = other._len; make_shareable(); } return *this; } - list& operator= (list&& other) noexcept { _buffers = std::move(other._buffers); _len = other._len; @@ -729,8 +764,8 @@ namespace buffer CEPH_BUFFER_API { uint64_t get_wasted_space() const; unsigned get_num_buffers() const { return _buffers.size(); } - const ptr& front() const { return _buffers.front(); } - const ptr& back() const { return _buffers.back(); } + const ptr_node& front() const { return _buffers.front(); } + const ptr_node& back() const { return _buffers.back(); } int get_mempool() const; void reassign_to_mempool(int pool); @@ -775,7 +810,7 @@ namespace buffer CEPH_BUFFER_API { // modifiers void clear() noexcept { - _buffers.clear(); + _buffers.clear_and_dispose(ptr_node::disposer()); _len = 0; _memcopy_count = 0; last_p = begin(); @@ -784,17 +819,18 @@ namespace buffer CEPH_BUFFER_API { void push_back(const ptr& bp) { if (bp.length() == 0) return; - _buffers.push_back(bp); + _buffers.push_back(ptr_node::create(bp)); _len += bp.length(); } void push_back(ptr&& bp) { if (bp.length() == 0) return; _len += bp.length(); - _buffers.push_back(std::move(bp)); + _buffers.push_back(ptr_node::create(std::move(bp))); } void push_back(raw *r) { - push_back(ptr(r)); + _buffers.push_back(ptr_node::create(r)); + _len += _buffers.back().length(); } void zero(); diff --git a/src/test/rgw/test_rgw_compression.cc b/src/test/rgw/test_rgw_compression.cc index 1718ea1a889..936b8f7c979 100644 --- a/src/test/rgw/test_rgw_compression.cc +++ b/src/test/rgw/test_rgw_compression.cc @@ -12,7 +12,7 @@ public: int handle_data(bufferlist& bl, off_t bl_ofs, off_t bl_len) override { - auto bl_buffers = bl.buffers(); + auto& bl_buffers = bl.buffers(); auto i = bl_buffers.begin(); while (bl_len > 0) { -- 2.39.5