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
diff --git a/build/Android.gtest.mk b/build/Android.gtest.mk
index 011c487..19af14d 100644
--- a/build/Android.gtest.mk
+++ b/build/Android.gtest.mk
@@ -27,6 +27,8 @@
AllFields \
ExceptionHandle \
GetMethodSignature \
+ ImageLayoutA \
+ ImageLayoutB \
Instrumentation \
Interfaces \
Lookup \
@@ -74,6 +76,7 @@
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 bf29e1c..bcd8940 100644
--- a/compiler/common_compiler_test.cc
+++ b/compiler/common_compiler_test.cc
@@ -225,6 +225,7 @@
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 e8b7729..a68ab7c 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<std::string> dex_file_locations;
+ std::vector<ScratchFile> image_locations;
+ std::vector<std::unique_ptr<const DexFile>> extra_dex_files;
+ std::vector<ScratchFile> image_files;
+ std::vector<ScratchFile> oat_files;
+ std::string image_dir;
+
+ void Compile(CompilerDriver* driver,
+ ImageHeader::StorageMode storage_mode);
+
+ std::vector<size_t> 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<std::string>* GetImageClasses() OVERRIDE {
+ return new std::unordered_set<std::string>(image_classes_);
+ }
+
+ private:
+ std::unordered_set<std::string> 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<size_t> CompilationHelper::GetImageObjectSectionSizes() {
+ std::vector<size_t> ret;
+ for (ScratchFile& image_file : image_files) {
+ std::unique_ptr<File> 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<const DexFile*>& boot_class_path = class_linker->GetBootClassPath();
- const size_t num_images = boot_class_path.size();
+ std::vector<const DexFile*> class_path = class_linker->GetBootClassPath();
+
+ for (const std::unique_ptr<const DexFile>& 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<ScratchFile> 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<int>(num_images); ++i) {
+ for (int i = 0; i < static_cast<int>(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<std::string> image_filenames;
- std::vector<ScratchFile> 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 @@
}
std::vector<std::string> oat_filenames;
- std::vector<ScratchFile> 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<const DexFile*, size_t> dex_file_to_oat_index_map;
std::vector<const char*> oat_filename_vector;
for (const std::string& file : oat_filenames) {
@@ -108,13 +164,13 @@
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<ImageWriter> writer(new ImageWriter(*compiler_driver_,
- requested_image_base,
+ std::unique_ptr<ImageWriter> writer(new ImageWriter(*driver,
+ kRequestedImageBase,
/*compile_pic*/false,
/*compile_app_image*/false,
storage_mode,
@@ -125,13 +181,13 @@
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<std::string, std::string> key_value_store;
std::vector<const char*> 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 @@
oat_filename_vector,
image_filename_vector));
- const std::vector<const DexFile*>& dex_files = class_linker->GetBootClassPath();
std::vector<std::unique_ptr<ElfWriter>> elf_writers;
std::vector<std::unique_ptr<OatWriter>> 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 @@
std::vector<std::unique_ptr<const DexFile>> 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<const uint8_t> raw_dex_file(
reinterpret_cast<const uint8_t*>(&dex_file->GetHeader()),
@@ -171,8 +226,8 @@
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 @@
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<const DexFile*> cur_dex_files(1u, dex_files[i]);
- oat_writer->PrepareLayout(compiler_driver_.get(), writer.get(), cur_dex_files, &patcher);
+ std::vector<const DexFile*> 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 @@
ASSERT_TRUE(success);
}
}
- }
- {
bool success_image = writer->Write(kInvalidFd,
image_filename_vector,
oat_filename_vector);
@@ -250,9 +303,39 @@
<< 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<uint64_t> image_file_sizes;
- for (ScratchFile& image_file : image_files) {
+ for (ScratchFile& image_file : helper.image_files) {
std::unique_ptr<File> file(OS::OpenFileForReading(image_file.GetFilename().c_str()));
ASSERT_TRUE(file.get() != nullptr);
ImageHeader image_header;
@@ -283,8 +366,8 @@
// 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 @@
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<void*>(nullptr)));
// By default the compiler this creates will not include patch information.
options.push_back(std::make_pair("-Xnorelocate", nullptr));
@@ -315,17 +398,18 @@
// 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<const DexFile> 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 @@
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<uintptr_t>(image_begin));
+ CHECK_EQ(kRequestedImageBase, reinterpret_cast<uintptr_t>(image_begin));
}
for (size_t j = 0; j < dex->NumClassDefs(); ++j) {
const DexFile::ClassDef& class_def = dex->GetClassDef(j);
@@ -352,15 +436,6 @@
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 @@
TestWriteRead(ImageHeader::kStorageModeLZ4HC);
}
+TEST_F(ImageTest, TestImageLayout) {
+ std::vector<size_t> image_sizes;
+ std::vector<size_t> 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 063eb11..61cf009 100644
--- a/compiler/image_writer.cc
+++ b/compiler/image_writer.cc
@@ -387,7 +387,6 @@
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 @@
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 @@
// 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 @@
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<Object>* ImageWriter::CreateImageRoots(size_t oat_index) const {
Runtime* runtime = Runtime::Current();
@@ -1090,61 +1059,33 @@
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<mirror::Class> h_class(hs.NewHandle(klass));
- mirror::Class* super = h_class->GetSuperClass();
- if (super != nullptr) {
- WalkInstanceFields(obj, super);
+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;
}
- //
- 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<mirror::Object>(field_offset);
- if (value != nullptr) {
- WalkFieldsInOrder(value);
- }
- field_offset = MemberOffset(field_offset.Uint32Value() +
- sizeof(mirror::HeapReference<mirror::Object>));
- }
-}
-
-// 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;
- }
- // 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<mirror::Object> h_obj(hs.NewHandle(obj));
- Handle<mirror::Class> 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<mirror::Object>(field_offset);
- if (value != nullptr) {
- WalkFieldsInOrder(value);
- }
- field_offset = MemberOffset(field_offset.Uint32Value() +
- sizeof(mirror::HeapReference<mirror::Object>));
+ // 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 @@
LengthPrefixedArray<ArtField>* 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<ArtField>* cur_fields : fields) {
// Total array length including header.
@@ -1249,26 +1190,26 @@
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<mirror::Object>()->GetLength();
- for (int32_t i = 0; i < length; i++) {
- mirror::ObjectArray<mirror::Object>* obj_array = h_obj->AsObjectArray<mirror::Object>();
- 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 @@
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<ImageWriter*>(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 @@
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<mirror::Object>* root) const
+ SHARED_REQUIRES(Locks::mutator_lock_) {
+ if (!root->IsNull()) {
+ VisitRoot(root);
+ }
+ }
+
+ ALWAYS_INLINE void VisitRoot(mirror::CompressedReference<mirror::Object>* 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<mirror::Object, kVerifyNone, kWithoutReadBarrier>(offset);
+ obj->SetFieldObject</*kTransactionActive*/false>(offset, VisitReference(ref));
+ }
+
+ ALWAYS_INLINE void operator() (mirror::Class* klass ATTRIBUTE_UNUSED,
+ mirror::Reference* ref) const
+ SHARED_REQUIRES(Locks::mutator_lock_) {
+ ref->SetReferent</*kTransactionActive*/false>(
+ VisitReference(ref->GetReferent<kWithoutReadBarrier>()));
+ }
+
+ 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<mirror::Object*>* 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<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]->AsMirrorPtr());
+ }
+ }
+
+ private:
+ std::vector<mirror::Object*>* const roots_;
+};
+
+void ImageWriter::ProcessWorkStack(WorkStack* work_stack) {
+ while (!work_stack->empty()) {
+ std::pair<mirror::Object*, size_t> 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</*kVisitNativeRoots*/true, kVerifyNone, kWithoutReadBarrier>(
+ visitor,
+ visitor);
+ }
+}
+
void ImageWriter::CalculateNewObjectOffsets() {
Thread* const self = Thread::Current();
StackHandleScopeCollection handles(self);
@@ -1360,8 +1389,8 @@
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 @@
}
}
- // 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 <object, oat_index> 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<mirror::Object*> 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<uintptr_t>(space->Begin()),
+ reinterpret_cast<uintptr_t>(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</*kVisitNativeRoots*/true, kVerifyNone, kWithoutReadBarrier>(
+ 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 @@
}
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 1efdc22..37f108f 100644
--- a/compiler/image_writer.h
+++ b/compiler/image_writer.h
@@ -23,6 +23,7 @@
#include <cstddef>
#include <memory>
#include <set>
+#include <stack>
#include <string>
#include <ostream>
@@ -143,6 +144,8 @@
void UpdateOatFileHeader(size_t oat_index, const OatHeader& oat_header);
private:
+ using WorkStack = std::stack<std::pair<mirror::Object*, size_t>>;
+
bool AllocMemory();
// Mark the objects defined in this space in the given live bitmap.
@@ -321,7 +324,10 @@
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 @@
// 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<mirror::Object>* 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 @@
std::unordered_set<mirror::Class*>* 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 @@
// forwarding addresses as well as copying over hash codes.
std::unordered_map<mirror::Object*, uint32_t> saved_hashcode_map_;
+ // Oat index map for objects.
+ std::unordered_map<mirror::Object*, uint32_t> oat_index_map_;
+
// Boolean flags.
const bool compile_pic_;
const bool compile_app_image_;
@@ -573,8 +584,10 @@
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 4dafc77..8aceffb 100644
--- a/runtime/class_linker.h
+++ b/runtime/class_linker.h
@@ -1171,6 +1171,7 @@
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 e9154cb..909511c 100644
--- a/runtime/class_table.cc
+++ b/runtime/class_table.cc
@@ -107,6 +107,10 @@
classes_.back().Insert(GcRoot<mirror::Class>(klass));
}
+void ClassTable::InsertWithoutLocks(mirror::Class* klass) {
+ classes_.back().Insert(GcRoot<mirror::Class>(klass));
+}
+
void ClassTable::InsertWithHash(mirror::Class* klass, size_t hash) {
WriterMutexLock mu(Thread::Current(), lock_);
classes_.back().InsertWithHash(GcRoot<mirror::Class>(klass), hash);
diff --git a/runtime/class_table.h b/runtime/class_table.h
index 6fb4206..e3fc217 100644
--- a/runtime/class_table.h
+++ b/runtime/class_table.h
@@ -163,6 +163,8 @@
}
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 @@
// 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<GcRoot<mirror::Object>> 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 0a94900..e290928 100644
--- a/runtime/common_runtime_test.h
+++ b/runtime/common_runtime_test.h
@@ -119,8 +119,7 @@
std::string GetTestDexFileName(const char* name) const;
- std::vector<std::unique_ptr<const DexFile>> OpenTestDexFiles(const char* name)
- SHARED_REQUIRES(Locks::mutator_lock_);
+ std::vector<std::unique_ptr<const DexFile>> OpenTestDexFiles(const char* name);
std::unique_ptr<const DexFile> 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 0000000..0784ec2
--- /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 0000000..a21c5e2
--- /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<String, String> map = new HashMap<String, String>();
+ static {
+ map.put("KEY_FOR_HASH_MAP", "VALUE_FOR_HASH_MAP");
+ }
+}