diff options
| -rw-r--r-- | build/Android.oat.mk | 3 | ||||
| -rw-r--r-- | build/Android.oattest.mk | 1 | ||||
| -rw-r--r-- | src/class_linker.cc | 11 | ||||
| -rw-r--r-- | src/common_test.h | 6 | ||||
| -rw-r--r-- | src/dex2oat.cc | 95 | ||||
| -rw-r--r-- | src/image_test.cc | 2 | ||||
| -rw-r--r-- | src/oat_test.cc | 2 | ||||
| -rw-r--r-- | src/oat_writer.cc | 76 | ||||
| -rw-r--r-- | src/oat_writer.h | 8 | ||||
| -rw-r--r-- | src/oatopt.cc | 8 |
10 files changed, 151 insertions, 61 deletions
diff --git a/build/Android.oat.mk b/build/Android.oat.mk index 50c05ab..6d72ded 100644 --- a/build/Android.oat.mk +++ b/build/Android.oat.mk @@ -49,11 +49,13 @@ TARGET_CORE_IMG := $(ART_TEST_OUT)/core.art $(HOST_CORE_OAT): $(HOST_CORE_DEX) $(DEX2OAT_DEPENDENCY) @echo "host dex2oat: $@ ($?)" @mkdir -p $(dir $@) + @rm -f $@ $(hide) $(DEX2OAT) --runtime-arg -Xms16m --runtime-arg -Xmx16m $(addprefix --dex-file=,$(filter-out $(DEX2OAT),$^)) --oat=$@ --image=$(HOST_CORE_IMG) --base=$(IMG_HOST_BASE_ADDRESS) $(TARGET_CORE_OAT): $(TARGET_CORE_DEX) $(DEX2OAT_DEPENDENCY) @echo "target dex2oat: $@ ($?)" @mkdir -p $(dir $@) + @rm -f $@ $(hide) $(DEX2OAT) --runtime-arg -Xms32m --runtime-arg -Xmx32m $(addprefix --dex-file=,$(filter-out $(DEX2OAT),$^)) --oat=$@ --image=$(TARGET_CORE_IMG) --base=$(IMG_TARGET_BASE_ADDRESS) --host-prefix=$(PRODUCT_OUT) $(HOST_CORE_IMG): $(HOST_CORE_OAT) @@ -70,6 +72,7 @@ TARGET_BOOT_IMG := $(ART_CACHE_OUT)/boot.art $(TARGET_BOOT_OAT): $(TARGET_BOOT_DEX) $(DEX2OAT_DEPENDENCY) @echo "target dex2oat: $@ ($?)" @mkdir -p $(dir $@) + @rm -f $@ $(hide) $(DEX2OAT) --runtime-arg -Xms256m --runtime-arg -Xmx256m $(addprefix --dex-file=,$(filter-out $(DEX2OAT),$^)) --oat=$@ --image=$(TARGET_BOOT_IMG) --base=$(IMG_TARGET_BASE_ADDRESS) --host-prefix=$(PRODUCT_OUT) $(TARGET_BOOT_IMG): $(TARGET_BOOT_OAT) diff --git a/build/Android.oattest.mk b/build/Android.oattest.mk index bb6695e..7d467bc 100644 --- a/build/Android.oattest.mk +++ b/build/Android.oattest.mk @@ -40,6 +40,7 @@ $(foreach dir,$(TEST_DEX_DIRECTORIES), $(eval $(call build-art-test-dex,$(dir))) define build-art-oat $(2): $(1) $(3) $(DEX2OAT_DEPENDENCY) @echo "target dex2oat: $$@ ($$?)" + @rm -f $$@ $(hide) $(DEX2OAT) --runtime-arg -Xms64m --runtime-arg -Xmx64m --boot-image=$(3) $(addprefix --dex-file=,$$<) --oat=$$@ --host-prefix=$(PRODUCT_OUT) endef diff --git a/src/class_linker.cc b/src/class_linker.cc index a94607a..4f387db 100644 --- a/src/class_linker.cc +++ b/src/class_linker.cc @@ -568,8 +568,12 @@ const OatFile* ClassLinker::GenerateOatFile(const std::string& filename) { std::string oat_file_option("--oat="); oat_file_option += oat_filename; - execl("/system/bin/dex2oatd", - "/system/bin/dex2oatd", + std::string dex2oat("/system/bin/dex2oat"); +#ifndef NDEBUG + dex2oat += 'd'; +#endif + + execl(dex2oat.c_str(), dex2oat.c_str(), "--runtime-arg", "-Xms64m", "--runtime-arg", "-Xmx64m", boot_image_option.c_str(), @@ -636,6 +640,9 @@ const OatFile* ClassLinker::FindOatFile(const DexFile& dex_file) { } LOG(WARNING) << ".oat file " << oat_file->GetLocation() << " is older than " << dex_file.GetLocation() << " --- regenerating"; + if (TEMP_FAILURE_RETRY(unlink(oat_file->GetLocation().c_str())) != 0) { + PLOG(FATAL) << "Couldn't remove obsolete .oat file " << oat_file->GetLocation(); + } // Fall through... } // Generate oat file if it wasn't found or was obsolete. diff --git a/src/common_test.h b/src/common_test.h index 928cb51..3966742 100644 --- a/src/common_test.h +++ b/src/common_test.h @@ -56,6 +56,7 @@ class ScratchFile { filename_ += "/TmpFile-XXXXXX"; fd_ = mkstemp(&filename_[0]); CHECK_NE(-1, fd_); + file_.reset(OS::FileFromFd(GetFilename(), fd_)); } ~ScratchFile() { @@ -69,6 +70,10 @@ class ScratchFile { return filename_.c_str(); } + File* GetFile() const { + return file_.get(); + } + int GetFd() const { return fd_; } @@ -76,6 +81,7 @@ class ScratchFile { private: std::string filename_; int fd_; + UniquePtr<File> file_; }; class CommonTest : public testing::Test { diff --git a/src/dex2oat.cc b/src/dex2oat.cc index 80b21e2..f05f99a 100644 --- a/src/dex2oat.cc +++ b/src/dex2oat.cc @@ -2,6 +2,7 @@ #include <stdio.h> #include <stdlib.h> +#include <sys/file.h> #include <string> #include <vector> @@ -62,6 +63,31 @@ static void usage() { exit(EXIT_FAILURE); } +class FileJanitor { +public: + FileJanitor(const std::string& filename, int fd) + : filename_(filename), fd_(fd), do_unlink_(true) { + } + + void KeepFile() { + do_unlink_ = false; + } + + ~FileJanitor() { + if (fd_ != -1) { + flock(fd_, LOCK_UN); + } + if (do_unlink_) { + unlink(filename_.c_str()); + } + } + +private: + std::string filename_; + int fd_; + bool do_unlink_; +}; + int dex2oat(int argc, char** argv) { // Skip over argv[0]. argv++; @@ -143,17 +169,62 @@ int dex2oat(int argc, char** argv) { } } - // Check early that the result of compilation can be written - // TODO: implement a proper locking scheme here, probably hold onto the open file.. - if (OS::FileExists(oat_filename.c_str())) { - // File exists, check we can write to it - UniquePtr<File> file(OS::OpenFile(oat_filename.c_str(), true)); - if (file.get() == NULL) { + // Create the output file if we can, or open it read-only if we weren't first. + bool did_create = true; + int fd = open(oat_filename.c_str(), O_EXCL | O_CREAT | O_TRUNC | O_RDWR, 0666); + if (fd == -1) { + if (errno != EEXIST) { PLOG(ERROR) << "Unable to create oat file " << oat_filename; return EXIT_FAILURE; } + did_create = false; + fd = open(oat_filename.c_str(), O_RDONLY); + if (fd == -1) { + PLOG(ERROR) << "Unable to open oat file for reading " << oat_filename; + return EXIT_FAILURE; + } } - LOG(INFO) << "dex2oat: " << oat_filename; + + // Handles removing the file on failure and unlocking on both failure and success. + FileJanitor file_janitor(oat_filename, fd); + + // TODO: TEMP_FAILURE_RETRY on flock calls + + // If we won the creation race, block trying to take the lock (since we're going to be doing + // the work, we need the lock). If we lost the creation race, spin trying to take the lock + // non-blocking until we fail -- at which point we know the other guy has the lock -- and then + // block trying to take the now-taken lock. + if (did_create) { + LOG(INFO) << "This process created " << oat_filename; + while (flock(fd, LOCK_EX) != 0) { + // Try again. + } + LOG(INFO) << "This process created and locked " << oat_filename; + } else { + LOG(INFO) << "Another process has already created " << oat_filename; + while (flock(fd, LOCK_EX | LOCK_NB) == 0) { + // Give up the lock and hope the creator has taken the lock next time round. + flock(fd, LOCK_UN); + } + // Now a non-blocking attempt to take the lock has failed, we know the other guy has the + // lock, so block waiting to take it. + LOG(INFO) << "Another process is already working on " << oat_filename; + if (flock(fd, LOCK_EX) != 0) { + PLOG(ERROR) << "Waiter unable to wait for creator to finish " << oat_filename; + return EXIT_FAILURE; + } + // We have the lock and the creator has finished. + // TODO: check the creator did a good job by checking the header. + LOG(INFO) << "Another process finished working on " << oat_filename; + // Job done. + file_janitor.KeepFile(); + return EXIT_SUCCESS; + } + + // If we get this far, we won the creation race and have locked the file. + UniquePtr<File> oat_file(OS::FileFromFd(oat_filename.c_str(), fd)); + + LOG(INFO) << "dex2oat: " << oat_file->name(); Runtime::Options options; options.push_back(std::make_pair("compiler", reinterpret_cast<void*>(NULL))); @@ -266,22 +337,26 @@ int dex2oat(int argc, char** argv) { } } - if (!OatWriter::Create(oat_filename, class_loader.get(), compiler)) { - LOG(ERROR) << "Failed to create oat file " << oat_filename; + if (!OatWriter::Create(oat_file.get(), class_loader.get(), compiler)) { + LOG(ERROR) << "Failed to create oat file " << oat_file->name(); return EXIT_FAILURE; } if (image_filename == NULL) { + LOG(INFO) << "No image filename supplied; exiting"; return EXIT_SUCCESS; } CHECK(compiler.IsImage()); ImageWriter image_writer; - if (!image_writer.Write(image_filename, image_base, oat_filename, host_prefix)) { + if (!image_writer.Write(image_filename, image_base, oat_file->name(), host_prefix)) { LOG(ERROR) << "Failed to create image file " << image_filename; return EXIT_FAILURE; } + // We wrote the file successfully, and want to keep it. + LOG(INFO) << "Image written successfully " << image_filename; + file_janitor.KeepFile(); return EXIT_SUCCESS; } diff --git a/src/image_test.cc b/src/image_test.cc index 2ef7489..3e64d95 100644 --- a/src/image_test.cc +++ b/src/image_test.cc @@ -18,7 +18,7 @@ class ImageTest : public CommonTest {}; TEST_F(ImageTest, WriteRead) { ScratchFile tmp_oat; - bool success_oat = OatWriter::Create(tmp_oat.GetFilename(), NULL, *compiler_.get()); + bool success_oat = OatWriter::Create(tmp_oat.GetFile(), NULL, *compiler_.get()); ASSERT_TRUE(success_oat); ImageWriter writer; diff --git a/src/oat_test.cc b/src/oat_test.cc index 3433e44..e90d2ed 100644 --- a/src/oat_test.cc +++ b/src/oat_test.cc @@ -19,7 +19,7 @@ TEST_F(OatTest, WriteRead) { } ScratchFile tmp; - bool success = OatWriter::Create(tmp.GetFilename(), class_loader.get(), *compiler_.get()); + bool success = OatWriter::Create(tmp.GetFile(), class_loader.get(), *compiler_.get()); ASSERT_TRUE(success); if (compile) { // OatWriter strips the code, regenerate to compare diff --git a/src/oat_writer.cc b/src/oat_writer.cc index 214d177..664dcfe 100644 --- a/src/oat_writer.cc +++ b/src/oat_writer.cc @@ -10,12 +10,12 @@ namespace art { -bool OatWriter::Create(const std::string& filename, +bool OatWriter::Create(File* file, const ClassLoader* class_loader, const Compiler& compiler) { const std::vector<const DexFile*>& dex_files = ClassLoader::GetCompileTimeClassPath(class_loader); OatWriter oat_writer(dex_files, class_loader, compiler); - return oat_writer.Write(filename); + return oat_writer.Write(file); } OatWriter::OatWriter(const std::vector<const DexFile*>& dex_files, @@ -293,32 +293,26 @@ size_t OatWriter::InitOatCodeMethod(size_t offset, #define DCHECK_CODE_OFFSET() \ DCHECK_EQ(static_cast<off_t>(code_offset), lseek(file->Fd(), 0, SEEK_CUR)) -bool OatWriter::Write(const std::string& filename) { - UniquePtr<File> file(OS::OpenFile(filename.c_str(), true)); - if (file.get() == NULL) { - PLOG(ERROR) << "Failed to open file " << filename << " for writing"; - return false; - } - +bool OatWriter::Write(File* file) { if (!file->WriteFully(oat_header_, sizeof(*oat_header_))) { - PLOG(ERROR) << "Failed to write oat header to " << filename; + PLOG(ERROR) << "Failed to write oat header to " << file->name(); return false; } - if (!WriteTables(file.get())) { - LOG(ERROR) << "Failed to write oat tables to " << filename; + if (!WriteTables(file)) { + LOG(ERROR) << "Failed to write oat tables to " << file->name(); return false; } - size_t code_offset = WriteCode(file.get()); + size_t code_offset = WriteCode(file); if (code_offset == 0) { - LOG(ERROR) << "Failed to write oat code to " << filename; + LOG(ERROR) << "Failed to write oat code to " << file->name(); return false; } - code_offset = WriteCodeDexFiles(file.get(), code_offset); + code_offset = WriteCodeDexFiles(file, code_offset); if (code_offset == 0) { - LOG(ERROR) << "Failed to write oat code for dex files to " << filename; + LOG(ERROR) << "Failed to write oat code for dex files to " << file->name(); return false; } @@ -328,19 +322,19 @@ bool OatWriter::Write(const std::string& filename) { bool OatWriter::WriteTables(File* file) { for (size_t i = 0; i != oat_dex_files_.size(); ++i) { if (!oat_dex_files_[i]->Write(file)) { - PLOG(ERROR) << "Failed to write oat dex information"; + PLOG(ERROR) << "Failed to write oat dex information to " << file->name(); return false; } } for (size_t i = 0; i != oat_classes_.size(); ++i) { if (!oat_classes_[i]->Write(file)) { - PLOG(ERROR) << "Failed to write oat classes information"; + PLOG(ERROR) << "Failed to write oat classes information to " << file->name(); return false; } } for (size_t i = 0; i != oat_methods_.size(); ++i) { if (!oat_methods_[i]->Write(file)) { - PLOG(ERROR) << "Failed to write oat methods information"; + PLOG(ERROR) << "Failed to write oat methods information to " << file->name(); return false; } } @@ -352,7 +346,7 @@ size_t OatWriter::WriteCode(File* file) { off_t new_offset = lseek(file->Fd(), executable_offset_padding_length_, SEEK_CUR); if (static_cast<uint32_t>(new_offset) != code_offset) { PLOG(ERROR) << "Failed to seek to oat code section. Actual: " << new_offset - << " Expected: " << code_offset; + << " Expected: " << code_offset << " File: " << file->name(); return 0; } DCHECK_CODE_OFFSET(); @@ -371,12 +365,8 @@ size_t OatWriter::WriteCodeDexFiles(File* file, size_t code_offset) { return code_offset; } -size_t OatWriter::WriteCodeDexFile(File* file, - size_t code_offset, - const DexFile& dex_file) { - for (size_t class_def_index = 0; - class_def_index < dex_file.NumClassDefs(); - class_def_index++) { +size_t OatWriter::WriteCodeDexFile(File* file, size_t code_offset, const DexFile& dex_file) { + for (size_t class_def_index = 0; class_def_index < dex_file.NumClassDefs(); class_def_index++) { const DexFile::ClassDef& class_def = dex_file.GetClassDef(class_def_index); code_offset = WriteCodeClassDef(file, code_offset, dex_file, class_def); if (code_offset == 0) { @@ -386,6 +376,10 @@ size_t OatWriter::WriteCodeDexFile(File* file, return code_offset; } +void OatWriter::ReportWriteFailure(const char* what, Method* m, File* f) { + PLOG(ERROR) << "Failed to write " << what << " for " << PrettyMethod(m) << " to " << f->name(); +} + size_t OatWriter::WriteCodeClassDef(File* file, size_t code_offset, const DexFile& dex_file, @@ -436,7 +430,7 @@ size_t OatWriter::WriteCodeMethod(File* file, size_t code_offset, Method* method off_t new_offset = lseek(file->Fd(), aligned_code_delta, SEEK_CUR); if (static_cast<uint32_t>(new_offset) != aligned_code_offset) { PLOG(ERROR) << "Failed to seek to align oat code. Actual: " << new_offset - << " Expected: " << aligned_code_offset; + << " Expected: " << aligned_code_offset << " File: " << file->name(); return false; } code_offset += aligned_code_delta; @@ -456,7 +450,7 @@ size_t OatWriter::WriteCodeMethod(File* file, size_t code_offset, Method* method DCHECK((code_size == 0 && method->GetOatCodeOffset() == 0) || offset == method->GetOatCodeOffset()) << PrettyMethod(method); if (!file->WriteFully(&code[0], code_size)) { - PLOG(ERROR) << "Failed to write method code for " << PrettyMethod(method); + ReportWriteFailure("method code", method, file); return false; } code_offset += code_size; @@ -469,22 +463,22 @@ size_t OatWriter::WriteCodeMethod(File* file, size_t code_offset, Method* method uint32_t core_spill_mask = method->GetCoreSpillMask(); uint32_t fp_spill_mask = method->GetFpSpillMask(); if (!file->WriteFully(&frame_size_in_bytes, sizeof(frame_size_in_bytes))) { - PLOG(ERROR) << "Failed to write method frame size for " << PrettyMethod(method); + ReportWriteFailure("method frame size", method, file); return false; } code_offset += sizeof(frame_size_in_bytes); if (!file->WriteFully(&return_pc_offset_in_bytes, sizeof(return_pc_offset_in_bytes))) { - PLOG(ERROR) << "Failed to write method return pc offset for " << PrettyMethod(method); + ReportWriteFailure("method return pc", method, file); return false; } code_offset += sizeof(return_pc_offset_in_bytes); if (!file->WriteFully(&core_spill_mask, sizeof(core_spill_mask))) { - PLOG(ERROR) << "Failed to write method core spill mask for " << PrettyMethod(method); + ReportWriteFailure("method core spill mask", method, file); return false; } code_offset += sizeof(core_spill_mask); if (!file->WriteFully(&fp_spill_mask, sizeof(fp_spill_mask))) { - PLOG(ERROR) << "Failed to write method fp spill mask for " << PrettyMethod(method); + ReportWriteFailure("method fp spill mask", method, file); return false; } code_offset += sizeof(fp_spill_mask); @@ -502,7 +496,7 @@ size_t OatWriter::WriteCodeMethod(File* file, size_t code_offset, Method* method DCHECK((mapping_table_size == 0 && method->GetOatMappingTableOffset() == 0) || code_offset == method->GetOatMappingTableOffset()) << PrettyMethod(method); if (!file->WriteFully(&mapping_table[0], mapping_table_size)) { - PLOG(ERROR) << "Failed to write mapping table for " << PrettyMethod(method); + ReportWriteFailure("mapping table", method, file); return false; } code_offset += mapping_table_size; @@ -521,7 +515,7 @@ size_t OatWriter::WriteCodeMethod(File* file, size_t code_offset, Method* method DCHECK((vmap_table_size == 0 && method->GetOatVmapTableOffset() == 0) || code_offset == method->GetOatVmapTableOffset()) << PrettyMethod(method); if (!file->WriteFully(&vmap_table[0], vmap_table_size)) { - PLOG(ERROR) << "Failed to write vmap table for " << PrettyMethod(method); + ReportWriteFailure("vmap table", method, file); return false; } code_offset += vmap_table_size; @@ -557,7 +551,7 @@ size_t OatWriter::WriteCodeMethod(File* file, size_t code_offset, Method* method DCHECK((invoke_stub_size == 0 && method->GetOatInvokeStubOffset() == 0) || code_offset == method->GetOatInvokeStubOffset()) << PrettyMethod(method); if (!file->WriteFully(&invoke_stub[0], invoke_stub_size)) { - PLOG(ERROR) << "Failed to write invoke stub code for " << PrettyMethod(method); + ReportWriteFailure("invoke stub code", method, file); return false; } code_offset += invoke_stub_size; @@ -598,19 +592,19 @@ void OatWriter::OatDexFile::UpdateChecksum(OatHeader& oat_header) const { bool OatWriter::OatDexFile::Write(File* file) const { if (!file->WriteFully(&dex_file_location_size_, sizeof(dex_file_location_size_))) { - PLOG(ERROR) << "Failed to write dex file location length"; + PLOG(ERROR) << "Failed to write dex file location length to " << file->name(); return false; } if (!file->WriteFully(dex_file_location_data_, dex_file_location_size_)) { - PLOG(ERROR) << "Failed to write dex file location data"; + PLOG(ERROR) << "Failed to write dex file location data to " << file->name(); return false; } if (!file->WriteFully(&dex_file_checksum_, sizeof(dex_file_checksum_))) { - PLOG(ERROR) << "Failed to write dex file checksum"; + PLOG(ERROR) << "Failed to write dex file checksum to " << file->name(); return false; } if (!file->WriteFully(&classes_offset_, sizeof(classes_offset_))) { - PLOG(ERROR) << "Failed to write classes offset"; + PLOG(ERROR) << "Failed to write classes offset to " << file->name(); return false; } return true; @@ -630,7 +624,7 @@ void OatWriter::OatClasses::UpdateChecksum(OatHeader& oat_header) const { bool OatWriter::OatClasses::Write(File* file) const { if (!file->WriteFully(&methods_offsets_[0], SizeOf())) { - PLOG(ERROR) << "Failed to methods offsets"; + PLOG(ERROR) << "Failed to write methods offsets to " << file->name(); return false; } return true; @@ -650,7 +644,7 @@ void OatWriter::OatMethods::UpdateChecksum(OatHeader& oat_header) const { bool OatWriter::OatMethods::Write(File* file) const { if (!file->WriteFully(&method_offsets_[0], SizeOf())) { - PLOG(ERROR) << "Failed to method offsets"; + PLOG(ERROR) << "Failed to write method offsets to " << file->name(); return false; } return true; diff --git a/src/oat_writer.h b/src/oat_writer.h index 1c7ee9a..0642fa4 100644 --- a/src/oat_writer.h +++ b/src/oat_writer.h @@ -49,9 +49,7 @@ namespace art { class OatWriter { public: // Write an oat file. Returns true on success, false on failure. - static bool Create(const std::string& filename, - const ClassLoader* class_loader, - const Compiler& compiler); + static bool Create(File* file, const ClassLoader* class_loader, const Compiler& compiler); private: @@ -78,7 +76,7 @@ class OatWriter { size_t class_def_method_index, Method* method); - bool Write(const std::string& filename); + bool Write(File* file); bool WriteTables(File* file); size_t WriteCode(File* file); size_t WriteCodeDexFiles(File* file, @@ -94,6 +92,8 @@ class OatWriter { size_t offset, Method* method); + void ReportWriteFailure(const char* what, Method* m, File* f); + class OatDexFile { public: explicit OatDexFile(const DexFile& dex_file); diff --git a/src/oatopt.cc b/src/oatopt.cc index c792b4d..dd702d4 100644 --- a/src/oatopt.cc +++ b/src/oatopt.cc @@ -49,8 +49,12 @@ int ProcessZipFile(int zip_fd, int cache_fd, const char* zip_name, const char *f std::string oat_file_option("--oat="); oat_file_option += GetArtCacheFilenameOrDie(OatFile::DexFilenameToOatFilename(dex_file.get()->GetLocation())); - execl("/system/bin/dex2oatd", - "/system/bin/dex2oatd", + std::string dex2oat("/system/bin/dex2oat"); +#ifndef NDEBUG + dex2oat += 'd'; +#endif + + execl(dex2oat.c_str(), dex2oat.c_str(), "--runtime-arg", "-Xms64m", "--runtime-arg", "-Xmx64m", "--boot-image=/data/art-cache/boot.art", |