From d8c8f08c12b7f84825e044e202e40f8fc7e3ac8a Mon Sep 17 00:00:00 2001
From: sdong <siying.d@fb.com>
Date: Mon, 8 Jun 2015 16:57:44 -0700
Subject: [PATCH] GetSnapshot() and ReleaseSnapshot() to move new and free out
 of DB mutex

Summary: We currently issue malloc and free inside DB mutex in GetSnapshot() and ReleaseSnapshot(). Move them out.

Test Plan:
Go through all tests
make valgrind_check

Reviewers: yhchiang, rven, IslamAbdelRahman, anthony, igor

Reviewed By: igor

Subscribers: maykov, hermanlee4, MarkCallaghan, yoshinorim, leveldb, dhruba

Differential Revision: https://reviews.facebook.net/D39753
---
 db/db_impl.cc | 16 ++++++++++++----
 db/snapshot.h |  6 +++---
 2 files changed, 15 insertions(+), 7 deletions(-)

diff --git a/db/db_impl.cc b/db/db_impl.cc
index b4f7a4c12..76382ecf4 100644
--- a/db/db_impl.cc
+++ b/db/db_impl.cc
@@ -3212,16 +3212,24 @@ Status DBImpl::NewIterators(
 const Snapshot* DBImpl::GetSnapshot() {
   int64_t unix_time = 0;
   env_->GetCurrentTime(&unix_time);  // Ignore error
+  SnapshotImpl* s = new SnapshotImpl;
 
   InstrumentedMutexLock l(&mutex_);
   // returns null if the underlying memtable does not support snapshot.
-  if (!is_snapshot_supported_) return nullptr;
-  return snapshots_.New(versions_->LastSequence(), unix_time);
+  if (!is_snapshot_supported_) {
+    delete s;
+    return nullptr;
+  }
+  return snapshots_.New(s, versions_->LastSequence(), unix_time);
 }
 
 void DBImpl::ReleaseSnapshot(const Snapshot* s) {
-  InstrumentedMutexLock l(&mutex_);
-  snapshots_.Delete(reinterpret_cast<const SnapshotImpl*>(s));
+  const SnapshotImpl* casted_s = reinterpret_cast<const SnapshotImpl*>(s);
+  {
+    InstrumentedMutexLock l(&mutex_);
+    snapshots_.Delete(casted_s);
+  }
+  delete casted_s;
 }
 
 // Convenience methods
diff --git a/db/snapshot.h b/db/snapshot.h
index c6852f5e5..b4d58fdf0 100644
--- a/db/snapshot.h
+++ b/db/snapshot.h
@@ -49,8 +49,8 @@ class SnapshotList {
   SnapshotImpl* oldest() const { assert(!empty()); return list_.next_; }
   SnapshotImpl* newest() const { assert(!empty()); return list_.prev_; }
 
-  const SnapshotImpl* New(SequenceNumber seq, uint64_t unix_time) {
-    SnapshotImpl* s = new SnapshotImpl;
+  const SnapshotImpl* New(SnapshotImpl* s, SequenceNumber seq,
+                          uint64_t unix_time) {
     s->number_ = seq;
     s->unix_time_ = unix_time;
     s->list_ = this;
@@ -62,12 +62,12 @@ class SnapshotList {
     return s;
   }
 
+  // Do not responsible to free the object.
   void Delete(const SnapshotImpl* s) {
     assert(s->list_ == this);
     s->prev_->next_ = s->next_;
     s->next_->prev_ = s->prev_;
     count_--;
-    delete s;
   }
 
   // retrieve all snapshot numbers. They are sorted in ascending order.