You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@quickstep.apache.org by zu...@apache.org on 2016/05/16 20:55:54 UTC

[17/46] incubator-quickstep git commit: Refactored makeRoomForBlock. (#192)

Refactored makeRoomForBlock. (#192)

Project: http://git-wip-us.apache.org/repos/asf/incubator-quickstep/repo
Commit: http://git-wip-us.apache.org/repos/asf/incubator-quickstep/commit/3bd1586e
Tree: http://git-wip-us.apache.org/repos/asf/incubator-quickstep/tree/3bd1586e
Diff: http://git-wip-us.apache.org/repos/asf/incubator-quickstep/diff/3bd1586e

Branch: refs/heads/master
Commit: 3bd1586ecfa343f467398892a8887b3ea3c7a5f5
Parents: 7ac1d22
Author: Zuyu ZHANG <zu...@users.noreply.github.com>
Authored: Wed Apr 27 23:00:23 2016 -0700
Committer: Jignesh Patel <pa...@users.noreply.github.com>
Committed: Thu Apr 28 01:00:23 2016 -0500

----------------------------------------------------------------------
 storage/StorageManager.cpp | 89 ++++++++++++++++++++---------------------
 storage/StorageManager.hpp | 10 +++--
 2 files changed, 50 insertions(+), 49 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/incubator-quickstep/blob/3bd1586e/storage/StorageManager.cpp
----------------------------------------------------------------------
diff --git a/storage/StorageManager.cpp b/storage/StorageManager.cpp
index b98a28c..b990a22 100644
--- a/storage/StorageManager.cpp
+++ b/storage/StorageManager.cpp
@@ -397,7 +397,7 @@ void* StorageManager::allocateSlots(const std::size_t num_slots,
       = MAP_PRIVATE | MAP_ANONYMOUS | MAP_ALIGNED_SUPER;
 #endif
 
-  makeRoomForBlock(num_slots);
+  makeRoomForBlockOrBlob(num_slots);
   void *slots = nullptr;
 
 #if defined(QUICKSTEP_HAVE_MMAP_LINUX_HUGETLB) || defined(QUICKSTEP_HAVE_MMAP_BSD_SUPERPAGE)
@@ -568,65 +568,62 @@ MutableBlobReference StorageManager::getBlobInternal(const block_id blob,
   return ret;
 }
 
-void StorageManager::makeRoomForBlock(const size_t slots) {
+void StorageManager::makeRoomForBlockOrBlob(const size_t slots) {
+  block_id block_to_evict;
   while (total_memory_usage_ + slots > max_memory_usage_) {
-    block_id block_index;
-    EvictionPolicy::Status status = eviction_policy_->chooseBlockToEvict(&block_index);
-
-    if (status == EvictionPolicy::Status::kOk) {
-      bool has_collision = false;
-      SpinSharedMutexExclusiveLock<false> eviction_lock(*lock_manager_.get(block_index, &has_collision));
-      if (has_collision) {
-        // We have a collision in the shared lock manager, where some callers
-        // of this function (i.e., getBlockInternal or getBlobInternal) has
-        // acquired an exclusive lock, and we are trying to evict a block that
-        // hashes to the same location. This will cause a deadlock.
-
-        // For now simply treat this situation as the case where there is not
-        // enough memory and we temporarily go over the memory limit.
-        break;
-      }
+    const EvictionPolicy::Status status = eviction_policy_->chooseBlockToEvict(&block_to_evict);
+    if (status != EvictionPolicy::Status::kOk) {
+      // If status was not ok, then we must not have been able to evict enough
+      // blocks; therefore, we return anyway, temporarily going over the memory
+      // limit.
+      break;
+    }
 
-      StorageBlockBase* block;
-      {
-        SpinSharedMutexSharedLock<false> read_lock(blocks_shared_mutex_);
-        if (blocks_.find(block_index) == blocks_.end()) {
-          // another thread must have jumped in and evicted it before us
-
-          // NOTE(zuyu): It is ok to release the shard for a block or blob,
-          // before 'eviction_lock' destructs, because we will never encounter a
-          // self-deadlock in a single thread, and in multiple-thread case some
-          // thread will block but not deadlock if there is a shard collision.
-          lock_manager_.release(block_index);
-          continue;
-        }
-        block = blocks_[block_index].block;
-      }
-      if (eviction_policy_->getRefCount(block->getID()) > 0) {
-        // Someone sneaked in and referenced the block before we could evict it.
+    bool has_collision = false;
+    SpinSharedMutexExclusiveLock<false> eviction_lock(*lock_manager_.get(block_to_evict, &has_collision));
+    if (has_collision) {
+      // We have a collision in the shared lock manager, where some callers
+      // of this function (i.e., getBlockInternal or getBlobInternal) has
+      // acquired an exclusive lock, and we are trying to evict a block that
+      // hashes to the same location. This will cause a deadlock.
+
+      // For now simply treat this situation as the case where there is not
+      // enough memory and we temporarily go over the memory limit.
+      break;
+    }
 
-        // NOTE(zuyu): It is ok to release the shard for a block or blob, before
+    {
+      SpinSharedMutexSharedLock<false> read_lock(blocks_shared_mutex_);
+      if (blocks_.find(block_to_evict) == blocks_.end()) {
+        // another thread must have jumped in and evicted it before us
+
+        // NOTE(zuyu): It is ok to release the shard for a block or blob,
         // before 'eviction_lock' destructs, because we will never encounter a
         // self-deadlock in a single thread, and in multiple-thread case some
         // thread will block but not deadlock if there is a shard collision.
-        lock_manager_.release(block_index);
+        lock_manager_.release(block_to_evict);
         continue;
       }
-      if (saveBlockOrBlob(block->getID())) {
-        evictBlockOrBlob(block->getID());
-      }  // else : Someone sneaked in and evicted the block before we could.
+    }
+    if (eviction_policy_->getRefCount(block_to_evict) > 0) {
+      // Someone sneaked in and referenced the block before we could evict it.
 
       // NOTE(zuyu): It is ok to release the shard for a block or blob, before
       // before 'eviction_lock' destructs, because we will never encounter a
       // self-deadlock in a single thread, and in multiple-thread case some
       // thread will block but not deadlock if there is a shard collision.
-      lock_manager_.release(block_index);
-    } else {
-      // If status was not ok, then we must not have been able to evict enough
-      // blocks; therefore, we return anyway, temporarily going over the memory
-      // limit.
-      break;
+      lock_manager_.release(block_to_evict);
+      continue;
     }
+    if (saveBlockOrBlob(block_to_evict)) {
+      evictBlockOrBlob(block_to_evict);
+    }  // else : Someone sneaked in and evicted the block before we could.
+
+    // NOTE(zuyu): It is ok to release the shard for a block or blob, before
+    // before 'eviction_lock' destructs, because we will never encounter a
+    // self-deadlock in a single thread, and in multiple-thread case some
+    // thread will block but not deadlock if there is a shard collision.
+    lock_manager_.release(block_to_evict);
   }
 }
 

http://git-wip-us.apache.org/repos/asf/incubator-quickstep/blob/3bd1586e/storage/StorageManager.hpp
----------------------------------------------------------------------
diff --git a/storage/StorageManager.hpp b/storage/StorageManager.hpp
index dab33f6..0b68b76 100644
--- a/storage/StorageManager.hpp
+++ b/storage/StorageManager.hpp
@@ -402,12 +402,16 @@ class StorageManager {
                                        const int numa_node);
 
   /**
-   * @brief Evict blocks until there is enough space for a new block of the
-   *        requested size.
+   * @brief Evict blocks or blobs until there is enough space for a new block
+   *        or blob of the requested size.
+   *
+   * @note This non-blocking method gives up evictions if there is a shard
+   *       collision, and thus the buffer pool size may temporarily go beyond
+   *       the memory limit.
    *
    * @param slots Number of slots to make room for.
    */
-  void makeRoomForBlock(const std::size_t slots);
+  void makeRoomForBlockOrBlob(const std::size_t slots);
 
   /**
    * @brief Load a block from the persistent storage into memory.