Completed a TODO on barrier free SkipList::Insert() and cleaned some unused headers

db/db_impl.cc

@@ -27,11 +27,9 @@
 #include "leveldb/status.h"
 #include "leveldb/table.h"
 #include "leveldb/table_builder.h"
-#include "port/port.h"
 #include "table/block.h"
 #include "table/merger.h"
 #include "table/two_level_iterator.h"
-#include "util/coding.h"
 #include "util/logging.h"
 #include "util/mutexlock.h"
 

db/db_impl.h

@@ -15,7 +15,6 @@
 #include "db/snapshot.h"
 #include "leveldb/db.h"
 #include "leveldb/env.h"
-#include "port/port.h"
 #include "port/thread_annotations.h"
 
 namespace leveldb {

db/memtable.cc

@@ -115,8 +115,8 @@ bool MemTable::Get(const LookupKey& key, std::string* value, Status* s) {
     // all entries with overly large sequence numbers.
     const char* entry = iter.key();
     uint32_t key_length;
-    const char* key_ptr = GetVarint32Ptr(entry, entry + 5, &key_length);
-    if (comparator_.comparator.user_comparator()->Compare(
+    if (const char* key_ptr = GetVarint32Ptr(entry, entry + 5, &key_length);
+        comparator_.comparator.user_comparator()->Compare(
             Slice(key_ptr, key_length - 8), key.user_key()) == 0) {
       // Correct user key
       const uint64_t tag = DecodeFixed64(key_ptr + key_length - 8);

db/memtable.h

@@ -9,7 +9,6 @@
 
 #include "db/dbformat.h"
 #include "db/skiplist.h"
-#include "leveldb/db.h"
 #include "util/arena.h"
 
 namespace leveldb {

db/skiplist.h

@@ -29,7 +29,6 @@
 
 #include <atomic>
 #include <cassert>
-#include <cstdlib>
 
 #include "util/arena.h"
 #include "util/random.h"
@@ -117,6 +116,9 @@ class SkipList {
   // node at "level" for every level in [0..max_height_-1].
   Node* FindGreaterOrEqual(const Key& key, Node** prev) const;
 
+  // used inside of Insert() as it is externally synchronized
+  Node* FindGreaterOrEqualBarrierFree(const Key& key, Node** prev) const;
+
   // Return the latest node with a key < key.
   // Return head_ if there is no such node.
   Node* FindLessThan(const Key& key) const;
@@ -257,8 +259,7 @@ bool SkipList<Key, Comparator>::KeyIsAfterNode(const Key& key, Node* n) const {
 
 template <typename Key, class Comparator>
 typename SkipList<Key, Comparator>::Node*
-SkipList<Key, Comparator>::FindGreaterOrEqual(const Key& key,
-                                              Node** prev) const {
+SkipList<Key, Comparator>::FindGreaterOrEqual(const Key& key, Node** prev) const {
   Node* x = head_;
   int level = GetMaxHeight() - 1;
   while (true) {
@@ -278,6 +279,28 @@ SkipList<Key, Comparator>::FindGreaterOrEqual(const Key& key,
   }
 }
 
+template <typename Key, class Comparator>
+typename SkipList<Key, Comparator>::Node*
+SkipList<Key, Comparator>::FindGreaterOrEqualBarrierFree(const Key& key, Node** prev) const {
+  Node* x = head_;
+  int level = GetMaxHeight() - 1;
+  while (true) {
+    Node* next = x->NoBarrier_Next(level);
+    if (KeyIsAfterNode(key, next)) {
+      // Keep searching in this list
+      x = next;
+    } else {
+      if (prev != nullptr) prev[level] = x;
+      if (level == 0) {
+        return next;
+      } else {
+        // Switch to next list
+        level--;
+      }
+    }
+  }
+}
+
 template <typename Key, class Comparator>
 typename SkipList<Key, Comparator>::Node*
 SkipList<Key, Comparator>::FindLessThan(const Key& key) const {
@@ -333,10 +356,9 @@ SkipList<Key, Comparator>::SkipList(Comparator cmp, Arena* arena)
 
 template <typename Key, class Comparator>
 void SkipList<Key, Comparator>::Insert(const Key& key) {
-  // TODO(opt): We can use a barrier-free variant of FindGreaterOrEqual()
-  // here since Insert() is externally synchronized.
   Node* prev[kMaxHeight];
-  Node* x = FindGreaterOrEqual(key, prev);
+  // Use the barrier-free variant since Insert() is externally synchronized.
+  Node* x = FindGreaterOrEqualBarrierFree(key, prev);
 
   // Our data structure does not allow duplicate insertion
   assert(x == nullptr || !Equal(key, x->key));