Revert "Support class unloading in dex2oat for quicken multidex"

This reverts commit 0b1c341d2d89a483142cd14bdeb4650ab00184f1.

New test Dex2oatDeterminism.UnloadCompile fails on bots gtest-debug-gc and friends.

Change-Id: Ib101fc4390d90f88fe017d8482775d5e975f2ccb

12 files changed, 21 insertions, 201 deletions

diff --git a/compiler/dex/quick_compiler_callbacks.cc b/compiler/dex/quick_compiler_callbacks.cc
index c7e9f4fc07..872f7ea15d 100644
--- a/compiler/dex/quick_compiler_callbacks.cc
+++ b/compiler/dex/quick_compiler_callbacks.cc
@@ -16,7 +16,6 @@
 
 #include "quick_compiler_callbacks.h"
 
-#include "driver/compiler_driver.h"
 #include "verification_results.h"
 #include "verifier/method_verifier-inl.h"
 
@@ -34,17 +33,4 @@ 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 578aff45e5..a3a6c0972c 100644
--- a/compiler/dex/quick_compiler_callbacks.h
+++ b/compiler/dex/quick_compiler_callbacks.h
@@ -22,7 +22,6 @@
 
 namespace art {
 
-class CompilerDriver;
 class VerificationResults;
 
 class QuickCompilerCallbacks FINAL : public CompilerCallbacks {
@@ -54,19 +53,8 @@ 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 037e45840d..6e087c5785 100644
--- a/compiler/driver/compiler_driver.cc
+++ b/compiler/driver/compiler_driver.cc
@@ -3040,10 +3040,4 @@ 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 ba4581c9f4..d9886a2fba 100644
--- a/compiler/driver/compiler_driver.h
+++ b/compiler/driver/compiler_driver.h
@@ -377,8 +377,6 @@ 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 392d57c0f2..4da3e0df39 100644
--- a/compiler/driver/compiler_driver_test.cc
+++ b/compiler/driver/compiler_driver_test.cc
@@ -23,7 +23,6 @@
 #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"
@@ -369,9 +368,7 @@ TEST_F(CompilerDriverVerifyTest, VerifyCompilation) {
 
 // Test that a class of status kStatusRetryVerificationAtRuntime is indeed recorded that way in the
 // driver.
-// 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) {
+TEST_F(CompilerDriverVerifyTest, RetryVerifcationStatus) {
   Thread* const self = Thread::Current();
   jobject class_loader;
   std::vector<const DexFile*> dex_files;
@@ -385,7 +382,6 @@ TEST_F(CompilerDriverVerifyTest, RetryVerifcationStatusCheckVerified) {
     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;
@@ -401,12 +397,6 @@ TEST_F(CompilerDriverVerifyTest, RetryVerifcationStatusCheckVerified) {
     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 f50e88fdaf..ff193e933a 100644
--- a/dex2oat/dex2oat.cc
+++ b/dex2oat/dex2oat.cc
@@ -1774,8 +1774,10 @@ 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, and verify, will use the individual compilation mode (to reduce RAM used
-    // by the compiler).
+    // 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.
     return !IsImage() &&
         dex_files_.size() > 1 &&
         !CompilerFilter::IsAnyCompilationEnabled(compiler_options_->GetCompilerFilter());
@@ -1854,16 +1856,6 @@ 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(
@@ -1880,7 +1872,7 @@ class Dex2Oat FINAL {
       callbacks_->SetVerifierDeps(new verifier::VerifierDeps(dex_files_));
     }
     // Invoke the compilation.
-    if (compile_individually) {
+    if (ShouldCompileDexFilesIndividually()) {
       CompileDexFilesIndividually();
       // Return a null classloader since we already freed released it.
       return nullptr;
@@ -2974,8 +2966,6 @@ 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()) {
@@ -3024,8 +3014,6 @@ 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 1673fc120f..12bceb3c1f 100644
--- a/dex2oat/dex2oat_test.cc
+++ b/dex2oat/dex2oat_test.cc
@@ -41,7 +41,6 @@
 namespace art {
 
 static constexpr size_t kMaxMethodIds = 65535;
-static constexpr bool kDebugArgs = false;
 
 using android::base::StringPrintf;
 
@@ -56,7 +55,7 @@ class Dex2oatTest : public Dex2oatEnvironmentTest {
   }
 
  protected:
-  int GenerateOdexForTestWithStatus(const std::vector<std::string>& dex_locations,
+  int GenerateOdexForTestWithStatus(const std::string& dex_location,
                                     const std::string& odex_location,
                                     CompilerFilter::Filter filter,
                                     std::string* error_msg,
@@ -64,10 +63,7 @@ class Dex2oatTest : public Dex2oatEnvironmentTest {
                                     bool use_fd = false) {
     std::unique_ptr<File> oat_file;
     std::vector<std::string> args;
-    // Add dex file args.
-    for (const std::string& dex_location : dex_locations) {
-      args.push_back("--dex-file=" + dex_location);
-    }
+    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;
@@ -97,7 +93,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,
@@ -191,14 +187,6 @@ 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) {
@@ -963,7 +951,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,
@@ -1119,66 +1107,4 @@ TEST_F(Dex2oatClassLoaderContextTest, ChainContext) {
   RunTest(context.c_str(), expected_classpath_key.c_str(), true);
 }
 
-class Dex2oatDeterminism : public Dex2oatTest {};
-
-TEST_F(Dex2oatDeterminism, UnloadCompile) {
-  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 9396565eee..b1bc3f8f2e 100644
--- a/runtime/aot_class_linker.cc
+++ b/runtime/aot_class_linker.cc
@@ -16,12 +16,10 @@
 
 #include "aot_class_linker.h"
 
-#include "class_reference.h"
-#include "compiler_callbacks.h"
 #include "handle_scope-inl.h"
-#include "mirror/class-inl.h"
+#include "mirror/class.h"
+#include "mirror/object-inl.h"
 #include "runtime.h"
-#include "verifier/verifier_enums.h"
 
 namespace art {
 
@@ -68,18 +66,4 @@ 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 927b53302b..11bea86fc4 100644
--- a/runtime/aot_class_linker.h
+++ b/runtime/aot_class_linker.h
@@ -27,16 +27,6 @@ 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 ed673d4505..3ac87c5137 100644
--- a/runtime/class_linker.cc
+++ b/runtime/class_linker.cc
@@ -3694,8 +3694,7 @@ 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_);
-  DexCacheData dex_cache_data = FindDexCacheDataLocked(dex_file);
-  ObjPtr<mirror::DexCache> dex_cache = DecodeDexCache(self, dex_cache_data);
+  ObjPtr<mirror::DexCache> dex_cache = DecodeDexCache(self, FindDexCacheDataLocked(dex_file));
   if (dex_cache != nullptr) {
     return dex_cache;
   }
@@ -3705,8 +3704,7 @@ 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()
-             << " " << &dex_file << " " << dex_cache_data.dex_file;
+  LOG(FATAL) << "Failed to find DexCache for DexFile " << dex_file.GetLocation();
   UNREACHABLE();
 }
 
@@ -4282,7 +4280,13 @@ verifier::FailureKind ClassLinker::VerifyClass(
   std::string error_msg;
   verifier::FailureKind verifier_failure = verifier::FailureKind::kNoFailure;
   if (!preverified) {
-    verifier_failure = PerformClassVerification(self, klass, log_level, &error_msg);
+    Runtime* runtime = Runtime::Current();
+    verifier_failure = verifier::MethodVerifier::VerifyClass(self,
+                                                             klass.Get(),
+                                                             runtime->GetCompilerCallbacks(),
+                                                             runtime->IsAotCompiler(),
+                                                             log_level,
+                                                             &error_msg);
   }
 
   // Verification is done, grab the lock again.
@@ -4350,19 +4354,6 @@ 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 2fbbe79e47..bf14aebb52 100644
--- a/runtime/class_linker.h
+++ b/runtime/class_linker.h
@@ -710,12 +710,6 @@ 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 c51bb5e176..806653a265 100644
--- a/runtime/compiler_callbacks.h
+++ b/runtime/compiler_callbacks.h
@@ -22,8 +22,6 @@
 
 namespace art {
 
-class CompilerDriver;
-
 namespace verifier {
 
 class MethodVerifier;
@@ -51,13 +49,6 @@ 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;
   }