From 0866ea41c21df6672b302eabe5b98ebb40c92613 Mon Sep 17 00:00:00 2001 From: Vladimir Marko Date: Tue, 15 Oct 2019 12:04:17 +0000 Subject: [PATCH] Revert^2 "Refactor oat file writing." This reverts commit 50c812abbe8a8f64d2def49c549eaa005b5f32c7. Fix the VerificationResults to be created before Runtime. This is needed for recording verification data during early Runtime initialization when compiling the boot image. This also cleans up a TODO for bug 29790079. Test: m test-art-host-gtest Test: testrunner.py --host --optimizing Test: Manual, prebuilt files do not change with this CL. Bug: 142680736 Bug: 29790079 Change-Id: I4350e7a67a16ee1653a8f2fce6244e353a3597c4 --- dex2oat/dex2oat.cc | 180 +++++++++++++++--------------- dex2oat/linker/image_test.h | 9 +- dex2oat/linker/oat_writer.cc | 67 +++++------ dex2oat/linker/oat_writer.h | 21 ++-- dex2oat/linker/oat_writer_test.cc | 10 +- 5 files changed, 149 insertions(+), 138 deletions(-) diff --git a/dex2oat/dex2oat.cc b/dex2oat/dex2oat.cc index 91d6513fc8..8c8bc4e2fa 100644 --- a/dex2oat/dex2oat.cc +++ b/dex2oat/dex2oat.cc @@ -1448,6 +1448,79 @@ class Dex2Oat final { return dex2oat::ReturnCode::kOther; } + { + TimingLogger::ScopedTiming t_dex("Writing and opening dex files", timings_); + for (size_t i = 0, size = oat_writers_.size(); i != size; ++i) { + // Unzip or copy dex files straight to the oat file. + std::vector opened_dex_files_map; + std::vector> opened_dex_files; + // No need to verify the dex file when we have a vdex file, which means it was already + // verified. + const bool verify = (input_vdex_file_ == nullptr); + if (!oat_writers_[i]->WriteAndOpenDexFiles( + vdex_files_[i].get(), + verify, + update_input_vdex_, + copy_dex_files_, + &opened_dex_files_map, + &opened_dex_files)) { + return dex2oat::ReturnCode::kOther; + } + dex_files_per_oat_file_.push_back(MakeNonOwningPointerVector(opened_dex_files)); + if (opened_dex_files_map.empty()) { + DCHECK(opened_dex_files.empty()); + } else { + for (MemMap& map : opened_dex_files_map) { + opened_dex_files_maps_.push_back(std::move(map)); + } + for (std::unique_ptr& dex_file : opened_dex_files) { + dex_file_oat_index_map_.emplace(dex_file.get(), i); + opened_dex_files_.push_back(std::move(dex_file)); + } + } + } + } + + compiler_options_->dex_files_for_oat_file_ = MakeNonOwningPointerVector(opened_dex_files_); + const std::vector& dex_files = compiler_options_->dex_files_for_oat_file_; + + // Check if we need to downgrade the compiler-filter for size reasons. + // Note: This does not affect the compiler filter already stored in the key-value + // store which is used for determining whether the oat file is up to date, + // together with the boot class path locations and checksums stored below. + CompilerFilter::Filter original_compiler_filter = compiler_options_->GetCompilerFilter(); + if (!IsBootImage() && IsVeryLarge(dex_files)) { + // Disable app image to make sure dex2oat unloading is enabled. + compiler_options_->image_type_ = CompilerOptions::ImageType::kNone; + + // If we need to downgrade the compiler-filter for size reasons, do that early before we read + // it below for creating verification callbacks. + if (!CompilerFilter::IsAsGoodAs(kLargeAppFilter, compiler_options_->GetCompilerFilter())) { + LOG(INFO) << "Very large app, downgrading to verify."; + compiler_options_->SetCompilerFilter(kLargeAppFilter); + } + } + + if (CompilerFilter::IsAnyCompilationEnabled(compiler_options_->GetCompilerFilter())) { + // Only modes with compilation require verification results, do this here instead of when we + // create the compilation callbacks since the compilation mode may have been changed by the + // very large app logic. + // Avoiding setting the verification results saves RAM by not adding the dex files later in + // the function. + // Note: When compiling boot image, this must be done before creating the Runtime. + verification_results_.reset(new VerificationResults(compiler_options_.get())); + callbacks_->SetVerificationResults(verification_results_.get()); + } + + if (IsBootImage()) { + // For boot image, pass opened dex files to the Runtime::Create(). + // Note: Runtime acquires ownership of these dex files. + runtime_options.Set(RuntimeArgumentMap::BootClassPathDexList, &opened_dex_files_); + } + if (!CreateRuntime(std::move(runtime_options))) { + return dex2oat::ReturnCode::kCreateRuntime; + } + if (!compilation_reason_.empty()) { key_value_store_->Put(OatHeader::kCompilationReasonKey, compilation_reason_); } @@ -1459,13 +1532,7 @@ class Dex2Oat final { } if (!IsBootImage()) { - // When compiling an app, create the runtime early to retrieve - // the boot image checksums needed for the oat header. - if (!CreateRuntime(std::move(runtime_options))) { - return dex2oat::ReturnCode::kCreateRuntime; - } - - if (CompilerFilter::DependsOnImageChecksum(compiler_options_->GetCompilerFilter())) { + if (CompilerFilter::DependsOnImageChecksum(original_compiler_filter)) { TimingLogger::ScopedTiming t3("Loading image checksum", timings_); Runtime* runtime = Runtime::Current(); key_value_store_->Put(OatHeader::kBootClassPathKey, @@ -1518,74 +1585,6 @@ class Dex2Oat final { key_value_store_->Put(OatHeader::kClassPathKey, class_path_key); } - // Now that we have finalized key_value_store_, start writing the oat file. - { - TimingLogger::ScopedTiming t_dex("Writing and opening dex files", timings_); - rodata_.reserve(oat_writers_.size()); - for (size_t i = 0, size = oat_writers_.size(); i != size; ++i) { - rodata_.push_back(elf_writers_[i]->StartRoData()); - // Unzip or copy dex files straight to the oat file. - std::vector opened_dex_files_map; - std::vector> opened_dex_files; - // No need to verify the dex file when we have a vdex file, which means it was already - // verified. - const bool verify = (input_vdex_file_ == nullptr); - if (!oat_writers_[i]->WriteAndOpenDexFiles( - vdex_files_[i].get(), - rodata_.back(), - (i == 0u) ? key_value_store_.get() : nullptr, - verify, - update_input_vdex_, - copy_dex_files_, - &opened_dex_files_map, - &opened_dex_files)) { - return dex2oat::ReturnCode::kOther; - } - dex_files_per_oat_file_.push_back(MakeNonOwningPointerVector(opened_dex_files)); - if (opened_dex_files_map.empty()) { - DCHECK(opened_dex_files.empty()); - } else { - for (MemMap& map : opened_dex_files_map) { - opened_dex_files_maps_.push_back(std::move(map)); - } - for (std::unique_ptr& dex_file : opened_dex_files) { - dex_file_oat_index_map_.emplace(dex_file.get(), i); - opened_dex_files_.push_back(std::move(dex_file)); - } - } - } - } - - compiler_options_->dex_files_for_oat_file_ = MakeNonOwningPointerVector(opened_dex_files_); - const std::vector& dex_files = compiler_options_->dex_files_for_oat_file_; - - // If we need to downgrade the compiler-filter for size reasons. - if (!IsBootImage() && IsVeryLarge(dex_files)) { - // Disable app image to make sure dex2oat unloading is enabled. - compiler_options_->image_type_ = CompilerOptions::ImageType::kNone; - - // If we need to downgrade the compiler-filter for size reasons, do that early before we read - // it below for creating verification callbacks. - if (!CompilerFilter::IsAsGoodAs(kLargeAppFilter, compiler_options_->GetCompilerFilter())) { - LOG(INFO) << "Very large app, downgrading to verify."; - // Note: this change won't be reflected in the key-value store, as that had to be - // finalized before loading the dex files. This setup is currently required - // to get the size from the DexFile objects. - // TODO: refactor. b/29790079 - compiler_options_->SetCompilerFilter(kLargeAppFilter); - } - } - - if (CompilerFilter::IsAnyCompilationEnabled(compiler_options_->GetCompilerFilter())) { - // Only modes with compilation require verification results, do this here instead of when we - // create the compilation callbacks since the compilation mode may have been changed by the - // very large app logic. - // Avoiding setting the verification results saves RAM by not adding the dex files later in - // the function. - verification_results_.reset(new VerificationResults(compiler_options_.get())); - callbacks_->SetVerificationResults(verification_results_.get()); - } - // We had to postpone the swap decision till now, as this is the point when we actually // know about the dex files we're going to use. @@ -1602,14 +1601,6 @@ class Dex2Oat final { } } // Note that dex2oat won't close the swap_fd_. The compiler driver's swap space will do that. - if (IsBootImage()) { - // For boot image, pass opened dex files to the Runtime::Create(). - // Note: Runtime acquires ownership of these dex files. - runtime_options.Set(RuntimeArgumentMap::BootClassPathDexList, &opened_dex_files_); - if (!CreateRuntime(std::move(runtime_options))) { - return dex2oat::ReturnCode::kOther; - } - } // If we're doing the image, override the compiler filter to force full compilation. Must be // done ahead of WellKnownClasses::Init that causes verification. Note: doesn't force @@ -1934,12 +1925,21 @@ class Dex2Oat final { } } - // Initialize the writers with the compiler driver, image writer, and their - // dex files. The writers were created without those being there yet. - for (size_t i = 0, size = oat_files_.size(); i != size; ++i) { - std::unique_ptr& oat_writer = oat_writers_[i]; - std::vector& dex_files = dex_files_per_oat_file_[i]; - oat_writer->Initialize(driver_.get(), image_writer_.get(), dex_files); + // Initialize the writers with the compiler driver, image writer and their dex files + // and start writing the .rodata sections. + { + TimingLogger::ScopedTiming t2("dex2oat Starting oat .rodata section(s)", timings_); + rodata_.reserve(oat_writers_.size()); + for (size_t i = 0, size = oat_files_.size(); i != size; ++i) { + rodata_.push_back(elf_writers_[i]->StartRoData()); + if (!oat_writers_[i]->StartRoData(driver_.get(), + image_writer_.get(), + dex_files_per_oat_file_[i], + rodata_.back(), + (i == 0u) ? key_value_store_.get() : nullptr)) { + return false; + } + } } { @@ -2008,7 +2008,7 @@ class Dex2Oat final { debug::DebugInfo debug_info = oat_writer->GetDebugInfo(); // Keep the variable alive. elf_writer->PrepareDebugInfo(debug_info); // Processes the data on background thread. - OutputStream*& rodata = rodata_[i]; + OutputStream* rodata = rodata_[i]; DCHECK(rodata != nullptr); if (!oat_writer->WriteRodata(rodata)) { LOG(ERROR) << "Failed to write .rodata section to the ELF file " << oat_file->GetPath(); diff --git a/dex2oat/linker/image_test.h b/dex2oat/linker/image_test.h index 4c984d34a9..d1d22a5820 100644 --- a/dex2oat/linker/image_test.h +++ b/dex2oat/linker/image_test.h @@ -257,8 +257,6 @@ inline void ImageTest::DoCompile(ImageHeader::StorageMode storage_mode, std::vector> cur_opened_dex_files; bool dex_files_ok = oat_writers[i]->WriteAndOpenDexFiles( out_helper.vdex_files[i].GetFile(), - rodata.back(), - (i == 0u) ? &key_value_store : nullptr, /* verify */ false, // Dex files may be dex-to-dex-ed, don't verify. /* update_input_vdex */ false, /* copy_dex_files */ CopyOption::kOnlyIfCompressed, @@ -289,7 +287,12 @@ inline void ImageTest::DoCompile(ImageHeader::StorageMode storage_mode, OatWriter* const oat_writer = oat_writers[i].get(); ElfWriter* const elf_writer = elf_writers[i].get(); std::vector cur_dex_files(1u, class_path[i]); - oat_writer->Initialize(driver, writer.get(), cur_dex_files); + bool initialize_ok = oat_writer->StartRoData(driver, + writer.get(), + cur_dex_files, + rodata[i], + (i == 0u) ? &key_value_store : nullptr); + ASSERT_TRUE(initialize_ok); std::unique_ptr vdex_out = std::make_unique( diff --git a/dex2oat/linker/oat_writer.cc b/dex2oat/linker/oat_writer.cc index b478e28da1..2c1837ec37 100644 --- a/dex2oat/linker/oat_writer.cc +++ b/dex2oat/linker/oat_writer.cc @@ -658,8 +658,6 @@ bool OatWriter::MayHaveCompiledMethods() const { bool OatWriter::WriteAndOpenDexFiles( File* vdex_file, - OutputStream* oat_rodata, - SafeMap* key_value_store, bool verify, bool update_input_vdex, CopyOption copy_dex_files, @@ -667,23 +665,9 @@ bool OatWriter::WriteAndOpenDexFiles( /*out*/ std::vector>* opened_dex_files) { CHECK(write_state_ == WriteState::kAddingDexFileSources); - // Record the ELF rodata section offset, i.e. the beginning of the OAT data. - if (!RecordOatDataOffset(oat_rodata)) { - return false; - } - - // Record whether this is the primary oat file. - primary_oat_file_ = (key_value_store != nullptr); - - // Initialize VDEX and OAT headers. - // Reserve space for Vdex header and checksums. vdex_size_ = sizeof(VdexFile::VerifierDepsHeader) + oat_dex_files_.size() * sizeof(VdexFile::VdexChecksum); - oat_size_ = InitOatHeader(dchecked_integral_cast(oat_dex_files_.size()), - key_value_store); - - ChecksumUpdatingOutputStream checksum_updating_rodata(oat_rodata, this); std::unique_ptr vdex_out = std::make_unique(std::make_unique(vdex_file)); @@ -695,6 +679,37 @@ bool OatWriter::WriteAndOpenDexFiles( return false; } + *opened_dex_files_map = std::move(dex_files_map); + *opened_dex_files = std::move(dex_files); + write_state_ = WriteState::kStartRoData; + return true; +} + +// Initialize the writer with the given parameters. +bool OatWriter::StartRoData(const CompilerDriver* compiler_driver, + ImageWriter* image_writer, + const std::vector& dex_files, + OutputStream* oat_rodata, + SafeMap* key_value_store) { + CHECK(write_state_ == WriteState::kStartRoData); + compiler_driver_ = compiler_driver; + image_writer_ = image_writer; + dex_files_ = &dex_files; + + // Record the ELF rodata section offset, i.e. the beginning of the OAT data. + if (!RecordOatDataOffset(oat_rodata)) { + return false; + } + + // Record whether this is the primary oat file. + primary_oat_file_ = (key_value_store != nullptr); + + // Initialize OAT header. + oat_size_ = InitOatHeader(dchecked_integral_cast(oat_dex_files_.size()), + key_value_store); + + ChecksumUpdatingOutputStream checksum_updating_rodata(oat_rodata, this); + // Write type lookup tables into the oat file. if (!WriteTypeLookupTables(&checksum_updating_rodata, dex_files)) { return false; @@ -705,21 +720,10 @@ bool OatWriter::WriteAndOpenDexFiles( return false; } - *opened_dex_files_map = std::move(dex_files_map); - *opened_dex_files = std::move(dex_files); write_state_ = WriteState::kPrepareLayout; return true; } -// Initialize the writer with the given parameters. -void OatWriter::Initialize(const CompilerDriver* compiler_driver, - ImageWriter* image_writer, - const std::vector& dex_files) { - compiler_driver_ = compiler_driver; - image_writer_ = image_writer; - dex_files_ = &dex_files; -} - void OatWriter::PrepareLayout(MultiOatRelativePatcher* relative_patcher) { CHECK(write_state_ == WriteState::kPrepareLayout); @@ -3768,9 +3772,8 @@ bool OatWriter::OpenDexFiles( return true; } -bool OatWriter::WriteTypeLookupTables( - OutputStream* oat_rodata, - const std::vector>& opened_dex_files) { +bool OatWriter::WriteTypeLookupTables(OutputStream* oat_rodata, + const std::vector& opened_dex_files) { TimingLogger::ScopedTiming split("WriteTypeLookupTables", timings_); uint32_t expected_offset = oat_data_offset_ + oat_size_; @@ -3852,11 +3855,11 @@ bool OatWriter::WriteTypeLookupTables( bool OatWriter::WriteDexLayoutSections( OutputStream* oat_rodata, - const std::vector>& opened_dex_files) { + const std::vector& opened_dex_files) { TimingLogger::ScopedTiming split(__FUNCTION__, timings_); if (!kWriteDexLayoutInfo) { - return true;; + return true; } uint32_t expected_offset = oat_data_offset_ + oat_size_; diff --git a/dex2oat/linker/oat_writer.h b/dex2oat/linker/oat_writer.h index 84d13a8522..c95b4ddd76 100644 --- a/dex2oat/linker/oat_writer.h +++ b/dex2oat/linker/oat_writer.h @@ -132,7 +132,7 @@ class OatWriter { // - AddVdexDexFilesSource(). // Then the user must call in order // - WriteAndOpenDexFiles() - // - Initialize() + // - StartRoData() // - WriteVerifierDeps() // - WriteQuickeningInfo() // - WriteChecksumsAndVdexHeader() @@ -167,23 +167,23 @@ class OatWriter { dchecked_vector GetSourceLocations() const; // Write raw dex files to the vdex file, mmap the file and open the dex files from it. - // Supporting data structures are written into the .rodata section of the oat file. // The `verify` setting dictates whether the dex file verifier should check the dex files. // This is generally the case, and should only be false for tests. // If `update_input_vdex` is true, then this method won't actually write the dex files, // and the compiler will just re-use the existing vdex file. bool WriteAndOpenDexFiles(File* vdex_file, - OutputStream* oat_rodata, - SafeMap* key_value_store, bool verify, bool update_input_vdex, CopyOption copy_dex_files, /*out*/ std::vector* opened_dex_files_map, /*out*/ std::vector>* opened_dex_files); - // Initialize the writer with the given parameters. - void Initialize(const CompilerDriver* compiler_driver, - ImageWriter* image_writer, - const std::vector& dex_files); + // Initialize the writer with the given parameters and start writing .rodata. + // Supporting data structures for dex files are written into the .rodata section of the oat file. + bool StartRoData(const CompilerDriver* compiler_driver, + ImageWriter* image_writer, + const std::vector& dex_files, + OutputStream* oat_rodata, + SafeMap* key_value_store); bool WriteQuickeningInfo(OutputStream* vdex_out); bool WriteVerifierDeps(OutputStream* vdex_out, verifier::VerifierDeps* verifier_deps); bool WriteChecksumsAndVdexHeader(OutputStream* vdex_out); @@ -339,9 +339,9 @@ class OatWriter { bool RecordOatDataOffset(OutputStream* out); bool WriteTypeLookupTables(OutputStream* oat_rodata, - const std::vector>& opened_dex_files); + const std::vector& opened_dex_files); bool WriteDexLayoutSections(OutputStream* oat_rodata, - const std::vector>& opened_dex_files); + const std::vector& opened_dex_files); bool WriteCodeAlignment(OutputStream* out, uint32_t aligned_code_delta); bool WriteUpTo16BytesAlignment(OutputStream* out, uint32_t size, uint32_t* stat); void SetMultiOatRelativePatcherAdjustment(); @@ -355,6 +355,7 @@ class OatWriter { enum class WriteState { kAddingDexFileSources, + kStartRoData, kPrepareLayout, kWriteRoData, kWriteText, diff --git a/dex2oat/linker/oat_writer_test.cc b/dex2oat/linker/oat_writer_test.cc index 2142727dae..0a92baf7b8 100644 --- a/dex2oat/linker/oat_writer_test.cc +++ b/dex2oat/linker/oat_writer_test.cc @@ -178,8 +178,6 @@ class OatTest : public CommonCompilerDriverTest { std::vector> opened_dex_files; if (!oat_writer.WriteAndOpenDexFiles( vdex_file, - oat_rodata, - &key_value_store, verify, /*update_input_vdex=*/ false, copy, @@ -199,7 +197,13 @@ class OatTest : public CommonCompilerDriverTest { MultiOatRelativePatcher patcher(compiler_options_->GetInstructionSet(), compiler_options_->GetInstructionSetFeatures(), compiler_driver_->GetCompiledMethodStorage()); - oat_writer.Initialize(compiler_driver_.get(), nullptr, dex_files); + if (!oat_writer.StartRoData(compiler_driver_.get(), + /*image_writer=*/ nullptr, + dex_files, + oat_rodata, + &key_value_store)) { + return false; + } oat_writer.PrepareLayout(&patcher); elf_writer->PrepareDynamicSection(oat_writer.GetOatHeader().GetExecutableOffset(), oat_writer.GetCodeSize(),