diff --git a/paddle/fluid/framework/new_executor/new_executor_defs.h b/paddle/fluid/framework/new_executor/new_executor_defs.h index 37fb57072f5eceb52674f8c4b0a2ec44076bf472..c765b7fe4d2acc822308528db294be30c4dbdc7b 100644 --- a/paddle/fluid/framework/new_executor/new_executor_defs.h +++ b/paddle/fluid/framework/new_executor/new_executor_defs.h @@ -474,13 +474,12 @@ struct VariableMetaInfo { // TODO(zhiqiu): Maybe we need to add rwlock for VariableScope? // NOTE(xiongkun03): Use scope as a member of VariableScope, we don't need -// ScopeBase. -// Scope manager the variables and VariableScope is just a -// quick -// access machanism. -class VariableScope : public ScopeBase { +// ScopeBase. Scope manager the variables and VariableScope is just a quick +// access machanism. ScopeListener is the callback to sync changes in Original +// Scope. We can make it a membership of VariableScope. Here we use inherent. +class VariableScope : public ScopeBase, public ScopeListener { public: - VariableScope() { + VariableScope(Scope* outer_scope) { // for @EMPTY@ variable var_list_.push_back(nullptr); name2id_[kEmptyVarName] = 0; @@ -488,9 +487,20 @@ class VariableScope : public ScopeBase { info.var_ref_count_ = 0; info.vardesc_ = nullptr; vec_meta_info_.push_back(info); - scope_ptr_.reset(new Scope()); + outer_scope_ = outer_scope; + + PADDLE_ENFORCE_NE( + outer_scope_, nullptr, + platform::errors::PreconditionNotMet( + "You have passed a nullptr to construct VariableScope.")); + outer_scope->AddListener(this); } - const Scope* GetScope() const { return scope_ptr_.get(); } + + ~VariableScope() { + if (outer_scope_ != nullptr) outer_scope_->DelListener(this); + } + + const Scope* GetScope() const { return outer_scope_; } Variable* FindVar(const std::string& name) const { auto it = name2id_.find(name); @@ -548,8 +558,9 @@ class VariableScope : public ScopeBase { size_t VarSize() const { return var_list_.size(); } void AddVar(const std::string& name, VarDesc* var_desc) { // NOLINT - name2id_[name] = VarSize(); - auto v = scope_ptr_->Var(name); + // AddVar -> Scope::Var -> onCreateVariable. + VLOG(4) << "Add variable: " << name << " through AddVar()"; + auto v = outer_scope_->Var(name); if (nullptr == var_desc) { v->GetMutable(); } else { @@ -558,26 +569,13 @@ class VariableScope : public ScopeBase { var_desc ->GetType()); // Scope don't initialize variable recently created } - var_list_.push_back(v); - - VariableMetaInfo info; - info.var_ref_count_ = 0; - info.vardesc_ = var_desc; - vec_meta_info_.push_back(info); + SetVarDesc(name, var_desc); } void AddVar(const std::string& name, Variable& var) { // NOLINT - // must copy. - VLOG(4) << "Add variable: " << name << " through AddVar()"; - auto v = scope_ptr_->Var(name); - *v = var; - name2id_[name] = VarSize(); - var_list_.push_back(v); - - VariableMetaInfo info; - info.var_ref_count_ = 0; - info.vardesc_ = nullptr; - vec_meta_info_.push_back(info); + // Though name existed in outer_scope_, we need + // add again to create name2id map. + outer_scope_->Var(name); } void SetVarDesc(const std::string& name, framework::VarDesc* var_desc) { @@ -607,6 +605,32 @@ class VariableScope : public ScopeBase { platform::errors::NotFound("%s not in VariableScope.", name)); } + public: // callbacks from ScopeListener class + void onCreateVariable(const std::string& name) override { + auto v = outer_scope_->GetVar(name); // must exsit in outer_scope_ + if (!HasVar(name)) { // may exist in variable scope. + VLOG(4) << "Calling VariableScope::onCreateVariable with var_name: " + << name; + name2id_[name] = VarSize(); + var_list_.push_back(v); + + VariableMetaInfo info; + info.var_ref_count_ = 0; + info.vardesc_ = nullptr; // set nullptr, then modifty it in AddVar() + vec_meta_info_.push_back(info); + } + } + void onDeleteVariable(const std::string& name) override { + if (HasVar(name)) { + VLOG(4) << "Calling VariableScope::onDeleteVariable with var_name: " + << name; + } + } + void onRenameVariable(const std::string& old_name, + const std::string& new_name) override {} + void onCreateScope(Scope* Scope) override {} + void onDeleteScope(Scope* Scope) override {} + void onClear() override {} std::vector& MutableVecMetaInfo() { return vec_meta_info_; } const std::vector& VecMetaInfo() const { @@ -617,7 +641,7 @@ class VariableScope : public ScopeBase { std::vector var_list_; std::map name2id_; std::vector vec_meta_info_; - std::unique_ptr scope_ptr_; + Scope* outer_scope_ = nullptr; }; class NextInstruction { diff --git a/paddle/fluid/framework/new_executor/standalone_executor.cc b/paddle/fluid/framework/new_executor/standalone_executor.cc index e3bcbaec7ad700b0521d585161d7a9f5e44fbb25..1c9f6b3d901bd4f517313502e5e1b752333ed219 100644 --- a/paddle/fluid/framework/new_executor/standalone_executor.cc +++ b/paddle/fluid/framework/new_executor/standalone_executor.cc @@ -23,9 +23,9 @@ StandaloneExecutor::StandaloneExecutor(const platform::Place& place, : place_(place), startup_prog_(startup_prog), main_prog_(main_prog), - outer_scope_(scope) { + outer_scope_(scope), + global_scope_(scope) { paddle::framework::InitDevices(); - // init scope BuildVariableOuterScope(startup_prog, &global_scope_, scope); diff --git a/paddle/fluid/framework/scope.cc b/paddle/fluid/framework/scope.cc index 932974855a28eca53c99e7fd3186a749a15be6d3..7a36354d7e6695070e60fde626f7dc260d300ee3 100644 --- a/paddle/fluid/framework/scope.cc +++ b/paddle/fluid/framework/scope.cc @@ -59,18 +59,35 @@ std::unique_ptr Scope::NewTmpScope() const { } Variable* Scope::Var(const std::string& name) { - SCOPE_VARS_WRITER_LOCK - return VarInternal(name); + // NOTE(xiongkun03): add {} here to unlock. With {}, scope + // will do callback after unlock. + Variable* ret = nullptr; + { + SCOPE_VARS_WRITER_LOCK + ret = VarInternal(name); + } + for (auto l : listeners_) { + l->onCreateVariable(name); + } + return ret; } Variable* Scope::Var(std::string* name) { - SCOPE_VARS_WRITER_LOCK - auto new_name = std::to_string(reinterpret_cast(this)) + "." + - std::to_string(vars_.size()); - if (name != nullptr) { - *name = new_name; + Variable* ret = nullptr; + std::string new_name; + { + SCOPE_VARS_WRITER_LOCK + new_name = std::to_string(reinterpret_cast(this)) + "." + + std::to_string(vars_.size()); + if (name != nullptr) { + *name = new_name; + } + ret = VarInternal(new_name); + } + for (auto l : listeners_) { + l->onCreateVariable(new_name); } - return VarInternal(new_name); + return ret; } Variable* Scope::FindVar(const std::string& name) const { @@ -101,9 +118,14 @@ const Scope* Scope::FindScope(const std::string& name) const { } void Scope::DropKids() { - SCOPE_KIDS_WRITER_LOCK - for (Scope* s : kids_) delete s; - kids_.clear(); + { + SCOPE_KIDS_WRITER_LOCK + for (Scope* s : kids_) delete s; + kids_.clear(); + } + for (auto l : listeners_) { + l->onClear(); + } } bool Scope::HasKid(const Scope* scope) const { @@ -125,42 +147,64 @@ std::vector Scope::LocalVarNames() const { } void Scope::DeleteScope(Scope* scope) const { - SCOPE_KIDS_WRITER_LOCK - auto it = std::find(this->kids_.begin(), this->kids_.end(), scope); - PADDLE_ENFORCE_NE(it, this->kids_.end(), - platform::errors::NotFound( - "%p is not found in %p as kid scope", scope, this)); - this->kids_.erase(it); - // When making memory benchmark on Fluid, we have to delete scope sync. - if (FLAGS_benchmark || FLAGS_eager_delete_scope) { - delete scope; - } else { - Async([scope] { delete scope; }); + { + SCOPE_KIDS_WRITER_LOCK + auto it = std::find(this->kids_.begin(), this->kids_.end(), scope); + PADDLE_ENFORCE_NE(it, this->kids_.end(), + platform::errors::NotFound( + "%p is not found in %p as kid scope", scope, this)); + this->kids_.erase(it); + // When making memory benchmark on Fluid, we have to delete scope sync. + if (FLAGS_benchmark || FLAGS_eager_delete_scope) { + delete scope; + } else { + Async([scope] { delete scope; }); + } + } + for (auto l : listeners_) { + l->onDeleteScope(scope); } } void Scope::EraseVars(const std::vector& var_names) { - std::set var_set(var_names.begin(), var_names.end()); - SCOPE_VARS_WRITER_LOCK - for (auto it = vars_.begin(); it != vars_.end();) { - if (var_set.find(it->first) != var_set.end()) { - it = vars_.erase(it); - } else { - ++it; + { + std::set var_set(var_names.begin(), var_names.end()); + SCOPE_VARS_WRITER_LOCK + for (auto it = vars_.begin(); it != vars_.end();) { + if (var_set.find(it->first) != var_set.end()) { + it = vars_.erase(it); + } else { + ++it; + } + } + } + for (auto l : listeners_) { + for (auto& var_name : var_names) { + l->onDeleteVariable(var_name); } } } void Scope::Rename(const std::string& origin_name, const std::string& new_name) const { - SCOPE_VARS_WRITER_LOCK - RenameInternal(origin_name, new_name); + { + SCOPE_VARS_WRITER_LOCK + RenameInternal(origin_name, new_name); + } + for (auto l : listeners_) { + l->onRenameVariable(origin_name, new_name); + } } std::string Scope::Rename(const std::string& origin_name) const { - SCOPE_VARS_WRITER_LOCK auto new_name = string::Sprintf("%p.%d", this, vars_.size()); - RenameInternal(origin_name, new_name); + { + SCOPE_VARS_WRITER_LOCK + RenameInternal(origin_name, new_name); + } + for (auto l : listeners_) { + l->onRenameVariable(origin_name, new_name); + } return new_name; } @@ -222,6 +266,17 @@ Variable* Scope::FindVarLocally(const std::string& name) const { return nullptr; } +void Scope::AddListener(ScopeListener* listener) { + auto it = std::find(listeners_.begin(), listeners_.end(), listener); + if (it == listeners_.end()) { + listeners_.push_back(listener); + } +} + +void Scope::DelListener(ScopeListener* listener) { + listeners_.remove(listener); +} + void Scope::EraseVarsExcept(const std::unordered_set& vars) { SCOPE_VARS_WRITER_LOCK for (auto iter = vars_.begin(); iter != vars_.end();) { diff --git a/paddle/fluid/framework/scope.h b/paddle/fluid/framework/scope.h index ab29a7a88fc0003c7de5d80d598ba9d5877e5d2d..ca486aec8c27e17669e7e1446d8acdbd93135891 100644 --- a/paddle/fluid/framework/scope.h +++ b/paddle/fluid/framework/scope.h @@ -51,6 +51,22 @@ class ScopeBase { class Scope; +class ScopeListener { + // NOTE(xiongkun03) Abstract Class, doesn't have any attributes. + // Used by VariableScope. If we modify the original scope, we + // need synchronize changes to VariableScope. So we add listerer + // in original Scope. + public: + virtual ~ScopeListener() {} + virtual void onCreateVariable(const std::string& name) {} + virtual void onDeleteVariable(const std::string& name) {} + virtual void onRenameVariable(const std::string& old_name, + const std::string& new_name) {} + virtual void onCreateScope(Scope* Scope) {} + virtual void onDeleteScope(Scope* Scope) {} + virtual void onClear() {} +}; + /** * @brief Scope that manage all variables. * @@ -128,6 +144,10 @@ class Scope : public ScopeBase { // Rename variable to a new name and return the new name std::string Rename(const std::string& origin_name) const; + void AddListener(ScopeListener* listener); + + void DelListener(ScopeListener* listener); + protected: struct KeyHasher { std::size_t operator()(const std::string& key) const { @@ -164,6 +184,7 @@ class Scope : public ScopeBase { // Scope in `kids_` are owned by this class. mutable std::list kids_; const Scope* parent_{nullptr}; + std::list listeners_; DISABLE_COPY_AND_ASSIGN(Scope);