diff --git a/paddle/fluid/framework/scope.cc b/paddle/fluid/framework/scope.cc index 4d6a7172308fbb36c9225d937e3d3debcd450d1a..bb2d866c824e0fec1b241caea407a38c88a3cb51 100644 --- a/paddle/fluid/framework/scope.cc +++ b/paddle/fluid/framework/scope.cc @@ -34,13 +34,7 @@ DEFINE_bool( namespace paddle { namespace framework { -Scope::~Scope() { - DropKids(); - for (auto& kv : vars_) { - VLOG(3) << "Destroy variable " << kv.first; - delete kv.second; - } -} +Scope::~Scope() { DropKids(); } Scope& Scope::NewScope() const { std::unique_lock lock(mutex_); @@ -53,8 +47,9 @@ Variable* Scope::Var(const std::string& name) { std::unique_lock lock(mutex_); auto* v = FindVarLocally(name); if (v != nullptr) return v; + v = new Variable(); - vars_[name] = v; + vars_[name].reset(v); VLOG(3) << "Create variable " << name; v->name_ = &(vars_.find(name)->first); return v; @@ -84,7 +79,7 @@ Variable* Scope::FindVarInternal(const std::string& name) const { const Scope* Scope::FindScope(const Variable* var) const { for (auto& kv : vars_) { - if (kv.second == var) { + if (kv.second.get() == var) { return this; } } @@ -123,7 +118,6 @@ void Scope::EraseVars(const std::vector& var_names) { std::set var_set(var_names.begin(), var_names.end()); for (auto it = vars_.begin(); it != vars_.end();) { if (var_set.find(it->first) != var_set.end()) { - delete it->second; it = vars_.erase(it); } else { ++it; @@ -139,7 +133,7 @@ void Scope::Rename(const std::string& origin_name, auto new_it = vars_.find(new_name); PADDLE_ENFORCE(new_it == vars_.end(), "The variable with name %s is already in the scope", new_name); - vars_[new_name] = origin_it->second; + vars_[new_name].reset(origin_it->second.release()); vars_.erase(origin_it); } @@ -151,7 +145,7 @@ std::string Scope::Rename(const std::string& origin_name) const { Variable* Scope::FindVarLocally(const std::string& name) const { auto it = vars_.find(name); - if (it != vars_.end()) return it->second; + if (it != vars_.end()) return it->second.get(); return nullptr; } diff --git a/paddle/fluid/framework/scope.h b/paddle/fluid/framework/scope.h index 9b8402ce6838a59c3c58bf40d071858fe0a452dc..98d103d867987fc02dc66df5ac855a14b66b8f03 100644 --- a/paddle/fluid/framework/scope.h +++ b/paddle/fluid/framework/scope.h @@ -47,15 +47,18 @@ class Scope { Scope& NewScope() const; /// Create a variable with given name if it doesn't exist. + /// Caller doesn't own the returned Variable. Variable* Var(const std::string& name); /// Create a variable with a scope-unique name. + /// Caller doesn't own the returned Variable. Variable* Var(std::string* name = nullptr); void EraseVars(const std::vector& var_names); /// Find a variable in the scope or any of its ancestors. Returns /// nullptr if cannot find. + /// Caller doesn't own the returned Variable. Variable* FindVar(const std::string& name) const; const Scope* parent() const { return parent_; } @@ -82,13 +85,17 @@ class Scope { // Call Scope::NewScope for a sub-scope. explicit Scope(Scope const* parent) : parent_(parent) {} - // Called by FindVar recursively + // Called by FindVar recursively. + // Caller doesn't own the returned Variable. Variable* FindVarInternal(const std::string& name) const; - // Called by FindVarInternal and Var + // Called by FindVarInternal and Var. + // Caller doesn't own the returned Variable. Variable* FindVarLocally(const std::string& name) const; - mutable std::unordered_map vars_; + mutable std::unordered_map> vars_; + + // Scope in `kids_` are owned by this class. mutable std::list kids_; Scope const* parent_{nullptr};