From 4e9c4e746617bad6a012d799d2f5cf9e01d24ea2 Mon Sep 17 00:00:00 2001 From: Mathieu Chartier Date: Tue, 20 Sep 2016 15:33:31 -0700 Subject: [PATCH] Smarter image layout Put strings in the dex file that resolves them. Depth first traversal with overrides for class and dex cache. The work list keeps track of what oat_index with each pushed item. This means the static fields of a class will usually be in the same image. Added layout test to image_test to make sure things are somewhat reasonably attributed. Bug: 28640955 Test: test-art-host Change-Id: I67a536c33aeed603b252d8e0f75622c9efbf2559 --- build/Android.gtest.mk | 3 + compiler/common_compiler_test.cc | 1 + compiler/image_test.cc | 210 +++++++++++++++++------ compiler/image_writer.cc | 325 ++++++++++++++++++++++-------------- compiler/image_writer.h | 27 ++- runtime/class_linker.h | 1 + runtime/class_table.cc | 4 + runtime/class_table.h | 4 + runtime/common_runtime_test.h | 3 +- test/ImageLayoutA/ImageLayoutA.java | 21 +++ test/ImageLayoutB/ImageLayoutB.java | 25 +++ 11 files changed, 440 insertions(+), 184 deletions(-) create mode 100644 test/ImageLayoutA/ImageLayoutA.java create mode 100644 test/ImageLayoutB/ImageLayoutB.java diff --git a/build/Android.gtest.mk b/build/Android.gtest.mk index 011c487d5..19af14de1 100644 --- a/build/Android.gtest.mk +++ b/build/Android.gtest.mk @@ -27,6 +27,8 @@ GTEST_DEX_DIRECTORIES := \ AllFields \ ExceptionHandle \ GetMethodSignature \ + ImageLayoutA \ + ImageLayoutB \ Instrumentation \ Interfaces \ Lookup \ @@ -74,6 +76,7 @@ ART_GTEST_dex_cache_test_DEX_DEPS := Main Packages ART_GTEST_dex_file_test_DEX_DEPS := GetMethodSignature Main Nested ART_GTEST_dex2oat_test_DEX_DEPS := $(ART_GTEST_dex2oat_environment_tests_DEX_DEPS) ART_GTEST_exception_test_DEX_DEPS := ExceptionHandle +ART_GTEST_image_test_DEX_DEPS := ImageLayoutA ImageLayoutB ART_GTEST_instrumentation_test_DEX_DEPS := Instrumentation ART_GTEST_jni_compiler_test_DEX_DEPS := MyClassNatives ART_GTEST_jni_internal_test_DEX_DEPS := AllFields StaticLeafMethods diff --git a/compiler/common_compiler_test.cc b/compiler/common_compiler_test.cc index bf29e1c31..bcd8940b5 100644 --- a/compiler/common_compiler_test.cc +++ b/compiler/common_compiler_test.cc @@ -225,6 +225,7 @@ void CommonCompilerTest::TearDown() { method_inliner_map_.reset(); verification_results_.reset(); compiler_options_.reset(); + image_reservation_.reset(); CommonRuntimeTest::TearDown(); } diff --git a/compiler/image_test.cc b/compiler/image_test.cc index e8b772936..a68ab7cc9 100644 --- a/compiler/image_test.cc +++ b/compiler/image_test.cc @@ -39,43 +39,101 @@ namespace art { +static const uintptr_t kRequestedImageBase = ART_BASE_ADDRESS; + +struct CompilationHelper { + std::vector dex_file_locations; + std::vector image_locations; + std::vector> extra_dex_files; + std::vector image_files; + std::vector oat_files; + std::string image_dir; + + void Compile(CompilerDriver* driver, + ImageHeader::StorageMode storage_mode); + + std::vector GetImageObjectSectionSizes(); + + ~CompilationHelper(); +}; + class ImageTest : public CommonCompilerTest { protected: virtual void SetUp() { ReserveImageSpace(); CommonCompilerTest::SetUp(); } + void TestWriteRead(ImageHeader::StorageMode storage_mode); + + void Compile(ImageHeader::StorageMode storage_mode, + CompilationHelper& out_helper, + const std::string& extra_dex = "", + const std::string& image_class = ""); + + std::unordered_set* GetImageClasses() OVERRIDE { + return new std::unordered_set(image_classes_); + } + + private: + std::unordered_set image_classes_; }; -void ImageTest::TestWriteRead(ImageHeader::StorageMode storage_mode) { - CreateCompilerDriver(Compiler::kOptimizing, kRuntimeISA, kIsTargetBuild ? 2U : 16U); +CompilationHelper::~CompilationHelper() { + for (ScratchFile& image_file : image_files) { + image_file.Unlink(); + } + for (ScratchFile& oat_file : oat_files) { + oat_file.Unlink(); + } + const int rmdir_result = rmdir(image_dir.c_str()); + CHECK_EQ(0, rmdir_result); +} - // Set inline filter values. - compiler_options_->SetInlineDepthLimit(CompilerOptions::kDefaultInlineDepthLimit); - compiler_options_->SetInlineMaxCodeUnits(CompilerOptions::kDefaultInlineMaxCodeUnits); +std::vector CompilationHelper::GetImageObjectSectionSizes() { + std::vector ret; + for (ScratchFile& image_file : image_files) { + std::unique_ptr file(OS::OpenFileForReading(image_file.GetFilename().c_str())); + CHECK(file.get() != nullptr); + ImageHeader image_header; + CHECK_EQ(file->ReadFully(&image_header, sizeof(image_header)), true); + CHECK(image_header.IsValid()); + ret.push_back(image_header.GetImageSize()); + } + return ret; +} +void CompilationHelper::Compile(CompilerDriver* driver, + ImageHeader::StorageMode storage_mode) { ClassLinker* class_linker = Runtime::Current()->GetClassLinker(); - const std::vector& boot_class_path = class_linker->GetBootClassPath(); - const size_t num_images = boot_class_path.size(); + std::vector class_path = class_linker->GetBootClassPath(); + + for (const std::unique_ptr& dex_file : extra_dex_files) { + { + ScopedObjectAccess soa(Thread::Current()); + // Inject in boot class path so that the compiler driver can see it. + class_linker->AppendToBootClassPath(soa.Self(), *dex_file.get()); + } + class_path.push_back(dex_file.get()); + } // Enable write for dex2dex. - for (const DexFile* dex_file : boot_class_path) { - dex_file->EnableWrite(); + for (const DexFile* dex_file : class_path) { + dex_file_locations.push_back(dex_file->GetLocation()); + if (dex_file->IsReadOnly()) { + dex_file->EnableWrite(); + } } - // Create a generic location tmp file, to be the base of the .art and .oat temporary files. - std::vector image_locations; { + // Create a generic tmp file, to be the base of the .art and .oat temporary files. ScratchFile location; - for (int i = 0; i < static_cast(num_images); ++i) { + for (int i = 0; i < static_cast(class_path.size()); ++i) { std::string cur_location(StringPrintf("%s-%d.art", location.GetFilename().c_str(), i)); image_locations.push_back(ScratchFile(cur_location)); } } std::vector image_filenames; - std::vector image_files; - std::string image_dir; for (ScratchFile& file : image_locations) { std::string image_filename(GetSystemImageFilename(file.GetFilename().c_str(), kRuntimeISA)); image_filenames.push_back(image_filename); @@ -90,14 +148,12 @@ void ImageTest::TestWriteRead(ImageHeader::StorageMode storage_mode) { } std::vector oat_filenames; - std::vector oat_files; for (const std::string& image_filename : image_filenames) { std::string oat_filename(image_filename.substr(0, image_filename.size() - strlen("art")) + "oat"); oat_files.push_back(ScratchFile(OS::CreateEmptyFile(oat_filename.c_str()))); oat_filenames.push_back(oat_filename); } - const uintptr_t requested_image_base = ART_BASE_ADDRESS; std::unordered_map dex_file_to_oat_index_map; std::vector oat_filename_vector; for (const std::string& file : oat_filenames) { @@ -108,13 +164,13 @@ void ImageTest::TestWriteRead(ImageHeader::StorageMode storage_mode) { image_filename_vector.push_back(file.c_str()); } size_t image_idx = 0; - for (const DexFile* dex_file : boot_class_path) { + for (const DexFile* dex_file : class_path) { dex_file_to_oat_index_map.emplace(dex_file, image_idx); ++image_idx; } // TODO: compile_pic should be a test argument. - std::unique_ptr writer(new ImageWriter(*compiler_driver_, - requested_image_base, + std::unique_ptr writer(new ImageWriter(*driver, + kRequestedImageBase, /*compile_pic*/false, /*compile_app_image*/false, storage_mode, @@ -125,13 +181,13 @@ void ImageTest::TestWriteRead(ImageHeader::StorageMode storage_mode) { jobject class_loader = nullptr; TimingLogger timings("ImageTest::WriteRead", false, false); TimingLogger::ScopedTiming t("CompileAll", &timings); - compiler_driver_->SetDexFilesForOatFile(class_linker->GetBootClassPath()); - compiler_driver_->CompileAll(class_loader, class_linker->GetBootClassPath(), &timings); + driver->SetDexFilesForOatFile(class_path); + driver->CompileAll(class_loader, class_path, &timings); t.NewTiming("WriteElf"); SafeMap key_value_store; std::vector dex_filename_vector; - for (size_t i = 0; i < boot_class_path.size(); ++i) { + for (size_t i = 0; i < class_path.size(); ++i) { dex_filename_vector.push_back(""); } key_value_store.Put(OatHeader::kBootClassPathKey, @@ -140,14 +196,13 @@ void ImageTest::TestWriteRead(ImageHeader::StorageMode storage_mode) { oat_filename_vector, image_filename_vector)); - const std::vector& dex_files = class_linker->GetBootClassPath(); std::vector> elf_writers; std::vector> oat_writers; for (ScratchFile& oat_file : oat_files) { - elf_writers.emplace_back(CreateElfWriterQuick(compiler_driver_->GetInstructionSet(), - compiler_driver_->GetInstructionSetFeatures(), - &compiler_driver_->GetCompilerOptions(), - oat_file.GetFile())); + elf_writers.emplace_back(CreateElfWriterQuick(driver->GetInstructionSet(), + driver->GetInstructionSetFeatures(), + &driver->GetCompilerOptions(), + oat_file.GetFile())); elf_writers.back()->Start(); oat_writers.emplace_back(new OatWriter(/*compiling_boot_image*/true, &timings)); } @@ -157,7 +212,7 @@ void ImageTest::TestWriteRead(ImageHeader::StorageMode storage_mode) { std::vector> opened_dex_files; // Now that we have finalized key_value_store_, start writing the oat file. for (size_t i = 0, size = oat_writers.size(); i != size; ++i) { - const DexFile* dex_file = dex_files[i]; + const DexFile* dex_file = class_path[i]; rodata.push_back(elf_writers[i]->StartRoData()); ArrayRef raw_dex_file( reinterpret_cast(&dex_file->GetHeader()), @@ -171,8 +226,8 @@ void ImageTest::TestWriteRead(ImageHeader::StorageMode storage_mode) { bool dex_files_ok = oat_writers[i]->WriteAndOpenDexFiles( rodata.back(), oat_files[i].GetFile(), - compiler_driver_->GetInstructionSet(), - compiler_driver_->GetInstructionSetFeatures(), + driver->GetInstructionSet(), + driver->GetInstructionSetFeatures(), &key_value_store, /* verify */ false, // Dex files may be dex-to-dex-ed, don't verify. &cur_opened_dex_files_map, @@ -194,12 +249,12 @@ void ImageTest::TestWriteRead(ImageHeader::StorageMode storage_mode) { ASSERT_TRUE(image_space_ok); for (size_t i = 0, size = oat_files.size(); i != size; ++i) { - linker::MultiOatRelativePatcher patcher(compiler_driver_->GetInstructionSet(), - instruction_set_features_.get()); + linker::MultiOatRelativePatcher patcher(driver->GetInstructionSet(), + driver->GetInstructionSetFeatures()); OatWriter* const oat_writer = oat_writers[i].get(); ElfWriter* const elf_writer = elf_writers[i].get(); - std::vector cur_dex_files(1u, dex_files[i]); - oat_writer->PrepareLayout(compiler_driver_.get(), writer.get(), cur_dex_files, &patcher); + std::vector cur_dex_files(1u, class_path[i]); + oat_writer->PrepareLayout(driver, writer.get(), cur_dex_files, &patcher); size_t rodata_size = oat_writer->GetOatHeader().GetExecutableOffset(); size_t text_size = oat_writer->GetSize() - rodata_size; elf_writer->SetLoadedSectionSizes(rodata_size, text_size, oat_writer->GetBssSize()); @@ -231,9 +286,7 @@ void ImageTest::TestWriteRead(ImageHeader::StorageMode storage_mode) { ASSERT_TRUE(success); } } - } - { bool success_image = writer->Write(kInvalidFd, image_filename_vector, oat_filename_vector); @@ -250,9 +303,39 @@ void ImageTest::TestWriteRead(ImageHeader::StorageMode storage_mode) { << oat_filename; } } +} + +void ImageTest::Compile(ImageHeader::StorageMode storage_mode, + CompilationHelper& helper, + const std::string& extra_dex, + const std::string& image_class) { + if (!image_class.empty()) { + image_classes_.insert(image_class); + } + CreateCompilerDriver(Compiler::kOptimizing, kRuntimeISA, kIsTargetBuild ? 2U : 16U); + // Set inline filter values. + compiler_options_->SetInlineDepthLimit(CompilerOptions::kDefaultInlineDepthLimit); + compiler_options_->SetInlineMaxCodeUnits(CompilerOptions::kDefaultInlineMaxCodeUnits); + image_classes_.clear(); + if (!extra_dex.empty()) { + helper.extra_dex_files = OpenTestDexFiles(extra_dex.c_str()); + } + helper.Compile(compiler_driver_.get(), storage_mode); + if (!image_class.empty()) { + // Make sure the class got initialized. + ScopedObjectAccess soa(Thread::Current()); + ClassLinker* const class_linker = Runtime::Current()->GetClassLinker(); + mirror::Class* klass = class_linker->FindSystemClass(Thread::Current(), image_class.c_str()); + EXPECT_TRUE(klass != nullptr); + EXPECT_TRUE(klass->IsInitialized()); + } +} +void ImageTest::TestWriteRead(ImageHeader::StorageMode storage_mode) { + CompilationHelper helper; + Compile(storage_mode, /*out*/ helper); std::vector image_file_sizes; - for (ScratchFile& image_file : image_files) { + for (ScratchFile& image_file : helper.image_files) { std::unique_ptr file(OS::OpenFileForReading(image_file.GetFilename().c_str())); ASSERT_TRUE(file.get() != nullptr); ImageHeader image_header; @@ -283,8 +366,8 @@ void ImageTest::TestWriteRead(ImageHeader::StorageMode storage_mode) { // Remove the reservation of the memory for use to load the image. // Need to do this before we reset the runtime. UnreserveImageSpace(); - writer.reset(nullptr); + helper.extra_dex_files.clear(); runtime_.reset(); java_lang_dex_file_ = nullptr; @@ -292,7 +375,7 @@ void ImageTest::TestWriteRead(ImageHeader::StorageMode storage_mode) { RuntimeOptions options; std::string image("-Ximage:"); - image.append(image_locations[0].GetFilename()); + image.append(helper.image_locations[0].GetFilename()); options.push_back(std::make_pair(image.c_str(), static_cast(nullptr))); // By default the compiler this creates will not include patch information. options.push_back(std::make_pair("-Xnorelocate", nullptr)); @@ -315,17 +398,18 @@ void ImageTest::TestWriteRead(ImageHeader::StorageMode storage_mode) { // We loaded the runtime with an explicit image, so it must exist. ASSERT_EQ(heap->GetBootImageSpaces().size(), image_file_sizes.size()); - for (size_t i = 0; i < image_file_sizes.size(); ++i) { + for (size_t i = 0; i < helper.dex_file_locations.size(); ++i) { std::unique_ptr dex( - LoadExpectSingleDexFile(GetLibCoreDexFileNames()[i].c_str())); + LoadExpectSingleDexFile(helper.dex_file_locations[i].c_str())); + ASSERT_TRUE(dex != nullptr); uint64_t image_file_size = image_file_sizes[i]; gc::space::ImageSpace* image_space = heap->GetBootImageSpaces()[i]; ASSERT_TRUE(image_space != nullptr); if (storage_mode == ImageHeader::kStorageModeUncompressed) { // Uncompressed, image should be smaller than file. ASSERT_LE(image_space->GetImageHeader().GetImageSize(), image_file_size); - } else { - // Compressed, file should be smaller than image. + } else if (image_file_size > 16 * KB) { + // Compressed, file should be smaller than image. Not really valid for small images. ASSERT_LE(image_file_size, image_space->GetImageHeader().GetImageSize()); } @@ -334,7 +418,7 @@ void ImageTest::TestWriteRead(ImageHeader::StorageMode storage_mode) { uint8_t* image_end = image_space->End(); if (i == 0) { // This check is only valid for image 0. - CHECK_EQ(requested_image_base, reinterpret_cast(image_begin)); + CHECK_EQ(kRequestedImageBase, reinterpret_cast(image_begin)); } for (size_t j = 0; j < dex->NumClassDefs(); ++j) { const DexFile::ClassDef& class_def = dex->GetClassDef(j); @@ -352,15 +436,6 @@ void ImageTest::TestWriteRead(ImageHeader::StorageMode storage_mode) { EXPECT_TRUE(Monitor::IsValidLockWord(klass->GetLockWord(false))); } } - - for (ScratchFile& image_file : image_files) { - image_file.Unlink(); - } - for (ScratchFile& oat_file : oat_files) { - oat_file.Unlink(); - } - int rmdir_result = rmdir(image_dir.c_str()); - CHECK_EQ(0, rmdir_result); } TEST_F(ImageTest, WriteReadUncompressed) { @@ -375,6 +450,35 @@ TEST_F(ImageTest, WriteReadLZ4HC) { TestWriteRead(ImageHeader::kStorageModeLZ4HC); } +TEST_F(ImageTest, TestImageLayout) { + std::vector image_sizes; + std::vector image_sizes_extra; + // Compile multi-image with ImageLayoutA being the last image. + { + CompilationHelper helper; + Compile(ImageHeader::kStorageModeUncompressed, helper, "ImageLayoutA", "LMyClass;"); + image_sizes = helper.GetImageObjectSectionSizes(); + } + TearDown(); + runtime_.reset(); + SetUp(); + // Compile multi-image with ImageLayoutB being the last image. + { + CompilationHelper helper; + Compile(ImageHeader::kStorageModeUncompressed, helper, "ImageLayoutB", "LMyClass;"); + image_sizes_extra = helper.GetImageObjectSectionSizes(); + } + // Make sure that the new stuff in the clinit in ImageLayoutB is in the last image and not in the + // first two images. + ASSERT_EQ(image_sizes.size(), image_sizes.size()); + // Sizes of the images should be the same. These sizes are for the whole image unrounded. + for (size_t i = 0; i < image_sizes.size() - 1; ++i) { + EXPECT_EQ(image_sizes[i], image_sizes_extra[i]); + } + // Last image should be larger since it has a hash map and a string. + EXPECT_LT(image_sizes.back(), image_sizes_extra.back()); +} + TEST_F(ImageTest, ImageHeaderIsValid) { uint32_t image_begin = ART_BASE_ADDRESS; uint32_t image_size_ = 16 * KB; diff --git a/compiler/image_writer.cc b/compiler/image_writer.cc index 063eb1171..61cf00942 100644 --- a/compiler/image_writer.cc +++ b/compiler/image_writer.cc @@ -387,7 +387,6 @@ void ImageWriter::SetImageBinSlot(mirror::Object* object, BinSlot bin_slot) { DCHECK(!IsImageBinSlotAssigned(object)); // Before we stomp over the lock word, save the hash code for later. - Monitor::Deflate(Thread::Current(), object);; LockWord lw(object->GetLockWord(false)); switch (lw.GetState()) { case LockWord::kFatLocked: { @@ -488,7 +487,7 @@ void ImageWriter::AddMethodPointerArray(mirror::PointerArray* arr) { pointer_arrays_.emplace(arr, kBinArtMethodClean); } -void ImageWriter::AssignImageBinSlot(mirror::Object* object) { +void ImageWriter::AssignImageBinSlot(mirror::Object* object, size_t oat_index) { DCHECK(object != nullptr); size_t object_size = object->SizeOf(); @@ -591,7 +590,10 @@ void ImageWriter::AssignImageBinSlot(mirror::Object* object) { // else bin = kBinRegular } - size_t oat_index = GetOatIndex(object); + // Assign the oat index too. + DCHECK(oat_index_map_.find(object) == oat_index_map_.end()); + oat_index_map_.emplace(object, oat_index); + ImageInfo& image_info = GetImageInfo(oat_index); size_t offset_delta = RoundUp(object_size, kObjectAlignment); // 64-bit alignment @@ -972,39 +974,6 @@ mirror::String* ImageWriter::FindInternedString(mirror::String* string) { return nullptr; } -void ImageWriter::CalculateObjectBinSlots(Object* obj) { - DCHECK(obj != nullptr); - // if it is a string, we want to intern it if its not interned. - if (obj->GetClass()->IsStringClass()) { - size_t oat_index = GetOatIndex(obj); - ImageInfo& image_info = GetImageInfo(oat_index); - - // we must be an interned string that was forward referenced and already assigned - if (IsImageBinSlotAssigned(obj)) { - DCHECK_EQ(obj, FindInternedString(obj->AsString())); - return; - } - // Need to check if the string is already interned in another image info so that we don't have - // the intern tables of two different images contain the same string. - mirror::String* interned = FindInternedString(obj->AsString()); - if (interned == nullptr) { - // Not in another image space, insert to our table. - interned = image_info.intern_table_->InternStrongImageString(obj->AsString()); - } - if (obj != interned) { - if (!IsImageBinSlotAssigned(interned)) { - // interned obj is after us, allocate its location early - AssignImageBinSlot(interned); - } - // point those looking for this object to the interned version. - SetImageBinSlot(obj, GetImageBinSlot(interned)); - return; - } - // else (obj == interned), nothing to do but fall through to the normal case - } - - AssignImageBinSlot(obj); -} ObjectArray* ImageWriter::CreateImageRoots(size_t oat_index) const { Runtime* runtime = Runtime::Current(); @@ -1090,61 +1059,33 @@ ObjectArray* ImageWriter::CreateImageRoots(size_t oat_index) const { return image_roots.Get(); } -// Walk instance fields of the given Class. Separate function to allow recursion on the super -// class. -void ImageWriter::WalkInstanceFields(mirror::Object* obj, mirror::Class* klass) { - // Visit fields of parent classes first. - StackHandleScope<1> hs(Thread::Current()); - Handle h_class(hs.NewHandle(klass)); - mirror::Class* super = h_class->GetSuperClass(); - if (super != nullptr) { - WalkInstanceFields(obj, super); - } - // - size_t num_reference_fields = h_class->NumReferenceInstanceFields(); - MemberOffset field_offset = h_class->GetFirstReferenceInstanceFieldOffset(); - for (size_t i = 0; i < num_reference_fields; ++i) { - mirror::Object* value = obj->GetFieldObject(field_offset); - if (value != nullptr) { - WalkFieldsInOrder(value); - } - field_offset = MemberOffset(field_offset.Uint32Value() + - sizeof(mirror::HeapReference)); - } -} - -// For an unvisited object, visit it then all its children found via fields. -void ImageWriter::WalkFieldsInOrder(mirror::Object* obj) { - if (IsInBootImage(obj)) { - // Object is in the image, don't need to fix it up. - return; +mirror::Object* ImageWriter::TryAssignBinSlot(WorkStack& work_stack, + mirror::Object* obj, + size_t oat_index) { + if (obj == nullptr || IsInBootImage(obj)) { + // Object is null or already in the image, there is no work to do. + return obj; } - // Use our own visitor routine (instead of GC visitor) to get better locality between - // an object and its fields if (!IsImageBinSlotAssigned(obj)) { - // Walk instance fields of all objects - StackHandleScope<2> hs(Thread::Current()); - Handle h_obj(hs.NewHandle(obj)); - Handle klass(hs.NewHandle(obj->GetClass())); - // visit the object itself. - CalculateObjectBinSlots(h_obj.Get()); - WalkInstanceFields(h_obj.Get(), klass.Get()); - // Walk static fields of a Class. - if (h_obj->IsClass()) { - size_t num_reference_static_fields = klass->NumReferenceStaticFields(); - MemberOffset field_offset = klass->GetFirstReferenceStaticFieldOffset(target_ptr_size_); - for (size_t i = 0; i < num_reference_static_fields; ++i) { - mirror::Object* value = h_obj->GetFieldObject(field_offset); - if (value != nullptr) { - WalkFieldsInOrder(value); - } - field_offset = MemberOffset(field_offset.Uint32Value() + - sizeof(mirror::HeapReference)); + // We want to intern all strings but also assign offsets for the source string. Since the + // pruning phase has already happened, if we intern a string to one in the image we still + // end up copying an unreachable string. + if (obj->IsString()) { + // Need to check if the string is already interned in another image info so that we don't have + // the intern tables of two different images contain the same string. + mirror::String* interned = FindInternedString(obj->AsString()); + if (interned == nullptr) { + // Not in another image space, insert to our table. + interned = GetImageInfo(oat_index).intern_table_->InternStrongImageString(obj->AsString()); + DCHECK_EQ(interned, obj); } + } else if (obj->IsDexCache()) { + oat_index = GetOatIndexForDexCache(obj->AsDexCache()); + } else if (obj->IsClass()) { // Visit and assign offsets for fields and field arrays. - auto* as_klass = h_obj->AsClass(); + mirror::Class* as_klass = obj->AsClass(); mirror::DexCache* dex_cache = as_klass->GetDexCache(); - DCHECK_NE(klass->GetStatus(), mirror::Class::kStatusError); + 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)) << PrettyClass(as_klass); @@ -1152,14 +1093,14 @@ void ImageWriter::WalkFieldsInOrder(mirror::Object* obj) { LengthPrefixedArray* fields[] = { as_klass->GetSFieldsPtr(), as_klass->GetIFieldsPtr(), }; - size_t oat_index = GetOatIndexForDexCache(dex_cache); + // Overwrite the oat index value since the class' dex cache is more accurate of where it + // belongs. + oat_index = GetOatIndexForDexCache(dex_cache); ImageInfo& image_info = GetImageInfo(oat_index); { - // Note: This table is only accessed from the image writer, so the lock is technically - // unnecessary. - WriterMutexLock mu(Thread::Current(), *Locks::classlinker_classes_lock_); - // Insert in the class table for this iamge. - image_info.class_table_->Insert(as_klass); + // Note: This table is only accessed from the image writer, avoid locking to prevent lock + // order violations from root visiting. + image_info.class_table_->InsertWithoutLocks(as_klass); } for (LengthPrefixedArray* cur_fields : fields) { // Total array length including header. @@ -1249,26 +1190,26 @@ void ImageWriter::WalkFieldsInOrder(mirror::Object* obj) { ImTable* imt = as_klass->GetImt(target_ptr_size_); TryAssignImTableOffset(imt, oat_index); } - } else if (h_obj->IsObjectArray()) { - // Walk elements of an object array. - int32_t length = h_obj->AsObjectArray()->GetLength(); - for (int32_t i = 0; i < length; i++) { - mirror::ObjectArray* obj_array = h_obj->AsObjectArray(); - mirror::Object* value = obj_array->Get(i); - if (value != nullptr) { - WalkFieldsInOrder(value); - } - } - } else if (h_obj->IsClassLoader()) { + } else if (obj->IsClassLoader()) { // Register the class loader if it has a class table. // The fake boot class loader should not get registered and we should end up with only one // class loader. - mirror::ClassLoader* class_loader = h_obj->AsClassLoader(); + mirror::ClassLoader* class_loader = obj->AsClassLoader(); if (class_loader->GetClassTable() != nullptr) { class_loaders_.insert(class_loader); } } + AssignImageBinSlot(obj, oat_index); + work_stack.emplace(obj, oat_index); } + if (obj->IsString()) { + // Always return the interned string if there exists one. + mirror::String* interned = FindInternedString(obj->AsString()); + if (interned != nullptr) { + return interned; + } + } + return obj; } bool ImageWriter::NativeRelocationAssigned(void* ptr) const { @@ -1325,10 +1266,16 @@ void ImageWriter::AssignMethodOffset(ArtMethod* method, offset += ArtMethod::Size(target_ptr_size_); } -void ImageWriter::WalkFieldsCallback(mirror::Object* obj, void* arg) { +void ImageWriter::EnsureBinSlotAssignedCallback(mirror::Object* obj, void* arg) { ImageWriter* writer = reinterpret_cast(arg); DCHECK(writer != nullptr); - writer->WalkFieldsInOrder(obj); + if (!Runtime::Current()->GetHeap()->ObjectIsInBootImageSpace(obj)) { + CHECK(writer->IsImageBinSlotAssigned(obj)) << PrettyTypeOf(obj) << " " << obj; + } +} + +void ImageWriter::DeflateMonitorCallback(mirror::Object* obj, void* arg ATTRIBUTE_UNUSED) { + Monitor::Deflate(Thread::Current(), obj); } void ImageWriter::UnbinObjectsIntoOffsetCallback(mirror::Object* obj, void* arg) { @@ -1352,6 +1299,88 @@ void ImageWriter::UnbinObjectsIntoOffset(mirror::Object* obj) { AssignImageOffset(obj, bin_slot); } +class ImageWriter::VisitReferencesVisitor { + public: + VisitReferencesVisitor(ImageWriter* image_writer, WorkStack* work_stack, size_t oat_index) + : image_writer_(image_writer), work_stack_(work_stack), oat_index_(oat_index) {} + + // Fix up separately since we also need to fix up method entrypoints. + ALWAYS_INLINE void VisitRootIfNonNull(mirror::CompressedReference* root) const + SHARED_REQUIRES(Locks::mutator_lock_) { + if (!root->IsNull()) { + VisitRoot(root); + } + } + + ALWAYS_INLINE void VisitRoot(mirror::CompressedReference* root) const + SHARED_REQUIRES(Locks::mutator_lock_) { + root->Assign(VisitReference(root->AsMirrorPtr())); + } + + ALWAYS_INLINE void operator() (mirror::Object* obj, + MemberOffset offset, + bool is_static ATTRIBUTE_UNUSED) const + SHARED_REQUIRES(Locks::mutator_lock_) { + mirror::Object* ref = + obj->GetFieldObject(offset); + obj->SetFieldObject(offset, VisitReference(ref)); + } + + ALWAYS_INLINE void operator() (mirror::Class* klass ATTRIBUTE_UNUSED, + mirror::Reference* ref) const + SHARED_REQUIRES(Locks::mutator_lock_) { + ref->SetReferent( + VisitReference(ref->GetReferent())); + } + + private: + mirror::Object* VisitReference(mirror::Object* ref) const SHARED_REQUIRES(Locks::mutator_lock_) { + return image_writer_->TryAssignBinSlot(*work_stack_, ref, oat_index_); + } + + ImageWriter* const image_writer_; + WorkStack* const work_stack_; + const size_t oat_index_; +}; + +class ImageWriter::GetRootsVisitor : public RootVisitor { + public: + explicit GetRootsVisitor(std::vector* roots) : roots_(roots) {} + + void VisitRoots(mirror::Object*** roots, + size_t count, + const RootInfo& info ATTRIBUTE_UNUSED) OVERRIDE + SHARED_REQUIRES(Locks::mutator_lock_) { + for (size_t i = 0; i < count; ++i) { + roots_->push_back(*roots[i]); + } + } + + void VisitRoots(mirror::CompressedReference** roots, + size_t count, + const RootInfo& info ATTRIBUTE_UNUSED) OVERRIDE + SHARED_REQUIRES(Locks::mutator_lock_) { + for (size_t i = 0; i < count; ++i) { + roots_->push_back(roots[i]->AsMirrorPtr()); + } + } + + private: + std::vector* const roots_; +}; + +void ImageWriter::ProcessWorkStack(WorkStack* work_stack) { + while (!work_stack->empty()) { + std::pair pair(work_stack->top()); + work_stack->pop(); + VisitReferencesVisitor visitor(this, work_stack, /*oat_index*/ pair.second); + // Walk references and assign bin slots for them. + pair.first->VisitReferences( + visitor, + visitor); + } +} + void ImageWriter::CalculateNewObjectOffsets() { Thread* const self = Thread::Current(); StackHandleScopeCollection handles(self); @@ -1360,8 +1389,8 @@ void ImageWriter::CalculateNewObjectOffsets() { image_roots.push_back(handles.NewHandle(CreateImageRoots(i))); } - auto* runtime = Runtime::Current(); - auto* heap = runtime->GetHeap(); + Runtime* const runtime = Runtime::Current(); + gc::Heap* const heap = runtime->GetHeap(); // Leave space for the header, but do not write it yet, we need to // know where image_roots is going to end up @@ -1387,8 +1416,64 @@ void ImageWriter::CalculateNewObjectOffsets() { } } - // Clear any pre-existing monitors which may have been in the monitor words, assign bin slots. - heap->VisitObjects(WalkFieldsCallback, this); + // Deflate monitors before we visit roots since deflating acquires the monitor lock. Acquiring + // this lock while holding other locks may cause lock order violations. + heap->VisitObjects(DeflateMonitorCallback, this); + + // Work list of for objects. Everything on the stack must already be + // assigned a bin slot. + WorkStack work_stack; + + // Special case interned strings to put them in the image they are likely to be resolved from. + for (const DexFile* dex_file : compiler_driver_.GetDexFilesForOatFile()) { + auto it = dex_file_oat_index_map_.find(dex_file); + DCHECK(it != dex_file_oat_index_map_.end()) << dex_file->GetLocation(); + const size_t oat_index = it->second; + InternTable* const intern_table = runtime->GetInternTable(); + for (size_t i = 0, count = dex_file->NumStringIds(); i < count; ++i) { + uint32_t utf16_length; + const char* utf8_data = dex_file->StringDataAndUtf16LengthByIdx(i, &utf16_length); + mirror::String* string = intern_table->LookupStrong(self, utf16_length, utf8_data); + TryAssignBinSlot(work_stack, string, oat_index); + } + } + + // Get the GC roots and then visit them separately to avoid lock violations since the root visitor + // visits roots while holding various locks. + { + std::vector roots; + GetRootsVisitor root_visitor(&roots); + runtime->VisitRoots(&root_visitor); + for (mirror::Object* obj : roots) { + TryAssignBinSlot(work_stack, obj, GetDefaultOatIndex()); + } + } + ProcessWorkStack(&work_stack); + + // For app images, there may be objects that are only held live by the by the boot image. One + // example is finalizer references. Forward these objects so that EnsureBinSlotAssignedCallback + // does not fail any checks. TODO: We should probably avoid copying these objects. + if (compile_app_image_) { + for (gc::space::ImageSpace* space : heap->GetBootImageSpaces()) { + DCHECK(space->IsImageSpace()); + gc::accounting::ContinuousSpaceBitmap* live_bitmap = space->GetLiveBitmap(); + live_bitmap->VisitMarkedRange(reinterpret_cast(space->Begin()), + reinterpret_cast(space->Limit()), + [this, &work_stack](mirror::Object* obj) + SHARED_REQUIRES(Locks::mutator_lock_) { + VisitReferencesVisitor visitor(this, &work_stack, GetDefaultOatIndex()); + // Visit all references and try to assign bin slots for them (calls TryAssignBinSlot). + obj->VisitReferences( + visitor, + visitor); + }); + } + // Process the work stack in case anything was added by TryAssignBinSlot. + ProcessWorkStack(&work_stack); + } + + // Verify that all objects have assigned image bin slots. + heap->VisitObjects(EnsureBinSlotAssignedCallback, this); // Calculate size of the dex cache arrays slot and prepare offsets. PrepareDexCacheArraySlots(); @@ -2272,25 +2357,21 @@ ImageWriter::Bin ImageWriter::BinTypeForNativeRelocationType(NativeObjectRelocat } size_t ImageWriter::GetOatIndex(mirror::Object* obj) const { - if (compile_app_image_) { + if (!IsMultiImage()) { return GetDefaultOatIndex(); - } else { - mirror::DexCache* dex_cache = - obj->IsDexCache() ? obj->AsDexCache() - : obj->IsClass() ? obj->AsClass()->GetDexCache() - : obj->GetClass()->GetDexCache(); - return GetOatIndexForDexCache(dex_cache); } + auto it = oat_index_map_.find(obj); + DCHECK(it != oat_index_map_.end()); + return it->second; } size_t ImageWriter::GetOatIndexForDexFile(const DexFile* dex_file) const { - if (compile_app_image_) { + if (!IsMultiImage()) { return GetDefaultOatIndex(); - } else { - auto it = dex_file_oat_index_map_.find(dex_file); - DCHECK(it != dex_file_oat_index_map_.end()) << dex_file->GetLocation(); - return it->second; } + auto it = dex_file_oat_index_map_.find(dex_file); + DCHECK(it != dex_file_oat_index_map_.end()) << dex_file->GetLocation(); + return it->second; } size_t ImageWriter::GetOatIndexForDexCache(mirror::DexCache* dex_cache) const { diff --git a/compiler/image_writer.h b/compiler/image_writer.h index 1efdc22c0..37f108f66 100644 --- a/compiler/image_writer.h +++ b/compiler/image_writer.h @@ -23,6 +23,7 @@ #include #include #include +#include #include #include @@ -143,6 +144,8 @@ class ImageWriter FINAL { void UpdateOatFileHeader(size_t oat_index, const OatHeader& oat_header); private: + using WorkStack = std::stack>; + bool AllocMemory(); // Mark the objects defined in this space in the given live bitmap. @@ -321,7 +324,10 @@ class ImageWriter FINAL { SHARED_REQUIRES(Locks::mutator_lock_); void PrepareDexCacheArraySlots() SHARED_REQUIRES(Locks::mutator_lock_); - void AssignImageBinSlot(mirror::Object* object) SHARED_REQUIRES(Locks::mutator_lock_); + void AssignImageBinSlot(mirror::Object* object, size_t oat_index) + SHARED_REQUIRES(Locks::mutator_lock_); + mirror::Object* TryAssignBinSlot(WorkStack& work_stack, mirror::Object* obj, size_t oat_index) + SHARED_REQUIRES(Locks::mutator_lock_); void SetImageBinSlot(mirror::Object* object, BinSlot bin_slot) SHARED_REQUIRES(Locks::mutator_lock_); bool IsImageBinSlotAssigned(mirror::Object* object) const @@ -378,20 +384,18 @@ class ImageWriter FINAL { // Lays out where the image objects will be at runtime. void CalculateNewObjectOffsets() SHARED_REQUIRES(Locks::mutator_lock_); + void ProcessWorkStack(WorkStack* work_stack) + SHARED_REQUIRES(Locks::mutator_lock_); void CreateHeader(size_t oat_index) SHARED_REQUIRES(Locks::mutator_lock_); mirror::ObjectArray* CreateImageRoots(size_t oat_index) const SHARED_REQUIRES(Locks::mutator_lock_); - void CalculateObjectBinSlots(mirror::Object* obj) - SHARED_REQUIRES(Locks::mutator_lock_); void UnbinObjectsIntoOffset(mirror::Object* obj) SHARED_REQUIRES(Locks::mutator_lock_); - void WalkInstanceFields(mirror::Object* obj, mirror::Class* klass) + static void EnsureBinSlotAssignedCallback(mirror::Object* obj, void* arg) SHARED_REQUIRES(Locks::mutator_lock_); - void WalkFieldsInOrder(mirror::Object* obj) - SHARED_REQUIRES(Locks::mutator_lock_); - static void WalkFieldsCallback(mirror::Object* obj, void* arg) + static void DeflateMonitorCallback(mirror::Object* obj, void* arg) SHARED_REQUIRES(Locks::mutator_lock_); static void UnbinObjectsIntoOffsetCallback(mirror::Object* obj, void* arg) SHARED_REQUIRES(Locks::mutator_lock_); @@ -461,6 +465,10 @@ class ImageWriter FINAL { std::unordered_set* visited) SHARED_REQUIRES(Locks::mutator_lock_); + bool IsMultiImage() const { + return image_infos_.size() > 1; + } + static Bin BinTypeForNativeRelocationType(NativeObjectRelocationType type); uintptr_t NativeOffsetInImage(void* obj) SHARED_REQUIRES(Locks::mutator_lock_); @@ -519,6 +527,9 @@ class ImageWriter FINAL { // forwarding addresses as well as copying over hash codes. std::unordered_map saved_hashcode_map_; + // Oat index map for objects. + std::unordered_map oat_index_map_; + // Boolean flags. const bool compile_pic_; const bool compile_app_image_; @@ -573,8 +584,10 @@ class ImageWriter FINAL { friend class FixupClassVisitor; friend class FixupRootVisitor; friend class FixupVisitor; + class GetRootsVisitor; friend class NativeLocationVisitor; friend class NonImageClassesVisitor; + class VisitReferencesVisitor; DISALLOW_COPY_AND_ASSIGN(ImageWriter); }; diff --git a/runtime/class_linker.h b/runtime/class_linker.h index 4dafc77c3..8aceffbdc 100644 --- a/runtime/class_linker.h +++ b/runtime/class_linker.h @@ -1171,6 +1171,7 @@ class ClassLinker { size_t image_pointer_size_; class FindVirtualMethodHolderVisitor; + friend struct CompilationHelper; // For Compile in ImageTest. friend class ImageDumper; // for DexLock friend class ImageWriter; // for GetClassRoots friend class JniCompilerTest; // for GetRuntimeQuickGenericJniStub diff --git a/runtime/class_table.cc b/runtime/class_table.cc index e9154cb40..909511c01 100644 --- a/runtime/class_table.cc +++ b/runtime/class_table.cc @@ -107,6 +107,10 @@ void ClassTable::Insert(mirror::Class* klass) { classes_.back().Insert(GcRoot(klass)); } +void ClassTable::InsertWithoutLocks(mirror::Class* klass) { + classes_.back().Insert(GcRoot(klass)); +} + void ClassTable::InsertWithHash(mirror::Class* klass, size_t hash) { WriterMutexLock mu(Thread::Current(), lock_); classes_.back().InsertWithHash(GcRoot(klass), hash); diff --git a/runtime/class_table.h b/runtime/class_table.h index 6fb420605..e3fc217cc 100644 --- a/runtime/class_table.h +++ b/runtime/class_table.h @@ -163,6 +163,8 @@ class ClassTable { } private: + void InsertWithoutLocks(mirror::Class* klass) NO_THREAD_SAFETY_ANALYSIS; + // Lock to guard inserting and removing. mutable ReaderWriterMutex lock_; // We have a vector to help prevent dirty pages after the zygote forks by calling FreezeSnapshot. @@ -171,6 +173,8 @@ class ClassTable { // loader which may not be owned by the class loader must be held strongly live. Also dex caches // are held live to prevent them being unloading once they have classes in them. std::vector> strong_roots_ GUARDED_BY(lock_); + + friend class ImageWriter; // for InsertWithoutLocks. }; } // namespace art diff --git a/runtime/common_runtime_test.h b/runtime/common_runtime_test.h index 0a949008e..e29092833 100644 --- a/runtime/common_runtime_test.h +++ b/runtime/common_runtime_test.h @@ -119,8 +119,7 @@ class CommonRuntimeTestImpl { std::string GetTestDexFileName(const char* name) const; - std::vector> OpenTestDexFiles(const char* name) - SHARED_REQUIRES(Locks::mutator_lock_); + std::vector> OpenTestDexFiles(const char* name); std::unique_ptr OpenTestDexFile(const char* name) SHARED_REQUIRES(Locks::mutator_lock_); diff --git a/test/ImageLayoutA/ImageLayoutA.java b/test/ImageLayoutA/ImageLayoutA.java new file mode 100644 index 000000000..0784ec267 --- /dev/null +++ b/test/ImageLayoutA/ImageLayoutA.java @@ -0,0 +1,21 @@ +/* + * Copyright (C) 2016 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import java.util.HashMap; + +class MyClass { + static int i = 123; +} diff --git a/test/ImageLayoutB/ImageLayoutB.java b/test/ImageLayoutB/ImageLayoutB.java new file mode 100644 index 000000000..a21c5e20f --- /dev/null +++ b/test/ImageLayoutB/ImageLayoutB.java @@ -0,0 +1,25 @@ +/* + * Copyright (C) 2016 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import java.util.HashMap; + +class MyClass { + public static String string = "ASDF_UNIQUE_STRING"; + public static HashMap map = new HashMap(); + static { + map.put("KEY_FOR_HASH_MAP", "VALUE_FOR_HASH_MAP"); + } +} -- 2.11.0