Fix folly::ThreadLocal to have unique singleton in dev builds
authorAndrii Grynenko <andrii@fb.com>
Fri, 26 Feb 2016 18:28:31 +0000 (10:28 -0800)
committerFacebook Github Bot 3 <facebook-github-bot-3-bot@fb.com>
Fri, 26 Feb 2016 18:35:27 +0000 (10:35 -0800)
Summary:This re-uses StaticSingletonManager which was previously used to fix the same issue in folly::Singleton.

Because of the same issue we can no longer use static thread_local for the ThreadEntry. We now rely on pthread_getspecific/pthread_setspecific instead and use static thread_local ThreadEntry* only as a cache (to improve perf).

Reviewed By: yfeldblum

Differential Revision: D2978526

fb-gh-sync-id: cf1d9044afc27b62bd50a1ed931c0c420ae7107e
shipit-source-id: cf1d9044afc27b62bd50a1ed931c0c420ae7107e

folly/Makefile.am
folly/Singleton.cpp
folly/Singleton.h
folly/detail/StaticSingletonManager.cpp [new file with mode: 0644]
folly/detail/StaticSingletonManager.h [new file with mode: 0644]
folly/detail/ThreadLocalDetail.h

index f46b13ad25f9e3175c1a9d3dd288fc4636957c51..e910a2cb3ceb84618529efe7e580290f8390a345 100644 (file)
@@ -70,6 +70,7 @@ nobase_follyinclude_HEADERS = \
        detail/Sleeper.h \
        detail/SlowFingerprint.h \
        detail/SpinLockImpl.h \
+       detail/StaticSingletonManager.h \
        detail/Stats.h \
        detail/ThreadLocalDetail.h \
        detail/TurnSequencer.h \
@@ -360,6 +361,7 @@ libfolly_la_SOURCES = \
        futures/QueuedImmediateExecutor.cpp \
        futures/ThreadWheelTimekeeper.cpp \
        detail/Futex.cpp \
+       detail/StaticSingletonManager.cpp \
        detail/ThreadLocalDetail.cpp \
        GroupVarint.cpp \
        GroupVarintTables.cpp \
index cf1ce727fe31295e47d6f965e87a510126082b34..8e9cd99b0763138bd8eb7c61819da5c293a61703 100644 (file)
@@ -25,12 +25,6 @@ namespace folly {
 
 namespace detail {
 
-// This implementation should always live in .cpp file.
-StaticSingletonManager& StaticSingletonManager::instance() {
-  static StaticSingletonManager* instance = new StaticSingletonManager();
-  return *instance;
-}
-
 constexpr std::chrono::seconds SingletonHolderBase::kDestroyWaitTime;
 
 }
index bb04862d5c9be8185634d7ac4a134598072db47a..c5fe79db274c0d15ebcbebf3a7efea282229e2ff 100644 (file)
 #include <folly/Demangle.h>
 #include <folly/Executor.h>
 #include <folly/experimental/ReadMostlySharedPtr.h>
+#include <folly/detail/StaticSingletonManager.h>
 
 #include <algorithm>
 #include <atomic>
@@ -160,62 +161,6 @@ class SingletonVault;
 
 namespace detail {
 
-// This internal-use-only class is used to create all leaked Meyers singletons.
-// It guarantees that only one instance of every such singleton will ever be
-// created, even when requested from different compilation units linked
-// dynamically.
-class StaticSingletonManager {
- public:
-  static StaticSingletonManager& instance();
-
-  template <typename T, typename Tag, typename F>
-  inline T* create(F&& creator) {
-    auto& entry = [&]() mutable -> Entry<T>& {
-      std::lock_guard<std::mutex> lg(mutex_);
-
-      auto& id = typeid(TypePair<T, Tag>);
-      auto& entryPtr = reinterpret_cast<Entry<T>*&>(map_[id]);
-      if (!entryPtr) {
-        entryPtr = new Entry<T>();
-      }
-      return *entryPtr;
-    }();
-
-    std::lock_guard<std::mutex> lg(entry.mutex);
-
-    if (!entry.ptr) {
-      entry.ptr = creator();
-    }
-    return entry.ptr;
-  }
-
- private:
-  template <typename A, typename B>
-  class TypePair {};
-
-  StaticSingletonManager() {}
-
-  template <typename T>
-  struct Entry {
-    T* ptr{nullptr};
-    std::mutex mutex;
-  };
-
-  std::unordered_map<std::type_index, intptr_t> map_;
-  std::mutex mutex_;
-};
-
-template <typename T, typename Tag, typename F>
-inline T* createGlobal(F&& creator) {
-  return StaticSingletonManager::instance().create<T, Tag>(
-      std::forward<F>(creator));
-}
-
-template <typename T, typename Tag>
-inline T* createGlobal() {
-  return createGlobal<T, Tag>([]() { return new T(); });
-}
-
 struct DefaultTag {};
 
 // A TypeDescriptor is the unique handle for a given singleton.  It is
diff --git a/folly/detail/StaticSingletonManager.cpp b/folly/detail/StaticSingletonManager.cpp
new file mode 100644 (file)
index 0000000..23464cc
--- /dev/null
@@ -0,0 +1,27 @@
+/*
+ * Copyright 2016 Facebook, Inc.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+#include <folly/detail/StaticSingletonManager.h>
+
+namespace folly {
+namespace detail {
+
+// This implementation should always live in .cpp file.
+StaticSingletonManager& StaticSingletonManager::instance() {
+  static StaticSingletonManager* instance = new StaticSingletonManager();
+  return *instance;
+}
+}
+}
diff --git a/folly/detail/StaticSingletonManager.h b/folly/detail/StaticSingletonManager.h
new file mode 100644 (file)
index 0000000..6e4b63d
--- /dev/null
@@ -0,0 +1,87 @@
+/*
+ * Copyright 2016 Facebook, Inc.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#pragma once
+
+#include <mutex>
+#include <typeindex>
+#include <unordered_map>
+
+namespace folly {
+namespace detail {
+
+// This internal-use-only class is used to create all leaked Meyers singletons.
+// It guarantees that only one instance of every such singleton will ever be
+// created, even when requested from different compilation units linked
+// dynamically.
+class StaticSingletonManager {
+ public:
+  static StaticSingletonManager& instance();
+
+  template <typename T, typename Tag, typename F>
+  inline T* create(F&& creator) {
+    auto& entry = [&]() mutable -> Entry<T>& {
+      std::lock_guard<std::mutex> lg(mutex_);
+
+      auto& id = typeid(TypePair<T, Tag>);
+      auto& entryPtr = map_[id];
+      if (!entryPtr) {
+        entryPtr = new Entry<T>();
+      }
+      assert(dynamic_cast<Entry<T>*>(entryPtr) != nullptr);
+      return *static_cast<Entry<T>*>(entryPtr);
+    }();
+
+    std::lock_guard<std::mutex> lg(entry.mutex);
+
+    if (!entry.ptr) {
+      entry.ptr = creator();
+    }
+    return entry.ptr;
+  }
+
+ private:
+  template <typename A, typename B>
+  class TypePair {};
+
+  StaticSingletonManager() {}
+
+  struct EntryIf {
+    virtual ~EntryIf() {}
+  };
+
+  template <typename T>
+  struct Entry : public EntryIf {
+    T* ptr{nullptr};
+    std::mutex mutex;
+  };
+
+  std::unordered_map<std::type_index, EntryIf*> map_;
+  std::mutex mutex_;
+};
+
+template <typename T, typename Tag, typename F>
+inline T* createGlobal(F&& creator) {
+  return StaticSingletonManager::instance().create<T, Tag>(
+      std::forward<F>(creator));
+}
+
+template <typename T, typename Tag>
+inline T* createGlobal() {
+  return createGlobal<T, Tag>([]() { return new T(); });
+}
+}
+}
index 90e57849b67445e799760e3e5073a1a6f0015f78..e9843269a2ee6937816c2316fe63127b920f9e44 100644 (file)
@@ -31,6 +31,8 @@
 #include <folly/Malloc.h>
 #include <folly/MicroSpinLock.h>
 
+#include <folly/detail/StaticSingletonManager.h>
+
 // In general, emutls cleanup is not guaranteed to play nice with the way
 // StaticMeta mixes direct pthread calls and the use of __thread. This has
 // caused problems on multiple platforms so don't use __thread there.
@@ -156,10 +158,10 @@ struct ElementWrapper {
  * (under the lock).
  */
 struct ThreadEntry {
-  ElementWrapper* elements;
-  size_t elementsCapacity;
-  ThreadEntry* next;
-  ThreadEntry* prev;
+  ElementWrapper* elements{nullptr};
+  size_t elementsCapacity{0};
+  ThreadEntry* next{nullptr};
+  ThreadEntry* prev{nullptr};
 };
 
 constexpr uint32_t kEntryIDInvalid = std::numeric_limits<uint32_t>::max();
@@ -273,9 +275,8 @@ struct StaticMeta {
   static StaticMeta<Tag>& instance() {
     // Leak it on exit, there's only one per process and we don't have to
     // worry about synchronization with exiting threads.
-    static bool constructed = (inst_ = new StaticMeta<Tag>());
-    (void)constructed; // suppress unused warning
-    return *inst_;
+    static auto instance = detail::createGlobal<StaticMeta<Tag>, void>();
+    return *instance;
   }
 
   uint32_t nextId_;
@@ -297,11 +298,6 @@ struct StaticMeta {
     t->next = t->prev = t;
   }
 
-#ifdef FOLLY_TLD_USE_FOLLY_TLS
-  static FOLLY_TLS ThreadEntry threadEntry_;
-#endif
-  static StaticMeta<Tag>* inst_;
-
   StaticMeta() : nextId_(1) {
     head_.next = head_.prev = &head_;
     int ret = pthread_key_create(&pthreadKey_, &onThreadExit);
@@ -326,10 +322,7 @@ struct StaticMeta {
     LOG(FATAL) << "StaticMeta lives forever!";
   }
 
-  static ThreadEntry* getThreadEntry() {
-#ifdef FOLLY_TLD_USE_FOLLY_TLS
-    return &threadEntry_;
-#else
+  static ThreadEntry* getThreadEntrySlow() {
     auto key = instance().pthreadKey_;
     ThreadEntry* threadEntry =
       static_cast<ThreadEntry*>(pthread_getspecific(key));
@@ -339,6 +332,17 @@ struct StaticMeta {
         checkPosixError(ret, "pthread_setspecific failed");
     }
     return threadEntry;
+  }
+
+  static ThreadEntry* getThreadEntry() {
+#ifdef FOLLY_TLD_USE_FOLLY_TLS
+    static FOLLY_TLS ThreadEntry* threadEntryCache{nullptr};
+    if (UNLIKELY(threadEntryCache == nullptr)) {
+      threadEntryCache = getThreadEntrySlow();
+    }
+    return threadEntryCache;
+#else
+    return getThreadEntrySlow();
 #endif
   }
 
@@ -346,37 +350,32 @@ struct StaticMeta {
     instance().lock_.lock();  // Make sure it's created
   }
 
-  static void onForkParent(void) {
-    inst_->lock_.unlock();
-  }
+  static void onForkParent(void) { instance().lock_.unlock(); }
 
   static void onForkChild(void) {
     // only the current thread survives
-    inst_->head_.next = inst_->head_.prev = &inst_->head_;
+    instance().head_.next = instance().head_.prev = &instance().head_;
     ThreadEntry* threadEntry = getThreadEntry();
     // If this thread was in the list before the fork, add it back.
     if (threadEntry->elementsCapacity != 0) {
-      inst_->push_back(threadEntry);
+      instance().push_back(threadEntry);
     }
-    inst_->lock_.unlock();
+    instance().lock_.unlock();
   }
 
   static void onThreadExit(void* ptr) {
     auto& meta = instance();
-#ifdef FOLLY_TLD_USE_FOLLY_TLS
-    ThreadEntry* threadEntry = getThreadEntry();
 
-    DCHECK_EQ(ptr, &meta);
-    DCHECK_GT(threadEntry->elementsCapacity, 0);
-#else
     // pthread sets the thread-specific value corresponding
     // to meta.pthreadKey_ to NULL before calling onThreadExit.
     // We need to set it back to ptr to enable the correct behaviour
     // of the subsequent calls of getThreadEntry
     // (which may happen in user-provided custom deleters)
     pthread_setspecific(meta.pthreadKey_, ptr);
-    ThreadEntry* threadEntry = static_cast<ThreadEntry*>(ptr);
-#endif
+
+    ThreadEntry* threadEntry = getThreadEntry();
+    DCHECK_GT(threadEntry->elementsCapacity, 0);
+
     {
       std::lock_guard<std::mutex> g(meta.lock_);
       meta.erase(threadEntry);
@@ -399,10 +398,7 @@ struct StaticMeta {
     threadEntry->elements = nullptr;
     pthread_setspecific(meta.pthreadKey_, nullptr);
 
-#ifndef FOLLY_TLD_USE_FOLLY_TLS
-    // Allocated in getThreadEntry() when not using folly TLS; free it
     delete threadEntry;
-#endif
   }
 
   static uint32_t allocate(EntryID* ent) {
@@ -558,12 +554,6 @@ struct StaticMeta {
     }
 
     free(reallocated);
-
-#ifdef FOLLY_TLD_USE_FOLLY_TLS
-    if (prevCapacity == 0) {
-      pthread_setspecific(meta.pthreadKey_, &meta);
-    }
-#endif
   }
 
   static ElementWrapper& get(EntryID* ent) {
@@ -580,13 +570,6 @@ struct StaticMeta {
   }
 };
 
-#ifdef FOLLY_TLD_USE_FOLLY_TLS
-template <class Tag>
-FOLLY_TLS ThreadEntry StaticMeta<Tag>::threadEntry_ = {nullptr, 0,
-                                                       nullptr, nullptr};
-#endif
-template <class Tag> StaticMeta<Tag>* StaticMeta<Tag>::inst_ = nullptr;
-
 }  // namespace threadlocal_detail
 }  // namespace folly