Merged PR 1590: Make Env::Default() returns a const ref

This change makes the Env::Default() to return a const reference to the environment.

Related work items: #153

Author: jaliyae

lotus/core/lib/threadpool.cc

@@ -25,7 +25,8 @@ limitations under the License.
 namespace Lotus {
 namespace thread {
 
-struct EigenEnvironment {
+class EigenEnvironment {
+ public:
   typedef Thread EnvThread;
 
   struct TaskImpl {
@@ -38,16 +39,12 @@ struct EigenEnvironment {
     std::unique_ptr<TaskImpl> f;
   };
 
-  Env* const env_;
-  const ThreadOptions thread_options_;
-  const std::string name_;
-
-  EigenEnvironment(Env* env, const ThreadOptions& thread_options,
+  EigenEnvironment(const Env& env, const ThreadOptions& thread_options,
                    const std::string& name)
       : env_(env), thread_options_(thread_options), name_(name) {}
 
   EnvThread* CreateThread(std::function<void()> f) {
-    return env_->StartThread(thread_options_, name_, [=]() {
+    return env_.StartThread(thread_options_, name_, [=]() {
       // TODO
       // Set the processor flag to flush denormals to zero.
       // port::ScopedFlushDenormal flush;
@@ -85,24 +82,29 @@ struct EigenEnvironment {
       t.f->f();
     }
   }
+
+ private:
+  const Env& env_;
+  const ThreadOptions thread_options_;
+  const std::string name_;
 };
 
 struct ThreadPool::Impl : Eigen::ThreadPoolTempl<EigenEnvironment> {
-  Impl(Env* env, const ThreadOptions& thread_options, const std::string& name,
+  Impl(const Env& env, const ThreadOptions& thread_options, const std::string& name,
        int num_threads, bool low_latency_hint)
       : Eigen::ThreadPoolTempl<EigenEnvironment>(
             num_threads, low_latency_hint,
             EigenEnvironment(env, thread_options, name)) {}
 };
 
-ThreadPool::ThreadPool(Env* env, const std::string& name, int num_threads)
+ThreadPool::ThreadPool(const Env& env, const std::string& name, int num_threads)
     : ThreadPool(env, ThreadOptions(), name, num_threads, true) {}
 
-ThreadPool::ThreadPool(Env* env, const ThreadOptions& thread_options,
+ThreadPool::ThreadPool(const Env& env, const ThreadOptions& thread_options,
                        const std::string& name, int num_threads)
     : ThreadPool(env, thread_options, name, num_threads, true) {}
 
-ThreadPool::ThreadPool(Env* env, const ThreadOptions& thread_options,
+ThreadPool::ThreadPool(const Env& env, const ThreadOptions& thread_options,
                        const std::string& name, int num_threads,
                        bool low_latency_hint) {
   LOTUS_ENFORCE(num_threads >= 1);

lotus/core/lib/threadpool.h

@@ -36,20 +36,20 @@ class ThreadPool {
   // operations like I/O the hint should be set to false.
   //
   // REQUIRES: num_threads > 0
-  ThreadPool(Env* env, const ThreadOptions& thread_options, const std::string& name,
+  ThreadPool(const Env& env, const ThreadOptions& thread_options, const std::string& name,
              int num_threads, bool low_latency_hint);
 
   // Constructs a pool for low-latency ops that contains "num_threads" threads
   // with specified "name". env->StartThread() is used to create individual
   // threads.
   // REQUIRES: num_threads > 0
-  ThreadPool(Env* env, const std::string& name, int num_threads);
+  ThreadPool(const Env& env, const std::string& name, int num_threads);
 
   // Constructs a pool for low-latency ops that contains "num_threads" threads
   // with specified "name". env->StartThread() is used to create individual
   // threads with the given ThreadOptions.
   // REQUIRES: num_threads > 0
-  ThreadPool(Env* env, const ThreadOptions& thread_options, const std::string& name,
+  ThreadPool(const Env& env, const ThreadOptions& thread_options, const std::string& name,
              int num_threads);
 
   // Waits until all scheduled work has finished and then destroy the

lotus/core/platform/env.h

@@ -40,7 +40,6 @@ struct ThreadOptions;
 /// multiple threads without any external synchronization.
 class Env {
  public:
-  Env();
   virtual ~Env() = default;
 
   /// \brief Returns a default environment suitable for the current operating
@@ -51,18 +50,18 @@ class Env {
   ///
   /// The result of Default() belongs to this library and must never be deleted.
   // TODO(Task:153) Lotus::Env::Default() should return const reference
-  static Env* Default();
+  static const Env& Default();
 
-  virtual int GetNumCpuCores() = 0;
+  virtual int GetNumCpuCores() const = 0;
 
   /// \brief Returns the number of micro-seconds since the Unix epoch.
-  virtual uint64 NowMicros() { return env_time_->NowMicros(); }
+  virtual uint64 NowMicros() const { return env_time_->NowMicros(); }
 
   /// \brief Returns the number of seconds since the Unix epoch.
-  virtual uint64 NowSeconds() { return env_time_->NowSeconds(); }
+  virtual uint64 NowSeconds() const { return env_time_->NowSeconds(); }
 
   /// Sleeps/delays the thread for the prescribed number of micro-seconds.
-  virtual void SleepForMicroseconds(int64 micros) = 0;
+  virtual void SleepForMicroseconds(int64 micros) const = 0;
 
   /// \brief Returns a new thread that is running fn() and is identified
   /// (for debugging/performance-analysis) by "name".
@@ -71,10 +70,13 @@ class Env {
   /// (the deletion will block until fn() stops running).
   virtual Thread* StartThread(const ThreadOptions& thread_options,
                               const std::string& name,
-                              std::function<void()> fn) = 0;
+                              std::function<void()> fn) const = 0;
 
   // TODO add filesystem related functions
 
+protected:
+  Env();
+
  private:
   LOTUS_DISALLOW_COPY_ASSIGN_AND_MOVE(Env);
   EnvTime* env_time_ = EnvTime::Default();

lotus/core/platform/posix/env.cc

@@ -43,13 +43,13 @@ class PosixEnv : public Env {
     return default_env;
   }
 
-  virtual int GetNumCpuCores() override {
+  virtual int GetNumCpuCores() const override {
     // TODO if you need the number of physical cores you'll need to parse
     // /proc/cpuinfo and grep for "cpu cores".
     return std::thread::hardware_concurrency();
   }
 
-  void SleepForMicroseconds(int64 micros) override {
+  void SleepForMicroseconds(int64 micros) const override {
     while (micros > 0) {
       timespec sleep_time;
       sleep_time.tv_sec = 0;
@@ -71,7 +71,7 @@ class PosixEnv : public Env {
   }
 
   Thread* StartThread(const ThreadOptions& thread_options, const std::string& name,
-                      std::function<void()> fn) override {
+                      std::function<void()> fn) const override {
     return new StdThread(thread_options, name, fn);
   }
 
@@ -84,8 +84,8 @@ class PosixEnv : public Env {
 #if defined(PLATFORM_POSIX) || defined(__ANDROID__)
 // REGISTER_FILE_SYSTEM("", PosixFileSystem);
 // REGISTER_FILE_SYSTEM("file", LocalPosixFileSystem);
-Env* Env::Default() {
-  return &PosixEnv::Instance();
+const Env& Env::Default() {
+  return PosixEnv::Instance();
 }
 #endif
 

lotus/core/platform/windows/env.cc

@@ -43,14 +43,14 @@ class StdThread : public Thread {
 
 class WindowsEnv : public Env {
  public:
-  void SleepForMicroseconds(int64 micros) override { Sleep(static_cast<DWORD>(micros) / 1000); }
+  void SleepForMicroseconds(int64 micros) const override { Sleep(static_cast<DWORD>(micros) / 1000); }
 
   Thread* StartThread(const ThreadOptions& thread_options, const std::string& name,
-                      std::function<void()> fn) override {
+                      std::function<void()> fn) const override {
     return new StdThread(thread_options, name, fn);
   }
 
-  virtual int GetNumCpuCores() override {
+  virtual int GetNumCpuCores() const override {
     SYSTEM_LOGICAL_PROCESSOR_INFORMATION buffer[256];
     DWORD returnLength = sizeof(buffer);
     if (GetLogicalProcessorInformation(buffer, &returnLength) == FALSE) {
@@ -101,8 +101,8 @@ class WindowsEnv : public Env {
 }  // namespace
 
 #if defined(PLATFORM_WINDOWS)
-Env* Env::Default() {
-  return &WindowsEnv::Instance();
+const Env& Env::Default() {
+  return WindowsEnv::Instance();
 }
 #endif
 

lotus/test/onnx/main.cc

@@ -62,8 +62,8 @@ int main(int argc, char* argv[]) {
   std::string provider = kCpuExecutionProvider;
   //if this var is not empty, only run the tests with name in this list
   std::vector<std::string> whitelisted_test_cases;
-  int concurrent_session_runs = Env::Default()->GetNumCpuCores();
-  int p_models = Env::Default()->GetNumCpuCores();
+  int concurrent_session_runs = Env::Default().GetNumCpuCores();
+  int p_models = Env::Default().GetNumCpuCores();
   {
     int ch;
     while ((ch = getopt(argc, argv, "c:hj:m:n:p:e:")) != -1) {

lotus/test/onnx/vstest_main.cc

@@ -59,11 +59,12 @@ static void run(const std::string& provider) {
 
 TEST_CLASS(ONNX_TEST){
   public :
-  TEST_METHOD(test_sequential_planner){
-     run<Lotus::AllocationPlannerType::SEQUENTIAL_PLANNER>(LotusIR::kCpuExecutionProvider);
-  }
+      TEST_METHOD(test_sequential_planner){
+          run<Lotus::AllocationPlannerType::SEQUENTIAL_PLANNER>(LotusIR::kCpuExecutionProvider);
+}
 
-  TEST_METHOD(test_simple_sequential_planner) {
-     run<Lotus::AllocationPlannerType::SIMPLE_SEQUENTIAL_PLANNER>(LotusIR::kCpuExecutionProvider);
-  }
-};
+TEST_METHOD(test_simple_sequential_planner) {
+  run<Lotus::AllocationPlannerType::SIMPLE_SEQUENTIAL_PLANNER>(LotusIR::kCpuExecutionProvider);
+}
+}
+;