Revert "Revert "Support class unloading in dex2oat for quicken multidex""

Bug: 63467744
Test: test-art-host

This reverts commit 6921d90a241f0307ac25120f8f976744d4a57706.

Change-Id: If70e31d3a15579dc75fd40bfef186e0124568c87

12 files changed, 207 insertions, 21 deletions

diff --git a/compiler/dex/quick_compiler_callbacks.cc b/compiler/dex/quick_compiler_callbacks.cc
index 872f7ea15d..c7e9f4fc07 100644
--- a/compiler/dex/quick_compiler_callbacks.cc
+++ b/compiler/dex/quick_compiler_callbacks.cc
@@ -16,6 +16,7 @@
 
 #include "quick_compiler_callbacks.h"
 
+#include "driver/compiler_driver.h"
 #include "verification_results.h"
 #include "verifier/method_verifier-inl.h"
 
@@ -33,4 +34,17 @@ void QuickCompilerCallbacks::ClassRejected(ClassReference ref) {
   }
 }
 
+bool QuickCompilerCallbacks::CanAssumeVerified(ClassReference ref) {
+  // If we don't have class unloading enabled in the compiler, we will never see class that were
+  // previously verified. Return false to avoid overhead from the lookup in the compiler driver.
+  if (!does_class_unloading_) {
+    return false;
+  }
+  DCHECK(compiler_driver_ != nullptr);
+  // In the case of the quicken filter: avoiding verification of quickened instructions, which the
+  // verifier doesn't currently support.
+  // In the case of the verify filter, avoiding verifiying twice.
+  return compiler_driver_->CanAssumeVerified(ref);
+}
+
 }  // namespace art
diff --git a/compiler/dex/quick_compiler_callbacks.h b/compiler/dex/quick_compiler_callbacks.h
index a3a6c0972c..578aff45e5 100644
--- a/compiler/dex/quick_compiler_callbacks.h
+++ b/compiler/dex/quick_compiler_callbacks.h
@@ -22,6 +22,7 @@
 
 namespace art {
 
+class CompilerDriver;
 class VerificationResults;
 
 class QuickCompilerCallbacks FINAL : public CompilerCallbacks {
@@ -53,8 +54,19 @@ class QuickCompilerCallbacks FINAL : public CompilerCallbacks {
       verification_results_ = verification_results;
     }
 
+    bool CanAssumeVerified(ClassReference ref) OVERRIDE;
+
+    void SetDoesClassUnloading(bool does_class_unloading, CompilerDriver* compiler_driver)
+        OVERRIDE {
+      does_class_unloading_ = does_class_unloading;
+      compiler_driver_ = compiler_driver;
+      DCHECK(!does_class_unloading || compiler_driver_ != nullptr);
+    }
+
   private:
     VerificationResults* verification_results_ = nullptr;
+    bool does_class_unloading_ = false;
+    CompilerDriver* compiler_driver_ = nullptr;
     std::unique_ptr<verifier::VerifierDeps> verifier_deps_;
 };
 
diff --git a/compiler/driver/compiler_driver.cc b/compiler/driver/compiler_driver.cc
index 6e087c5785..037e45840d 100644
--- a/compiler/driver/compiler_driver.cc
+++ b/compiler/driver/compiler_driver.cc
@@ -3040,4 +3040,10 @@ void CompilerDriver::SetDexFilesForOatFile(const std::vector<const DexFile*>& de
   }
 }
 
+bool CompilerDriver::CanAssumeVerified(ClassReference ref) const {
+  mirror::Class::Status existing = mirror::Class::kStatusNotReady;
+  compiled_classes_.Get(DexFileReference(ref.first, ref.second), &existing);
+  return existing >= mirror::Class::kStatusVerified;
+}
+
 }  // namespace art
diff --git a/compiler/driver/compiler_driver.h b/compiler/driver/compiler_driver.h
index d9886a2fba..ba4581c9f4 100644
--- a/compiler/driver/compiler_driver.h
+++ b/compiler/driver/compiler_driver.h
@@ -377,6 +377,8 @@ class CompilerDriver {
     return profile_compilation_info_;
   }
 
+  bool CanAssumeVerified(ClassReference ref) const;
+
  private:
   void PreCompile(jobject class_loader,
                   const std::vector<const DexFile*>& dex_files,
diff --git a/compiler/driver/compiler_driver_test.cc b/compiler/driver/compiler_driver_test.cc
index 4da3e0df39..392d57c0f2 100644
--- a/compiler/driver/compiler_driver_test.cc
+++ b/compiler/driver/compiler_driver_test.cc
@@ -23,6 +23,7 @@
 #include "art_method-inl.h"
 #include "class_linker-inl.h"
 #include "common_compiler_test.h"
+#include "compiler_callbacks.h"
 #include "dex_file.h"
 #include "dex_file_types.h"
 #include "gc/heap.h"
@@ -368,7 +369,9 @@ TEST_F(CompilerDriverVerifyTest, VerifyCompilation) {
 
 // Test that a class of status kStatusRetryVerificationAtRuntime is indeed recorded that way in the
 // driver.
-TEST_F(CompilerDriverVerifyTest, RetryVerifcationStatus) {
+// Test that checks that classes can be assumed as verified if unloading mode is enabled and
+// the class status is at least verified.
+TEST_F(CompilerDriverVerifyTest, RetryVerifcationStatusCheckVerified) {
   Thread* const self = Thread::Current();
   jobject class_loader;
   std::vector<const DexFile*> dex_files;
@@ -382,6 +385,7 @@ TEST_F(CompilerDriverVerifyTest, RetryVerifcationStatus) {
     dex_file = dex_files.front();
   }
   compiler_driver_->SetDexFilesForOatFile(dex_files);
+  callbacks_->SetDoesClassUnloading(true, compiler_driver_.get());
   ClassReference ref(dex_file, 0u);
   // Test that the status is read from the compiler driver as expected.
   for (size_t i = mirror::Class::kStatusRetryVerificationAtRuntime;
@@ -397,6 +401,12 @@ TEST_F(CompilerDriverVerifyTest, RetryVerifcationStatus) {
     mirror::Class::Status status = {};
     ASSERT_TRUE(compiler_driver_->GetCompiledClass(ref, &status));
     EXPECT_EQ(status, expected_status);
+
+    // Check that we can assume verified if we are a status that is at least verified.
+    if (status >= mirror::Class::kStatusVerified) {
+      // Check that the class can be assumed as verified in the compiler driver.
+      EXPECT_TRUE(callbacks_->CanAssumeVerified(ref)) << status;
+    }
   }
 }
 
diff --git a/dex2oat/dex2oat.cc b/dex2oat/dex2oat.cc
index ff193e933a..f50e88fdaf 100644
--- a/dex2oat/dex2oat.cc
+++ b/dex2oat/dex2oat.cc
@@ -1774,10 +1774,8 @@ class Dex2Oat FINAL {
   bool ShouldCompileDexFilesIndividually() const {
     // Compile individually if we are not building an image, not using any compilation, and are
     // using multidex.
-    // This means extract, verify, and quicken will use the individual compilation mode (to reduce
-    // RAM used by the compiler).
-    // TODO: Still do it for app images to get testing coverage. Note that this will generate empty
-    // app images.
+    // This means extract, and verify, will use the individual compilation mode (to reduce RAM used
+    // by the compiler).
     return !IsImage() &&
         dex_files_.size() > 1 &&
         !CompilerFilter::IsAnyCompilationEnabled(compiler_options_->GetCompilerFilter());
@@ -1856,6 +1854,16 @@ class Dex2Oat FINAL {
                                      profile_compilation_info_.get()));
     driver_->SetDexFilesForOatFile(dex_files_);
 
+    const bool compile_individually = ShouldCompileDexFilesIndividually();
+    if (compile_individually) {
+      // Set the compiler driver in the callbacks so that we can avoid re-verification. This not
+      // only helps performance but also prevents reverifying quickened bytecodes. Attempting
+      // verify quickened bytecode causes verification failures.
+      // Only set the compiler filter if we are doing separate compilation since there is a bit
+      // of overhead when checking if a class was previously verified.
+      callbacks_->SetDoesClassUnloading(true, driver_.get());
+    }
+
     // Setup vdex for compilation.
     if (!DoEagerUnquickeningOfVdex() && input_vdex_file_ != nullptr) {
       callbacks_->SetVerifierDeps(
@@ -1872,7 +1880,7 @@ class Dex2Oat FINAL {
       callbacks_->SetVerifierDeps(new verifier::VerifierDeps(dex_files_));
     }
     // Invoke the compilation.
-    if (ShouldCompileDexFilesIndividually()) {
+    if (compile_individually) {
       CompileDexFilesIndividually();
       // Return a null classloader since we already freed released it.
       return nullptr;
@@ -2966,6 +2974,8 @@ class ScopedGlobalRef {
 
 static dex2oat::ReturnCode CompileImage(Dex2Oat& dex2oat) {
   dex2oat.LoadClassProfileDescriptors();
+  // Keep the class loader that was used for compilation live for the rest of the compilation
+  // process.
   ScopedGlobalRef class_loader(dex2oat.Compile());
 
   if (!dex2oat.WriteOutputFiles()) {
@@ -3014,6 +3024,8 @@ static dex2oat::ReturnCode CompileImage(Dex2Oat& dex2oat) {
 }
 
 static dex2oat::ReturnCode CompileApp(Dex2Oat& dex2oat) {
+  // Keep the class loader that was used for compilation live for the rest of the compilation
+  // process.
   ScopedGlobalRef class_loader(dex2oat.Compile());
 
   if (!dex2oat.WriteOutputFiles()) {
diff --git a/dex2oat/dex2oat_test.cc b/dex2oat/dex2oat_test.cc
index 12bceb3c1f..6a9d979d52 100644
--- a/dex2oat/dex2oat_test.cc
+++ b/dex2oat/dex2oat_test.cc
@@ -41,6 +41,7 @@
 namespace art {
 
 static constexpr size_t kMaxMethodIds = 65535;
+static constexpr bool kDebugArgs = false;
 
 using android::base::StringPrintf;
 
@@ -55,7 +56,7 @@ class Dex2oatTest : public Dex2oatEnvironmentTest {
   }
 
  protected:
-  int GenerateOdexForTestWithStatus(const std::string& dex_location,
+  int GenerateOdexForTestWithStatus(const std::vector<std::string>& dex_locations,
                                     const std::string& odex_location,
                                     CompilerFilter::Filter filter,
                                     std::string* error_msg,
@@ -63,7 +64,10 @@ class Dex2oatTest : public Dex2oatEnvironmentTest {
                                     bool use_fd = false) {
     std::unique_ptr<File> oat_file;
     std::vector<std::string> args;
-    args.push_back("--dex-file=" + dex_location);
+    // Add dex file args.
+    for (const std::string& dex_location : dex_locations) {
+      args.push_back("--dex-file=" + dex_location);
+    }
     if (use_fd) {
       oat_file.reset(OS::CreateEmptyFile(odex_location.c_str()));
       CHECK(oat_file != nullptr) << odex_location;
@@ -93,7 +97,7 @@ class Dex2oatTest : public Dex2oatEnvironmentTest {
                            bool use_fd = false,
                            std::function<void(const OatFile&)> check_oat = [](const OatFile&) {}) {
     std::string error_msg;
-    int status = GenerateOdexForTestWithStatus(dex_location,
+    int status = GenerateOdexForTestWithStatus({dex_location},
                                                odex_location,
                                                filter,
                                                &error_msg,
@@ -187,6 +191,14 @@ class Dex2oatTest : public Dex2oatEnvironmentTest {
     CHECK(android_root != nullptr);
     argv.push_back("--android-root=" + std::string(android_root));
 
+    if (kDebugArgs) {
+      std::string all_args;
+      for (const std::string& arg : argv) {
+        all_args += arg + " ";
+      }
+      LOG(ERROR) << all_args;
+    }
+
     int link[2];
 
     if (pipe(link) == -1) {
@@ -951,7 +963,7 @@ class Dex2oatReturnCodeTest : public Dex2oatTest {
     Copy(GetTestDexFileName(), dex_location);
 
     std::string error_msg;
-    return GenerateOdexForTestWithStatus(dex_location,
+    return GenerateOdexForTestWithStatus({dex_location},
                                          odex_location,
                                          CompilerFilter::kSpeed,
                                          &error_msg,
@@ -1107,4 +1119,72 @@ TEST_F(Dex2oatClassLoaderContextTest, ChainContext) {
   RunTest(context.c_str(), expected_classpath_key.c_str(), true);
 }
 
+class Dex2oatDeterminism : public Dex2oatTest {};
+
+TEST_F(Dex2oatDeterminism, UnloadCompile) {
+  if (!kUseReadBarrier &&
+      gc::kCollectorTypeDefault != gc::kCollectorTypeCMS &&
+      gc::kCollectorTypeDefault != gc::kCollectorTypeMS) {
+    LOG(INFO) << "Test requires determinism support.";
+    return;
+  }
+  Runtime* const runtime = Runtime::Current();
+  std::string out_dir = GetScratchDir();
+  const std::string base_oat_name = out_dir + "/base.oat";
+  const std::string base_vdex_name = out_dir + "/base.vdex";
+  const std::string unload_oat_name = out_dir + "/unload.oat";
+  const std::string unload_vdex_name = out_dir + "/unload.vdex";
+  const std::string no_unload_oat_name = out_dir + "/nounload.oat";
+  const std::string no_unload_vdex_name = out_dir + "/nounload.vdex";
+  const std::string app_image_name = out_dir + "/unload.art";
+  std::string error_msg;
+  const std::vector<gc::space::ImageSpace*>& spaces = runtime->GetHeap()->GetBootImageSpaces();
+  ASSERT_GT(spaces.size(), 0u);
+  const std::string image_location = spaces[0]->GetImageLocation();
+  // Without passing in an app image, it will unload in between compilations.
+  const int res = GenerateOdexForTestWithStatus(
+      GetLibCoreDexFileNames(),
+      base_oat_name,
+      CompilerFilter::Filter::kQuicken,
+      &error_msg,
+      {"--force-determinism", "--avoid-storing-invocation"});
+  EXPECT_EQ(res, 0);
+  Copy(base_oat_name, unload_oat_name);
+  Copy(base_vdex_name, unload_vdex_name);
+  std::unique_ptr<File> unload_oat(OS::OpenFileForReading(unload_oat_name.c_str()));
+  std::unique_ptr<File> unload_vdex(OS::OpenFileForReading(unload_vdex_name.c_str()));
+  ASSERT_TRUE(unload_oat != nullptr);
+  ASSERT_TRUE(unload_vdex != nullptr);
+  EXPECT_GT(unload_oat->GetLength(), 0u);
+  EXPECT_GT(unload_vdex->GetLength(), 0u);
+  // Regenerate with an app image to disable the dex2oat unloading and verify that the output is
+  // the same.
+  const int res2 = GenerateOdexForTestWithStatus(
+      GetLibCoreDexFileNames(),
+      base_oat_name,
+      CompilerFilter::Filter::kQuicken,
+      &error_msg,
+      {"--force-determinism", "--avoid-storing-invocation", "--app-image-file=" + app_image_name});
+  EXPECT_EQ(res2, 0);
+  Copy(base_oat_name, no_unload_oat_name);
+  Copy(base_vdex_name, no_unload_vdex_name);
+  std::unique_ptr<File> no_unload_oat(OS::OpenFileForReading(no_unload_oat_name.c_str()));
+  std::unique_ptr<File> no_unload_vdex(OS::OpenFileForReading(no_unload_vdex_name.c_str()));
+  ASSERT_TRUE(no_unload_oat != nullptr);
+  ASSERT_TRUE(no_unload_vdex != nullptr);
+  EXPECT_GT(no_unload_oat->GetLength(), 0u);
+  EXPECT_GT(no_unload_vdex->GetLength(), 0u);
+  // Verify that both of the files are the same (odex and vdex).
+  EXPECT_EQ(unload_oat->GetLength(), no_unload_oat->GetLength());
+  EXPECT_EQ(unload_vdex->GetLength(), no_unload_vdex->GetLength());
+  EXPECT_EQ(unload_oat->Compare(no_unload_oat.get()), 0)
+      << unload_oat_name << " " << no_unload_oat_name;
+  EXPECT_EQ(unload_vdex->Compare(no_unload_vdex.get()), 0)
+      << unload_vdex_name << " " << no_unload_vdex_name;
+  // App image file.
+  std::unique_ptr<File> app_image_file(OS::OpenFileForReading(app_image_name.c_str()));
+  ASSERT_TRUE(app_image_file != nullptr);
+  EXPECT_GT(app_image_file->GetLength(), 0u);
+}
+
 }  // namespace art
diff --git a/runtime/aot_class_linker.cc b/runtime/aot_class_linker.cc
index b1bc3f8f2e..9396565eee 100644
--- a/runtime/aot_class_linker.cc
+++ b/runtime/aot_class_linker.cc
@@ -16,10 +16,12 @@
 
 #include "aot_class_linker.h"
 
+#include "class_reference.h"
+#include "compiler_callbacks.h"
 #include "handle_scope-inl.h"
-#include "mirror/class.h"
-#include "mirror/object-inl.h"
+#include "mirror/class-inl.h"
 #include "runtime.h"
+#include "verifier/verifier_enums.h"
 
 namespace art {
 
@@ -66,4 +68,18 @@ bool AotClassLinker::InitializeClass(Thread* self, Handle<mirror::Class> klass,
   }
   return success;
 }
+
+verifier::FailureKind AotClassLinker::PerformClassVerification(Thread* self,
+                                                               Handle<mirror::Class> klass,
+                                                               verifier::HardFailLogMode log_level,
+                                                               std::string* error_msg) {
+  Runtime* const runtime = Runtime::Current();
+  CompilerCallbacks* callbacks = runtime->GetCompilerCallbacks();
+  if (callbacks->CanAssumeVerified(ClassReference(&klass->GetDexFile(),
+                                                  klass->GetDexClassDefIndex()))) {
+    return verifier::FailureKind::kNoFailure;
+  }
+  return ClassLinker::PerformClassVerification(self, klass, log_level, error_msg);
+}
+
 }  // namespace art
diff --git a/runtime/aot_class_linker.h b/runtime/aot_class_linker.h
index 11bea86fc4..927b53302b 100644
--- a/runtime/aot_class_linker.h
+++ b/runtime/aot_class_linker.h
@@ -27,6 +27,16 @@ class AotClassLinker : public ClassLinker {
   explicit AotClassLinker(InternTable *intern_table);
   ~AotClassLinker();
 
+ protected:
+  // Overridden version of PerformClassVerification allows skipping verification if the class was
+  // previously verified but unloaded.
+  verifier::FailureKind PerformClassVerification(Thread* self,
+                                                 Handle<mirror::Class> klass,
+                                                 verifier::HardFailLogMode log_level,
+                                                 std::string* error_msg)
+      OVERRIDE
+      REQUIRES_SHARED(Locks::mutator_lock_);
+
   bool InitializeClass(Thread *self,
                        Handle<mirror::Class> klass,
                        bool can_run_clinit,
diff --git a/runtime/class_linker.cc b/runtime/class_linker.cc
index fc14da1067..9756f5787f 100644
--- a/runtime/class_linker.cc
+++ b/runtime/class_linker.cc
@@ -3694,7 +3694,8 @@ bool ClassLinker::IsDexFileRegistered(Thread* self, const DexFile& dex_file) {
 
 ObjPtr<mirror::DexCache> ClassLinker::FindDexCache(Thread* self, const DexFile& dex_file) {
   ReaderMutexLock mu(self, *Locks::dex_lock_);
-  ObjPtr<mirror::DexCache> dex_cache = DecodeDexCache(self, FindDexCacheDataLocked(dex_file));
+  DexCacheData dex_cache_data = FindDexCacheDataLocked(dex_file);
+  ObjPtr<mirror::DexCache> dex_cache = DecodeDexCache(self, dex_cache_data);
   if (dex_cache != nullptr) {
     return dex_cache;
   }
@@ -3704,7 +3705,8 @@ ObjPtr<mirror::DexCache> ClassLinker::FindDexCache(Thread* self, const DexFile&
       LOG(FATAL_WITHOUT_ABORT) << "Registered dex file " << data.dex_file->GetLocation();
     }
   }
-  LOG(FATAL) << "Failed to find DexCache for DexFile " << dex_file.GetLocation();
+  LOG(FATAL) << "Failed to find DexCache for DexFile " << dex_file.GetLocation()
+             << " " << &dex_file << " " << dex_cache_data.dex_file;
   UNREACHABLE();
 }
 
@@ -4280,13 +4282,7 @@ verifier::FailureKind ClassLinker::VerifyClass(
   std::string error_msg;
   verifier::FailureKind verifier_failure = verifier::FailureKind::kNoFailure;
   if (!preverified) {
-    Runtime* runtime = Runtime::Current();
-    verifier_failure = verifier::MethodVerifier::VerifyClass(self,
-                                                             klass.Get(),
-                                                             runtime->GetCompilerCallbacks(),
-                                                             runtime->IsAotCompiler(),
-                                                             log_level,
-                                                             &error_msg);
+    verifier_failure = PerformClassVerification(self, klass, log_level, &error_msg);
   }
 
   // Verification is done, grab the lock again.
@@ -4354,6 +4350,19 @@ verifier::FailureKind ClassLinker::VerifyClass(
   return verifier_failure;
 }
 
+verifier::FailureKind ClassLinker::PerformClassVerification(Thread* self,
+                                                            Handle<mirror::Class> klass,
+                                                            verifier::HardFailLogMode log_level,
+                                                            std::string* error_msg) {
+  Runtime* const runtime = Runtime::Current();
+  return verifier::MethodVerifier::VerifyClass(self,
+                                               klass.Get(),
+                                               runtime->GetCompilerCallbacks(),
+                                               runtime->IsAotCompiler(),
+                                               log_level,
+                                               error_msg);
+}
+
 bool ClassLinker::VerifyClassUsingOatFile(const DexFile& dex_file,
                                           ObjPtr<mirror::Class> klass,
                                           mirror::Class::Status& oat_file_class_status) {
diff --git a/runtime/class_linker.h b/runtime/class_linker.h
index bf14aebb52..2fbbe79e47 100644
--- a/runtime/class_linker.h
+++ b/runtime/class_linker.h
@@ -710,6 +710,12 @@ class ClassLinker {
       REQUIRES_SHARED(Locks::mutator_lock_)
       REQUIRES(!Locks::dex_lock_);
 
+  virtual verifier::FailureKind PerformClassVerification(Thread* self,
+                                                         Handle<mirror::Class> klass,
+                                                         verifier::HardFailLogMode log_level,
+                                                         std::string* error_msg)
+      REQUIRES_SHARED(Locks::mutator_lock_);
+
  private:
   class LinkInterfaceMethodsHelper;
 
diff --git a/runtime/compiler_callbacks.h b/runtime/compiler_callbacks.h
index 806653a265..c51bb5e176 100644
--- a/runtime/compiler_callbacks.h
+++ b/runtime/compiler_callbacks.h
@@ -22,6 +22,8 @@
 
 namespace art {
 
+class CompilerDriver;
+
 namespace verifier {
 
 class MethodVerifier;
@@ -49,6 +51,13 @@ class CompilerCallbacks {
   virtual verifier::VerifierDeps* GetVerifierDeps() const = 0;
   virtual void SetVerifierDeps(verifier::VerifierDeps* deps ATTRIBUTE_UNUSED) {}
 
+  virtual bool CanAssumeVerified(ClassReference ref ATTRIBUTE_UNUSED) {
+    return false;
+  }
+
+  virtual void SetDoesClassUnloading(bool does_class_unloading ATTRIBUTE_UNUSED,
+                                     CompilerDriver* compiler_driver ATTRIBUTE_UNUSED) {}
+
   bool IsBootImage() {
     return mode_ == CallbackMode::kCompileBootImage;
   }