From: g0dil Date: Thu, 2 Sep 2010 09:09:35 +0000 (+0000) Subject: Packets: Refactor refcounting for performance X-Git-Url: http://g0dil.de/git?a=commitdiff_plain;h=720400ed1b874ea96481a29f0812145bb2785d40;p=senf.git Packets: Refactor refcounting for performance git-svn-id: https://svn.berlios.de/svnroot/repos/senf/trunk@1707 270642c3-0616-0410-b53a-bc976706d245 --- diff --git a/senf/Packets/PacketImpl.cc b/senf/Packets/PacketImpl.cc index f9c598f..b1c9791 100644 --- a/senf/Packets/PacketImpl.cc +++ b/senf/Packets/PacketImpl.cc @@ -47,6 +47,19 @@ prefix_ senf::detail::PacketImpl::~PacketImpl() eraseInterpreters(interpreters_.begin(), interpreters_.end()); } +prefix_ void senf::detail::PacketImpl::release() +{ + SENF_ASSERT(refcount_ >= 1, "Internal failure: Releasing dead PacketImpl ??"); + // uah ... we need to be extremely careful here. If refcount_ is 1, we want to commit suicide, + // however the destructor will remove all PacketInterpreters from the list and will thereby + // decrement refcount -> only decrement refcount_ when *not* calling delete (otherwise + // the assert above will fail) + if (refcount_ == 1) + delete this; + else + -- refcount_; +} + // interpreter chain prefix_ void senf::detail::PacketImpl::appendInterpreter(PacketInterpreterBase * p) diff --git a/senf/Packets/PacketImpl.cci b/senf/Packets/PacketImpl.cci index 63c330c..70ac003 100644 --- a/senf/Packets/PacketImpl.cci +++ b/senf/Packets/PacketImpl.cci @@ -126,9 +126,9 @@ prefix_ senf::detail::PacketImpl::PacketImpl(size_type size, byte initValue) // reference/memory management -prefix_ void senf::detail::PacketImpl::add_ref(refcount_t n) +prefix_ void senf::detail::PacketImpl::add_ref() { - refcount_ += n; + ++ refcount_; } prefix_ senf::detail::PacketImpl::refcount_t senf::detail::PacketImpl::refcount() @@ -231,21 +231,6 @@ prefix_ senf::detail::PacketImpl::size_type senf::detail::PacketImpl::capacity() return data_.capacity(); } -// This function has a problem being inlined. Somehow, often when calling this, the size of the -// resulting inlined code would be huge? - -prefix_ void senf::detail::PacketImpl::release(refcount_t n) -{ - SENF_ASSERT(refcount_ >= n, "Internal failure: Releasing dead PacketImpl ??"); - // uah ... we need to be extremely careful here. If refcount_ is n, we want to commit suicide, - // however the destructor will remove all PacketInterpreters from the list and will thereby - // decrement refcount -> only decrement refcount_ when *not* calling delete - if (refcount_ == n) - delete this; - else - refcount_ -= n; -} - // Annotations prefix_ void * senf::detail::PacketImpl::annotation(AnnotationRegistry::key_type key) diff --git a/senf/Packets/PacketImpl.hh b/senf/Packets/PacketImpl.hh index 0b4f905..2900044 100644 --- a/senf/Packets/PacketImpl.hh +++ b/senf/Packets/PacketImpl.hh @@ -131,8 +131,8 @@ namespace detail { // rerference/memory management - void add_ref(refcount_t n=1); - void release(refcount_t n=1); + void add_ref(); + void release(); refcount_t refcount() const; // Interpreter chain diff --git a/senf/Packets/PacketImpl.test.cc b/senf/Packets/PacketImpl.test.cc index bfaaa36..c87accf 100644 --- a/senf/Packets/PacketImpl.test.cc +++ b/senf/Packets/PacketImpl.test.cc @@ -59,9 +59,9 @@ SENF_AUTO_UNIT_TEST(packetImpl_mem) // refcount .. - p->add_ref(2); - BOOST_CHECK_EQUAL(p->refcount(), 3); - p->release(2); + p->add_ref(); + BOOST_CHECK_EQUAL(p->refcount(), 2); + p->release(); BOOST_CHECK_EQUAL(p->refcount(), 1); { @@ -75,7 +75,7 @@ SENF_AUTO_UNIT_TEST(packetImpl_mem) senf::pool_alloc_mixin< senf::PacketInterpreter >::allocCounter(), 1u); #endif senf::PacketInterpreterBase::ptr pi2 (pi); - BOOST_CHECK_EQUAL(p->refcount(), 3); + BOOST_CHECK_EQUAL(p->refcount(), 2); } BOOST_CHECK_EQUAL(p->refcount(),1); diff --git a/senf/Packets/PacketInterpreter.cc b/senf/Packets/PacketInterpreter.cc index 62b912c..77627d3 100644 --- a/senf/Packets/PacketInterpreter.cc +++ b/senf/Packets/PacketInterpreter.cc @@ -101,6 +101,24 @@ prefix_ void senf::PacketInterpreterBase::finalizeTo(ptr other) finalizeThis(); } +// reference/memory management + +prefix_ void senf::PacketInterpreterBase::add_ref() +{ + if (impl_ && !refcount()) + impl_->add_ref(); + intrusive_refcount_t::add_ref(); +} + +prefix_ bool senf::PacketInterpreterBase::release() +{ + if (impl_ && refcount()==1) + // This call will set impl_ to 0 if we just removed the last reference ... + impl_->release(); + if (intrusive_refcount_t::release() && !impl_) + delete this; +} + /////////////////////////////////////////////////////////////////////////// // senf::PacketInterpreterBase::Factory diff --git a/senf/Packets/PacketInterpreter.cci b/senf/Packets/PacketInterpreter.cci index b5e84bb..40d2531 100644 --- a/senf/Packets/PacketInterpreter.cci +++ b/senf/Packets/PacketInterpreter.cci @@ -143,45 +143,45 @@ senf::PacketInterpreterBase::appendClone(detail::PacketImpl * impl, range r) //////////////////////////////////////// // private members -// reference/memory management - -prefix_ void senf::PacketInterpreterBase::add_ref() -{ - intrusive_refcount_t::add_ref(); - if (impl_) - impl_->add_ref(); -} - -prefix_ bool senf::PacketInterpreterBase::release() -{ - if (impl_) - // This call will set impl_ to 0 if we just removed the last reference ... - impl_->release(); - return intrusive_refcount_t::release() && !impl_; -} - // containment management. Only to be called by PacketImpl. prefix_ void senf::PacketInterpreterBase::assignImpl(detail::PacketImpl * impl) { SENF_ASSERT(!impl_, "Internal failure: PacketInterpreter added to two Packets"); impl_ = impl; - impl_->add_ref(refcount()); + if (refcount()) + impl_->add_ref(); } prefix_ void senf::PacketInterpreterBase::releaseImpl() { SENF_ASSERT(impl_, "Internal failure: release of lone PacketInterpreter"); - refcount_t refc (refcount()); - if (refc) { - impl_->release(refc); + if (refcount()) { + detail::PacketImpl * i (impl_); impl_ = 0; + // This call might delete this ... + i->release(); } else { impl_ = 0; delete this; } } +prefix_ void senf::intrusive_ptr_add_ref(PacketInterpreterBase const * p) +{ + if (! p->refcount()) + const_cast(p)->add_ref(); + else + const_cast(p)->intrusive_refcount_base::add_ref(); +} + +prefix_ void senf::intrusive_ptr_release(PacketInterpreterBase const * p) +{ + if (p->refcount() <= 1) + const_cast(p)->release(); + else + const_cast(p)->intrusive_refcount_base::release(); +} ///////////////////////////////cci.e/////////////////////////////////////// #undef prefix_ diff --git a/senf/Packets/PacketInterpreter.hh b/senf/Packets/PacketInterpreter.hh index 17621a4..d8fd813 100644 --- a/senf/Packets/PacketInterpreter.hh +++ b/senf/Packets/PacketInterpreter.hh @@ -226,8 +226,14 @@ namespace senf { template friend class PacketInterpreter; friend class detail::packet::test::TestDriver; friend class PacketParserBase; + + friend void senf::intrusive_ptr_add_ref(PacketInterpreterBase const *); + friend void senf::intrusive_ptr_release(PacketInterpreterBase const *); }; + void intrusive_ptr_add_ref(PacketInterpreterBase const * p); + void intrusive_ptr_release(PacketInterpreterBase const * p); + /** \brief Internal: Concrete packet interpreter \internal diff --git a/senf/Utils/intrusive_refcount.cci b/senf/Utils/intrusive_refcount.cci index 69cb1f6..9dbc6e8 100644 --- a/senf/Utils/intrusive_refcount.cci +++ b/senf/Utils/intrusive_refcount.cci @@ -32,11 +32,13 @@ ///////////////////////////////cci.p/////////////////////////////////////// prefix_ senf::intrusive_refcount_base::refcount_t senf::intrusive_refcount_base::refcount() + const { return refcount_; } prefix_ bool senf::intrusive_refcount_base::is_shared() + const { return refcount()>1; } diff --git a/senf/Utils/intrusive_refcount.hh b/senf/Utils/intrusive_refcount.hh index d612f4a..b1cb0a9 100644 --- a/senf/Utils/intrusive_refcount.hh +++ b/senf/Utils/intrusive_refcount.hh @@ -49,8 +49,8 @@ namespace senf { virtual ~intrusive_refcount_base(); - refcount_t refcount(); ///< current refcount - bool is_shared(); ///< return \c true if refcount() > 1 + refcount_t refcount() const; ///< current refcount + bool is_shared() const; ///< return \c true if refcount() > 1 protected: intrusive_refcount_base();