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
(cherry picked from commit 32c26b8f9b995250479c185172f4ffd881a59996)
Change-Id: Ic51c96b84042f769a7d33ec53fe587b68cd69df4
diff --git a/runtime/dex_file.cc b/runtime/dex_file.cc
index 0589cdd..20098e7 100644
--- a/runtime/dex_file.cc
+++ b/runtime/dex_file.cc
@@ -296,6 +296,12 @@
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<std::unique_ptr<const DexFile>>* dex_files) {
@@ -310,14 +316,13 @@
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<const DexFile> 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 @@
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<size_t>::max()) {
+ LOG(ERROR) << "Overflow in number of dex files!";
+ break;
+ }
}
return true;
@@ -973,11 +987,19 @@
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 0d07358..6b3f883 100644
--- a/runtime/dex_file.h
+++ b/runtime/dex_file.h
@@ -888,7 +888,13 @@
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 4d099e1..90b35a3 100644
--- a/runtime/dex_file_test.cc
+++ b/runtime/dex_file_test.cc
@@ -350,11 +350,20 @@
}
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 @@
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 @@
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 37e85ab..2f67263 100644
--- a/runtime/oat_file_assistant.cc
+++ b/runtime/oat_file_assistant.cc
@@ -230,8 +230,8 @@
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 @@
}
// 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) {