Singleton: refine "eager" initialization
authorSteve O'Brien <steveo@fb.com>
Fri, 25 Sep 2015 18:02:46 +0000 (11:02 -0700)
committerfacebook-github-bot-9 <folly-bot@fb.com>
Fri, 25 Sep 2015 18:20:17 +0000 (11:20 -0700)
Summary: * `registrationComplete` has a slightly different interface (uses enums now)
* `void doEagerInit()` method initializes synchronously; `Future<Unit> doEagerInitVia(Executor*)` now available.

Reviewed By: @luciang, @meyering

Differential Revision: D2463464

folly/Singleton.h
folly/test/SingletonTest.cpp
folly/test/SingletonTestGlobal.cpp [new file with mode: 0644]
folly/test/SingletonTestStructs.h [new file with mode: 0644]

index baff2a0a64ec4c7d915032481c81679268edda10..1d8ffa2749ccb67eec936e0c3c8665b1c424c09b 100644 (file)
 //     .shouldEagerInit();
 // }
 //
-// This way the singleton's instance is built at program initialization
-// time, or more accurately, when "registrationComplete()" or
-// "startEagerInit()" is called. (More about that below; see the
-// section starting with "A vault goes through a few stages of life".)
+// This way the singleton's instance is built at program initialization,
+// if the program opted-in to that feature by calling "doEagerInit" or
+// "doEagerInitVia" during its startup.
 //
 // What if you need to destroy all of your singletons?  Say, some of
 // your singletons manage threads, but you need to fork?  Or your unit
 #include <folly/RWSpinLock.h>
 #include <folly/Demangle.h>
 #include <folly/Executor.h>
+#include <folly/futures/Future.h>
 #include <folly/io/async/Request.h>
 
 #include <algorithm>
@@ -334,7 +334,7 @@ class SingletonVault {
 
   /**
    * Called by `Singleton<T>.shouldEagerInit()` to ensure the instance
-   * is built when registrationComplete() is called; see that method
+   * is built when `doEagerInit[Via]` is called; see those methods
    * for more info.
    */
   void addEagerInitSingleton(detail::SingletonHolderBase* entry) {
@@ -344,7 +344,7 @@ class SingletonVault {
 
     if (UNLIKELY(registrationComplete_)) {
       throw std::logic_error(
-        "Registering for eager-load after registrationComplete().");
+          "Registering for eager-load after registrationComplete().");
     }
 
     RWSpinLock::ReadHolder rhMutex(&mutex_);
@@ -356,75 +356,75 @@ class SingletonVault {
   }
 
   // Mark registration is complete; no more singletons can be
-  // registered at this point.  Kicks off eagerly-initialized singletons
-  // (if requested; default behavior is to do so).
-  void registrationComplete(bool autoStartEagerInit = true) {
+  // registered at this point.
+  void registrationComplete() {
     RequestContext::saveContext();
     std::atexit([](){ SingletonVault::singleton()->destroyInstances(); });
 
-    {
-      RWSpinLock::WriteHolder wh(&stateMutex_);
+    RWSpinLock::WriteHolder wh(&stateMutex_);
 
-      stateCheck(SingletonVaultState::Running);
+    stateCheck(SingletonVaultState::Running);
 
-      if (type_ == Type::Strict) {
-        for (const auto& p: singletons_) {
-          if (p.second->hasLiveInstance()) {
-            throw std::runtime_error(
+    if (type_ == Type::Strict) {
+      for (const auto& p : singletons_) {
+        if (p.second->hasLiveInstance()) {
+          throw std::runtime_error(
               "Singleton created before registration was complete.");
-          }
         }
       }
-
-      registrationComplete_ = true;
     }
 
-    if (autoStartEagerInit) {
-      startEagerInit();
-    }
+    registrationComplete_ = true;
   }
 
- /**
-  * If eagerInitExecutor_ is non-nullptr (default is nullptr) then
-  * schedule eager singletons' initializations through it.
-  * Otherwise, initializes them synchronously, in a loop.
-  */
-  void startEagerInit() {
+  /**
+   * Initialize all singletons which were marked as eager-initialized
+   * (using `shouldEagerInit()`).  No return value.  Propagates exceptions
+   * from constructors / create functions, as is the usual case when calling
+   * for example `Singleton<Foo>::get_weak()`.
+   */
+  void doEagerInit() {
     std::unordered_set<detail::SingletonHolderBase*> singletonSet;
     {
       RWSpinLock::ReadHolder rh(&stateMutex_);
       stateCheck(SingletonVaultState::Running);
       if (UNLIKELY(!registrationComplete_)) {
-        throw std::logic_error(
-          "registrationComplete() not yet called");
+        throw std::logic_error("registrationComplete() not yet called");
       }
-      singletonSet = eagerInitSingletons_;  // copy set of pointers
+      singletonSet = eagerInitSingletons_; // copy set of pointers
     }
 
-    auto *exe = eagerInitExecutor_;  // default value is nullptr
     for (auto *single : singletonSet) {
-      if (exe) {
-        eagerInitExecutor_->add([single] {
-          if (!single->creationStarted()) {
-            single->createInstance();
-          }
-        });
-      } else {
-        single->createInstance();
-      }
+      single->createInstance();
     }
   }
 
   /**
-   * Provide an executor through which startEagerInit would run tasks.
-   * If there are several singletons which may be independently initialized,
-   * and their construction takes long, they could possibly be run in parallel
-   * to cut down on startup time.  Unusual; default (synchronous initialization
-   * in a loop) is probably fine for most use cases, and most apps can most
-   * likely avoid using this.
+   * Schedule eager singletons' initializations through the given executor.
+   * Return a future which is fulfilled after all the initialization functions
+   * complete.
    */
-  void setEagerInitExecutor(folly::Executor *exe) {
-    eagerInitExecutor_ = exe;
+  Future<Unit> doEagerInitVia(Executor* exe) {
+    std::unordered_set<detail::SingletonHolderBase*> singletonSet;
+    {
+      RWSpinLock::ReadHolder rh(&stateMutex_);
+      stateCheck(SingletonVaultState::Running);
+      if (UNLIKELY(!registrationComplete_)) {
+        throw std::logic_error("registrationComplete() not yet called");
+      }
+      singletonSet = eagerInitSingletons_; // copy set of pointers
+    }
+
+    std::vector<Future<Unit>> resultFutures;
+    for (auto* single : singletonSet) {
+      resultFutures.emplace_back(via(exe).then([single] {
+        if (!single->creationStarted()) {
+          single->createInstance();
+        }
+      }));
+    }
+
+    return collectAll(resultFutures).via(exe).then();
   }
 
   // Destroy all singletons; when complete, the vault can't create
@@ -515,7 +515,6 @@ class SingletonVault {
   mutable folly::RWSpinLock mutex_;
   SingletonMap singletons_;
   std::unordered_set<detail::SingletonHolderBase*> eagerInitSingletons_;
-  folly::Executor* eagerInitExecutor_{nullptr};
   std::vector<detail::TypeDescriptor> creation_order_;
   SingletonVaultState state_{SingletonVaultState::Running};
   bool registrationComplete_{false};
@@ -584,7 +583,7 @@ class Singleton {
   }
 
   /**
-   * Should be instantiated as soon as "registrationComplete()" is called.
+   * Should be instantiated as soon as "doEagerInit[Via]" is called.
    * Singletons are usually lazy-loaded (built on-demand) but for those which
    * are known to be needed, to avoid the potential lag for objects that take
    * long to construct during runtime, there is an option to make sure these
@@ -594,12 +593,10 @@ class Singleton {
    *   Singleton<Foo> gFooInstance = Singleton<Foo>(...).shouldEagerInit();
    *
    * Or alternately, define the singleton as usual, and say
-   *   gFooInstance.shouldEagerInit()
+   *   gFooInstance.shouldEagerInit();
    *
    * at some point prior to calling registrationComplete().
-   * Then registrationComplete can be called (by default it will kick off
-   * init of the eager singletons); alternately, you can use
-   * startEagerInit().
+   * Then doEagerInit() or doEagerInitVia(Executor*) can be called.
    */
   Singleton& shouldEagerInit() {
     auto vault = SingletonVault::singleton<VaultTag>();
index dbdb579b6ac7d378755ccc9e8c21c91d423eeff8..55152751567bcc7d00a7849aabc1f8afb943ea2a 100644 (file)
@@ -18,7 +18,7 @@
 
 #include <folly/Singleton.h>
 #include <folly/io/async/EventBase.h>
-
+#include <folly/test/SingletonTestStructs.h>
 #include <folly/Benchmark.h>
 
 #include <glog/logging.h>
 
 using namespace folly;
 
-// A simple class that tracks how often instances of the class and
-// subclasses are created, and the ordering.  Also tracks a global
-// unique counter for each object.
-std::atomic<size_t> global_counter(19770326);
-struct Watchdog {
-  static std::vector<Watchdog*> creation_order;
-  Watchdog() : serial_number(++global_counter) {
-    creation_order.push_back(this);
-  }
-
-  ~Watchdog() {
-    if (creation_order.back() != this) {
-      throw std::out_of_range("Watchdog destruction order mismatch");
-    }
-    creation_order.pop_back();
-  }
-
-  const size_t serial_number;
-  size_t livingWatchdogCount() const { return creation_order.size(); }
-
-  Watchdog(const Watchdog&) = delete;
-  Watchdog& operator=(const Watchdog&) = delete;
-  Watchdog(Watchdog&&) noexcept = default;
-};
-
-std::vector<Watchdog*> Watchdog::creation_order;
-
-// Some basic types we use for tracking.
-struct ChildWatchdog : public Watchdog {};
-struct GlobalWatchdog : public Watchdog {};
-struct UnregisteredWatchdog : public Watchdog {};
-
-namespace {
-Singleton<GlobalWatchdog> global_watchdog;
-}
-
-// Test basic global usage (the default way singletons will generally
-// be used).
-TEST(Singleton, BasicGlobalUsage) {
-  EXPECT_EQ(Watchdog::creation_order.size(), 0);
-  EXPECT_EQ(SingletonVault::singleton()->registeredSingletonCount(), 1);
-  EXPECT_EQ(SingletonVault::singleton()->livingSingletonCount(), 0);
-
-  {
-    std::shared_ptr<GlobalWatchdog> wd1 = Singleton<GlobalWatchdog>::try_get();
-    EXPECT_NE(wd1, nullptr);
-    EXPECT_EQ(Watchdog::creation_order.size(), 1);
-    std::shared_ptr<GlobalWatchdog> wd2 = Singleton<GlobalWatchdog>::try_get();
-    EXPECT_NE(wd2, nullptr);
-    EXPECT_EQ(wd1.get(), wd2.get());
-    EXPECT_EQ(Watchdog::creation_order.size(), 1);
-  }
-
-  SingletonVault::singleton()->destroyInstances();
-  EXPECT_EQ(Watchdog::creation_order.size(), 0);
-}
-
 TEST(Singleton, MissingSingleton) {
   EXPECT_DEATH([]() { auto u = Singleton<UnregisteredWatchdog>::try_get(); }(),
       "");
@@ -496,6 +439,8 @@ TEST(Singleton, SingletonEagerInitSync) {
                   [&] {didEagerInit = true; return new std::string("foo"); })
               .shouldEagerInit();
   vault.registrationComplete();
+  EXPECT_FALSE(didEagerInit);
+  vault.doEagerInit();
   EXPECT_TRUE(didEagerInit);
   sing.get_weak();  // (avoid compile error complaining about unused var 'sing')
 }
@@ -512,10 +457,11 @@ TEST(Singleton, SingletonEagerInitAsync) {
                   [&] {didEagerInit = true; return new std::string("foo"); })
               .shouldEagerInit();
   folly::EventBase eb;
-  vault.setEagerInitExecutor(&eb);
   vault.registrationComplete();
   EXPECT_FALSE(didEagerInit);
+  auto result = vault.doEagerInitVia(&eb); // a Future<Unit> is returned
   eb.loop();
+  result.get(); // ensure this completed successfully and didn't hang forever
   EXPECT_TRUE(didEagerInit);
   sing.get_weak();  // (avoid compile error complaining about unused var 'sing')
 }
@@ -582,23 +528,23 @@ TEST(Singleton, SingletonEagerInitParallel) {
     initCounter.store(0);
 
     {
-      boost::barrier barrier(kThreads + 1);
+      std::vector<std::shared_ptr<std::thread>> threads;
+      boost::barrier barrier(kThreads);
       TestEagerInitParallelExecutor exe(kThreads);
-      vault.setEagerInitExecutor(&exe);
-      vault.registrationComplete(false);
+      vault.registrationComplete();
 
       EXPECT_EQ(0, initCounter.load());
 
       for (size_t j = 0; j < kThreads; j++) {
-        exe.add([&] {
+        threads.push_back(std::make_shared<std::thread>([&] {
           barrier.wait();
-          vault.startEagerInit();
-          barrier.wait();
-        });
+          vault.doEagerInitVia(&exe).get();
+        }));
       }
 
-      barrier.wait();  // to await all threads' readiness
-      barrier.wait();  // to await all threads' completion
+      for (auto thread : threads) {
+        thread->join();
+      }
     }
 
     EXPECT_EQ(1, initCounter.load());
diff --git a/folly/test/SingletonTestGlobal.cpp b/folly/test/SingletonTestGlobal.cpp
new file mode 100644 (file)
index 0000000..2422844
--- /dev/null
@@ -0,0 +1,59 @@
+/*
+ * Copyright 2015 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 <memory>
+#include <vector>
+
+#include <folly/Singleton.h>
+#include <folly/Benchmark.h>
+#include <folly/test/SingletonTestStructs.h>
+
+#include <glog/logging.h>
+#include <gtest/gtest.h>
+
+/*
+ * This test needs to be in its own file, as a standalone program.
+ * We want to ensure no other singletons are registered, so we can
+ * rely on some expectations about registered and living counts, etc.
+ * All other tests should go in `SingletonTest.cpp`.
+ */
+
+using namespace folly;
+
+namespace {
+Singleton<GlobalWatchdog> global_watchdog;
+}
+
+// Test basic global usage (the default way singletons will generally
+// be used).
+TEST(Singleton, BasicGlobalUsage) {
+  EXPECT_EQ(Watchdog::creation_order().size(), 0);
+  EXPECT_EQ(SingletonVault::singleton()->registeredSingletonCount(), 1);
+  EXPECT_EQ(SingletonVault::singleton()->livingSingletonCount(), 0);
+
+  {
+    std::shared_ptr<GlobalWatchdog> wd1 = Singleton<GlobalWatchdog>::try_get();
+    EXPECT_NE(wd1, nullptr);
+    EXPECT_EQ(Watchdog::creation_order().size(), 1);
+    std::shared_ptr<GlobalWatchdog> wd2 = Singleton<GlobalWatchdog>::try_get();
+    EXPECT_NE(wd2, nullptr);
+    EXPECT_EQ(wd1.get(), wd2.get());
+    EXPECT_EQ(Watchdog::creation_order().size(), 1);
+  }
+
+  SingletonVault::singleton()->destroyInstances();
+  EXPECT_EQ(Watchdog::creation_order().size(), 0);
+}
diff --git a/folly/test/SingletonTestStructs.h b/folly/test/SingletonTestStructs.h
new file mode 100644 (file)
index 0000000..cca975d
--- /dev/null
@@ -0,0 +1,72 @@
+/*
+ * Copyright 2015 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
+
+/*
+ * Copyright 2015 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 <memory>
+#include <vector>
+#include <glog/logging.h>
+#include <gtest/gtest.h>
+
+// A simple class that tracks how often instances of the class and
+// subclasses are created, and the ordering.  Also tracks a global
+// unique counter for each object.
+std::atomic<size_t> global_counter(19770326);
+struct Watchdog {
+  static std::vector<Watchdog*>& creation_order() {
+    static std::vector<Watchdog*> ret;
+    return ret;
+  }
+
+  Watchdog() : serial_number(++global_counter) {
+    creation_order().push_back(this);
+  }
+
+  ~Watchdog() {
+    if (creation_order().back() != this) {
+      throw std::out_of_range("Watchdog destruction order mismatch");
+    }
+    creation_order().pop_back();
+  }
+
+  const size_t serial_number;
+  size_t livingWatchdogCount() const { return creation_order().size(); }
+
+  Watchdog(const Watchdog&) = delete;
+  Watchdog& operator=(const Watchdog&) = delete;
+  Watchdog(Watchdog&&) noexcept = default;
+  Watchdog& operator=(Watchdog&&) noexcept = default;
+};
+
+// Some basic types we use for tracking.
+struct ChildWatchdog : public Watchdog {};
+struct GlobalWatchdog : public Watchdog {};
+struct UnregisteredWatchdog : public Watchdog {};