From: Andrii Grynenko Date: Wed, 17 Dec 2014 04:33:44 +0000 (-0800) Subject: Replace singleton names with type tags X-Git-Tag: v0.22.0~52 X-Git-Url: http://demsky.eecs.uci.edu/git/?a=commitdiff_plain;h=df2469a3b81a5c55d21975caa3d359af9bb2ca35;p=folly.git Replace singleton names with type tags Summary: This change simplifies Singleton API (methods don't need to accept name) and the actual implementation. It also makes it similar to folly::ThreadLocalPtr. Additionally misspelled singleton name becomes compilation error, not runtime error. Some users were actually naming singletons, when that was neccessary, this should also be fixed. Test Plan: unit tests for all touched projects Reviewed By: chip@fb.com Subscribers: trunkagent, fugalh, jsedgwick, fbcode-common-diffs@, mcduff, hitesh, mshneer, folly-diffs@ FB internal diff: D1744978 Signature: t1:1744978:1419282587:bd29dd8a70d7572530ac371a96a21764229bc397 --- diff --git a/folly/experimental/Singleton.h b/folly/experimental/Singleton.h index a996674d..54eb74b5 100644 --- a/folly/experimental/Singleton.h +++ b/folly/experimental/Singleton.h @@ -34,9 +34,18 @@ // std::weak_ptr instance = // Singleton::get_weak(); // -// You also can directly access it by the variable defining the -// singleton rather than via get(), and even treat that variable like -// a smart pointer (dereferencing it or using the -> operator). +// Within same compilation unit you should directly access it by the variable +// defining the singleton via get_fast()/get_weak_fast(), and even treat that +// variable like a smart pointer (dereferencing it or using the -> operator): +// +// MyExpensiveService* instance = the_singleton.get_fast(); +// or +// std::weak_ptr instance = the_singleton.get_weak_fast(); +// or even +// the_singleton->doSomething(); +// +// *_fast() accessors are faster than static accessors, and have performance +// similar to Meyers singletons/static objects. // // Please note, however, that all non-weak_ptr interfaces are // inherently subject to races with destruction. Use responsibly. @@ -48,15 +57,19 @@ // circular dependency, a runtime error will occur. // // You can have multiple singletons of the same underlying type, but -// each must be given a unique name: +// each must be given a unique tag. If no tag is specified - default tag is used // // namespace { -// folly::Singleton s1("name1"); -// folly::Singleton s2("name2"); +// struct Tag1 {}; +// struct Tag2 {}; +// folly::Singleton s_default(); +// folly::Singleton s1(); +// folly::Singleton s2(); // } // ... -// MyExpensiveService* svc1 = Singleton::get("name1"); -// MyExpensiveService* svc2 = Singleton::get("name2"); +// MyExpensiveService* svc_default = s_default.get_fast(); +// MyExpensiveService* svc1 = s1.get_fast(); +// MyExpensiveService* svc2 = s2.get_fast(); // // By default, the singleton instance is constructed via new and // deleted via delete, but this is configurable: @@ -122,29 +135,26 @@ namespace folly { namespace detail { -const char* const kDefaultTypeDescriptorName = "(default)"; +struct DefaultTag {}; + // A TypeDescriptor is the unique handle for a given singleton. It is // a combinaiton of the type and of the optional name, and is used as // a key in unordered_maps. class TypeDescriptor { public: - TypeDescriptor(const std::type_info& ti, std::string name__) - : ti_(ti), name_(name__) { - if (name_ == kDefaultTypeDescriptorName) { - LOG(DFATAL) << "Caller used the default name as their literal name; " - << "name your singleton something other than " - << kDefaultTypeDescriptorName; - } + TypeDescriptor(const std::type_info& ti, + const std::type_info& tag_ti) + : ti_(ti), tag_ti_(tag_ti) { } TypeDescriptor(const TypeDescriptor& other) - : ti_(other.ti_), name_(other.name_) { + : ti_(other.ti_), tag_ti_(other.tag_ti_) { } TypeDescriptor& operator=(const TypeDescriptor& other) { if (this != &other) { - name_ = other.name_; ti_ = other.ti_; + tag_ti_ = other.tag_ti_; } return *this; @@ -152,34 +162,28 @@ class TypeDescriptor { std::string name() const { std::string ret = ti_.name(); - ret += "/"; - if (name_.empty()) { - ret += kDefaultTypeDescriptorName; - } else { - ret += name_; + if (tag_ti_ != std::type_index(typeid(DefaultTag))) { + ret += "/"; + ret += tag_ti_.name(); } return ret; } - std::string name_raw() const { - return name_; - } - friend class TypeDescriptorHasher; bool operator==(const TypeDescriptor& other) const { - return ti_ == other.ti_ && name_ == other.name_; + return ti_ == other.ti_ && tag_ti_ == other.tag_ti_; } private: std::type_index ti_; - std::string name_; + std::type_index tag_ti_; }; class TypeDescriptorHasher { public: size_t operator()(const TypeDescriptor& ti) const { - return folly::hash::hash_combine(ti.ti_, ti.name_); + return folly::hash::hash_combine(ti.ti_, ti.tag_ti_); } }; @@ -515,7 +519,7 @@ class SingletonVault { // singletons. Create instances of this class in the global scope of // type Singleton to register your singleton for later access via // Singleton::get(). -template +template class Singleton { public: typedef std::function CreateFunc; @@ -525,19 +529,17 @@ class Singleton { // get() repeatedly rather than saving the reference, and then not // call get() during process shutdown. static T* get(SingletonVault* vault = nullptr /* for testing */) { - return get_ptr({typeid(T), ""}, vault); - } - - static T* get(const char* name, - SingletonVault* vault = nullptr /* for testing */) { - return get_ptr({typeid(T), name}, vault); + return static_cast( + (vault ?: SingletonVault::singleton())->get_ptr(typeDescriptor())); } + // Same as get, but should be preffered to it in the same compilation + // unit, where Singleton is registered. T* get_fast() { if (LIKELY(entry_->state == detail::SingletonEntryState::Living)) { return reinterpret_cast(entry_->instance_ptr); } else { - return get(type_descriptor_.name_raw().c_str(), vault_); + return get(vault_); } } @@ -547,13 +549,8 @@ class Singleton { // signal that the vault has been destroyed. static std::weak_ptr get_weak( SingletonVault* vault = nullptr /* for testing */) { - return get_weak("", vault); - } - - static std::weak_ptr get_weak( - const char* name, SingletonVault* vault = nullptr /* for testing */) { auto weak_void_ptr = - (vault ?: SingletonVault::singleton())->get_weak({typeid(T), name}); + (vault ?: SingletonVault::singleton())->get_weak(typeDescriptor()); // This is ugly and inefficient, but there's no other way to do it, because // there's no static_pointer_cast for weak_ptr. @@ -564,6 +561,8 @@ class Singleton { return std::static_pointer_cast(shared_void_ptr); } + // Same as get_weak, but should be preffered to it in the same compilation + // unit, where Singleton is registered. std::weak_ptr get_weak_fast() { if (LIKELY(entry_->state == detail::SingletonEntryState::Living)) { // This is ugly and inefficient, but there's no other way to do it, @@ -574,39 +573,39 @@ class Singleton { } return std::static_pointer_cast(shared_void_ptr); } else { - return get_weak(type_descriptor_.name_raw().c_str(), vault_); + return get_weak(vault_); } } // Allow the Singleton instance to also retrieve the underlying // singleton, if desired. - T* ptr() { return get_ptr(type_descriptor_, vault_); } + T* ptr() { return get_fast(); } T& operator*() { return *ptr(); } T* operator->() { return ptr(); } - template - explicit Singleton(CreateFunc c = nullptr, + explicit Singleton(std::nullptr_t _ = nullptr, Singleton::TeardownFunc t = nullptr, - SingletonVault* vault = nullptr /* for testing */) - : Singleton({typeid(T), ""}, c, t, vault) {} + SingletonVault* vault = nullptr) : + Singleton ([]() { return new T; }, + std::move(t), + vault) { + } - template - explicit Singleton(const char* name, - CreateFunc c = nullptr, + explicit Singleton(Singleton::CreateFunc c, Singleton::TeardownFunc t = nullptr, - SingletonVault* vault = nullptr /* for testing */) - : Singleton({typeid(T), name}, c, t, vault) {} + SingletonVault* vault = nullptr) { + if (c == nullptr) { + throw std::logic_error( + "nullptr_t should be passed if you want T to be default constructed"); + } - /** - * Construct and inject a mock singleton which should be used only from tests. - * See overloaded method for more details. - */ - template - static void make_mock(CreateFunc c = nullptr, - typename Singleton::TeardownFunc t = nullptr, - SingletonVault* vault = nullptr /* for testing */) { + if (vault == nullptr) { + vault = SingletonVault::singleton(); + } - make_mock("", c, t, vault); + vault_ = vault; + entry_ = + &(vault->registerSingleton(typeDescriptor(), c, getTeardownFunc(t))); } /** @@ -619,38 +618,15 @@ class Singleton { * the injection. The returned mock singleton is functionality identical to * regular singletons. */ - template - static void make_mock(const char* name, - CreateFunc c = nullptr, - typename Singleton::TeardownFunc t = nullptr, - SingletonVault* vault = nullptr /* for testing */ ) { - - Singleton mockSingleton({typeid(T), name}, c, t, vault, false); - mockSingleton.vault_->registerMockSingleton( - mockSingleton.type_descriptor_, - c, - getTeardownFunc(t)); + static void make_mock(std::nullptr_t c = nullptr, + typename Singleton::TeardownFunc t = nullptr, + SingletonVault* vault = nullptr /* for testing */ ) { + make_mock([]() { return new T; }, t, vault); } - private: - explicit Singleton(detail::TypeDescriptor type, - std::nullptr_t, - Singleton::TeardownFunc t, - SingletonVault* vault, - bool registerSingleton = true) : - Singleton (type, - []() { return new T; }, - std::move(t), - vault, - registerSingleton) { - } - - explicit Singleton(detail::TypeDescriptor type, - Singleton::CreateFunc c, - Singleton::TeardownFunc t, - SingletonVault* vault, - bool registerSingleton = true) - : type_descriptor_(type) { + static void make_mock(CreateFunc c, + typename Singleton::TeardownFunc t = nullptr, + SingletonVault* vault = nullptr /* for testing */ ) { if (c == nullptr) { throw std::logic_error( "nullptr_t should be passed if you want T to be default constructed"); @@ -660,24 +636,20 @@ class Singleton { vault = SingletonVault::singleton(); } - vault_ = vault; - if (registerSingleton) { - entry_ = &(vault->registerSingleton(type, c, getTeardownFunc(t))); - } + vault->registerMockSingleton( + typeDescriptor(), + c, + getTeardownFunc(t)); } - static inline void make_mock(const char* name, - std::nullptr_t c, - typename Singleton::TeardownFunc t = nullptr, - SingletonVault* vault = nullptr /* for testing */ ) { - make_mock(name, []() { return new T; }, std::move(t), vault); + private: + static detail::TypeDescriptor typeDescriptor() { + return {typeid(T), typeid(Tag)}; } - -private: // Construct SingletonVault::TeardownFunc. static SingletonVault::TeardownFunc getTeardownFunc( - Singleton::TeardownFunc t) { + TeardownFunc t) { SingletonVault::TeardownFunc teardown; if (t == nullptr) { teardown = [](void* v) { delete static_cast(v); }; @@ -688,30 +660,6 @@ private: return teardown; } - static T* get_ptr(detail::TypeDescriptor type_descriptor = {typeid(T), ""}, - SingletonVault* vault = nullptr /* for testing */) { - return static_cast( - (vault ?: SingletonVault::singleton())->get_ptr(type_descriptor)); - } - - // Don't use this function, it's private for a reason! Using it - // would defeat the *entire purpose* of the vault in that we lose - // the ability to guarantee that, after a destroyInstances is - // called, all instances are, in fact, destroyed. You should use - // weak_ptr if you need to hold a reference to the singleton and - // guarantee briefly that it exists. - // - // Yes, you can just get the weak pointer and lock it, but hopefully - // if you have taken the time to read this far, you see why that - // would be bad. - static std::shared_ptr get_shared( - detail::TypeDescriptor type_descriptor = {typeid(T), ""}, - SingletonVault* vault = nullptr /* for testing */) { - return std::static_pointer_cast( - (vault ?: SingletonVault::singleton())->get_weak(type_descriptor).lock()); - } - - detail::TypeDescriptor type_descriptor_; // This is pointing to SingletonEntry paired with this singleton object. This // is never reset, so each SingletonEntry should never be destroyed. // We rely on the fact that Singleton destructor won't reset this pointer, so diff --git a/folly/experimental/test/SingletonTest.cpp b/folly/experimental/test/SingletonTest.cpp index 9e195747..5d43c1be 100644 --- a/folly/experimental/test/SingletonTest.cpp +++ b/folly/experimental/test/SingletonTest.cpp @@ -126,7 +126,8 @@ TEST(Singleton, DirectUsage) { // Verify we can get to the underlying singletons via directly using // the singleton definition. Singleton watchdog(nullptr, nullptr, &vault); - Singleton named_watchdog("named", nullptr, nullptr, &vault); + struct TestTag {}; + Singleton named_watchdog(nullptr, nullptr, &vault); EXPECT_EQ(vault.registeredSingletonCount(), 2); vault.registrationComplete(); @@ -143,31 +144,32 @@ TEST(Singleton, NamedUsage) { EXPECT_EQ(vault.registeredSingletonCount(), 0); // Define two named Watchdog singletons and one unnamed singleton. - Singleton watchdog1_singleton( - "watchdog1", nullptr, nullptr, &vault); + struct Watchdog1 {}; + struct Watchdog2 {}; + typedef detail::DefaultTag Watchdog3; + Singleton watchdog1_singleton(nullptr, nullptr, &vault); EXPECT_EQ(vault.registeredSingletonCount(), 1); - Singleton watchdog2_singleton( - "watchdog2", nullptr, nullptr, &vault); + Singleton watchdog2_singleton(nullptr, nullptr, &vault); EXPECT_EQ(vault.registeredSingletonCount(), 2); - Singleton watchdog3_singleton(nullptr, nullptr, &vault); + Singleton watchdog3_singleton(nullptr, nullptr, &vault); EXPECT_EQ(vault.registeredSingletonCount(), 3); vault.registrationComplete(); // Verify our three singletons are distinct and non-nullptr. - Watchdog* s1 = Singleton::get("watchdog1", &vault); + Watchdog* s1 = Singleton::get(&vault); EXPECT_EQ(s1, watchdog1_singleton.ptr()); - Watchdog* s2 = Singleton::get("watchdog2", &vault); + Watchdog* s2 = Singleton::get(&vault); EXPECT_EQ(s2, watchdog2_singleton.ptr()); EXPECT_NE(s1, s2); - Watchdog* s3 = Singleton::get(&vault); + Watchdog* s3 = Singleton::get(&vault); EXPECT_EQ(s3, watchdog3_singleton.ptr()); EXPECT_NE(s3, s1); EXPECT_NE(s3, s2); - // Verify the "default" singleton is the same as the empty string + // Verify the "default" singleton is the same as the DefaultTag-tagged // singleton. - Watchdog* s4 = Singleton::get("", &vault); + Watchdog* s4 = Singleton::get(&vault); EXPECT_EQ(s4, watchdog3_singleton.ptr()); } @@ -216,8 +218,8 @@ TEST(Singleton, SharedPtrUsage) { Singleton child_watchdog_singleton(nullptr, nullptr, &vault); EXPECT_EQ(vault.registeredSingletonCount(), 2); - Singleton named_watchdog_singleton( - "a_name", nullptr, nullptr, &vault); + struct ATag {}; + Singleton named_watchdog_singleton(nullptr, nullptr, &vault); vault.registrationComplete(); Watchdog* s1 = Singleton::get(&vault); @@ -234,7 +236,7 @@ TEST(Singleton, SharedPtrUsage) { EXPECT_EQ(shared_s1.use_count(), 2); { - auto named_weak_s1 = Singleton::get_weak("a_name", &vault); + auto named_weak_s1 = Singleton::get_weak(&vault); auto locked = named_weak_s1.lock(); EXPECT_NE(locked.get(), shared_s1.get()); } diff --git a/folly/wangle/concurrent/GlobalExecutor.cpp b/folly/wangle/concurrent/GlobalExecutor.cpp index e8ac292e..35455921 100644 --- a/folly/wangle/concurrent/GlobalExecutor.cpp +++ b/folly/wangle/concurrent/GlobalExecutor.cpp @@ -24,7 +24,6 @@ using namespace folly::wangle; namespace { Singleton globalIOThreadPoolSingleton( - "GlobalIOThreadPool", [](){ return new IOThreadPoolExecutor( sysconf(_SC_NPROCESSORS_ONLN), @@ -42,7 +41,7 @@ IOExecutor* getIOExecutor() { IOExecutor* nullIOExecutor = nullptr; singleton->compare_exchange_strong( nullIOExecutor, - Singleton::get("GlobalIOThreadPool")); + globalIOThreadPoolSingleton.get_fast()); executor = singleton->load(); } return executor; diff --git a/folly/wangle/concurrent/IOExecutor.cpp b/folly/wangle/concurrent/IOExecutor.cpp index c5c3e796..d1b3283b 100644 --- a/folly/wangle/concurrent/IOExecutor.cpp +++ b/folly/wangle/concurrent/IOExecutor.cpp @@ -24,7 +24,6 @@ using folly::wangle::IOExecutor; namespace { Singleton> globalIOExecutorSingleton( - "GlobalIOExecutor", [](){ return new std::atomic(nullptr); }); @@ -44,7 +43,7 @@ IOExecutor::~IOExecutor() { } std::atomic* IOExecutor::getSingleton() { - return Singleton>::get("GlobalIOExecutor"); + return globalIOExecutorSingleton.get_fast(); } }} // folly::wangle