From 425b40d9a28cf189836582614f5d6934e0a6317a Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Tobias=20G=C3=B6dderz?= <github@tobias.goedderz.info>
Date: Thu, 18 Apr 2019 14:32:30 +0200
Subject: [PATCH] Feature/improve refcount performance (#8783)

---
 arangod/Aql/InputAqlItemRow.h         |  6 +++---
 arangod/Aql/SharedAqlItemBlockPtr.cpp | 18 ++++++++++--------
 arangod/Aql/SharedAqlItemBlockPtr.h   | 21 ++++++++++++++++-----
 3 files changed, 29 insertions(+), 16 deletions(-)

diff --git a/arangod/Aql/InputAqlItemRow.h b/arangod/Aql/InputAqlItemRow.h
index 53bbaad924..988fa52c00 100644
--- a/arangod/Aql/InputAqlItemRow.h
+++ b/arangod/Aql/InputAqlItemRow.h
@@ -97,7 +97,7 @@ class InputAqlItemRow {
     return a;
   }
 
-  std::size_t getNrRegisters() const { return block().getNrRegs(); }
+  std::size_t getNrRegisters() const noexcept { return block().getNrRegs(); }
 
   bool operator==(InputAqlItemRow const& other) const noexcept {
     TRI_ASSERT(isInitialized());
@@ -149,12 +149,12 @@ class InputAqlItemRow {
 
  private:
 
-  inline AqlItemBlock& block() {
+  inline AqlItemBlock& block() noexcept {
     TRI_ASSERT(_block != nullptr);
     return *_block;
   }
 
-  inline AqlItemBlock const& block() const {
+  inline AqlItemBlock const& block() const noexcept {
     TRI_ASSERT(_block != nullptr);
     return *_block;
   }
diff --git a/arangod/Aql/SharedAqlItemBlockPtr.cpp b/arangod/Aql/SharedAqlItemBlockPtr.cpp
index 233aaa8db3..17c50958f6 100644
--- a/arangod/Aql/SharedAqlItemBlockPtr.cpp
+++ b/arangod/Aql/SharedAqlItemBlockPtr.cpp
@@ -27,14 +27,16 @@
 using namespace arangodb;
 using namespace arangodb::aql;
 
-void SharedAqlItemBlockPtr::decrRefCount() noexcept {
-  if (_aqlItemBlock != nullptr) {
-    _aqlItemBlock->decrRefCount();
-    if (_aqlItemBlock->getRefCount() == 0) {
-      itemBlockManager().returnBlock(_aqlItemBlock);
-      TRI_ASSERT(_aqlItemBlock == nullptr);
-    }
-  }
+/*
+ * returnBlock() and itemBlockManager() cannot be moved into the header file due
+ * to the circular dependency between SharedAqlItemBlockPtr and
+ * AqlItemBlockManager. However, by extracting returnBlock(), at least the often
+ * called part of decrRefCount() can by inlined.
+ */
+
+void SharedAqlItemBlockPtr::returnBlock() noexcept {
+  itemBlockManager().returnBlock(_aqlItemBlock);
+  TRI_ASSERT(_aqlItemBlock == nullptr);
 }
 
 AqlItemBlockManager& SharedAqlItemBlockPtr::itemBlockManager() const noexcept {
diff --git a/arangod/Aql/SharedAqlItemBlockPtr.h b/arangod/Aql/SharedAqlItemBlockPtr.h
index 21a94a2b9f..130497da2d 100644
--- a/arangod/Aql/SharedAqlItemBlockPtr.h
+++ b/arangod/Aql/SharedAqlItemBlockPtr.h
@@ -62,8 +62,8 @@ class SharedAqlItemBlockPtr {
 
   inline void swap(SharedAqlItemBlockPtr& other) noexcept;
 
-  inline bool operator==(std::nullptr_t) noexcept;
-  inline bool operator!=(std::nullptr_t) noexcept;
+  inline bool operator==(std::nullptr_t) const noexcept;
+  inline bool operator!=(std::nullptr_t) const noexcept;
 
   inline bool operator==(SharedAqlItemBlockPtr const&) const noexcept;
   inline bool operator!=(SharedAqlItemBlockPtr const&) const noexcept;
@@ -71,10 +71,12 @@ class SharedAqlItemBlockPtr {
  private:
   inline void incrRefCount() const noexcept;
   // decrRefCount returns ("frees") _aqlItemBlock if the ref count reaches 0
-  void decrRefCount() noexcept;
+  inline void decrRefCount() noexcept;
 
   AqlItemBlockManager& itemBlockManager() const noexcept;
 
+  void returnBlock() noexcept;
+
  private:
   AqlItemBlock* _aqlItemBlock;
 };
@@ -162,11 +164,11 @@ void SharedAqlItemBlockPtr::incrRefCount() const noexcept {
   }
 }
 
-bool SharedAqlItemBlockPtr::operator==(std::nullptr_t) noexcept {
+bool SharedAqlItemBlockPtr::operator==(std::nullptr_t) const noexcept {
   return _aqlItemBlock == nullptr;
 }
 
-bool SharedAqlItemBlockPtr::operator!=(std::nullptr_t) noexcept {
+bool SharedAqlItemBlockPtr::operator!=(std::nullptr_t) const noexcept {
   return _aqlItemBlock != nullptr;
 }
 
@@ -196,6 +198,15 @@ void SharedAqlItemBlockPtr::swap(SharedAqlItemBlockPtr& other) noexcept {
   other._aqlItemBlock = tmp;
 }
 
+void arangodb::aql::SharedAqlItemBlockPtr::decrRefCount() noexcept {
+  if (_aqlItemBlock != nullptr) {
+    _aqlItemBlock->decrRefCount();
+    if (_aqlItemBlock->getRefCount() == 0) {
+      returnBlock();
+    }
+  }
+}
+
 }  // namespace aql
 }  // namespace arangodb