OSDN Git Service

Revert "Revert "Use dex cache from compilation unit in RTP.""
authorVladimir Marko <vmarko@google.com>
Tue, 19 Apr 2016 14:12:13 +0000 (14:12 +0000)
committerVladimir Marko <vmarko@google.com>
Tue, 19 Apr 2016 18:25:00 +0000 (19:25 +0100)
The exposed issue has been fixed by
    https://android-review.googlesource.com/215877

Bug:28210356

This reverts commit 34d9b04d8d0006967486c0ad1b221e7b632652af.

Change-Id: I5288c923e45d9ef3190dabb89738350a1212a60d

compiler/optimizing/builder.h
compiler/optimizing/inliner.cc
compiler/optimizing/nodes.h
compiler/optimizing/reference_type_propagation.cc
compiler/optimizing/reference_type_propagation.h
compiler/optimizing/ssa_builder.cc
compiler/optimizing/ssa_builder.h

index 4f46d5e..580ef72 100644 (file)
@@ -51,7 +51,7 @@ class HGraphBuilder : public ValueObject {
         compiler_driver_(driver),
         compilation_stats_(compiler_stats),
         block_builder_(graph, dex_file, code_item),
-        ssa_builder_(graph, handles),
+        ssa_builder_(graph, dex_compilation_unit->GetDexCache(), handles),
         instruction_builder_(graph,
                              &block_builder_,
                              &ssa_builder_,
@@ -78,7 +78,7 @@ class HGraphBuilder : public ValueObject {
         null_dex_cache_(),
         compilation_stats_(nullptr),
         block_builder_(graph, nullptr, code_item),
-        ssa_builder_(graph, handles),
+        ssa_builder_(graph, null_dex_cache_, handles),
         instruction_builder_(graph,
                              &block_builder_,
                              &ssa_builder_,
index ba6ae6f..836bcfa 100644 (file)
@@ -411,7 +411,10 @@ bool HInliner::TryInlineMonomorphicCall(HInvoke* invoke_instruction,
 
   // Run type propagation to get the guard typed, and eventually propagate the
   // type of the receiver.
-  ReferenceTypePropagation rtp_fixup(graph_, handles_, /* is_first_run */ false);
+  ReferenceTypePropagation rtp_fixup(graph_,
+                                     outer_compilation_unit_.GetDexCache(),
+                                     handles_,
+                                     /* is_first_run */ false);
   rtp_fixup.Run();
 
   MaybeRecordStat(kInlinedMonomorphicCall);
@@ -532,7 +535,10 @@ bool HInliner::TryInlinePolymorphicCall(HInvoke* invoke_instruction,
   MaybeRecordStat(kInlinedPolymorphicCall);
 
   // Run type propagation to get the guards typed.
-  ReferenceTypePropagation rtp_fixup(graph_, handles_, /* is_first_run */ false);
+  ReferenceTypePropagation rtp_fixup(graph_,
+                                     outer_compilation_unit_.GetDexCache(),
+                                     handles_,
+                                     /* is_first_run */ false);
   rtp_fixup.Run();
   return true;
 }
@@ -709,7 +715,10 @@ bool HInliner::TryInlinePolymorphicCallToSameTarget(HInvoke* invoke_instruction,
   deoptimize->CopyEnvironmentFrom(invoke_instruction->GetEnvironment());
 
   // Run type propagation to get the guard typed.
-  ReferenceTypePropagation rtp_fixup(graph_, handles_, /* is_first_run */ false);
+  ReferenceTypePropagation rtp_fixup(graph_,
+                                     outer_compilation_unit_.GetDexCache(),
+                                     handles_,
+                                     /* is_first_run */ false);
   rtp_fixup.Run();
 
   MaybeRecordStat(kInlinedPolymorphicCall);
@@ -971,7 +980,8 @@ HInstanceFieldGet* HInliner::CreateInstanceFieldGet(Handle<mirror::DexCache> dex
       // dex pc for the associated stack map. 0 is bogus but valid. Bug: 26854537.
       /* dex_pc */ 0);
   if (iget->GetType() == Primitive::kPrimNot) {
-    ReferenceTypePropagation rtp(graph_, handles_, /* is_first_run */ false);
+    // Use the same dex_cache that we used for field lookup as the hint_dex_cache.
+    ReferenceTypePropagation rtp(graph_, dex_cache, handles_, /* is_first_run */ false);
     rtp.Visit(iget);
   }
   return iget;
@@ -1320,13 +1330,19 @@ void HInliner::FixUpReturnReferenceType(HInvoke* invoke_instruction,
         if (invoke_rti.IsStrictSupertypeOf(return_rti)
             || (return_rti.IsExact() && !invoke_rti.IsExact())
             || !return_replacement->CanBeNull()) {
-          ReferenceTypePropagation(graph_, handles_, /* is_first_run */ false).Run();
+          ReferenceTypePropagation(graph_,
+                                   outer_compilation_unit_.GetDexCache(),
+                                   handles_,
+                                   /* is_first_run */ false).Run();
         }
       }
     } else if (return_replacement->IsInstanceOf()) {
       if (do_rtp) {
         // Inlining InstanceOf into an If may put a tighter bound on reference types.
-        ReferenceTypePropagation(graph_, handles_, /* is_first_run */ false).Run();
+        ReferenceTypePropagation(graph_,
+                                 outer_compilation_unit_.GetDexCache(),
+                                 handles_,
+                                 /* is_first_run */ false).Run();
       }
     }
   }
index 1ea2247..808ab09 100644 (file)
@@ -169,7 +169,7 @@ class ReferenceTypeInfo : ValueObject {
     return handle.GetReference() != nullptr;
   }
 
-  bool IsValid() const SHARED_REQUIRES(Locks::mutator_lock_) {
+  bool IsValid() const {
     return IsValidHandle(type_handle_);
   }
 
@@ -1916,7 +1916,7 @@ class HInstruction : public ArenaObject<kArenaAllocInstruction> {
   ReferenceTypeInfo GetReferenceTypeInfo() const {
     DCHECK_EQ(GetType(), Primitive::kPrimNot);
     return ReferenceTypeInfo::CreateUnchecked(reference_type_handle_,
-                                              GetPackedFlag<kFlagReferenceTypeIsExact>());;
+                                              GetPackedFlag<kFlagReferenceTypeIsExact>());
   }
 
   void AddUseAt(HInstruction* user, size_t index) {
index 0f93a4b..21d7ee7 100644 (file)
 
 namespace art {
 
+static inline mirror::DexCache* FindDexCacheWithHint(Thread* self,
+                                                     const DexFile& dex_file,
+                                                     Handle<mirror::DexCache> hint_dex_cache)
+    SHARED_REQUIRES(Locks::mutator_lock_) {
+  if (LIKELY(hint_dex_cache->GetDexFile() == &dex_file)) {
+    return hint_dex_cache.Get();
+  } else {
+    return Runtime::Current()->GetClassLinker()->FindDexCache(self, dex_file);
+  }
+}
+
 static inline ReferenceTypeInfo::TypeHandle GetRootHandle(StackHandleScopeCollection* handles,
                                                           ClassLinker::ClassRoot class_root,
                                                           ReferenceTypeInfo::TypeHandle* cache) {
@@ -54,10 +65,12 @@ ReferenceTypeInfo::TypeHandle ReferenceTypePropagation::HandleCache::GetThrowabl
 class ReferenceTypePropagation::RTPVisitor : public HGraphDelegateVisitor {
  public:
   RTPVisitor(HGraph* graph,
+             Handle<mirror::DexCache> hint_dex_cache,
              HandleCache* handle_cache,
              ArenaVector<HInstruction*>* worklist,
              bool is_first_run)
     : HGraphDelegateVisitor(graph),
+      hint_dex_cache_(hint_dex_cache),
       handle_cache_(handle_cache),
       worklist_(worklist),
       is_first_run_(is_first_run) {}
@@ -86,16 +99,19 @@ class ReferenceTypePropagation::RTPVisitor : public HGraphDelegateVisitor {
                                bool is_exact);
 
  private:
+  Handle<mirror::DexCache> hint_dex_cache_;
   HandleCache* handle_cache_;
   ArenaVector<HInstruction*>* worklist_;
   const bool is_first_run_;
 };
 
 ReferenceTypePropagation::ReferenceTypePropagation(HGraph* graph,
+                                                   Handle<mirror::DexCache> hint_dex_cache,
                                                    StackHandleScopeCollection* handles,
                                                    bool is_first_run,
                                                    const char* name)
     : HOptimization(graph, name),
+      hint_dex_cache_(hint_dex_cache),
       handle_cache_(handles),
       worklist_(graph->GetArena()->Adapter(kArenaAllocReferenceTypePropagation)),
       is_first_run_(is_first_run) {
@@ -130,7 +146,7 @@ void ReferenceTypePropagation::ValidateTypes() {
 }
 
 void ReferenceTypePropagation::Visit(HInstruction* instruction) {
-  RTPVisitor visitor(graph_, &handle_cache_, &worklist_, is_first_run_);
+  RTPVisitor visitor(graph_, hint_dex_cache_, &handle_cache_, &worklist_, is_first_run_);
   instruction->Accept(&visitor);
 }
 
@@ -149,7 +165,7 @@ void ReferenceTypePropagation::Run() {
 }
 
 void ReferenceTypePropagation::VisitBasicBlock(HBasicBlock* block) {
-  RTPVisitor visitor(graph_, &handle_cache_, &worklist_, is_first_run_);
+  RTPVisitor visitor(graph_, hint_dex_cache_, &handle_cache_, &worklist_, is_first_run_);
   // Handle Phis first as there might be instructions in the same block who depend on them.
   for (HInstructionIterator it(block->GetPhis()); !it.Done(); it.Advance()) {
     VisitPhi(it.Current()->AsPhi());
@@ -358,7 +374,6 @@ void ReferenceTypePropagation::BoundTypeForIfInstanceOf(HBasicBlock* block) {
   HLoadClass* load_class = instanceOf->InputAt(1)->AsLoadClass();
   ReferenceTypeInfo class_rti = load_class->GetLoadedClassRTI();
   {
-    ScopedObjectAccess soa(Thread::Current());
     if (!class_rti.IsValid()) {
       // He have loaded an unresolved class. Don't bother bounding the type.
       return;
@@ -412,7 +427,7 @@ void ReferenceTypePropagation::RTPVisitor::SetClassAsTypeInfo(HInstruction* inst
       ScopedObjectAccess soa(Thread::Current());
       StackHandleScope<2> hs(soa.Self());
       Handle<mirror::DexCache> dex_cache(
-          hs.NewHandle(cl->FindDexCache(soa.Self(), invoke->GetDexFile(), false)));
+          hs.NewHandle(FindDexCacheWithHint(soa.Self(), invoke->GetDexFile(), hint_dex_cache_)));
       // Use a null loader. We should probably use the compiling method's class loader,
       // but then we would need to pass it to RTPVisitor just for this debug check. Since
       // the method is from the String class, the null loader is good enough.
@@ -451,8 +466,7 @@ void ReferenceTypePropagation::RTPVisitor::UpdateReferenceTypeInfo(HInstruction*
   DCHECK_EQ(instr->GetType(), Primitive::kPrimNot);
 
   ScopedObjectAccess soa(Thread::Current());
-  mirror::DexCache* dex_cache = Runtime::Current()->GetClassLinker()->FindDexCache(
-      soa.Self(), dex_file, false);
+  mirror::DexCache* dex_cache = FindDexCacheWithHint(soa.Self(), dex_file, hint_dex_cache_);
   // Get type from dex cache assuming it was populated by the verifier.
   SetClassAsTypeInfo(instr, dex_cache->GetResolvedType(type_idx), is_exact);
 }
@@ -465,24 +479,24 @@ void ReferenceTypePropagation::RTPVisitor::VisitNewArray(HNewArray* instr) {
   UpdateReferenceTypeInfo(instr, instr->GetTypeIndex(), instr->GetDexFile(), /* is_exact */ true);
 }
 
-static mirror::Class* GetClassFromDexCache(Thread* self, const DexFile& dex_file, uint16_t type_idx)
+static mirror::Class* GetClassFromDexCache(Thread* self,
+                                           const DexFile& dex_file,
+                                           uint16_t type_idx,
+                                           Handle<mirror::DexCache> hint_dex_cache)
     SHARED_REQUIRES(Locks::mutator_lock_) {
-  mirror::DexCache* dex_cache =
-      Runtime::Current()->GetClassLinker()->FindDexCache(self, dex_file, /* allow_failure */ true);
-  if (dex_cache == nullptr) {
-    // Dex cache could not be found. This should only happen during gtests.
-    return nullptr;
-  }
+  mirror::DexCache* dex_cache = FindDexCacheWithHint(self, dex_file, hint_dex_cache);
   // Get type from dex cache assuming it was populated by the verifier.
   return dex_cache->GetResolvedType(type_idx);
 }
 
 void ReferenceTypePropagation::RTPVisitor::VisitParameterValue(HParameterValue* instr) {
-  ScopedObjectAccess soa(Thread::Current());
   // We check if the existing type is valid: the inliner may have set it.
   if (instr->GetType() == Primitive::kPrimNot && !instr->GetReferenceTypeInfo().IsValid()) {
-    mirror::Class* resolved_class =
-        GetClassFromDexCache(soa.Self(), instr->GetDexFile(), instr->GetTypeIndex());
+    ScopedObjectAccess soa(Thread::Current());
+    mirror::Class* resolved_class = GetClassFromDexCache(soa.Self(),
+                                                         instr->GetDexFile(),
+                                                         instr->GetTypeIndex(),
+                                                         hint_dex_cache_);
     SetClassAsTypeInfo(instr, resolved_class, /* is_exact */ false);
   }
 }
@@ -493,11 +507,11 @@ void ReferenceTypePropagation::RTPVisitor::UpdateFieldAccessTypeInfo(HInstructio
     return;
   }
 
-  ScopedObjectAccess soa(Thread::Current());
   mirror::Class* klass = nullptr;
 
   // The field index is unknown only during tests.
   if (info.GetFieldIndex() != kUnknownFieldIndex) {
+    ScopedObjectAccess soa(Thread::Current());
     ClassLinker* cl = Runtime::Current()->GetClassLinker();
     ArtField* field = cl->GetResolvedField(info.GetFieldIndex(), info.GetDexCache().Get());
     // TODO: There are certain cases where we can't resolve the field.
@@ -537,8 +551,10 @@ void ReferenceTypePropagation::RTPVisitor::VisitUnresolvedStaticFieldGet(
 void ReferenceTypePropagation::RTPVisitor::VisitLoadClass(HLoadClass* instr) {
   ScopedObjectAccess soa(Thread::Current());
   // Get type from dex cache assuming it was populated by the verifier.
-  mirror::Class* resolved_class =
-      GetClassFromDexCache(soa.Self(), instr->GetDexFile(), instr->GetTypeIndex());
+  mirror::Class* resolved_class = GetClassFromDexCache(soa.Self(),
+                                                       instr->GetDexFile(),
+                                                       instr->GetTypeIndex(),
+                                                       hint_dex_cache_);
   if (resolved_class != nullptr && !resolved_class->IsErroneous()) {
     instr->SetLoadedClassRTI(ReferenceTypeInfo::Create(
         handle_cache_->NewHandle(resolved_class), /* is_exact */ true));
@@ -572,7 +588,6 @@ void ReferenceTypePropagation::RTPVisitor::VisitLoadException(HLoadException* in
 }
 
 void ReferenceTypePropagation::RTPVisitor::VisitNullCheck(HNullCheck* instr) {
-  ScopedObjectAccess soa(Thread::Current());
   ReferenceTypeInfo parent_rti = instr->InputAt(0)->GetReferenceTypeInfo();
   if (parent_rti.IsValid()) {
     instr->SetReferenceTypeInfo(parent_rti);
@@ -580,10 +595,9 @@ void ReferenceTypePropagation::RTPVisitor::VisitNullCheck(HNullCheck* instr) {
 }
 
 void ReferenceTypePropagation::RTPVisitor::VisitBoundType(HBoundType* instr) {
-  ScopedObjectAccess soa(Thread::Current());
-
   ReferenceTypeInfo class_rti = instr->GetUpperBound();
   if (class_rti.IsValid()) {
+    ScopedObjectAccess soa(Thread::Current());
     // Narrow the type as much as possible.
     HInstruction* obj = instr->InputAt(0);
     ReferenceTypeInfo obj_rti = obj->GetReferenceTypeInfo();
@@ -614,8 +628,6 @@ void ReferenceTypePropagation::RTPVisitor::VisitBoundType(HBoundType* instr) {
 }
 
 void ReferenceTypePropagation::RTPVisitor::VisitCheckCast(HCheckCast* check_cast) {
-  ScopedObjectAccess soa(Thread::Current());
-
   HLoadClass* load_class = check_cast->InputAt(1)->AsLoadClass();
   ReferenceTypeInfo class_rti = load_class->GetLoadedClassRTI();
   HBoundType* bound_type = check_cast->GetNext()->AsBoundType();
@@ -650,7 +662,6 @@ void ReferenceTypePropagation::VisitPhi(HPhi* phi) {
       // point the interpreter jumps to that loop header.
       return;
     }
-    ScopedObjectAccess soa(Thread::Current());
     // Set the initial type for the phi. Use the non back edge input for reaching
     // a fixed point faster.
     HInstruction* first_input = phi->InputAt(0);
@@ -765,7 +776,8 @@ void ReferenceTypePropagation::RTPVisitor::VisitInvoke(HInvoke* instr) {
 
   ScopedObjectAccess soa(Thread::Current());
   ClassLinker* cl = Runtime::Current()->GetClassLinker();
-  mirror::DexCache* dex_cache = cl->FindDexCache(soa.Self(), instr->GetDexFile());
+  mirror::DexCache* dex_cache =
+      FindDexCacheWithHint(soa.Self(), instr->GetDexFile(), hint_dex_cache_);
   size_t pointer_size = cl->GetImagePointerSize();
   ArtMethod* method = dex_cache->GetResolvedMethod(instr->GetDexMethodIndex(), pointer_size);
   mirror::Class* klass = (method == nullptr) ? nullptr : method->GetReturnType(false, pointer_size);
index 01b5781..2106be6 100644 (file)
@@ -32,6 +32,7 @@ namespace art {
 class ReferenceTypePropagation : public HOptimization {
  public:
   ReferenceTypePropagation(HGraph* graph,
+                           Handle<mirror::DexCache> hint_dex_cache,
                            StackHandleScopeCollection* handles,
                            bool is_first_run,
                            const char* name = kReferenceTypePropagationPassName);
@@ -90,6 +91,10 @@ class ReferenceTypePropagation : public HOptimization {
 
   void ValidateTypes();
 
+  // Note: hint_dex_cache_ is usually, but not necessarily, the dex cache associated with
+  // graph_->GetDexFile(). Since we may look up also in other dex files, it's used only
+  // as a hint, to reduce the number of calls to the costly ClassLinker::FindDexCache().
+  Handle<mirror::DexCache> hint_dex_cache_;
   HandleCache handle_cache_;
 
   ArenaVector<HInstruction*> worklist_;
index 2fe2f20..b0682a9 100644 (file)
@@ -509,7 +509,7 @@ GraphAnalysisResult SsaBuilder::BuildSsa() {
 
   // 4) Compute type of reference type instructions. The pass assumes that
   // NullConstant has been fixed up.
-  ReferenceTypePropagation(graph_, handles_, /* is_first_run */ true).Run();
+  ReferenceTypePropagation(graph_, dex_cache_, handles_, /* is_first_run */ true).Run();
 
   // 5) HInstructionBuilder duplicated ArrayGet instructions with ambiguous type
   // (int/float or long/double) and marked ArraySets with ambiguous input type.
index c37c28c..d7360ad 100644 (file)
@@ -47,8 +47,11 @@ namespace art {
  */
 class SsaBuilder : public ValueObject {
  public:
-  SsaBuilder(HGraph* graph, StackHandleScopeCollection* handles)
+  SsaBuilder(HGraph* graph,
+             Handle<mirror::DexCache> dex_cache,
+             StackHandleScopeCollection* handles)
       : graph_(graph),
+        dex_cache_(dex_cache),
         handles_(handles),
         agets_fixed_(false),
         ambiguous_agets_(graph->GetArena()->Adapter(kArenaAllocGraphBuilder)),
@@ -112,6 +115,7 @@ class SsaBuilder : public ValueObject {
   void RemoveRedundantUninitializedStrings();
 
   HGraph* graph_;
+  Handle<mirror::DexCache> dex_cache_;
   StackHandleScopeCollection* const handles_;
 
   // True if types of ambiguous ArrayGets have been resolved.