Merge pull request #1401 from mbutrovich/friday_night

RWSet performance fixes, LockFreeArray performance fixes and tests

src/common/container/lock_free_array.cpp

@@ -10,12 +10,10 @@
 //
 //===----------------------------------------------------------------------===//
 
-#include <memory>
-
 #include "common/container/lock_free_array.h"
+#include "common/internal_types.h"
 #include "common/logger.h"
 #include "common/macros.h"
-#include "common/internal_types.h"
 
 namespace peloton {
 
@@ -38,26 +36,24 @@ template <typename ValueType>
 LOCK_FREE_ARRAY_TYPE::~LockFreeArray() { lock_free_array.clear(); }
 
 template <typename ValueType>
-bool LOCK_FREE_ARRAY_TYPE::Update(const std::size_t &offset, ValueType value) {
+void LOCK_FREE_ARRAY_TYPE::Update(const std::size_t &offset,
+                                  const ValueType &value) {
   LOG_TRACE("Update at %lu", offset);
-  PELOTON_ASSERT(lock_free_array.size() >= offset + 1);
+  PELOTON_ASSERT(lock_free_array.size() > offset);
   lock_free_array.at(offset) = value;
-  return true;
 }
 
 template <typename ValueType>
-bool LOCK_FREE_ARRAY_TYPE::Append(ValueType value) {
+void LOCK_FREE_ARRAY_TYPE::Append(const ValueType &value) {
   LOG_TRACE("Appended value.");
   lock_free_array.push_back(value);
-  return true;
 }
 
 template <typename ValueType>
-bool LOCK_FREE_ARRAY_TYPE::Erase(const std::size_t &offset,
+void LOCK_FREE_ARRAY_TYPE::Erase(const std::size_t &offset,
                                  const ValueType &invalid_value) {
   LOG_TRACE("Erase at %lu", offset);
   lock_free_array.at(offset) = invalid_value;
-  return true;
 }
 
 template <typename ValueType>
@@ -73,23 +69,12 @@ ValueType LOCK_FREE_ARRAY_TYPE::FindValid(
     const std::size_t &offset, const ValueType &invalid_value) const {
   LOG_TRACE("Find Valid at %lu", offset);
 
-  std::size_t valid_array_itr = 0;
-  std::size_t array_itr;
-  auto lock_free_array_offset = lock_free_array.size();
-  for (array_itr = 0; array_itr < lock_free_array_offset; array_itr++) {
-    auto value = lock_free_array.at(array_itr);
-    if (value != invalid_value) {
-      // Check offset
-      if (valid_array_itr == offset) {
-        return value;
-      }
-
-      // Update valid value count
-      valid_array_itr++;
-    }
+  ValueType value = invalid_value;
+  if ((lock_free_array.size() > offset)) {
+    value = lock_free_array.at(offset);
   }
 
-  return invalid_value;
+  return value;
 }
 
 template <typename ValueType>
@@ -99,10 +84,14 @@ template <typename ValueType>
 bool LOCK_FREE_ARRAY_TYPE::IsEmpty() const { return lock_free_array.empty(); }
 
 template <typename ValueType>
-void LOCK_FREE_ARRAY_TYPE::Clear() { lock_free_array.clear(); }
+void LOCK_FREE_ARRAY_TYPE::Clear() {
+  // Intel docs: To free internal arrays, call shrink_to_fit() after clear().
+  lock_free_array.clear();
+  lock_free_array.shrink_to_fit();
+}
 
 template <typename ValueType>
-bool LOCK_FREE_ARRAY_TYPE::Contains(const ValueType &value) {
+bool LOCK_FREE_ARRAY_TYPE::Contains(const ValueType &value) const {
   bool exists = false;
 
   for (std::size_t array_itr = 0; array_itr < lock_free_array.size();

src/concurrency/timestamp_ordering_transaction_manager.cpp

@@ -653,14 +653,19 @@ ResultType TimestampOrderingTransactionManager::CommitTransaction(
   // 3. install a new tuple for insert operations.
   // Iterate through each item pointer in the read write set
 
-  // TODO (Pooja): This might be inefficient since we will have to get the
-  // tile_group_header for each entry. Check if this needs to be consolidated
+  oid_t last_tile_group_id = INVALID_OID;
+  storage::TileGroupHeader *tile_group_header = nullptr;
+
   for (const auto &tuple_entry : rw_set) {
     ItemPointer item_ptr = tuple_entry.first;
     oid_t tile_group_id = item_ptr.block;
     oid_t tuple_slot = item_ptr.offset;
 
-    auto tile_group_header = storage_manager->GetTileGroup(tile_group_id)->GetHeader();
+    if (tile_group_id != last_tile_group_id) {
+      tile_group_header =
+          storage_manager->GetTileGroup(tile_group_id)->GetHeader();
+      last_tile_group_id = tile_group_id;
+    }
 
     if (tuple_entry.second == RWType::READ_OWN) {
       // A read operation has acquired ownership but hasn't done any further
@@ -805,13 +810,20 @@ ResultType TimestampOrderingTransactionManager::AbortTransaction(
   }
 
   // Iterate through each item pointer in the read write set
-  // TODO (Pooja): This might be inefficient since we will have to get the
-  // tile_group_header for each entry. Check if this needs to be consolidated
+
+  oid_t last_tile_group_id = INVALID_OID;
+  storage::TileGroupHeader *tile_group_header = nullptr;
+
   for (const auto &tuple_entry : rw_set) {
     ItemPointer item_ptr = tuple_entry.first;
     oid_t tile_group_id = item_ptr.block;
     oid_t tuple_slot = item_ptr.offset;
-    auto tile_group_header = storage_manager->GetTileGroup(tile_group_id)->GetHeader();
+
+    if (tile_group_id != last_tile_group_id) {
+      tile_group_header =
+          storage_manager->GetTileGroup(tile_group_id)->GetHeader();
+      last_tile_group_id = tile_group_id;
+    }
 
     if (tuple_entry.second == RWType::READ_OWN) {
       // A read operation has acquired ownership but hasn't done any further

src/concurrency/transaction_context.cpp

@@ -61,8 +61,6 @@ TransactionContext::TransactionContext(const size_t thread_id,
   Init(thread_id, isolation, read_id, commit_id);
 }
 
-TransactionContext::~TransactionContext() {}
-
 void TransactionContext::Init(const size_t thread_id,
                               const IsolationLevelType isolation,
                               const cid_t &read_id, const cid_t &commit_id) {
@@ -78,103 +76,73 @@ void TransactionContext::Init(const size_t thread_id,
 
   thread_id_ = thread_id;
 
-  isolation_level_ = isolation;
-
   is_written_ = false;
 
-  insert_count_ = 0;
+  isolation_level_ = isolation;
 
-  gc_set_.reset(new GCSet());
-  gc_object_set_.reset(new GCObjectSet());
+  gc_set_ = std::make_shared<GCSet>();
+  gc_object_set_ = std::make_shared<GCObjectSet>();
 
   on_commit_triggers_.reset();
 }
 
 RWType TransactionContext::GetRWType(const ItemPointer &location) {
-  RWType rw_type = RWType::INVALID;
-
-  auto rw_set_it = rw_set_.find(location);
+  const auto rw_set_it = rw_set_.find(location);
   if (rw_set_it != rw_set_.end()) {
     return rw_set_it->second;
   }
-  return rw_type;
+  return RWType::INVALID;
 }
 
 void TransactionContext::RecordRead(const ItemPointer &location) {
-
+  PELOTON_ASSERT(rw_set_.find(location) == rw_set_.end() ||
+                 (rw_set_[location] != RWType::DELETE &&
+                  rw_set_[location] != RWType::INS_DEL));
   auto rw_set_it = rw_set_.find(location);
   if (rw_set_it != rw_set_.end()) {
-    UNUSED_ATTRIBUTE RWType rw_type = rw_set_it->second;
-    PELOTON_ASSERT(rw_type != RWType::DELETE && rw_type != RWType::INS_DEL);
     return;
   }
-  rw_set_.insert(rw_set_it, std::make_pair(location, RWType::READ));
+  rw_set_[location] = RWType::READ;
 }
 
 void TransactionContext::RecordReadOwn(const ItemPointer &location) {
-  auto rw_set_it = rw_set_.find(location);
-  if (rw_set_it != rw_set_.end()) {
-    RWType rw_type = rw_set_it->second;
-    PELOTON_ASSERT(rw_type != RWType::DELETE && rw_type != RWType::INS_DEL);
-    if (rw_type == RWType::READ) {
-      rw_set_it->second = RWType::READ_OWN;
-    }
-  } else {
-    rw_set_.insert(rw_set_it, std::make_pair(location, RWType::READ_OWN));
-  }
+  PELOTON_ASSERT(rw_set_.find(location) == rw_set_.end() ||
+                 (rw_set_[location] != RWType::DELETE &&
+                  rw_set_[location] != RWType::INS_DEL));
+  rw_set_[location] = RWType::READ_OWN;
 }
 
 void TransactionContext::RecordUpdate(const ItemPointer &location) {
+  PELOTON_ASSERT(rw_set_.find(location) == rw_set_.end() ||
+                 (rw_set_[location] != RWType::DELETE &&
+                  rw_set_[location] != RWType::INS_DEL));
   auto rw_set_it = rw_set_.find(location);
-  if (rw_set_it != rw_set_.end()) {
-    RWType rw_type = rw_set_it->second;
-    if (rw_type == RWType::READ || rw_type == RWType::READ_OWN) {
-      is_written_ = true;
-      rw_set_it->second = RWType::UPDATE;
-    } else if (rw_type == RWType::UPDATE || rw_type == RWType::INSERT) {
-      return;
-    } else {
-      // DELETE or INS_DELETE
-      PELOTON_ASSERT(false);
-    }
-  } else {
-    rw_set_.insert(rw_set_it, std::make_pair(location, RWType::UPDATE));
+  if (rw_set_it != rw_set_.end() && (rw_set_it->second == RWType::READ ||
+                                     rw_set_it->second == RWType::READ_OWN)) {
+    rw_set_it->second = RWType::UPDATE;
+    is_written_ = true;
   }
+  PELOTON_ASSERT(is_written_);
 }
 
 void TransactionContext::RecordInsert(const ItemPointer &location) {
-  auto rw_set_it = rw_set_.find(location);
-  if (rw_set_it != rw_set_.end()) {
-    PELOTON_ASSERT(false);
-    return;
-  }
-  rw_set_.insert(rw_set_it, std::make_pair(location, RWType::INSERT));
-  ++insert_count_;
+  PELOTON_ASSERT(rw_set_.find(location) == rw_set_.end());
+  rw_set_[location] = RWType::INSERT;
+  is_written_ = true;
 }
 
 bool TransactionContext::RecordDelete(const ItemPointer &location) {
+  PELOTON_ASSERT(rw_set_.find(location) == rw_set_.end() ||
+                 (rw_set_[location] != RWType::DELETE &&
+                  rw_set_[location] != RWType::INS_DEL));
   auto rw_set_it = rw_set_.find(location);
-  if (rw_set_it != rw_set_.end()) {
-    RWType rw_type = rw_set_it->second;
-
-    if (rw_type == RWType::READ || rw_type == RWType::READ_OWN) {
-      rw_set_it->second = RWType::DELETE;
-      is_written_ = true;
-      return false;
-    } else if (rw_type == RWType::UPDATE) {
-      rw_set_it->second = RWType::DELETE;
-      return false;
-    } else if (rw_type == RWType::INSERT) {
-      rw_set_it->second = RWType::INS_DEL;
-      --insert_count_;
-      return true;
-    } else {
-      // DELETE and INS_DEL
-      PELOTON_ASSERT(false);
-      return false;
-    }
+  if (rw_set_it != rw_set_.end() && rw_set_it->second == RWType::INSERT) {
+    PELOTON_ASSERT(is_written_);
+    rw_set_it->second = RWType::INS_DEL;
+    return true;
   } else {
-    rw_set_.insert(rw_set_it, std::make_pair(location, RWType::DELETE));
+    rw_set_[location] = RWType::DELETE;
+    is_written_ = true;
     return false;
   }
 }

src/include/common/container/lock_free_array.h

@@ -12,13 +12,6 @@
 
 #pragma once
 #include "tbb/concurrent_vector.h"
-#include "tbb/tbb_allocator.h"
-#include <cstdlib>
-#include <cstring>
-#include <cstdio>
-#include <array>
-#include <atomic>
-#include <memory>
 
 namespace peloton {
 
@@ -35,33 +28,74 @@ class LockFreeArray {
   LockFreeArray();
   ~LockFreeArray();
 
-  // Update a item
-  bool Update(const std::size_t &offset, ValueType value);
-
-  // Append an item
-  bool Append(ValueType value);
-
-  // Get a item
+  /**
+   * Assigns the provided value to the provided offset.
+   *
+   * @param offset  Element offset to update
+   * @param value   Value to be assigned
+   */
+  void Update(const std::size_t &offset, const ValueType &value);
+
+  /**
+   * Appends an element to the end of the array
+   *
+   * @param value   Value to be appended
+   */
+  void Append(const ValueType &value);
+
+  /**
+   * Returns the element at the offset
+   *
+   * @returns   Element at offset
+   */
   ValueType Find(const std::size_t &offset) const;
 
-  // Get a valid item
+  /**
+   * Returns the element at the offset, or invalid_value if
+   * the element does not exist.
+   *
+   * @param offset          Element offset to access
+   * @param invalid_value   Sentinel value to return if element
+   *                        does not exist or offset out of range
+   * @returns               Element at offset or invalid_value
+   */
   ValueType FindValid(const std::size_t &offset,
                       const ValueType &invalid_value) const;
 
-  // Delete key from the lock_free_array
-  bool Erase(const std::size_t &offset, const ValueType &invalid_value);
-
-  // Returns item count in the lock_free_array
+  /**
+   * Assigns the provided invalid_value to the provided offset.
+   *
+   * @param offset          Element offset to update
+   * @param invalid_value   Invalid value to be assigned
+   */
+  void Erase(const std::size_t &offset, const ValueType &invalid_value);
+
+  /**
+   *
+   * @return Number of elements in the underlying structure
+   */
   size_t GetSize() const;
 
-  // Checks if the lock_free_array is empty
+  /**
+   *
+   * @return True if empty, false otherwise
+   */
   bool IsEmpty() const;
 
-  // Clear all elements and reset them to default value
+  /**
+   * Resets the underlying data structure to have 0 elements
+   */
   void Clear();
 
-  // Exists ?
-  bool Contains(const ValueType &value);
+  /**
+   *
+   * Check the lock-free array for the provided value.
+   * O(n) time complexity.
+   *
+   * @param value  value to search for
+   * @return       True if element present, false otherwise
+   */
+  bool Contains(const ValueType &value) const;
 
  private:
   // lock free array