From afad5bd1c5f37782a96542a8663243e272c3edcb Mon Sep 17 00:00:00 2001 From: Andrew Kryczka Date: Thu, 28 Jul 2016 14:55:22 -0700 Subject: [PATCH] Simplify thread-local static initialization Summary: The call stack used to look like this during static initialization: #0 0x00000000008032d1 in rocksdb::ThreadLocalPtr::StaticMeta::StaticMeta() (this=0x7ffff683b300) at util/thread_local.cc:172 #1 0x00000000008030a7 in rocksdb::ThreadLocalPtr::Instance() () at util/thread_local.cc:135 #2 0x000000000080310f in rocksdb::ThreadLocalPtr::StaticMeta::Mutex() () at util/thread_local.cc:141 #3 0x0000000000803103 in rocksdb::ThreadLocalPtr::StaticMeta::InitSingletons() () at util/thread_local.cc:139 #4 0x000000000080305d in rocksdb::ThreadLocalPtr::InitSingletons() () at util/thread_local.cc:106 It involves outer/inner classes and the call stacks goes outer->inner->outer->inner, which is too difficult to understand. We can avoid a level of back-and-forth by skipping StaticMeta::InitSingletons(), which doesn't initialize anything beyond what ThreadLocalPtr::Instance() already initializes. Now the call stack looks like this during static initialization: #0 0x00000000008032c5 in rocksdb::ThreadLocalPtr::StaticMeta::StaticMeta() (this=0x7ffff683b300) at util/thread_local.cc:170 #1 0x00000000008030a7 in rocksdb::ThreadLocalPtr::Instance() () at util/thread_local.cc:135 #2 0x000000000080305d in rocksdb::ThreadLocalPtr::InitSingletons() () at util/thread_local.cc:106 Test Plan: unit tests verify StaticMeta::mutex_ is still initialized in DefaultEnv() (StaticMeta::mutex_ is the only variable intended to be initialized via StaticMeta::InitSingletons() which I removed) #0 0x00000000005cee17 in rocksdb::port::Mutex::Mutex(bool) (this=0x7ffff69500b0, adaptive=false) at port/port_posix.cc:52 #1 0x0000000000769cf8 in rocksdb::ThreadLocalPtr::StaticMeta::StaticMeta() (this=0x7ffff6950000) at util/thread_local.cc:168 #2 0x0000000000769a53 in rocksdb::ThreadLocalPtr::Instance() () at util/thread_local.cc:133 #3 0x0000000000769a09 in rocksdb::ThreadLocalPtr::InitSingletons() () at util/thread_local.cc:105 #4 0x0000000000647d98 in rocksdb::Env::Default() () at util/env_posix.cc:845 Reviewers: lightmark, yhchiang, sdong Reviewed By: sdong Subscribers: arahut, IslamAbdelRahman, yiwu, andrewkr, dhruba, leveldb Differential Revision: https://reviews.facebook.net/D60813 --- util/thread_local.cc | 6 +----- util/thread_local.h | 9 --------- 2 files changed, 1 insertion(+), 14 deletions(-) diff --git a/util/thread_local.cc b/util/thread_local.cc index 5f3fddae5..b69f41a77 100644 --- a/util/thread_local.cc +++ b/util/thread_local.cc @@ -102,9 +102,7 @@ PIMAGE_TLS_CALLBACK p_thread_callback_on_exit = wintlscleanup::WinOnThreadExit; #endif // OS_WIN -void ThreadLocalPtr::InitSingletons() { - ThreadLocalPtr::StaticMeta::InitSingletons(); -} +void ThreadLocalPtr::InitSingletons() { ThreadLocalPtr::Instance(); } ThreadLocalPtr::StaticMeta* ThreadLocalPtr::Instance() { // Here we prefer function static variable instead of global @@ -136,8 +134,6 @@ ThreadLocalPtr::StaticMeta* ThreadLocalPtr::Instance() { return inst; } -void ThreadLocalPtr::StaticMeta::InitSingletons() { Mutex(); } - port::Mutex* ThreadLocalPtr::StaticMeta::Mutex() { return &Instance()->mutex_; } void ThreadLocalPtr::StaticMeta::OnThreadExit(void* ptr) { diff --git a/util/thread_local.h b/util/thread_local.h index 3adf8ba85..08eabd06e 100644 --- a/util/thread_local.h +++ b/util/thread_local.h @@ -133,15 +133,6 @@ class ThreadLocalPtr { // Register the UnrefHandler for id void SetHandler(uint32_t id, UnrefHandler handler); - // Initialize all the singletons associated with StaticMeta. - // - // If this function is not called, then the singletons will be - // automatically initialized when they are used. - // - // Calling this function twice or after the singletons have been - // initialized will be no-op. - static void InitSingletons(); - // protect inst, next_instance_id_, free_instance_ids_, head_, // ThreadData.entries //