From 4ae935e2cf89e6cf55173e2f5f849e1f5f43f596 Mon Sep 17 00:00:00 2001
From: tensor-tang <tangjian03@baidu.com>
Date: Wed, 6 Jun 2018 17:22:44 +0800
Subject: [PATCH] refine the lock in scope

---
 paddle/fluid/framework/scope.cc | 14 +++++++++-----
 paddle/fluid/framework/scope.h  |  8 ++++++--
 2 files changed, 15 insertions(+), 7 deletions(-)

diff --git a/paddle/fluid/framework/scope.cc b/paddle/fluid/framework/scope.cc
index d1850b055..4d6a71723 100644
--- a/paddle/fluid/framework/scope.cc
+++ b/paddle/fluid/framework/scope.cc
@@ -49,10 +49,10 @@ Scope& Scope::NewScope() const {
 }
 
 Variable* Scope::Var(const std::string& name) {
-  auto* v = FindVarLocally(name);
-  if (v != nullptr) return v;
   // acquire the lock when new var under this scope
   std::unique_lock<std::mutex> lock(mutex_);
+  auto* v = FindVarLocally(name);
+  if (v != nullptr) return v;
   v = new Variable();
   vars_[name] = v;
   VLOG(3) << "Create variable " << name;
@@ -69,11 +69,17 @@ Variable* Scope::Var(std::string* name) {
 }
 
 Variable* Scope::FindVar(const std::string& name) const {
+  // acquire the lock when find var
+  std::unique_lock<std::mutex> lock(mutex_);
+  return FindVarInternal(name);
+}
+
+Variable* Scope::FindVarInternal(const std::string& name) const {
   auto var = FindVarLocally(name);
   if (var != nullptr) {
     return var;
   }
-  return (parent_ == nullptr) ? nullptr : parent_->FindVar(name);
+  return (parent_ == nullptr) ? nullptr : parent_->FindVarInternal(name);
 }
 
 const Scope* Scope::FindScope(const Variable* var) const {
@@ -144,8 +150,6 @@ std::string Scope::Rename(const std::string& origin_name) const {
 }
 
 Variable* Scope::FindVarLocally(const std::string& name) const {
-  // acquire the lock when find locally
-  std::unique_lock<std::mutex> lock(mutex_);
   auto it = vars_.find(name);
   if (it != vars_.end()) return it->second;
   return nullptr;
diff --git a/paddle/fluid/framework/scope.h b/paddle/fluid/framework/scope.h
index abc82e452..9b8402ce6 100644
--- a/paddle/fluid/framework/scope.h
+++ b/paddle/fluid/framework/scope.h
@@ -78,12 +78,16 @@ class Scope {
   // Rename variable to a new name and return the new name
   std::string Rename(const std::string& origin_name) const;
 
-  Variable* FindVarLocally(const std::string& name) const;
-
  private:
   // Call Scope::NewScope for a sub-scope.
   explicit Scope(Scope const* parent) : parent_(parent) {}
 
+  // Called by FindVar recursively
+  Variable* FindVarInternal(const std::string& name) const;
+
+  // Called by FindVarInternal and Var
+  Variable* FindVarLocally(const std::string& name) const;
+
   mutable std::unordered_map<std::string, Variable*> vars_;
   mutable std::list<Scope*> kids_;
   Scope const* parent_{nullptr};
-- 
GitLab