Make sure that const-class linkage is preserved, try again.
This CL causes occasional test failures on the build servers
which we were not able to reproduce locally. So we add some
some additional debug output to help pinpoint the cause.
Bug: 30627598
Bug: 33231647
Test: m test-art-host
This reverts commit 171cf811a1cdf8b1cbc5151505d8630741ce4cf3.
Change-Id: Id56a3f0e86e8212fd547e09c61794401bff47fb0
diff --git a/compiler/image_writer.cc b/compiler/image_writer.cc
index a706697..b8f725e 100644
--- a/compiler/image_writer.cc
+++ b/compiler/image_writer.cc
@@ -838,19 +838,64 @@
return true;
}
-class ImageWriter::NonImageClassesVisitor : public ClassVisitor {
+class ImageWriter::PruneClassesVisitor : public ClassVisitor {
public:
- explicit NonImageClassesVisitor(ImageWriter* image_writer) : image_writer_(image_writer) {}
+ PruneClassesVisitor(ImageWriter* image_writer, ObjPtr<mirror::ClassLoader> class_loader)
+ : image_writer_(image_writer),
+ class_loader_(class_loader),
+ classes_to_prune_(),
+ defined_class_count_(0u) { }
- bool operator()(ObjPtr<Class> klass) OVERRIDE REQUIRES_SHARED(Locks::mutator_lock_) {
+ bool operator()(ObjPtr<mirror::Class> klass) OVERRIDE REQUIRES_SHARED(Locks::mutator_lock_) {
if (!image_writer_->KeepClass(klass.Ptr())) {
classes_to_prune_.insert(klass.Ptr());
+ if (klass->GetClassLoader() == class_loader_) {
+ ++defined_class_count_;
+ }
}
return true;
}
- std::unordered_set<mirror::Class*> classes_to_prune_;
+ size_t Prune() REQUIRES_SHARED(Locks::mutator_lock_) {
+ ClassTable* class_table =
+ Runtime::Current()->GetClassLinker()->ClassTableForClassLoader(class_loader_);
+ for (mirror::Class* klass : classes_to_prune_) {
+ std::string storage;
+ const char* descriptor = klass->GetDescriptor(&storage);
+ bool result = class_table->Remove(descriptor);
+ DCHECK(result);
+ }
+ return defined_class_count_;
+ }
+
+ private:
ImageWriter* const image_writer_;
+ const ObjPtr<mirror::ClassLoader> class_loader_;
+ std::unordered_set<mirror::Class*> classes_to_prune_;
+ size_t defined_class_count_;
+};
+
+class ImageWriter::PruneClassLoaderClassesVisitor : public ClassLoaderVisitor {
+ public:
+ explicit PruneClassLoaderClassesVisitor(ImageWriter* image_writer)
+ : image_writer_(image_writer), removed_class_count_(0) {}
+
+ virtual void Visit(ObjPtr<mirror::ClassLoader> class_loader) OVERRIDE
+ REQUIRES_SHARED(Locks::mutator_lock_) {
+ PruneClassesVisitor classes_visitor(image_writer_, class_loader);
+ ClassTable* class_table =
+ Runtime::Current()->GetClassLinker()->ClassTableForClassLoader(class_loader);
+ class_table->Visit(classes_visitor);
+ removed_class_count_ += classes_visitor.Prune();
+ }
+
+ size_t GetRemovedClassCount() const {
+ return removed_class_count_;
+ }
+
+ private:
+ ImageWriter* const image_writer_;
+ size_t removed_class_count_;
};
void ImageWriter::PruneNonImageClasses() {
@@ -862,21 +907,13 @@
// path dex caches.
class_linker->ClearClassTableStrongRoots();
- // Make a list of classes we would like to prune.
- NonImageClassesVisitor visitor(this);
- class_linker->VisitClasses(&visitor);
-
// Remove the undesired classes from the class roots.
- VLOG(compiler) << "Pruning " << visitor.classes_to_prune_.size() << " classes";
- for (mirror::Class* klass : visitor.classes_to_prune_) {
- std::string temp;
- const char* name = klass->GetDescriptor(&temp);
- VLOG(compiler) << "Pruning class " << name;
- if (!compile_app_image_) {
- DCHECK(IsBootClassLoaderClass(klass));
- }
- bool result = class_linker->RemoveClass(name, klass->GetClassLoader());
- DCHECK(result);
+ {
+ ReaderMutexLock mu(self, *Locks::classlinker_classes_lock_);
+ PruneClassLoaderClassesVisitor class_loader_visitor(this);
+ class_loader_visitor.Visit(nullptr); // Visit boot class loader.
+ class_linker->VisitClassLoaders(&class_loader_visitor);
+ VLOG(compiler) << "Pruned " << class_loader_visitor.GetRemovedClassCount() << " classes";
}
// Clear references to removed classes from the DexCaches.
@@ -932,7 +969,8 @@
class_linker->DropFindArrayClassCache();
// Clear to save RAM.
- prune_class_memo_.clear();
+ // FIXME: This has been temporarily removed to provide extra debugging output. Bug 33231647.
+ // prune_class_memo_.clear();
}
void ImageWriter::CheckNonImageClassesRemoved() {
@@ -1104,7 +1142,40 @@
DCHECK_NE(as_klass->GetStatus(), mirror::Class::kStatusError);
if (compile_app_image_) {
// Extra sanity, no boot loader classes should be left!
- CHECK(!IsBootClassLoaderClass(as_klass)) << as_klass->PrettyClass();
+ // FIXME: Remove the extra logging. Bug 33231647.
+ struct Dumper {
+ std::string ImageRanges() const {
+ std::ostringstream oss;
+ const char* separator = "";
+ gc::Heap* const heap = Runtime::Current()->GetHeap();
+ for (gc::space::ImageSpace* boot_image_space : heap->GetBootImageSpaces()) {
+ const uint8_t* image_begin = boot_image_space->Begin();
+ // Real image end including ArtMethods and ArtField sections.
+ const uint8_t* image_end =
+ image_begin + boot_image_space->GetImageHeader().GetImageSize();
+ oss << separator << static_cast<const void*>(image_begin)
+ << "-" << static_cast<const void*>(image_end);
+ separator = ":";
+ }
+ return oss.str();
+ }
+ std::string PruneMemo(ImageWriter* writer,
+ mirror::Class* klass,
+ const std::unordered_map<mirror::Class*, bool>& map) const
+ REQUIRES_SHARED(Locks::mutator_lock_) {
+ auto it = map.find(klass);
+ if (it == map.end()) {
+ return writer->PruneAppImageClass(klass) ? "missing/true" : "missing/false";
+ } else {
+ return it->second ? "true" : "false";
+ }
+ }
+ };
+ CHECK(!IsBootClassLoaderClass(as_klass)) << as_klass->PrettyClass()
+ << " status:" << as_klass->GetStatus()
+ << " " << static_cast<const void*>(as_klass)
+ << " " << Dumper().ImageRanges()
+ << " prune_memo:" << Dumper().PruneMemo(this, as_klass, prune_class_memo_);
}
LengthPrefixedArray<ArtField>* fields[] = {
as_klass->GetSFieldsPtr(), as_klass->GetIFieldsPtr(),
@@ -1507,8 +1578,10 @@
}
// Calculate the size of the class table.
ReaderMutexLock mu(self, *Locks::classlinker_classes_lock_);
- DCHECK_EQ(image_info.class_table_->NumZygoteClasses(), 0u);
- if (image_info.class_table_->NumNonZygoteClasses() != 0u) {
+ CHECK_EQ(class_loaders_.size(), compile_app_image_ ? 1u : 0u);
+ mirror::ClassLoader* class_loader = compile_app_image_ ? *class_loaders_.begin() : nullptr;
+ DCHECK_EQ(image_info.class_table_->NumZygoteClasses(class_loader), 0u);
+ if (image_info.class_table_->NumNonZygoteClasses(class_loader) != 0u) {
image_info.class_table_bytes_ += image_info.class_table_->WriteToMemory(nullptr);
}
}
@@ -1853,8 +1926,10 @@
// above comment for intern tables.
ClassTable temp_class_table;
temp_class_table.ReadFromMemory(class_table_memory_ptr);
- CHECK_EQ(temp_class_table.NumZygoteClasses(), table->NumNonZygoteClasses() +
- table->NumZygoteClasses());
+ CHECK_EQ(class_loaders_.size(), compile_app_image_ ? 1u : 0u);
+ mirror::ClassLoader* class_loader = compile_app_image_ ? *class_loaders_.begin() : nullptr;
+ CHECK_EQ(temp_class_table.NumZygoteClasses(class_loader),
+ table->NumNonZygoteClasses(class_loader) + table->NumZygoteClasses(class_loader));
BufferedRootVisitor<kDefaultBufferedRootCount> buffered_visitor(&root_visitor,
RootInfo(kRootUnknown));
temp_class_table.VisitRoots(buffered_visitor);