From 947eb700bec9e214a72d4747864398dc238da60c Mon Sep 17 00:00:00 2001
From: Vladimir Marko <vmarko@google.com>
Date: Fri, 25 Mar 2016 15:31:35 +0000
Subject: Optimizing: Reduce arena memory used by GraphChecker.

Use member variables to reuse the storage instead of
repeatedly allocating new storage for local variables.

Bug: 27690481
Change-Id: I614db9b8614d585653cfbff62e9cf7d7f0c58810
---
 compiler/optimizing/graph_checker.cc | 25 +++++++++++--------------
 compiler/optimizing/graph_checker.h  |  8 +++++++-
 2 files changed, 18 insertions(+), 15 deletions(-)

(limited to 'compiler')

diff --git a/compiler/optimizing/graph_checker.cc b/compiler/optimizing/graph_checker.cc
index e5c8c47579..f2e77e28a6 100644
--- a/compiler/optimizing/graph_checker.cc
+++ b/compiler/optimizing/graph_checker.cc
@@ -32,11 +32,9 @@ void GraphChecker::VisitBasicBlock(HBasicBlock* block) {
 
   // Check consistency with respect to predecessors of `block`.
   // Note: Counting duplicates with a sorted vector uses up to 6x less memory
-  // than ArenaSafeMap<HBasicBlock*, size_t>.
-  ArenaVector<HBasicBlock*> sorted_predecessors(
-      block->GetPredecessors().begin(),
-      block->GetPredecessors().end(),
-      GetGraph()->GetArena()->Adapter(kArenaAllocGraphChecker));
+  // than ArenaSafeMap<HBasicBlock*, size_t> and also allows storage reuse.
+  ArenaVector<HBasicBlock*>& sorted_predecessors = blocks_storage_;
+  sorted_predecessors.assign(block->GetPredecessors().begin(), block->GetPredecessors().end());
   std::sort(sorted_predecessors.begin(), sorted_predecessors.end());
   for (auto it = sorted_predecessors.begin(), end = sorted_predecessors.end(); it != end; ) {
     HBasicBlock* p = *it++;
@@ -57,11 +55,9 @@ void GraphChecker::VisitBasicBlock(HBasicBlock* block) {
 
   // Check consistency with respect to successors of `block`.
   // Note: Counting duplicates with a sorted vector uses up to 6x less memory
-  // than ArenaSafeMap<HBasicBlock*, size_t>.
-  ArenaVector<HBasicBlock*> sorted_successors(
-      block->GetSuccessors().begin(),
-      block->GetSuccessors().end(),
-      GetGraph()->GetArena()->Adapter(kArenaAllocGraphChecker));
+  // than ArenaSafeMap<HBasicBlock*, size_t> and also allows storage reuse.
+  ArenaVector<HBasicBlock*>& sorted_successors = blocks_storage_;
+  sorted_successors.assign(block->GetSuccessors().begin(), block->GetSuccessors().end());
   std::sort(sorted_successors.begin(), sorted_successors.end());
   for (auto it = sorted_successors.begin(), end = sorted_successors.end(); it != end; ) {
     HBasicBlock* s = *it++;
@@ -811,10 +807,11 @@ void GraphChecker::VisitPhi(HPhi* phi) {
               phi->GetRegNumber(),
               type_str.str().c_str()));
         } else {
-          ArenaBitVector visited(GetGraph()->GetArena(),
-                                 0,
-                                 /* expandable */ true,
-                                 kArenaAllocGraphChecker);
+          // If we get here, make sure we allocate all the necessary storage at once
+          // because the BitVector reallocation strategy has very bad worst-case behavior.
+          ArenaBitVector& visited = visited_storage_;
+          visited.SetBit(GetGraph()->GetCurrentInstructionId());
+          visited.ClearAllBits();
           if (!IsConstantEquivalent(phi, other_phi, &visited)) {
             AddError(StringPrintf("Two phis (%d and %d) found for VReg %d but they "
                                   "are not equivalents of constants.",
diff --git a/compiler/optimizing/graph_checker.h b/compiler/optimizing/graph_checker.h
index 27d5621887..a536b30fcd 100644
--- a/compiler/optimizing/graph_checker.h
+++ b/compiler/optimizing/graph_checker.h
@@ -33,7 +33,9 @@ class GraphChecker : public HGraphDelegateVisitor {
       seen_ids_(graph->GetArena(),
                 graph->GetCurrentInstructionId(),
                 false,
-                kArenaAllocGraphChecker) {}
+                kArenaAllocGraphChecker),
+      blocks_storage_(graph->GetArena()->Adapter(kArenaAllocGraphChecker)),
+      visited_storage_(graph->GetArena(), 0u, true, kArenaAllocGraphChecker) {}
 
   // Check the whole graph (in reverse post-order).
   void Run() {
@@ -102,6 +104,10 @@ class GraphChecker : public HGraphDelegateVisitor {
   const char* const dump_prefix_;
   ArenaBitVector seen_ids_;
 
+  // To reduce the total arena memory allocation, we reuse the same storage.
+  ArenaVector<HBasicBlock*> blocks_storage_;
+  ArenaBitVector visited_storage_;
+
   DISALLOW_COPY_AND_ASSIGN(GraphChecker);
 };
 
-- 
cgit v1.2.3-59-g8ed1b