From 32c26b8f9b995250479c185172f4ffd881a59996 Mon Sep 17 00:00:00 2001 From: Andreas Gampe Date: Mon, 27 Apr 2015 20:01:52 -0700 Subject: [PATCH] ART: Remove multidex limit Remove the arbitrary multidex limit. If users want to use many files, allow them, but print a warning after a considerable amount. Bug: 20071800 Change-Id: Ic51c96b84042f769a7d33ec53fe587b68cd69df4 --- runtime/dex_file.cc | 38 ++++++++++++++++++++++++++++++-------- runtime/dex_file.h | 8 +++++++- runtime/dex_file_test.cc | 19 ++++++++++++++----- runtime/oat_file_assistant.cc | 8 ++++---- 4 files changed, 55 insertions(+), 18 deletions(-) diff --git a/runtime/dex_file.cc b/runtime/dex_file.cc index 0589cdd3a..20098e748 100644 --- a/runtime/dex_file.cc +++ b/runtime/dex_file.cc @@ -296,6 +296,12 @@ std::unique_ptr DexFile::Open(const ZipArchive& zip_archive, cons return dex_file; } +// Technically we do not have a limitation with respect to the number of dex files that can be in a +// multidex APK. However, it's bad practice, as each dex file requires its own tables for symbols +// (types, classes, methods, ...) and dex caches. So warn the user that we open a zip with what +// seems an excessive number. +static constexpr size_t kWarnOnManyDexFilesThreshold = 100; + bool DexFile::OpenFromZip(const ZipArchive& zip_archive, const std::string& location, std::string* error_msg, std::vector>* dex_files) { @@ -310,14 +316,13 @@ bool DexFile::OpenFromZip(const ZipArchive& zip_archive, const std::string& loca dex_files->push_back(std::move(dex_file)); // Now try some more. - size_t i = 2; // We could try to avoid std::string allocations by working on a char array directly. As we // do not expect a lot of iterations, this seems too involved and brittle. - while (i < 100) { - std::string name = StringPrintf("classes%zu.dex", i); - std::string fake_location = location + kMultiDexSeparator + name; + for (size_t i = 1; ; ++i) { + std::string name = GetMultiDexClassesDexName(i); + std::string fake_location = GetMultiDexLocation(i, location.c_str()); std::unique_ptr next_dex_file(Open(zip_archive, name.c_str(), fake_location, error_msg, &error_code)); if (next_dex_file.get() == nullptr) { @@ -329,7 +334,16 @@ bool DexFile::OpenFromZip(const ZipArchive& zip_archive, const std::string& loca dex_files->push_back(std::move(next_dex_file)); } - i++; + if (i == kWarnOnManyDexFilesThreshold) { + LOG(WARNING) << location << " has in excess of " << kWarnOnManyDexFilesThreshold + << " dex files. Please consider coalescing and shrinking the number to " + " avoid runtime overhead."; + } + + if (i == std::numeric_limits::max()) { + LOG(ERROR) << "Overflow in number of dex files!"; + break; + } } return true; @@ -973,11 +987,19 @@ bool DexFile::IsMultiDexLocation(const char* location) { return strrchr(location, kMultiDexSeparator) != nullptr; } -std::string DexFile::GetMultiDexClassesDexName(size_t number, const char* dex_location) { - if (number == 0) { +std::string DexFile::GetMultiDexClassesDexName(size_t index) { + if (index == 0) { + return "classes.dex"; + } else { + return StringPrintf("classes%zu.dex", index + 1); + } +} + +std::string DexFile::GetMultiDexLocation(size_t index, const char* dex_location) { + if (index == 0) { return dex_location; } else { - return StringPrintf("%s" kMultiDexSeparatorString "classes%zu.dex", dex_location, number + 1); + return StringPrintf("%s" kMultiDexSeparatorString "classes%zu.dex", dex_location, index + 1); } } diff --git a/runtime/dex_file.h b/runtime/dex_file.h index 0d0735828..6b3f883a5 100644 --- a/runtime/dex_file.h +++ b/runtime/dex_file.h @@ -888,7 +888,13 @@ class DexFile { return size_; } - static std::string GetMultiDexClassesDexName(size_t number, const char* dex_location); + // Return the name of the index-th classes.dex in a multidex zip file. This is classes.dex for + // index == 0, and classes{index + 1}.dex else. + static std::string GetMultiDexClassesDexName(size_t index); + + // Return the (possibly synthetic) dex location for a multidex entry. This is dex_location for + // index == 0, and dex_location + multi-dex-separator + GetMultiDexClassesDexName(index) else. + static std::string GetMultiDexLocation(size_t index, const char* dex_location); // Returns the canonical form of the given dex location. // diff --git a/runtime/dex_file_test.cc b/runtime/dex_file_test.cc index 4d099e1ca..90b35a3e0 100644 --- a/runtime/dex_file_test.cc +++ b/runtime/dex_file_test.cc @@ -350,11 +350,20 @@ TEST_F(DexFileTest, FindFieldId) { } TEST_F(DexFileTest, GetMultiDexClassesDexName) { + ASSERT_EQ("classes.dex", DexFile::GetMultiDexClassesDexName(0)); + ASSERT_EQ("classes2.dex", DexFile::GetMultiDexClassesDexName(1)); + ASSERT_EQ("classes3.dex", DexFile::GetMultiDexClassesDexName(2)); + ASSERT_EQ("classes100.dex", DexFile::GetMultiDexClassesDexName(99)); +} + +TEST_F(DexFileTest, GetMultiDexLocation) { std::string dex_location_str = "/system/app/framework.jar"; const char* dex_location = dex_location_str.c_str(); - ASSERT_EQ("/system/app/framework.jar", DexFile::GetMultiDexClassesDexName(0, dex_location)); - ASSERT_EQ("/system/app/framework.jar:classes2.dex", DexFile::GetMultiDexClassesDexName(1, dex_location)); - ASSERT_EQ("/system/app/framework.jar:classes101.dex", DexFile::GetMultiDexClassesDexName(100, dex_location)); + ASSERT_EQ("/system/app/framework.jar", DexFile::GetMultiDexLocation(0, dex_location)); + ASSERT_EQ("/system/app/framework.jar:classes2.dex", + DexFile::GetMultiDexLocation(1, dex_location)); + ASSERT_EQ("/system/app/framework.jar:classes101.dex", + DexFile::GetMultiDexLocation(100, dex_location)); } TEST_F(DexFileTest, GetDexCanonicalLocation) { @@ -363,7 +372,7 @@ TEST_F(DexFileTest, GetDexCanonicalLocation) { std::string dex_location(dex_location_real.get()); ASSERT_EQ(dex_location, DexFile::GetDexCanonicalLocation(dex_location.c_str())); - std::string multidex_location = DexFile::GetMultiDexClassesDexName(1, dex_location.c_str()); + std::string multidex_location = DexFile::GetMultiDexLocation(1, dex_location.c_str()); ASSERT_EQ(multidex_location, DexFile::GetDexCanonicalLocation(multidex_location.c_str())); std::string dex_location_sym = dex_location + "symlink"; @@ -371,7 +380,7 @@ TEST_F(DexFileTest, GetDexCanonicalLocation) { ASSERT_EQ(dex_location, DexFile::GetDexCanonicalLocation(dex_location_sym.c_str())); - std::string multidex_location_sym = DexFile::GetMultiDexClassesDexName(1, dex_location_sym.c_str()); + std::string multidex_location_sym = DexFile::GetMultiDexLocation(1, dex_location_sym.c_str()); ASSERT_EQ(multidex_location, DexFile::GetDexCanonicalLocation(multidex_location_sym.c_str())); ASSERT_EQ(0, unlink(dex_location_sym.c_str())); diff --git a/runtime/oat_file_assistant.cc b/runtime/oat_file_assistant.cc index 37e85ab37..2f6726328 100644 --- a/runtime/oat_file_assistant.cc +++ b/runtime/oat_file_assistant.cc @@ -230,8 +230,8 @@ std::vector> OatFileAssistant::LoadDexFiles( dex_files.push_back(std::move(dex_file)); // Load secondary multidex files - for (int i = 1; ; i++) { - std::string secondary_dex_location = DexFile::GetMultiDexClassesDexName(i, dex_location); + for (size_t i = 1; ; i++) { + std::string secondary_dex_location = DexFile::GetMultiDexLocation(i, dex_location); oat_dex_file = oat_file.GetOatDexFile(secondary_dex_location.c_str(), nullptr, false); if (oat_dex_file == nullptr) { // There are no more secondary dex files to load. @@ -403,9 +403,9 @@ bool OatFileAssistant::GivenOatFileIsOutOfDate(const OatFile& file) { } // Verify the dex checksums for any secondary multidex files - for (int i = 1; ; i++) { + for (size_t i = 1; ; i++) { std::string secondary_dex_location - = DexFile::GetMultiDexClassesDexName(i, dex_location_); + = DexFile::GetMultiDexLocation(i, dex_location_); const OatFile::OatDexFile* secondary_oat_dex_file = file.GetOatDexFile(secondary_dex_location.c_str(), nullptr, false); if (secondary_oat_dex_file == nullptr) { -- 2.11.0