From ceb07b3285eaab350a8cd12f7d74be3e40a255dd Mon Sep 17 00:00:00 2001 From: Mathieu Chartier Date: Thu, 10 Dec 2015 09:33:21 -0800 Subject: Revert "Revert "Add support for LZ4 compressed image files"" Needed to call compiler_driver_->SetSupportBootImageFixup(false). Bug: 22858531 This reverts commit 83d4d72aa0e4170209ab50c67ba22e46b71352c1. Change-Id: Iaed6a810a0c088f1f2c57cf2f12087f3978a3de1 --- compiler/Android.mk | 8 ++--- compiler/common_compiler_test.cc | 32 ++++++++++++------ compiler/common_compiler_test.h | 2 ++ compiler/image_test.cc | 38 ++++++++++++++++----- compiler/image_writer.cc | 71 +++++++++++++++++++++++++++++++++------- compiler/image_writer.h | 10 ++++-- 6 files changed, 125 insertions(+), 36 deletions(-) (limited to 'compiler') diff --git a/compiler/Android.mk b/compiler/Android.mk index 348eabdde5..bdd9a84433 100644 --- a/compiler/Android.mk +++ b/compiler/Android.mk @@ -258,9 +258,9 @@ define build-libart-compiler ifeq ($$(art_ndebug_or_debug),ndebug) LOCAL_MODULE := libart-compiler ifeq ($$(art_static_or_shared), static) - LOCAL_STATIC_LIBRARIES += libart + LOCAL_STATIC_LIBRARIES += libart liblz4 else - LOCAL_SHARED_LIBRARIES += libart + LOCAL_SHARED_LIBRARIES += libart liblz4 endif ifeq ($$(art_target_or_host),target) LOCAL_FDO_SUPPORT := true @@ -268,9 +268,9 @@ define build-libart-compiler else # debug LOCAL_MODULE := libartd-compiler ifeq ($$(art_static_or_shared), static) - LOCAL_STATIC_LIBRARIES += libartd + LOCAL_STATIC_LIBRARIES += libartd liblz4 else - LOCAL_SHARED_LIBRARIES += libartd + LOCAL_SHARED_LIBRARIES += libartd liblz4 endif endif diff --git a/compiler/common_compiler_test.cc b/compiler/common_compiler_test.cc index e6cc50cc5e..638b897eea 100644 --- a/compiler/common_compiler_test.cc +++ b/compiler/common_compiler_test.cc @@ -189,17 +189,29 @@ void CommonCompilerTest::SetUp() { } timer_.reset(new CumulativeLogger("Compilation times")); - compiler_driver_.reset(new CompilerDriver(compiler_options_.get(), - verification_results_.get(), - method_inliner_map_.get(), - compiler_kind_, instruction_set, - instruction_set_features_.get(), - true, - GetImageClasses(), - GetCompiledClasses(), - GetCompiledMethods(), - 2, true, true, "", false, timer_.get(), -1, "")); + CreateCompilerDriver(compiler_kind_, instruction_set); } +} + +void CommonCompilerTest::CreateCompilerDriver(Compiler::Kind kind, InstructionSet isa) { + compiler_driver_.reset(new CompilerDriver(compiler_options_.get(), + verification_results_.get(), + method_inliner_map_.get(), + kind, + isa, + instruction_set_features_.get(), + true, + GetImageClasses(), + GetCompiledClasses(), + GetCompiledMethods(), + 2, + true, + true, + "", + false, + timer_.get(), + -1, + "")); // We typically don't generate an image in unit tests, disable this optimization by default. compiler_driver_->SetSupportBootImageFixup(false); } diff --git a/compiler/common_compiler_test.h b/compiler/common_compiler_test.h index 1b57b7d1d2..b491946dc3 100644 --- a/compiler/common_compiler_test.h +++ b/compiler/common_compiler_test.h @@ -90,6 +90,8 @@ class CommonCompilerTest : public CommonRuntimeTest { const char* method_name, const char* signature) SHARED_REQUIRES(Locks::mutator_lock_); + void CreateCompilerDriver(Compiler::Kind kind, InstructionSet isa); + void ReserveImageSpace(); void UnreserveImageSpace(); diff --git a/compiler/image_test.cc b/compiler/image_test.cc index cda6240bd4..b596ba66a5 100644 --- a/compiler/image_test.cc +++ b/compiler/image_test.cc @@ -43,10 +43,17 @@ class ImageTest : public CommonCompilerTest { ReserveImageSpace(); CommonCompilerTest::SetUp(); } + void TestWriteRead(ImageHeader::StorageMode storage_mode); }; -TEST_F(ImageTest, WriteRead) { - TEST_DISABLED_FOR_NON_PIC_COMPILING_WITH_OPTIMIZING(); +void ImageTest::TestWriteRead(ImageHeader::StorageMode storage_mode) { + // TODO: Test does not currently work with optimizing. + CreateCompilerDriver(Compiler::kQuick, kRuntimeISA); + ClassLinker* class_linker = Runtime::Current()->GetClassLinker(); + // Enable write for dex2dex. + for (const DexFile* dex_file : class_linker->GetBootClassPath()) { + dex_file->EnableWrite(); + } // Create a generic location tmp file, to be the base of the .art and .oat temporary files. ScratchFile location; ScratchFile image_location(location, ".art"); @@ -68,17 +75,14 @@ TEST_F(ImageTest, WriteRead) { std::unique_ptr writer(new ImageWriter(*compiler_driver_, requested_image_base, /*compile_pic*/false, - /*compile_app_image*/false)); + /*compile_app_image*/false, + storage_mode)); // TODO: compile_pic should be a test argument. { { jobject class_loader = nullptr; - ClassLinker* class_linker = Runtime::Current()->GetClassLinker(); TimingLogger timings("ImageTest::WriteRead", false, false); TimingLogger::ScopedTiming t("CompileAll", &timings); - for (const DexFile* dex_file : class_linker->GetBootClassPath()) { - dex_file->EnableWrite(); - } compiler_driver_->SetDexFilesForOatFile(class_linker->GetBootClassPath()); compiler_driver_->CompileAll(class_loader, class_linker->GetBootClassPath(), &timings); @@ -209,7 +213,13 @@ TEST_F(ImageTest, WriteRead) { gc::space::ImageSpace* image_space = heap->GetBootImageSpace(); ASSERT_TRUE(image_space != nullptr); - ASSERT_LE(image_space->Size(), image_file_size); + if (storage_mode == ImageHeader::kStorageModeUncompressed) { + // Uncompressed, image should be smaller than file. + ASSERT_LE(image_space->Size(), image_file_size); + } else { + // Compressed, file should be smaller than image. + ASSERT_LE(image_file_size, image_space->Size()); + } image_space->VerifyImageAllocations(); uint8_t* image_begin = image_space->Begin(); @@ -237,6 +247,14 @@ TEST_F(ImageTest, WriteRead) { CHECK_EQ(0, rmdir_result); } +TEST_F(ImageTest, WriteReadUncompressed) { + TestWriteRead(ImageHeader::kStorageModeUncompressed); +} + +TEST_F(ImageTest, WriteReadLZ4) { + TestWriteRead(ImageHeader::kStorageModeLZ4); +} + TEST_F(ImageTest, ImageHeaderIsValid) { uint32_t image_begin = ART_BASE_ADDRESS; uint32_t image_size_ = 16 * KB; @@ -257,7 +275,9 @@ TEST_F(ImageTest, ImageHeaderIsValid) { oat_data_end, oat_file_end, sizeof(void*), - /*compile_pic*/false); + /*compile_pic*/false, + ImageHeader::kDefaultStorageMode, + /*data_size*/0u); ASSERT_TRUE(image_header.IsValid()); char* magic = const_cast(image_header.GetMagic()); diff --git a/compiler/image_writer.cc b/compiler/image_writer.cc index bf1fcdd5f5..f9f0eb8154 100644 --- a/compiler/image_writer.cc +++ b/compiler/image_writer.cc @@ -17,6 +17,7 @@ #include "image_writer.h" #include +#include #include #include @@ -225,27 +226,72 @@ bool ImageWriter::Write(int image_fd, return EXIT_FAILURE; } - // Write out the image + fields + methods. + std::unique_ptr compressed_data; + // Image data size excludes the bitmap and the header. ImageHeader* const image_header = reinterpret_cast(image_->Begin()); - const auto write_count = image_header->GetImageSize(); - if (!image_file->WriteFully(image_->Begin(), write_count)) { - PLOG(ERROR) << "Failed to write image file " << image_filename; + const size_t image_data_size = image_header->GetImageSize() - sizeof(ImageHeader); + char* image_data = reinterpret_cast(image_->Begin()) + sizeof(ImageHeader); + size_t data_size; + const char* image_data_to_write; + + CHECK_EQ(image_header->storage_mode_, image_storage_mode_); + switch (image_storage_mode_) { + case ImageHeader::kStorageModeLZ4: { + size_t compressed_max_size = LZ4_compressBound(image_data_size); + compressed_data.reset(new char[compressed_max_size]); + data_size = LZ4_compress( + reinterpret_cast(image_->Begin()) + sizeof(ImageHeader), + &compressed_data[0], + image_data_size); + image_data_to_write = &compressed_data[0]; + VLOG(compiler) << "Compressed from " << image_data_size << " to " << data_size; + break; + } + case ImageHeader::kStorageModeUncompressed: { + data_size = image_data_size; + image_data_to_write = image_data; + break; + } + default: { + LOG(FATAL) << "Unsupported"; + UNREACHABLE(); + } + } + + // Write header first, as uncompressed. + image_header->data_size_ = data_size; + if (!image_file->WriteFully(image_->Begin(), sizeof(ImageHeader))) { + PLOG(ERROR) << "Failed to write image file header " << image_filename; + image_file->Erase(); + return false; + } + + // Write out the image + fields + methods. + const bool is_compressed = compressed_data != nullptr; + if (!image_file->WriteFully(image_data_to_write, data_size)) { + PLOG(ERROR) << "Failed to write image file data " << image_filename; image_file->Erase(); return false; } - // Write out the image bitmap at the page aligned start of the image end. + // Write out the image bitmap at the page aligned start of the image end, also uncompressed for + // convenience. const ImageSection& bitmap_section = image_header->GetImageSection( ImageHeader::kSectionImageBitmap); - CHECK_ALIGNED(bitmap_section.Offset(), kPageSize); + // Align up since data size may be unaligned if the image is compressed. + size_t bitmap_position_in_file = RoundUp(sizeof(ImageHeader) + data_size, kPageSize); + if (!is_compressed) { + CHECK_EQ(bitmap_position_in_file, bitmap_section.Offset()); + } if (!image_file->Write(reinterpret_cast(image_bitmap_->Begin()), - bitmap_section.Size(), bitmap_section.Offset())) { + bitmap_section.Size(), + bitmap_position_in_file)) { PLOG(ERROR) << "Failed to write image file " << image_filename; image_file->Erase(); return false; } - - CHECK_EQ(bitmap_section.End(), static_cast(image_file->GetLength())); + CHECK_EQ(bitmap_position_in_file + bitmap_section.Size(), + static_cast(image_file->GetLength())); if (image_file->FlushCloseOrErase() != 0) { PLOG(ERROR) << "Failed to flush and close image file " << image_filename; return false; @@ -1247,7 +1293,8 @@ void ImageWriter::CreateHeader(size_t oat_loaded_size, size_t oat_data_offset) { } CHECK_EQ(AlignUp(image_begin_ + image_end, kPageSize), oat_file_begin) << "Oat file should be right after the image."; - // Create the header. + // Create the header, leave 0 for data size since we will fill this in as we are writing the + // image. new (image_->Begin()) ImageHeader(PointerToLowMemUInt32(image_begin_), image_end, sections, @@ -1258,7 +1305,9 @@ void ImageWriter::CreateHeader(size_t oat_loaded_size, size_t oat_data_offset) { PointerToLowMemUInt32(oat_data_end), PointerToLowMemUInt32(oat_file_end), target_ptr_size_, - compile_pic_); + compile_pic_, + image_storage_mode_, + /*data_size*/0u); } ArtMethod* ImageWriter::GetImageMethodAddress(ArtMethod* method) { diff --git a/compiler/image_writer.h b/compiler/image_writer.h index 386838fde0..c20d83639a 100644 --- a/compiler/image_writer.h +++ b/compiler/image_writer.h @@ -30,6 +30,7 @@ #include "base/macros.h" #include "driver/compiler_driver.h" #include "gc/space/space.h" +#include "image.h" #include "length_prefixed_array.h" #include "lock_word.h" #include "mem_map.h" @@ -54,7 +55,8 @@ class ImageWriter FINAL { ImageWriter(const CompilerDriver& compiler_driver, uintptr_t image_begin, bool compile_pic, - bool compile_app_image) + bool compile_app_image, + ImageHeader::StorageMode image_storage_mode) : compiler_driver_(compiler_driver), image_begin_(reinterpret_cast(image_begin)), image_end_(0), @@ -73,7 +75,8 @@ class ImageWriter FINAL { image_method_array_(ImageHeader::kImageMethodsCount), dirty_methods_(0u), clean_methods_(0u), - class_table_bytes_(0u) { + class_table_bytes_(0u), + image_storage_mode_(image_storage_mode) { CHECK_NE(image_begin, 0U); std::fill_n(image_methods_, arraysize(image_methods_), nullptr); std::fill_n(oat_address_offsets_, arraysize(oat_address_offsets_), 0); @@ -460,6 +463,9 @@ class ImageWriter FINAL { // Number of image class table bytes. size_t class_table_bytes_; + // Which mode the image is stored as, see image.h + const ImageHeader::StorageMode image_storage_mode_; + friend class ContainsBootClassLoaderNonImageClassVisitor; friend class FixupClassVisitor; friend class FixupRootVisitor; -- cgit v1.2.3-59-g8ed1b