From a9ec3be2fd845a2f7ebed3031ecc1802d0dc42a3 Mon Sep 17 00:00:00 2001 From: Florent Rougon Date: Mon, 13 Nov 2017 12:26:14 +0100 Subject: [PATCH] SGSharedPtr: the move constructor and move assignment operator are now 'noexcept' This automatically makes SGSharedPtr more efficient when used in standard containers (among others). See below for the benchmark details. Mark as 'noexcept' (after checking it's legitimate!) the SGSharedPtr and SGReferenced methods required for SGSharedPtr's move constructor and move assignment operator to be guaranteed 'noexcept'. Benchmark --------- I measured a 25 % speedup with g++ 6.3.0 on Linux amd64, CFLAGS=-Wall -O2 as compared to commit 18f04842490dd6c60a9a595f394d97bb4f699a06 (which is just before my changes to SGSharedPtr.hxx) on the following test code, called with: nbIterations = 3000000 minSize = 0 maxSize = 200 ------------------------------------------------------------------------ static std::default_random_engine randomNumbersGenerator; class SGReferencedTestClass : public SGReferenced { int i; }; void SGSharedPtr_perfTest(std::size_t nbIterations, std::size_t minSize, std::size_t maxSize) { using Ref = SGSharedPtr; std::uniform_int_distribution sizeDist(minSize, maxSize); auto randomSize = std::bind(sizeDist, randomNumbersGenerator); std::chrono::time_point start, end; start = std::chrono::system_clock::now(); std::vector v; for (std::size_t i=0; i < nbIterations; i++) { v = std::vector{}; // start anew for (std::size_t j=0; j < randomSize(); j++) { auto p = Ref(new SGReferencedTestClass()); v.emplace_back(std::move(p)); } std::shuffle(v.begin(), v.end(), randomNumbersGenerator); std::sort(v.begin(), v.end()); } end = std::chrono::system_clock::now(); std::chrono::duration elapsedSecs = end - start; std::cout << elapsedSecs.count() << "\n"; // duration in seconds } ------------------------------------------------------------------------ Basically, these gains can be explained by the fact that copying an SGSharedPtr requires to test SGReferenced::ref, increase the refcount, and then when the object is destroyed, test again SGReferenced::ref, decrease the refcount and test it in order to maybe delete. With the move constructor and move assignment operator, copying the argument is never necessary: its raw pointer can be swapped with the one contained in *this, which is very fast. For the move constructor, this is all that is needed; move assignment just needs one reset() call after that in order to release the resource from the moved-from shared pointer. --- simgear/structure/SGReferenced.hxx | 2 +- simgear/structure/SGSharedPtr.hxx | 17 ++++++++++------- 2 files changed, 11 insertions(+), 8 deletions(-) diff --git a/simgear/structure/SGReferenced.hxx b/simgear/structure/SGReferenced.hxx index 4c7fabd2..8e11cc66 100644 --- a/simgear/structure/SGReferenced.hxx +++ b/simgear/structure/SGReferenced.hxx @@ -43,7 +43,7 @@ public: static unsigned get(const SGReferenced* ref) { if (ref) return ++(ref->_refcount); else return 0; } - static unsigned put(const SGReferenced* ref) + static unsigned put(const SGReferenced* ref) noexcept { if (ref) return --(ref->_refcount); else return 0; } static unsigned count(const SGReferenced* ref) { if (ref) return ref->_refcount; else return 0; } diff --git a/simgear/structure/SGSharedPtr.hxx b/simgear/structure/SGSharedPtr.hxx index ce242f05..fdc64415 100644 --- a/simgear/structure/SGSharedPtr.hxx +++ b/simgear/structure/SGSharedPtr.hxx @@ -20,8 +20,9 @@ #ifndef SGSharedPtr_HXX #define SGSharedPtr_HXX +#include + #include "SGReferenced.hxx" -#include template class SGWeakPtr; @@ -50,13 +51,15 @@ class SGSharedPtr { public: typedef T element_type; - SGSharedPtr(void) : _ptr(0) + SGSharedPtr(void) noexcept + : _ptr(0) {} SGSharedPtr(T* ptr) : _ptr(ptr) { get(_ptr); } SGSharedPtr(const SGSharedPtr& p) : _ptr(p.get()) { get(_ptr); } - SGSharedPtr(SGSharedPtr&& other) : SGSharedPtr() + SGSharedPtr(SGSharedPtr&& other) noexcept + : SGSharedPtr() { swap(other); } template SGSharedPtr(const SGSharedPtr& p) : _ptr(p.get()) @@ -70,7 +73,7 @@ public: SGSharedPtr& operator=(const SGSharedPtr& p) { reset(p.get()); return *this; } - SGSharedPtr& operator=(SGSharedPtr&& p) + SGSharedPtr& operator=(SGSharedPtr&& p) noexcept { // Whether self-move is to be supported at all is controversial, let's // take the conservative approach for now if (this != &p) { @@ -99,7 +102,7 @@ public: { return _ptr; } T* release() { T* tmp = _ptr; _ptr = 0; T::put(tmp); return tmp; } - void reset() + void reset() noexcept { if (!T::put(_ptr)) delete _ptr; _ptr = 0; } void reset(T* p) { SGSharedPtr(p).swap(*this); } @@ -114,8 +117,8 @@ public: void clear() { reset(); } - void swap(SGSharedPtr& other) - { std::swap(_ptr, other._ptr); } + void swap(SGSharedPtr& other) noexcept + { simgear::noexceptSwap(_ptr, other._ptr); } private: void assignNonRef(T* p)