Commit 5d1db31b authored by Andrii Grynenko's avatar Andrii Grynenko Committed by Alecs King

Kill get_fast/get_weak_fast Singletonn API

Summary: After D1827390 regular get and get_weak is comparable to Meyers and static singletons, so there's no need to keey _fast APIs.

Test Plan: benchmark && fbmake

Reviewed By: mshneer@fb.com

Subscribers: trunkagent, configerator-diffs@, fbcode-common-diffs@, mcduff, hitesh, mshneer, fugalh, acampi, alikhtarov, folly-diffs@, jsedgwick, yfeldblum

FB internal diff: D1843219

Signature: t1:1843219:1424216566:f2f182a1c86bb5f0fb1f978d8c6b7a4388198f5f
parent b35c3434
...@@ -34,18 +34,9 @@ ...@@ -34,18 +34,9 @@
// std::weak_ptr<MyExpensiveService> instance = // std::weak_ptr<MyExpensiveService> instance =
// Singleton<MyExpensiveService>::get_weak(); // Singleton<MyExpensiveService>::get_weak();
// //
// Within same compilation unit you should directly access it by the variable // You also can directly access it by the variable defining the
// defining the singleton via get_fast()/get_weak_fast(), and even treat that // singleton rather than via get(), and even treat that variable like
// variable like a smart pointer (dereferencing it or using the -> operator): // a smart pointer (dereferencing it or using the -> operator).
//
// MyExpensiveService* instance = the_singleton.get_fast();
// or
// std::weak_ptr<MyExpensiveService> 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 // Please note, however, that all non-weak_ptr interfaces are
// inherently subject to races with destruction. Use responsibly. // inherently subject to races with destruction. Use responsibly.
...@@ -67,9 +58,9 @@ ...@@ -67,9 +58,9 @@
// folly::Singleton<MyExpensiveService, Tag2> s2(); // folly::Singleton<MyExpensiveService, Tag2> s2();
// } // }
// ... // ...
// MyExpensiveService* svc_default = s_default.get_fast(); // MyExpensiveService* svc_default = s_default.get();
// MyExpensiveService* svc1 = s1.get_fast(); // MyExpensiveService* svc1 = s1.get();
// MyExpensiveService* svc2 = s2.get_fast(); // MyExpensiveService* svc2 = s2.get();
// //
// By default, the singleton instance is constructed via new and // By default, the singleton instance is constructed via new and
// deleted via delete, but this is configurable: // deleted via delete, but this is configurable:
...@@ -448,12 +439,6 @@ class Singleton { ...@@ -448,12 +439,6 @@ class Singleton {
return getEntry().get(); return getEntry().get();
} }
// Same as get, but should be preffered to it in the same compilation
// unit, where Singleton is registered.
T* get_fast() {
return entry_.get();
}
// If, however, you do need to hold a reference to the specific // If, however, you do need to hold a reference to the specific
// singleton, you can try to do so with a weak_ptr. Avoid this when // singleton, you can try to do so with a weak_ptr. Avoid this when
// possible but the inability to lock the weak pointer can be a // possible but the inability to lock the weak pointer can be a
...@@ -462,17 +447,10 @@ class Singleton { ...@@ -462,17 +447,10 @@ class Singleton {
return getEntry().get_weak(); return getEntry().get_weak();
} }
// Same as get_weak, but should be preffered to it in the same compilation
// unit, where Singleton is registered.
std::weak_ptr<T> get_weak_fast() {
return entry_.get_weak();
}
// Allow the Singleton<t> instance to also retrieve the underlying // Allow the Singleton<t> instance to also retrieve the underlying
// singleton, if desired. // singleton, if desired.
T* ptr() { return get_fast(); } T& operator*() { return *get(); }
T& operator*() { return *ptr(); } T* operator->() { return get(); }
T* operator->() { return ptr(); }
explicit Singleton(std::nullptr_t _ = nullptr, explicit Singleton(std::nullptr_t _ = nullptr,
Singleton::TeardownFunc t = nullptr) : Singleton::TeardownFunc t = nullptr) :
...@@ -480,15 +458,15 @@ class Singleton { ...@@ -480,15 +458,15 @@ class Singleton {
} }
explicit Singleton(Singleton::CreateFunc c, explicit Singleton(Singleton::CreateFunc c,
Singleton::TeardownFunc t = nullptr) : entry_(getEntry()) { Singleton::TeardownFunc t = nullptr) {
if (c == nullptr) { if (c == nullptr) {
throw std::logic_error( throw std::logic_error(
"nullptr_t should be passed if you want T to be default constructed"); "nullptr_t should be passed if you want T to be default constructed");
} }
auto vault = SingletonVault::singleton<VaultTag>(); auto vault = SingletonVault::singleton<VaultTag>();
entry_.registerSingleton(std::move(c), getTeardownFunc(std::move(t))); getEntry().registerSingleton(std::move(c), getTeardownFunc(std::move(t)));
vault->registerSingleton(&entry_); vault->registerSingleton(&getEntry());
} }
/** /**
...@@ -532,12 +510,6 @@ class Singleton { ...@@ -532,12 +510,6 @@ class Singleton {
return t; return t;
} }
} }
// This is pointing to SingletonHolder paired with this singleton object. This
// is never reset, so each SingletonHolder should never be destroyed.
// We rely on the fact that Singleton destructor won't reset this pointer, so
// it can be "safely" used even after static Singleton object is destroyed.
detail::SingletonHolder<T>& entry_;
}; };
} }
......
...@@ -139,9 +139,9 @@ TEST(Singleton, DirectUsage) { ...@@ -139,9 +139,9 @@ TEST(Singleton, DirectUsage) {
EXPECT_EQ(vault.registeredSingletonCount(), 2); EXPECT_EQ(vault.registeredSingletonCount(), 2);
vault.registrationComplete(); vault.registrationComplete();
EXPECT_NE(watchdog.ptr(), nullptr); EXPECT_NE(watchdog.get(), nullptr);
EXPECT_EQ(watchdog.ptr(), SingletonDirectUsage<Watchdog>::get()); EXPECT_EQ(watchdog.get(), SingletonDirectUsage<Watchdog>::get());
EXPECT_NE(watchdog.ptr(), named_watchdog.ptr()); EXPECT_NE(watchdog.get(), named_watchdog.get());
EXPECT_EQ(watchdog->livingWatchdogCount(), 2); EXPECT_EQ(watchdog->livingWatchdogCount(), 2);
EXPECT_EQ((*watchdog).livingWatchdogCount(), 2); EXPECT_EQ((*watchdog).livingWatchdogCount(), 2);
...@@ -172,19 +172,19 @@ TEST(Singleton, NamedUsage) { ...@@ -172,19 +172,19 @@ TEST(Singleton, NamedUsage) {
// Verify our three singletons are distinct and non-nullptr. // Verify our three singletons are distinct and non-nullptr.
Watchdog* s1 = SingletonNamedUsage<Watchdog, Watchdog1>::get(); Watchdog* s1 = SingletonNamedUsage<Watchdog, Watchdog1>::get();
EXPECT_EQ(s1, watchdog1_singleton.ptr()); EXPECT_EQ(s1, watchdog1_singleton.get());
Watchdog* s2 = SingletonNamedUsage<Watchdog, Watchdog2>::get(); Watchdog* s2 = SingletonNamedUsage<Watchdog, Watchdog2>::get();
EXPECT_EQ(s2, watchdog2_singleton.ptr()); EXPECT_EQ(s2, watchdog2_singleton.get());
EXPECT_NE(s1, s2); EXPECT_NE(s1, s2);
Watchdog* s3 = SingletonNamedUsage<Watchdog, Watchdog3>::get(); Watchdog* s3 = SingletonNamedUsage<Watchdog, Watchdog3>::get();
EXPECT_EQ(s3, watchdog3_singleton.ptr()); EXPECT_EQ(s3, watchdog3_singleton.get());
EXPECT_NE(s3, s1); EXPECT_NE(s3, s1);
EXPECT_NE(s3, s2); EXPECT_NE(s3, s2);
// Verify the "default" singleton is the same as the DefaultTag-tagged // Verify the "default" singleton is the same as the DefaultTag-tagged
// singleton. // singleton.
Watchdog* s4 = SingletonNamedUsage<Watchdog>::get(); Watchdog* s4 = SingletonNamedUsage<Watchdog>::get();
EXPECT_EQ(s4, watchdog3_singleton.ptr()); EXPECT_EQ(s4, watchdog3_singleton.get());
vault.destroyInstances(); vault.destroyInstances();
} }
...@@ -499,23 +499,21 @@ struct BenchmarkTag {}; ...@@ -499,23 +499,21 @@ struct BenchmarkTag {};
template <typename T, typename Tag = detail::DefaultTag> template <typename T, typename Tag = detail::DefaultTag>
using SingletonBenchmark = Singleton <T, Tag, BenchmarkTag>; using SingletonBenchmark = Singleton <T, Tag, BenchmarkTag>;
SingletonBenchmark<BenchmarkSingleton> benchmark_singleton; struct GetTag{};
struct GetWeakTag{};
BENCHMARK_RELATIVE(FollySingletonSlow, n) { SingletonBenchmark<BenchmarkSingleton, GetTag> benchmark_singleton_get;
for (size_t i = 0; i < n; ++i) { SingletonBenchmark<BenchmarkSingleton, GetWeakTag> benchmark_singleton_get_weak;
doNotOptimizeAway(SingletonBenchmark<BenchmarkSingleton>::get());
}
}
BENCHMARK_RELATIVE(FollySingletonFast, n) { BENCHMARK_RELATIVE(FollySingleton, n) {
for (size_t i = 0; i < n; ++i) { for (size_t i = 0; i < n; ++i) {
doNotOptimizeAway(benchmark_singleton.get_fast()); doNotOptimizeAway(SingletonBenchmark<BenchmarkSingleton, GetTag>::get());
} }
} }
BENCHMARK_RELATIVE(FollySingletonFastWeak, n) { BENCHMARK_RELATIVE(FollySingletonWeak, n) {
for (size_t i = 0; i < n; ++i) { for (size_t i = 0; i < n; ++i) {
benchmark_singleton.get_weak_fast(); SingletonBenchmark<BenchmarkSingleton, GetWeakTag>::get_weak();
} }
} }
......
...@@ -87,7 +87,7 @@ Future<void> ThreadWheelTimekeeper::after(Duration dur) { ...@@ -87,7 +87,7 @@ Future<void> ThreadWheelTimekeeper::after(Duration dur) {
} }
Timekeeper* getTimekeeperSingleton() { Timekeeper* getTimekeeperSingleton() {
return timekeeperSingleton_.get_fast(); return timekeeperSingleton_.get();
} }
}} // folly::detail }} // folly::detail
...@@ -60,8 +60,8 @@ std::shared_ptr<Exe> getExecutor( ...@@ -60,8 +60,8 @@ std::shared_ptr<Exe> getExecutor(
Singleton<std::shared_ptr<DefaultExe>>& sDefaultExecutor, Singleton<std::shared_ptr<DefaultExe>>& sDefaultExecutor,
Singleton<RWSpinLock, LockTag>& sExecutorLock) { Singleton<RWSpinLock, LockTag>& sExecutorLock) {
std::shared_ptr<Exe> executor; std::shared_ptr<Exe> executor;
auto singleton = sExecutor.get_fast(); auto singleton = sExecutor.get();
auto lock = sExecutorLock.get_fast(); auto lock = sExecutorLock.get();
{ {
RWSpinLock::ReadHolder guard(lock); RWSpinLock::ReadHolder guard(lock);
...@@ -74,7 +74,7 @@ std::shared_ptr<Exe> getExecutor( ...@@ -74,7 +74,7 @@ std::shared_ptr<Exe> getExecutor(
RWSpinLock::WriteHolder guard(lock); RWSpinLock::WriteHolder guard(lock);
executor = singleton->lock(); executor = singleton->lock();
if (!executor) { if (!executor) {
executor = *sDefaultExecutor.get_fast(); executor = *sDefaultExecutor.get();
*singleton = executor; *singleton = executor;
} }
return executor; return executor;
...@@ -85,8 +85,8 @@ void setExecutor( ...@@ -85,8 +85,8 @@ void setExecutor(
std::shared_ptr<Exe> executor, std::shared_ptr<Exe> executor,
Singleton<std::weak_ptr<Exe>>& sExecutor, Singleton<std::weak_ptr<Exe>>& sExecutor,
Singleton<RWSpinLock, LockTag>& sExecutorLock) { Singleton<RWSpinLock, LockTag>& sExecutorLock) {
RWSpinLock::WriteHolder guard(sExecutorLock.get_fast()); RWSpinLock::WriteHolder guard(sExecutorLock.get());
*sExecutor.get_fast() = std::move(executor); *sExecutor.get() = std::move(executor);
} }
std::shared_ptr<Executor> getCPUExecutor() { std::shared_ptr<Executor> getCPUExecutor() {
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment