From 5f5e5fc2e95fdb048883928c05b127c4617ef290 Mon Sep 17 00:00:00 2001
From: kailiu <hfevers@gmail.com>
Date: Fri, 13 Dec 2013 15:43:05 -0800
Subject: [PATCH] Revert `atomic_size_t usage`

Summary:
By disassemble the function, we found that the atomic variables do invoke the `lock` that locks the memory bus.
As a tradeoff, we protect the GetUsage by mutex and leave usage_ as plain size_t.

Test Plan: passed `cache_test`

Reviewers: dhruba

CC: leveldb

Differential Revision: https://reviews.facebook.net/D14667
---
 util/cache.cc | 22 +++++++++++++++-------
 1 file changed, 15 insertions(+), 7 deletions(-)

diff --git a/util/cache.cc b/util/cache.cc
index 8fceefc82..34a12d345 100644
--- a/util/cache.cc
+++ b/util/cache.cc
@@ -156,7 +156,13 @@ class LRUCache {
   Cache::Handle* Lookup(const Slice& key, uint32_t hash);
   void Release(Cache::Handle* handle);
   void Erase(const Slice& key, uint32_t hash);
-  size_t GetUsage() const { return usage_.load(std::memory_order_relaxed); }
+  // Although in some platforms the update of size_t is atomic, to make sure
+  // GetUsage() works correctly under any platforms, we'll protect this
+  // function with mutex.
+  size_t GetUsage() const {
+    MutexLock l(&mutex_);
+    return usage_;
+  }
 
  private:
   void LRU_Remove(LRUHandle* e);
@@ -172,8 +178,10 @@ class LRUCache {
   uint32_t remove_scan_count_limit_;
 
   // mutex_ protects the following state.
-  port::Mutex mutex_;
-  std::atomic_size_t usage_;
+  // We don't count mutex_ as the cache's internal state so semantically we
+  // don't mind mutex_ invoking the non-const actions.
+  mutable port::Mutex mutex_;
+  size_t usage_;
 
   // Dummy head of LRU list.
   // lru.prev is newest entry, lru.next is oldest entry.
@@ -215,7 +223,7 @@ void LRUCache::FreeEntry(LRUHandle* e) {
 void LRUCache::LRU_Remove(LRUHandle* e) {
   e->next->prev = e->prev;
   e->prev->next = e->next;
-  usage_.fetch_sub(e->charge, std::memory_order_relaxed);
+  usage_ -= e->charge;
 }
 
 void LRUCache::LRU_Append(LRUHandle* e) {
@@ -224,7 +232,7 @@ void LRUCache::LRU_Append(LRUHandle* e) {
   e->prev = lru_.prev;
   e->prev->next = e;
   e->next->prev = e;
-  usage_.fetch_add(e->charge, std::memory_order_relaxed);
+  usage_ += e->charge;
 }
 
 Cache::Handle* LRUCache::Lookup(const Slice& key, uint32_t hash) {
@@ -283,7 +291,7 @@ Cache::Handle* LRUCache::Insert(
       // referenced by the cache first.
       LRUHandle* cur = lru_.next;
       for (unsigned int scanCount = 0;
-           GetUsage() > capacity_ && cur != &lru_
+           usage_ > capacity_ && cur != &lru_
            && scanCount < remove_scan_count_limit_; scanCount++) {
         LRUHandle* next = cur->next;
         if (cur->refs <= 1) {
@@ -299,7 +307,7 @@ Cache::Handle* LRUCache::Insert(
 
     // Free the space following strict LRU policy until enough space
     // is freed.
-    while (GetUsage() > capacity_ && lru_.next != &lru_) {
+    while (usage_ > capacity_ && lru_.next != &lru_) {
       LRUHandle* old = lru_.next;
       LRU_Remove(old);
       table_.Remove(old->key(), old->hash);