From f75d1822cc168222d48225d9a36ab42b589f9e08 Mon Sep 17 00:00:00 2001 From: Avi Kivity Date: Tue, 21 Oct 2014 11:20:12 +0300 Subject: [PATCH 1/4] core: special-case deleter for raw memory In many cases, a deleter is used to protect raw memory (e.g. a char array, not something with a destructor). In that case we can simply free() it, so, the deleter need not remember which destructor needs to be called. It does need to remember whether it's a raw object or not, so we take over the least significant bit and use it as a marker, and store the pointer to the object in the deleter, instead of using a proxy impl object to control actual deletion. If the deleter is subsequently share()d, we have to convert it back to the standard form, since the reference count lives in the impl object. --- core/deleter.hh | 38 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 38 insertions(+) diff --git a/core/deleter.hh b/core/deleter.hh index 7bc49bd5bb..b02f9e84b7 100644 --- a/core/deleter.hh +++ b/core/deleter.hh @@ -6,17 +6,23 @@ #define DELETER_HH_ #include +#include +#include class deleter { public: struct impl; + struct raw_object_tag {}; private: + // if bit 0 set, point to object to be freed directly. impl* _impl = nullptr; public: deleter() = default; deleter(const deleter&) = delete; deleter(deleter&& x) : _impl(x._impl) { x._impl = nullptr; } explicit deleter(impl* i) : _impl(i) {} + deleter(raw_object_tag tag, void* object) + : _impl(from_raw_object(object)) {} ~deleter(); deleter& operator=(deleter&& x); deleter& operator=(deleter&) = delete; @@ -30,6 +36,19 @@ public: this->~deleter(); new (this) deleter(i); } +private: + bool is_raw_object() const { + auto x = reinterpret_cast(_impl); + return x & 1; + } + void* to_raw_object() const { + auto x = reinterpret_cast(_impl); + return reinterpret_cast(x & ~uintptr_t(1)); + } + impl* from_raw_object(void* object) { + auto x = reinterpret_cast(object); + return reinterpret_cast(x | 1); + } }; struct deleter::impl { @@ -41,6 +60,10 @@ struct deleter::impl { inline deleter::~deleter() { + if (is_raw_object()) { + std::free(to_raw_object()); + return; + } if (_impl && --_impl->refs == 0) { delete _impl; } @@ -69,14 +92,29 @@ make_deleter(deleter next, Deleter d) { return deleter(new lambda_deleter_impl(std::move(next), std::move(d))); } +struct free_deleter_impl final : deleter::impl { + void* obj; + free_deleter_impl(void* obj) : impl(deleter()), obj(obj) {} + virtual ~free_deleter_impl() override { std::free(obj); } +}; + inline deleter deleter::share() { if (!_impl) { return deleter(); } + if (is_raw_object()) { + _impl = new free_deleter_impl(to_raw_object()); + } ++_impl->refs; return deleter(_impl); } +inline +deleter +make_free_deleter(void* obj) { + return deleter(deleter::raw_object_tag(), obj); +} + #endif /* DELETER_HH_ */ From fe8ed688787457d543d77f520dc89738109a8fd3 Mon Sep 17 00:00:00 2001 From: Avi Kivity Date: Tue, 21 Oct 2014 11:23:41 +0300 Subject: [PATCH 2/4] temporary_buffer: use raw object deleter to destroy allocated buffer Removes an allocation. --- core/temporary_buffer.hh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/core/temporary_buffer.hh b/core/temporary_buffer.hh index 73a88f4c49..9abde45e32 100644 --- a/core/temporary_buffer.hh +++ b/core/temporary_buffer.hh @@ -18,8 +18,8 @@ class temporary_buffer { deleter _deleter; public: explicit temporary_buffer(size_t size) - : _buffer(new CharType[size]), _size(size) - , _deleter(make_deleter(deleter(), [b = _buffer] { delete[] b; })) {} + : _buffer(static_cast(malloc(size * sizeof(CharType)))), _size(size) + , _deleter(make_free_deleter(_buffer)) {} //explicit temporary_buffer(CharType* borrow, size_t size) : _buffer(borrow), _size(size) {} temporary_buffer() = delete; temporary_buffer(const temporary_buffer&) = delete; From 61782fcc05d3f4b2302050aba7eef17b6d1936a8 Mon Sep 17 00:00:00 2001 From: Avi Kivity Date: Tue, 21 Oct 2014 11:24:27 +0300 Subject: [PATCH 3/4] packet: add a vectored constructor with a deleter Add packet(Iterator, Iterator, deleter). (unfortunately we have both a template version with a template parameter named Deleter, and a non-template version with a parameter called deleter. Need to sort the naming out). --- net/packet.hh | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/net/packet.hh b/net/packet.hh index a35601d352..1aa8e087b4 100644 --- a/net/packet.hh +++ b/net/packet.hh @@ -175,6 +175,9 @@ public: // build packet with iterator template packet(Iterator begin, Iterator end, Deleter del); + // build packet with iterator + template + packet(Iterator begin, Iterator end, deleter del); // append fragment (copying new fragment) packet(packet&& x, fragment frag); // prepend fragment (copying new fragment, with header optimization) @@ -317,6 +320,18 @@ packet::packet(Iterator begin, Iterator end, Deleter del) { std::copy(begin, end, _impl->_frags); } +template +inline +packet::packet(Iterator begin, Iterator end, deleter del) { + unsigned nr_frags = 0, len = 0; + nr_frags = std::distance(begin, end); + std::for_each(begin, end, [&] (fragment& frag) { len += frag.size; }); + _impl = impl::allocate(nr_frags); + _impl->_deleter = std::move(del); + _impl->_len = len; + _impl->_nr_frags = nr_frags; + std::copy(begin, end, _impl->_frags); +} inline packet::packet(packet&& x, fragment frag) From 91782ac6a2b8434b646dac0aae04ff30ddee2b30 Mon Sep 17 00:00:00 2001 From: Avi Kivity Date: Tue, 21 Oct 2014 11:25:43 +0300 Subject: [PATCH 4/4] virtio: optimize single-buffer packet deleter Instead of allocating a vector to store the buffers to be destroyed, in the case of a single buffer, use an ordinary free deleter. This doesn't currently help much because the packet is share()d later on, but if we may be able to eliminate the sharing one day. --- net/virtio.cc | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/net/virtio.cc b/net/virtio.cc index c622b43641..63abfa4b84 100644 --- a/net/virtio.cc +++ b/net/virtio.cc @@ -376,7 +376,7 @@ class virtio_net_device : public net::device { vring _ring; unsigned _remaining_buffers = 0; std::vector _fragments; - std::vector> _deleters; + std::vector> _deleters; public: rxq(virtio_net_device& _if, vring::config config, readable_eventfd notified, writeable_eventfd kicked); @@ -506,7 +506,7 @@ virtio_net_device::rxq::prepare_buffers() { struct single_buffer_and_comletion : std::array { promise completed; } bc; - std::unique_ptr buf(new char[4096]); + std::unique_ptr buf(reinterpret_cast(malloc(4096))); vring::buffer& b = bc[0]; b.addr = virt_to_phys(buf.get()); b.len = 4096; @@ -528,12 +528,19 @@ virtio_net_device::rxq::prepare_buffers() { // Append current buffer _fragments.emplace_back(fragment{frag_buf, frag_len}); - _deleters.emplace_back(buf.release()); + _deleters.push_back(std::move(buf)); _remaining_buffers--; // Last buffer if (_remaining_buffers == 0) { - packet p(_fragments.begin(), _fragments.end(), [deleters = std::move(_deleters)] () mutable { deleters.clear(); }); + deleter del; + if (_deleters.size() == 1) { + del = make_free_deleter(_deleters[0].release()); + _deleters.clear(); + } else { + del = make_deleter(deleter(), [deleters = std::move(_deleters)] {}); + } + packet p(_fragments.begin(), _fragments.end(), std::move(del)); _dev._rx_ready = _dev._rx_ready.then([this, p = std::move(p)] () mutable { return _dev.queue_rx_packet(std::move(p)); });