From 641038dc5f3ee853f64cbe9fb0a1542d76b5ead9 Mon Sep 17 00:00:00 2001
From: wanghuancoder <wanghuancoder@163.com>
Date: Fri, 26 Nov 2021 19:37:50 +0800
Subject: [PATCH] clear local scope every setp (#37569)

* clear local scope every setp, test=develop

* refine,test=develop

* refine, test=develop
---
 .../framework/new_executor/interpretercore.cc | 25 +++++++++++++++++++
 .../framework/new_executor/interpretercore.h  |  2 ++
 .../interpretercore_garbage_collector.cc      |  1 -
 .../new_executor/interpretercore_util.cc      |  1 -
 paddle/fluid/framework/scope.cc               | 12 +++++++++
 paddle/fluid/framework/scope.h                |  5 +++-
 6 files changed, 43 insertions(+), 3 deletions(-)

diff --git a/paddle/fluid/framework/new_executor/interpretercore.cc b/paddle/fluid/framework/new_executor/interpretercore.cc
index 06948545444..94b2118ba9d 100644
--- a/paddle/fluid/framework/new_executor/interpretercore.cc
+++ b/paddle/fluid/framework/new_executor/interpretercore.cc
@@ -88,6 +88,10 @@ paddle::framework::FetchList InterpreterCore::Run(
     ExecuteInstructionList(vec_instruction_);
   }
 
+  if (create_local_scope_) {
+    ClearLoDTensorArrayInLocalScope();
+  }
+
   // return Fetch Tensors
   auto* fetch_var = global_scope_->Var(interpreter::kFetchVarName);
   return std::move(*fetch_var->GetMutable<framework::FetchList>());
@@ -122,11 +126,28 @@ paddle::framework::FetchList InterpreterCore::Run(
     ExecuteInstructionList(vec_instruction_);
   }
 
+  if (create_local_scope_) {
+    ClearLoDTensorArrayInLocalScope();
+  }
+
   // return Fetch Tensors
   auto* fetch_var = global_scope_->Var(interpreter::kFetchVarName);
   return std::move(*fetch_var->GetMutable<framework::FetchList>());
 }
 
+// At the end of each step, the holder of Tensor in LoDTensorArray is null.
+// Clear these Tensors and leave LoDTensorArray empty, otherwise an exception
+// will occur in the next step
+void InterpreterCore::ClearLoDTensorArrayInLocalScope() {
+  auto vars = local_scope_->LocalVars();
+  for (auto var : vars) {
+    if (var->IsType<LoDTensorArray>()) {
+      auto* lod_tensor_arr = var->GetMutable<LoDTensorArray>();
+      lod_tensor_arr->clear();
+    }
+  }
+}
+
 void InterpreterCore::BuildOperatorDependences() {
   // analysis the dependences between ops, set the dependecy_count_ and Call
   // Schedule
@@ -609,6 +630,10 @@ interpreter::CostInfo InterpreterCore::DryRun(
     platform::DeviceContextPool::Instance().Get(place_)->Wait();
   }
 
+  if (create_local_scope_) {
+    ClearLoDTensorArrayInLocalScope();
+  }
+
   return cost_info;
 }
 
diff --git a/paddle/fluid/framework/new_executor/interpretercore.h b/paddle/fluid/framework/new_executor/interpretercore.h
index 70392faf6d4..656262d6381 100644
--- a/paddle/fluid/framework/new_executor/interpretercore.h
+++ b/paddle/fluid/framework/new_executor/interpretercore.h
@@ -86,6 +86,8 @@ class InterpreterCore {
 
   void SetFeedVarsInplaceSkip(const std::vector<std::string>& feed_names);
 
+  void ClearLoDTensorArrayInLocalScope();
+
   bool is_build_;
 
   const platform::Place& place_;
diff --git a/paddle/fluid/framework/new_executor/interpretercore_garbage_collector.cc b/paddle/fluid/framework/new_executor/interpretercore_garbage_collector.cc
index 1255ecfc9ae..f17f64dbcae 100644
--- a/paddle/fluid/framework/new_executor/interpretercore_garbage_collector.cc
+++ b/paddle/fluid/framework/new_executor/interpretercore_garbage_collector.cc
@@ -79,7 +79,6 @@ void InterpreterCoreGarbageCollector::Add(paddle::framework::Variable* var,
     for (auto& t : *tensor_arr) {
       Add(t.MoveMemoryHolder(), event, ctx);
     }
-    tensor_arr->clear();
   } else if (var->IsType<std::vector<Scope*>>()) {
     // NOTE(@xiongkun03) conditional_op / while_op will create a STEP_SCOPE
     // refer to executor.cc to see what old garbage collector does.
diff --git a/paddle/fluid/framework/new_executor/interpretercore_util.cc b/paddle/fluid/framework/new_executor/interpretercore_util.cc
index 774e4e5c9b4..98799e049df 100644
--- a/paddle/fluid/framework/new_executor/interpretercore_util.cc
+++ b/paddle/fluid/framework/new_executor/interpretercore_util.cc
@@ -411,7 +411,6 @@ void build_op_func_list(const platform::Place& place,
         for (auto& t : *lod_tensor_arr) {
           garbages->emplace_back(t.MoveMemoryHolder());
         }
-        lod_tensor_arr->clear();
       } else {
         PADDLE_THROW(platform::errors::Unimplemented(
             "Type %s of variable %s is not supported eager deletion.",
diff --git a/paddle/fluid/framework/scope.cc b/paddle/fluid/framework/scope.cc
index 49cca5018ce..b2062cc5120 100644
--- a/paddle/fluid/framework/scope.cc
+++ b/paddle/fluid/framework/scope.cc
@@ -146,6 +146,18 @@ std::vector<std::string> Scope::LocalVarNames() const {
   return known_vars;
 }
 
+std::vector<Variable*> Scope::LocalVars() {
+  std::vector<Variable*> known_vars;
+  {
+    SCOPE_VARS_READER_LOCK
+    known_vars.reserve(this->vars_.size());
+    for (auto& p : vars_) {
+      known_vars.emplace_back(p.second.get());
+    }
+  }
+  return known_vars;
+}
+
 void Scope::DeleteScope(Scope* scope) const {
   {
     SCOPE_KIDS_WRITER_LOCK
diff --git a/paddle/fluid/framework/scope.h b/paddle/fluid/framework/scope.h
index c18d1d588a3..b963c28d597 100644
--- a/paddle/fluid/framework/scope.h
+++ b/paddle/fluid/framework/scope.h
@@ -134,9 +134,12 @@ class Scope : public ScopeBase {
 
   const std::list<Scope*>& kids() const { return kids_; }
 
-  // enumerate all the variables current contains.
+  // enumerate all the variable names current contains.
   std::vector<std::string> LocalVarNames() const;
 
+  // enumerate all the variables current contains.
+  std::vector<Variable*> LocalVars();
+
   // Rename variable to a new name
   void Rename(const std::string& origin_name,
               const std::string& new_name) const;
-- 
GitLab