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
diff --git a/compiler/dex/quick_compiler_callbacks.cc b/compiler/dex/quick_compiler_callbacks.cc
index c7e9f4f..872f7ea 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 @@
}
}
-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 578aff4..a3a6c09 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 @@
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 037e458..6e087c5 100644
--- a/compiler/driver/compiler_driver.cc
+++ b/compiler/driver/compiler_driver.cc
@@ -3040,10 +3040,4 @@
}
}
-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 ba4581c..d9886a2 100644
--- a/compiler/driver/compiler_driver.h
+++ b/compiler/driver/compiler_driver.h
@@ -377,8 +377,6 @@
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 392d57c..4da3e0d 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 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 @@
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 @@
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 f50e88f..ff193e9 100644
--- a/dex2oat/dex2oat.cc
+++ b/dex2oat/dex2oat.cc
@@ -1774,8 +1774,10 @@
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 @@
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 @@
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 @@
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 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 1673fc1..12bceb3 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 @@
}
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 @@
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 @@
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 @@
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 @@
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 @@
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 9396565..b1bc3f8 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 @@
}
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 927b533..11bea86 100644
--- a/runtime/aot_class_linker.h
+++ b/runtime/aot_class_linker.h
@@ -27,16 +27,6 @@
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 ed673d4..3ac87c5 100644
--- a/runtime/class_linker.cc
+++ b/runtime/class_linker.cc
@@ -3694,8 +3694,7 @@
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 @@
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 @@
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 @@
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 2fbbe79..bf14aeb 100644
--- a/runtime/class_linker.h
+++ b/runtime/class_linker.h
@@ -710,12 +710,6 @@
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 c51bb5e..806653a 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 @@
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;
}