Revert "Revert "Hold dex caches live in class table""

Bug: 29083330

This reverts commit f102faf1bcbdb2149e3e7bf27b1819f621b7894b.

(cherry picked from commit c9dbb1df3b5c06ba122cacaf35b17cb53c6be3c6)

Change-Id: Ib5d82bde2e4a031c1cab0ea6116925ef99d9f07f

diff --git a/compiler/driver/compiler_driver-inl.h b/compiler/driver/compiler_driver-inl.h
index 3cb63e7..94f5acc 100644 (file)
--- a/compiler/driver/compiler_driver-inl.h
+++ b/compiler/driver/compiler_driver-inl.h
@@ -390,9 +390,8 @@ inline int CompilerDriver::IsFastInvoke(
           *devirt_target->dex_file, devirt_target->dex_method_index, dex_cache, class_loader,
           nullptr, kVirtual);
     } else {
-      auto target_dex_cache(hs.NewHandle(class_linker->RegisterDexFile(
-          *devirt_target->dex_file,
-          class_linker->GetOrCreateAllocatorForClassLoader(class_loader.Get()))));
+      auto target_dex_cache(hs.NewHandle(class_linker->RegisterDexFile(*devirt_target->dex_file,
+                                                                       class_loader.Get())));
       called_method = class_linker->ResolveMethod<ClassLinker::kNoICCECheckForCache>(
           *devirt_target->dex_file, devirt_target->dex_method_index, target_dex_cache,
           class_loader, nullptr, kVirtual);

diff --git a/compiler/driver/compiler_driver.cc b/compiler/driver/compiler_driver.cc
index 7708b97..a4b4889 100644 (file)
--- a/compiler/driver/compiler_driver.cc
+++ b/compiler/driver/compiler_driver.cc
@@ -1113,9 +1113,8 @@ void CompilerDriver::LoadImageClasses(TimingLogger* timings) {
       uint16_t exception_type_idx = exception_type.first;
       const DexFile* dex_file = exception_type.second;
       StackHandleScope<2> hs2(self);
-      Handle<mirror::DexCache> dex_cache(hs2.NewHandle(class_linker->RegisterDexFile(
-          *dex_file,
-          Runtime::Current()->GetLinearAlloc())));
+      Handle<mirror::DexCache> dex_cache(hs2.NewHandle(class_linker->RegisterDexFile(*dex_file,
+                                                                                     nullptr)));
       Handle<mirror::Class> klass(hs2.NewHandle(
           class_linker->ResolveType(*dex_file,
                                     exception_type_idx,
@@ -2156,7 +2155,7 @@ class ResolveTypeVisitor : public CompilationVisitor {
         hs.NewHandle(soa.Decode<mirror::ClassLoader*>(manager_->GetClassLoader())));
     Handle<mirror::DexCache> dex_cache(hs.NewHandle(class_linker->RegisterDexFile(
         dex_file,
-        class_linker->GetOrCreateAllocatorForClassLoader(class_loader.Get()))));
+        class_loader.Get())));
     mirror::Class* klass = class_linker->ResolveType(dex_file, type_idx, dex_cache, class_loader);
 
     if (klass == nullptr) {

diff --git a/compiler/image_writer.cc b/compiler/image_writer.cc
index eaeacc5..da10568 100644 (file)
--- a/compiler/image_writer.cc
+++ b/compiler/image_writer.cc
@@ -842,6 +842,10 @@ void ImageWriter::PruneNonImageClasses() {
   ClassLinker* class_linker = runtime->GetClassLinker();
   Thread* self = Thread::Current();
 
+  // Clear class table strong roots so that dex caches can get pruned. We require pruning the class
+  // path dex caches.
+  class_linker->ClearClassTableStrongRoots();
+
   // Make a list of classes we would like to prune.
   NonImageClassesVisitor visitor(this);
   class_linker->VisitClasses(&visitor);

diff --git a/compiler/oat_test.cc b/compiler/oat_test.cc
index 21e198c..6d1f944 100644 (file)
--- a/compiler/oat_test.cc
+++ b/compiler/oat_test.cc
@@ -199,7 +199,7 @@ class OatTest : public CommonCompilerTest {
     for (const std::unique_ptr<const DexFile>& dex_file : opened_dex_files) {
       dex_files.push_back(dex_file.get());
       ScopedObjectAccess soa(Thread::Current());
-      class_linker->RegisterDexFile(*dex_file, runtime->GetLinearAlloc());
+      class_linker->RegisterDexFile(*dex_file, nullptr);
     }
     linker::MultiOatRelativePatcher patcher(compiler_driver_->GetInstructionSet(),
                                             instruction_set_features_.get());
@@ -491,10 +491,7 @@ TEST_F(OatTest, EmptyTextSection) {
   ClassLinker* const class_linker = Runtime::Current()->GetClassLinker();
   for (const DexFile* dex_file : dex_files) {
     ScopedObjectAccess soa(Thread::Current());
-    class_linker->RegisterDexFile(
-        *dex_file,
-        class_linker->GetOrCreateAllocatorForClassLoader(
-            soa.Decode<mirror::ClassLoader*>(class_loader)));
+    class_linker->RegisterDexFile(*dex_file, soa.Decode<mirror::ClassLoader*>(class_loader));
   }
   compiler_driver_->SetDexFilesForOatFile(dex_files);
   compiler_driver_->CompileAll(class_loader, dex_files, &timings);

diff --git a/dex2oat/dex2oat.cc b/dex2oat/dex2oat.cc
index db26799..3db85c1 100644 (file)
--- a/dex2oat/dex2oat.cc
+++ b/dex2oat/dex2oat.cc
@@ -1456,7 +1456,8 @@ class Dex2Oat FINAL {
     for (const auto& dex_file : dex_files_) {
       ScopedObjectAccess soa(self);
       dex_caches_.push_back(soa.AddLocalReference<jobject>(
-          class_linker->RegisterDexFile(*dex_file, Runtime::Current()->GetLinearAlloc())));
+          class_linker->RegisterDexFile(*dex_file,
+                                        soa.Decode<mirror::ClassLoader*>(class_loader_))));
     }
 
     return true;

diff --git a/oatdump/oatdump.cc b/oatdump/oatdump.cc
index f5458c0..aa4635d 100644 (file)
--- a/oatdump/oatdump.cc
+++ b/oatdump/oatdump.cc
@@ -1118,8 +1118,7 @@ class OatDumper {
       ScopedObjectAccess soa(Thread::Current());
       Runtime* const runtime = Runtime::Current();
       Handle<mirror::DexCache> dex_cache(
-          hs->NewHandle(runtime->GetClassLinker()->RegisterDexFile(*dex_file,
-                                                                   runtime->GetLinearAlloc())));
+          hs->NewHandle(runtime->GetClassLinker()->RegisterDexFile(*dex_file, nullptr)));
       DCHECK(options_.class_loader_ != nullptr);
       return verifier::MethodVerifier::VerifyMethodAndDump(
           soa.Self(), vios, dex_method_idx, dex_file, dex_cache, *options_.class_loader_,
@@ -2283,7 +2282,7 @@ static int DumpOatWithRuntime(Runtime* runtime, OatFile* oat_file, OatDumperOpti
     std::string error_msg;
     const DexFile* const dex_file = OpenDexFile(odf, &error_msg);
     CHECK(dex_file != nullptr) << error_msg;
-    class_linker->RegisterDexFile(*dex_file, runtime->GetLinearAlloc());
+    class_linker->RegisterDexFile(*dex_file, nullptr);
     class_path.push_back(dex_file);
   }
 

diff --git a/runtime/class_linker.cc b/runtime/class_linker.cc
index 9e144dd..0a5874b 100644 (file)
--- a/runtime/class_linker.cc
+++ b/runtime/class_linker.cc
@@ -2469,9 +2469,7 @@ mirror::Class* ClassLinker::DefineClass(Thread* self,
     self->AssertPendingOOMException();
     return nullptr;
   }
-  mirror::DexCache* dex_cache = RegisterDexFile(
-      dex_file,
-      GetOrCreateAllocatorForClassLoader(class_loader.Get()));
+  mirror::DexCache* dex_cache = RegisterDexFile(dex_file, class_loader.Get());
   if (dex_cache == nullptr) {
     self->AssertPendingOOMException();
     return nullptr;
@@ -3230,7 +3228,8 @@ void ClassLinker::RegisterDexFileLocked(const DexFile& dex_file,
   dex_caches_.push_back(data);
 }
 
-mirror::DexCache* ClassLinker::RegisterDexFile(const DexFile& dex_file, LinearAlloc* linear_alloc) {
+mirror::DexCache* ClassLinker::RegisterDexFile(const DexFile& dex_file,
+                                               mirror::ClassLoader* class_loader) {
   Thread* self = Thread::Current();
   {
     ReaderMutexLock mu(self, dex_lock_);
@@ -3239,21 +3238,31 @@ mirror::DexCache* ClassLinker::RegisterDexFile(const DexFile& dex_file, LinearAl
       return dex_cache;
     }
   }
+  LinearAlloc* const linear_alloc = GetOrCreateAllocatorForClassLoader(class_loader);
+  DCHECK(linear_alloc != nullptr);
+  ClassTable* table;
+  {
+    WriterMutexLock mu(self, *Locks::classlinker_classes_lock_);
+    table = InsertClassTableForClassLoader(class_loader);
+  }
   // Don't alloc while holding the lock, since allocation may need to
   // suspend all threads and another thread may need the dex_lock_ to
   // get to a suspend point.
   StackHandleScope<1> hs(self);
   Handle<mirror::DexCache> h_dex_cache(hs.NewHandle(AllocDexCache(self, dex_file, linear_alloc)));
-  WriterMutexLock mu(self, dex_lock_);
-  mirror::DexCache* dex_cache = FindDexCacheLocked(self, dex_file, true);
-  if (dex_cache != nullptr) {
-    return dex_cache;
-  }
-  if (h_dex_cache.Get() == nullptr) {
-    self->AssertPendingOOMException();
-    return nullptr;
+  {
+    WriterMutexLock mu(self, dex_lock_);
+    mirror::DexCache* dex_cache = FindDexCacheLocked(self, dex_file, true);
+    if (dex_cache != nullptr) {
+      return dex_cache;
+    }
+    if (h_dex_cache.Get() == nullptr) {
+      self->AssertPendingOOMException();
+      return nullptr;
+    }
+    RegisterDexFileLocked(dex_file, h_dex_cache);
   }
-  RegisterDexFileLocked(dex_file, h_dex_cache);
+  table->InsertStrongRoot(h_dex_cache.Get());
   return h_dex_cache.Get();
 }
 
@@ -7973,6 +7982,16 @@ void ClassLinker::DropFindArrayClassCache() {
   find_array_class_cache_next_victim_ = 0;
 }
 
+void ClassLinker::ClearClassTableStrongRoots() const {
+  Thread* const self = Thread::Current();
+  WriterMutexLock mu(self, *Locks::classlinker_classes_lock_);
+  for (const ClassLoaderData& data : class_loaders_) {
+    if (data.class_table != nullptr) {
+      data.class_table->ClearStrongRoots();
+    }
+  }
+}
+
 void ClassLinker::VisitClassLoaders(ClassLoaderVisitor* visitor) const {
   Thread* const self = Thread::Current();
   for (const ClassLoaderData& data : class_loaders_) {
@@ -7991,7 +8010,7 @@ void ClassLinker::InsertDexFileInToClassLoader(mirror::Object* dex_file,
   WriterMutexLock mu(self, *Locks::classlinker_classes_lock_);
   ClassTable* const table = ClassTableForClassLoader(class_loader);
   DCHECK(table != nullptr);
-  if (table->InsertDexFile(dex_file) && class_loader != nullptr) {
+  if (table->InsertStrongRoot(dex_file) && class_loader != nullptr) {
     // It was not already inserted, perform the write barrier to let the GC know the class loader's
     // class table was modified.
     Runtime::Current()->GetHeap()->WriteBarrierEveryFieldOf(class_loader);

diff --git a/runtime/class_linker.h b/runtime/class_linker.h
index f6ce545..0715bab 100644 (file)
--- a/runtime/class_linker.h
+++ b/runtime/class_linker.h
@@ -377,7 +377,8 @@ class ClassLinker {
       SHARED_REQUIRES(Locks::mutator_lock_)
       REQUIRES(!dex_lock_, !Roles::uninterruptible_);
 
-  mirror::DexCache* RegisterDexFile(const DexFile& dex_file, LinearAlloc* linear_alloc)
+  mirror::DexCache* RegisterDexFile(const DexFile& dex_file,
+                                    mirror::ClassLoader* class_loader)
       REQUIRES(!dex_lock_)
       SHARED_REQUIRES(Locks::mutator_lock_);
   void RegisterDexFile(const DexFile& dex_file, Handle<mirror::DexCache> dex_cache)
@@ -634,6 +635,11 @@ class ClassLinker {
   // Create the IMT and conflict tables for a class.
   void FillIMTAndConflictTables(mirror::Class* klass) SHARED_REQUIRES(Locks::mutator_lock_);
 
+  // Clear class table strong roots (other than classes themselves). This is done by dex2oat to
+  // allow pruning dex caches.
+  void ClearClassTableStrongRoots() const
+      REQUIRES(!Locks::classlinker_classes_lock_)
+      SHARED_REQUIRES(Locks::mutator_lock_);
 
   struct DexCacheData {
     // Weak root to the DexCache. Note: Do not decode this unnecessarily or else class unloading may

diff --git a/runtime/class_table-inl.h b/runtime/class_table-inl.h
index 42e320a..d52365d 100644 (file)
--- a/runtime/class_table-inl.h
+++ b/runtime/class_table-inl.h
@@ -29,7 +29,7 @@ void ClassTable::VisitRoots(Visitor& visitor) {
       visitor.VisitRoot(root.AddressWithoutBarrier());
     }
   }
-  for (GcRoot<mirror::Object>& root : dex_files_) {
+  for (GcRoot<mirror::Object>& root : strong_roots_) {
     visitor.VisitRoot(root.AddressWithoutBarrier());
   }
 }
@@ -42,7 +42,7 @@ void ClassTable::VisitRoots(const Visitor& visitor) {
       visitor.VisitRoot(root.AddressWithoutBarrier());
     }
   }
-  for (GcRoot<mirror::Object>& root : dex_files_) {
+  for (GcRoot<mirror::Object>& root : strong_roots_) {
     visitor.VisitRoot(root.AddressWithoutBarrier());
   }
 }

diff --git a/runtime/class_table.cc b/runtime/class_table.cc
index 8267c68..e9154cb 100644 (file)
--- a/runtime/class_table.cc
+++ b/runtime/class_table.cc
@@ -146,15 +146,15 @@ uint32_t ClassTable::ClassDescriptorHashEquals::operator()(const char* descripto
   return ComputeModifiedUtf8Hash(descriptor);
 }
 
-bool ClassTable::InsertDexFile(mirror::Object* dex_file) {
+bool ClassTable::InsertStrongRoot(mirror::Object* obj) {
   WriterMutexLock mu(Thread::Current(), lock_);
-  DCHECK(dex_file != nullptr);
-  for (GcRoot<mirror::Object>& root : dex_files_) {
-    if (root.Read() == dex_file) {
+  DCHECK(obj != nullptr);
+  for (GcRoot<mirror::Object>& root : strong_roots_) {
+    if (root.Read() == obj) {
       return false;
     }
   }
-  dex_files_.push_back(GcRoot<mirror::Object>(dex_file));
+  strong_roots_.push_back(GcRoot<mirror::Object>(obj));
   return true;
 }
 
@@ -189,4 +189,8 @@ void ClassTable::AddClassSet(ClassSet&& set) {
   classes_.insert(classes_.begin(), std::move(set));
 }
 
+void ClassTable::ClearStrongRoots() {
+  WriterMutexLock mu(Thread::Current(), lock_);
+  strong_roots_.clear();
+}
 }  // namespace art

diff --git a/runtime/class_table.h b/runtime/class_table.h
index 686381d..6fb4206 100644 (file)
--- a/runtime/class_table.h
+++ b/runtime/class_table.h
@@ -133,8 +133,8 @@ class ClassTable {
       REQUIRES(!lock_)
       SHARED_REQUIRES(Locks::mutator_lock_);
 
-  // Return true if we inserted the dex file, false if it already exists.
-  bool InsertDexFile(mirror::Object* dex_file)
+  // Return true if we inserted the strong root, false if it already exists.
+  bool InsertStrongRoot(mirror::Object* obj)
       REQUIRES(!lock_)
       SHARED_REQUIRES(Locks::mutator_lock_);
 
@@ -153,6 +153,11 @@ class ClassTable {
       REQUIRES(!lock_)
       SHARED_REQUIRES(Locks::mutator_lock_);
 
+  // Clear strong roots (other than classes themselves).
+  void ClearStrongRoots()
+      REQUIRES(!lock_)
+      SHARED_REQUIRES(Locks::mutator_lock_);
+
   ReaderWriterMutex& GetLock() {
     return lock_;
   }
@@ -162,9 +167,10 @@ class ClassTable {
   mutable ReaderWriterMutex lock_;
   // We have a vector to help prevent dirty pages after the zygote forks by calling FreezeSnapshot.
   std::vector<ClassSet> classes_ GUARDED_BY(lock_);
-  // Dex files used by the class loader which may not be owned by the class loader. We keep these
-  // live so that we do not have issues closing any of the dex files.
-  std::vector<GcRoot<mirror::Object>> dex_files_ GUARDED_BY(lock_);
+  // Extra strong roots that can be either dex files or dex caches. Dex files used by the class
+  // loader which may not be owned by the class loader must be held strongly live. Also dex caches
+  // are held live to prevent them being unloading once they have classes in them.
+  std::vector<GcRoot<mirror::Object>> strong_roots_ GUARDED_BY(lock_);
 };
 
 }  // namespace art

diff --git a/runtime/native/dalvik_system_DexFile.cc b/runtime/native/dalvik_system_DexFile.cc
index f30f7a6..8c7c966 100644 (file)
--- a/runtime/native/dalvik_system_DexFile.cc
+++ b/runtime/native/dalvik_system_DexFile.cc
@@ -278,9 +278,7 @@ static jclass DexFile_defineClassNative(JNIEnv* env,
       StackHandleScope<1> hs(soa.Self());
       Handle<mirror::ClassLoader> class_loader(
           hs.NewHandle(soa.Decode<mirror::ClassLoader*>(javaLoader)));
-      class_linker->RegisterDexFile(
-          *dex_file,
-          class_linker->GetOrCreateAllocatorForClassLoader(class_loader.Get()));
+      class_linker->RegisterDexFile(*dex_file, class_loader.Get());
       mirror::Class* result = class_linker->DefineClass(soa.Self(),
                                                         descriptor.c_str(),
                                                         hash,

diff --git a/runtime/native/dalvik_system_VMRuntime.cc b/runtime/native/dalvik_system_VMRuntime.cc
index 6c943dc..79b18aa 100644 (file)
--- a/runtime/native/dalvik_system_VMRuntime.cc
+++ b/runtime/native/dalvik_system_VMRuntime.cc
@@ -504,8 +504,7 @@ static void VMRuntime_preloadDexCaches(JNIEnv* env, jobject) {
     const DexFile* dex_file = boot_class_path[i];
     CHECK(dex_file != nullptr);
     StackHandleScope<1> hs(soa.Self());
-    Handle<mirror::DexCache> dex_cache(
-        hs.NewHandle(linker->RegisterDexFile(*dex_file, runtime->GetLinearAlloc())));
+    Handle<mirror::DexCache> dex_cache(hs.NewHandle(linker->RegisterDexFile(*dex_file, nullptr)));
 
     if (kPreloadDexCachesStrings) {
       for (size_t j = 0; j < dex_cache->NumStrings(); j++) {