Add support for registering classpath classes status.
By doing class unloading after each dex file compilation, we are loosing
away verification done on classpath classes.
This change introduces a new table for keeping around class status of
classpath classes.
Multidex quickening compilation improved by ~5% by not re-verifying classpath
classes.
Bug: 63467744
test: test.py
test: golem successfully compiles FB
Change-Id: I629c0a7d86519bbc516f5e59f7cd92ca6ca842eb
diff --git a/compiler/dex/quick_compiler_callbacks.cc b/compiler/dex/quick_compiler_callbacks.cc
index 23511e5..92b1230 100644
--- a/compiler/dex/quick_compiler_callbacks.cc
+++ b/compiler/dex/quick_compiler_callbacks.cc
@@ -44,11 +44,14 @@
// 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.
- ClassStatus status;
- if (!compiler_driver_->GetCompiledClass(ref, &status)) {
- return ClassStatus::kStatusNotReady;
+ return compiler_driver_->GetClassStatus(ref);
+}
+
+void QuickCompilerCallbacks::UpdateClassState(ClassReference ref, ClassStatus status) {
+ // Driver is null when bootstrapping the runtime.
+ if (compiler_driver_ != nullptr) {
+ compiler_driver_->RecordClassStatus(ref, status);
}
- return status;
}
} // namespace art
diff --git a/compiler/dex/quick_compiler_callbacks.h b/compiler/dex/quick_compiler_callbacks.h
index 45456f2..6d22f95 100644
--- a/compiler/dex/quick_compiler_callbacks.h
+++ b/compiler/dex/quick_compiler_callbacks.h
@@ -26,48 +26,50 @@
class VerificationResults;
class QuickCompilerCallbacks FINAL : public CompilerCallbacks {
- public:
- explicit QuickCompilerCallbacks(CompilerCallbacks::CallbackMode mode)
- : CompilerCallbacks(mode) {}
+ public:
+ explicit QuickCompilerCallbacks(CompilerCallbacks::CallbackMode mode)
+ : CompilerCallbacks(mode) {}
- ~QuickCompilerCallbacks() { }
+ ~QuickCompilerCallbacks() { }
- void MethodVerified(verifier::MethodVerifier* verifier)
- REQUIRES_SHARED(Locks::mutator_lock_) OVERRIDE;
+ void MethodVerified(verifier::MethodVerifier* verifier)
+ REQUIRES_SHARED(Locks::mutator_lock_) OVERRIDE;
- void ClassRejected(ClassReference ref) OVERRIDE;
+ void ClassRejected(ClassReference ref) OVERRIDE;
- // We are running in an environment where we can call patchoat safely so we should.
- bool IsRelocationPossible() OVERRIDE {
- return true;
- }
+ // We are running in an environment where we can call patchoat safely so we should.
+ bool IsRelocationPossible() OVERRIDE {
+ return true;
+ }
- verifier::VerifierDeps* GetVerifierDeps() const OVERRIDE {
- return verifier_deps_.get();
- }
+ verifier::VerifierDeps* GetVerifierDeps() const OVERRIDE {
+ return verifier_deps_.get();
+ }
- void SetVerifierDeps(verifier::VerifierDeps* deps) OVERRIDE {
- verifier_deps_.reset(deps);
- }
+ void SetVerifierDeps(verifier::VerifierDeps* deps) OVERRIDE {
+ verifier_deps_.reset(deps);
+ }
- void SetVerificationResults(VerificationResults* verification_results) {
- verification_results_ = verification_results;
- }
+ void SetVerificationResults(VerificationResults* verification_results) {
+ verification_results_ = verification_results;
+ }
- ClassStatus GetPreviousClassState(ClassReference ref) OVERRIDE;
+ ClassStatus GetPreviousClassState(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);
- }
+ 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_;
+ void UpdateClassState(ClassReference ref, ClassStatus state) OVERRIDE;
+
+ private:
+ VerificationResults* verification_results_ = nullptr;
+ bool does_class_unloading_ = false;
+ CompilerDriver* compiler_driver_ = nullptr;
+ std::unique_ptr<verifier::VerifierDeps> verifier_deps_;
};
} // namespace art
diff --git a/compiler/driver/compiler_driver.cc b/compiler/driver/compiler_driver.cc
index 18b54ee..b9c0314 100644
--- a/compiler/driver/compiler_driver.cc
+++ b/compiler/driver/compiler_driver.cc
@@ -2900,6 +2900,14 @@
return true;
}
+mirror::Class::Status CompilerDriver::GetClassStatus(ClassReference ref) const {
+ mirror::Class::Status status = ClassStatus::kStatusNotReady;
+ if (!GetCompiledClass(ref, &status)) {
+ classpath_classes_.Get(DexFileReference(ref.first, ref.second), &status);
+ }
+ return status;
+}
+
void CompilerDriver::RecordClassStatus(ClassReference ref, mirror::Class::Status status) {
switch (status) {
case mirror::Class::kStatusErrorResolved:
@@ -2918,11 +2926,12 @@
}
ClassStateTable::InsertResult result;
+ ClassStateTable* table = &compiled_classes_;
do {
DexFileReference dex_ref(ref.first, ref.second);
mirror::Class::Status existing = mirror::Class::kStatusNotReady;
- if (!compiled_classes_.Get(dex_ref, &existing)) {
- // Probably a uses library class, bail.
+ if (!table->Get(dex_ref, &existing)) {
+ // A classpath class.
if (kIsDebugBuild) {
// Check to make sure it's not a dex file for an oat file we are compiling since these
// should always succeed. These do not include classes in for used libraries.
@@ -2930,7 +2939,12 @@
CHECK_NE(dex_ref.dex_file, dex_file) << dex_ref.dex_file->GetLocation();
}
}
- return;
+ if (!classpath_classes_.HaveDexFile(ref.first)) {
+ // Boot classpath dex file.
+ return;
+ }
+ table = &classpath_classes_;
+ table->Get(dex_ref, &existing);
}
if (existing >= status) {
// Existing status is already better than we expect, break.
@@ -2938,8 +2952,8 @@
}
// Update the status if we now have a greater one. This happens with vdex,
// which records a class is verified, but does not resolve it.
- result = compiled_classes_.Insert(dex_ref, existing, status);
- CHECK(result != ClassStateTable::kInsertResultInvalidDexFile);
+ result = table->Insert(dex_ref, existing, status);
+ CHECK(result != ClassStateTable::kInsertResultInvalidDexFile) << ref.first->GetLocation();
} while (result != ClassStateTable::kInsertResultSuccess);
}
@@ -3046,11 +3060,11 @@
void CompilerDriver::SetDexFilesForOatFile(const std::vector<const DexFile*>& dex_files) {
dex_files_for_oat_file_ = dex_files;
- for (const DexFile* dex_file : dex_files) {
- if (!compiled_classes_.HaveDexFile(dex_file)) {
- compiled_classes_.AddDexFile(dex_file, dex_file->NumClassDefs());
- }
- }
+ compiled_classes_.AddDexFiles(dex_files);
+}
+
+void CompilerDriver::SetClasspathDexFiles(const std::vector<const DexFile*>& dex_files) {
+ classpath_classes_.AddDexFiles(dex_files);
}
} // namespace art
diff --git a/compiler/driver/compiler_driver.h b/compiler/driver/compiler_driver.h
index d08d9d7..19fae69 100644
--- a/compiler/driver/compiler_driver.h
+++ b/compiler/driver/compiler_driver.h
@@ -106,6 +106,7 @@
// Set dex files that will be stored in the oat file after being compiled.
void SetDexFilesForOatFile(const std::vector<const DexFile*>& dex_files);
+ void SetClasspathDexFiles(const std::vector<const DexFile*>& dex_files);
// Get dex file that will be stored in the oat file after being compiled.
ArrayRef<const DexFile* const> GetDexFilesForOatFile() const {
@@ -152,6 +153,7 @@
std::unique_ptr<const std::vector<uint8_t>> CreateQuickToInterpreterBridge() const;
bool GetCompiledClass(ClassReference ref, mirror::Class::Status* status) const;
+ mirror::Class::Status GetClassStatus(ClassReference ref) const;
CompiledMethod* GetCompiledMethod(MethodReference ref) const;
size_t GetNonRelativeLinkerPatchCount() const;
@@ -488,6 +490,7 @@
// All class references that this compiler has compiled. Indexed by class defs.
using ClassStateTable = AtomicDexRefMap<mirror::Class::Status>;
ClassStateTable compiled_classes_;
+ ClassStateTable classpath_classes_;
typedef AtomicDexRefMap<CompiledMethod*> MethodTable;
diff --git a/compiler/utils/atomic_dex_ref_map-inl.h b/compiler/utils/atomic_dex_ref_map-inl.h
index c41d8fc..14f1f0b 100644
--- a/compiler/utils/atomic_dex_ref_map-inl.h
+++ b/compiler/utils/atomic_dex_ref_map-inl.h
@@ -54,6 +54,15 @@
}
template <typename T>
+inline void AtomicDexRefMap<T>::AddDexFiles(const std::vector<const DexFile*>& dex_files) {
+ for (const DexFile* dex_file : dex_files) {
+ if (!HaveDexFile(dex_file)) {
+ AddDexFile(dex_file, dex_file->NumClassDefs());
+ }
+ }
+}
+
+template <typename T>
inline typename AtomicDexRefMap<T>::ElementArray* AtomicDexRefMap<T>::GetArray(
const DexFile* dex_file) {
auto it = arrays_.find(dex_file);
diff --git a/compiler/utils/atomic_dex_ref_map.h b/compiler/utils/atomic_dex_ref_map.h
index 2da4ffa..b02c9b6 100644
--- a/compiler/utils/atomic_dex_ref_map.h
+++ b/compiler/utils/atomic_dex_ref_map.h
@@ -46,6 +46,7 @@
// Dex files must be added before method references belonging to them can be used as keys. Not
// thread safe.
void AddDexFile(const DexFile* dex_file, size_t max_index);
+ void AddDexFiles(const std::vector<const DexFile*>& dex_files);
bool HaveDexFile(const DexFile* dex_file) const {
return arrays_.find(dex_file) != arrays_.end();
diff --git a/dex2oat/dex2oat.cc b/dex2oat/dex2oat.cc
index 7ba165e..4ecb043 100644
--- a/dex2oat/dex2oat.cc
+++ b/dex2oat/dex2oat.cc
@@ -1865,6 +1865,9 @@
swap_fd_,
profile_compilation_info_.get()));
driver_->SetDexFilesForOatFile(dex_files_);
+ if (!IsBootImage()) {
+ driver_->SetClasspathDexFiles(class_loader_context_->FlattenOpenedDexFiles());
+ }
const bool compile_individually = ShouldCompileDexFilesIndividually();
if (compile_individually) {
@@ -2363,27 +2366,6 @@
return dex_files_size >= very_large_threshold_;
}
- std::vector<std::string> GetClassPathLocations(const std::string& class_path) {
- // This function is used only for apps and for an app we have exactly one oat file.
- DCHECK(!IsBootImage());
- DCHECK_EQ(oat_writers_.size(), 1u);
- std::vector<std::string> dex_files_canonical_locations;
- for (const std::string& location : oat_writers_[0]->GetSourceLocations()) {
- dex_files_canonical_locations.push_back(DexFile::GetDexCanonicalLocation(location.c_str()));
- }
-
- std::vector<std::string> parsed;
- Split(class_path, ':', &parsed);
- auto kept_it = std::remove_if(parsed.begin(),
- parsed.end(),
- [dex_files_canonical_locations](const std::string& location) {
- return ContainsElement(dex_files_canonical_locations,
- DexFile::GetDexCanonicalLocation(location.c_str()));
- });
- parsed.erase(kept_it, parsed.end());
- return parsed;
- }
-
bool PrepareImageClasses() {
// If --image-classes was specified, calculate the full list of classes to include in the image.
if (image_classes_filename_ != nullptr) {
diff --git a/runtime/class_linker.cc b/runtime/class_linker.cc
index f95b493..c48fcca 100644
--- a/runtime/class_linker.cc
+++ b/runtime/class_linker.cc
@@ -4309,6 +4309,12 @@
EnsureSkipAccessChecksMethods(klass, image_pointer_size_);
}
}
+ // Done verifying. Notify the compiler about the verification status, in case the class
+ // was verified implicitly (eg super class of a compiled class).
+ if (Runtime::Current()->IsAotCompiler()) {
+ Runtime::Current()->GetCompilerCallbacks()->UpdateClassState(
+ ClassReference(&klass->GetDexFile(), klass->GetDexClassDefIndex()), klass->GetStatus());
+ }
return verifier_failure;
}
diff --git a/runtime/compiler_callbacks.h b/runtime/compiler_callbacks.h
index 9b22714..9041df9 100644
--- a/runtime/compiler_callbacks.h
+++ b/runtime/compiler_callbacks.h
@@ -65,6 +65,9 @@
return mode_ == CallbackMode::kCompileBootImage;
}
+ virtual void UpdateClassState(ClassReference ref ATTRIBUTE_UNUSED,
+ ClassStatus state ATTRIBUTE_UNUSED) {}
+
protected:
explicit CompilerCallbacks(CallbackMode mode) : mode_(mode) { }