Make the runtime option -Ximage a ParseStringList
Previously, the string spliting is done in multiple code locations,
including ImageSpace::BootImageLayout::LoadOrValidate and
GetPrimaryImageLocation (which may be called multiple times).
This change converts the -Ximage option from a string into a
ParseStringList<':'>.
It may be worth pointing out that this doesn't change the current code
expectation that each image can have a profile (e.g.
"/path/to/foo.art!/some/profile.prof").
There is a later plan to introduce new options of boot image fds with
ParseIntList<':'>. This change would make them more consistent.
Bug: 187327262
Test: boot looks normal
Test: dexopt looks normal
Test: TH
Change-Id: I82657cb725cda2d3b782cbe7a6e6d9a871e80ee7
diff --git a/dex2oat/dex2oat_image_test.cc b/dex2oat/dex2oat_image_test.cc
index a44a3e4..cfb79e5 100644
--- a/dex2oat/dex2oat_image_test.cc
+++ b/dex2oat/dex2oat_image_test.cc
@@ -422,7 +422,7 @@
ScopedObjectAccess soa(Thread::Current());
return gc::space::ImageSpace::LoadBootImage(/*boot_class_path=*/ boot_class_path,
/*boot_class_path_locations=*/ libcore_dex_files,
- image_location,
+ android::base::Split(image_location, ":"),
kRuntimeISA,
relocate,
/*executable=*/ true,
diff --git a/dexoptanalyzer/dexoptanalyzer.cc b/dexoptanalyzer/dexoptanalyzer.cc
index 998f41a..5863e45 100644
--- a/dexoptanalyzer/dexoptanalyzer.cc
+++ b/dexoptanalyzer/dexoptanalyzer.cc
@@ -368,10 +368,11 @@
std::string error_msg;
const std::vector<std::string>& bcp = runtime->GetBootClassPath();
const std::vector<std::string>& bcp_locations = runtime->GetBootClassPathLocations();
+ const std::vector<std::string>& image_locations = runtime->GetImageLocations();
const std::string bcp_locations_path = android::base::Join(bcp_locations, ':');
if (!ImageSpace::VerifyBootClassPathChecksums(checksums,
bcp_locations_path,
- runtime->GetImageLocation(),
+ ArrayRef<const std::string>(image_locations),
ArrayRef<const std::string>(bcp_locations),
ArrayRef<const std::string>(bcp),
runtime->GetInstructionSet(),
diff --git a/runtime/dexopt_test.cc b/runtime/dexopt_test.cc
index cfd7f91..9b5b473 100644
--- a/runtime/dexopt_test.cc
+++ b/runtime/dexopt_test.cc
@@ -152,7 +152,7 @@
bool match = gc::space::ImageSpace::VerifyBootClassPathChecksums(
checksums,
oat_bcp,
- image_location,
+ ArrayRef<const std::string>(&image_location, 1),
ArrayRef<const std::string>(Runtime::Current()->GetBootClassPathLocations()),
ArrayRef<const std::string>(Runtime::Current()->GetBootClassPath()),
kRuntimeISA,
diff --git a/runtime/gc/heap.cc b/runtime/gc/heap.cc
index a3dd3c7..93a594e 100644
--- a/runtime/gc/heap.cc
+++ b/runtime/gc/heap.cc
@@ -262,7 +262,7 @@
size_t non_moving_space_capacity,
const std::vector<std::string>& boot_class_path,
const std::vector<std::string>& boot_class_path_locations,
- const std::string& image_file_name,
+ const std::vector<std::string>& image_file_names,
const InstructionSet image_instruction_set,
CollectorType foreground_collector_type,
CollectorType background_collector_type,
@@ -462,7 +462,7 @@
MemMap heap_reservation;
if (space::ImageSpace::LoadBootImage(boot_class_path,
boot_class_path_locations,
- image_file_name,
+ image_file_names,
image_instruction_set,
runtime->ShouldRelocate(),
/*executable=*/ !runtime->IsAotCompiler(),
diff --git a/runtime/gc/heap.h b/runtime/gc/heap.h
index 31075b1..bd899bc 100644
--- a/runtime/gc/heap.h
+++ b/runtime/gc/heap.h
@@ -201,7 +201,7 @@
size_t non_moving_space_capacity,
const std::vector<std::string>& boot_class_path,
const std::vector<std::string>& boot_class_path_locations,
- const std::string& image_file_name,
+ const std::vector<std::string>& image_file_names,
InstructionSet image_instruction_set,
CollectorType foreground_collector_type,
CollectorType background_collector_type,
diff --git a/runtime/gc/space/image_space.cc b/runtime/gc/space/image_space.cc
index 1627d78..5ee126c 100644
--- a/runtime/gc/space/image_space.cc
+++ b/runtime/gc/space/image_space.cc
@@ -1411,10 +1411,10 @@
mutable android::base::unique_fd oat_fd;
};
- BootImageLayout(const std::string& image_location,
+ BootImageLayout(ArrayRef<const std::string> image_locations,
ArrayRef<const std::string> boot_class_path,
ArrayRef<const std::string> boot_class_path_locations)
- : image_location_(image_location),
+ : image_locations_(image_locations),
boot_class_path_(boot_class_path),
boot_class_path_locations_(boot_class_path_locations) {}
@@ -1485,7 +1485,7 @@
return boot_class_path_[bcp_index].substr(0u, bcp_slash_pos + 1u);
}
- bool VerifyImageLocation(const std::vector<std::string>& components,
+ bool VerifyImageLocation(ArrayRef<const std::string> components,
/*out*/size_t* named_components_count,
/*out*/std::string* error_msg);
@@ -1512,7 +1512,7 @@
const std::string& base_filename,
size_t bcp_index,
const std::string& profile_filename,
- ArrayRef<std::string> dependencies,
+ ArrayRef<const std::string> dependencies,
/*out*/std::string* error_msg);
bool CheckAndRemoveLastChunkChecksum(/*inout*/std::string_view* oat_checksums,
@@ -1527,7 +1527,7 @@
/*inout*/std::string_view* oat_checksums,
/*out*/std::string* error_msg);
- const std::string& image_location_;
+ ArrayRef<const std::string> image_locations_;
ArrayRef<const std::string> boot_class_path_;
ArrayRef<const std::string> boot_class_path_locations_;
@@ -1539,15 +1539,8 @@
};
std::string ImageSpace::BootImageLayout::GetPrimaryImageLocation() {
- size_t location_start = 0u;
- size_t location_end = image_location_.find(kComponentSeparator);
- while (location_end == location_start) {
- ++location_start;
- location_end = image_location_.find(location_start, kComponentSeparator);
- }
- std::string location = (location_end == std::string::npos)
- ? image_location_.substr(location_start)
- : image_location_.substr(location_start, location_end - location_start);
+ DCHECK(!image_locations_.empty());
+ std::string location = image_locations_[0];
if (location.find('/') == std::string::npos) {
// No path, so use the path from the first boot class path component.
size_t slash_pos = boot_class_path_.empty()
@@ -1562,7 +1555,7 @@
}
bool ImageSpace::BootImageLayout::VerifyImageLocation(
- const std::vector<std::string>& components,
+ ArrayRef<const std::string> components,
/*out*/size_t* named_components_count,
/*out*/std::string* error_msg) {
DCHECK(named_components_count != nullptr);
@@ -1853,7 +1846,7 @@
const std::string& base_filename,
size_t bcp_index,
const std::string& profile_filename,
- ArrayRef<std::string> dependencies,
+ ArrayRef<const std::string> dependencies,
/*out*/std::string* error_msg) {
DCHECK_LE(total_component_count_, next_bcp_index_);
DCHECK_LE(next_bcp_index_, bcp_index);
@@ -2080,8 +2073,7 @@
static_assert(ImageSpace::kImageChecksumPrefix == 'i', "Format prefix check.");
DCHECK(!validate || StartsWith(*oat_checksums, "i"));
- std::vector<std::string> components;
- Split(image_location_, kComponentSeparator, &components);
+ ArrayRef<const std::string> components = image_locations_;
size_t named_components_count = 0u;
if (!VerifyImageLocation(components, &named_components_count, error_msg)) {
return false;
@@ -2099,7 +2091,7 @@
DCHECK_EQ(named_component_locations.size(), named_components.size());
const size_t bcp_component_count = boot_class_path_.size();
size_t bcp_pos = 0u;
- ArrayRef<std::string> extension_dependencies;
+ ArrayRef<const std::string> extension_dependencies;
for (size_t i = 0, size = named_components.size(); i != size; ++i) {
const std::string& base_location = named_component_locations[i].base_location;
size_t bcp_index = named_component_locations[i].bcp_index;
@@ -2107,8 +2099,7 @@
if (extension_dependencies.empty() && !profile_filename.empty()) {
// Each extension is compiled against the same dependencies, namely the leading
// named components that were specified without providing the profile filename.
- extension_dependencies =
- ArrayRef<std::string>(components).SubArray(/*pos=*/ 0, /*length=*/ i);
+ extension_dependencies = components.SubArray(/*pos=*/ 0, /*length=*/ i);
}
if (bcp_index < bcp_pos) {
DCHECK_NE(i, 0u);
@@ -2158,8 +2149,7 @@
}
// Look for remaining components if there are any wildcard specifications.
- ArrayRef<const std::string> search_paths =
- ArrayRef<const std::string>(components).SubArray(/*pos=*/ named_components_count);
+ ArrayRef<const std::string> search_paths = components.SubArray(/*pos=*/ named_components_count);
if (!search_paths.empty()) {
const std::string& primary_base_location = named_component_locations[0].base_location;
size_t base_slash_pos = primary_base_location.rfind('/');
@@ -2229,13 +2219,13 @@
public:
BootImageLoader(const std::vector<std::string>& boot_class_path,
const std::vector<std::string>& boot_class_path_locations,
- const std::string& image_location,
+ const std::vector<std::string>& image_locations,
InstructionSet image_isa,
bool relocate,
bool executable)
: boot_class_path_(boot_class_path),
boot_class_path_locations_(boot_class_path_locations),
- image_location_(image_location),
+ image_locations_(image_locations),
image_isa_(image_isa),
relocate_(relocate),
executable_(executable),
@@ -2243,7 +2233,7 @@
}
void FindImageFiles() {
- BootImageLayout layout(image_location_, boot_class_path_, boot_class_path_locations_);
+ BootImageLayout layout(image_locations_, boot_class_path_, boot_class_path_locations_);
std::string image_location = layout.GetPrimaryImageLocation();
std::string system_filename;
bool found_image = FindImageFilenameImpl(image_location.c_str(),
@@ -3101,7 +3091,7 @@
const ArrayRef<const std::string> boot_class_path_;
const ArrayRef<const std::string> boot_class_path_locations_;
- const std::string image_location_;
+ const ArrayRef<const std::string> image_locations_;
const InstructionSet image_isa_;
const bool relocate_;
const bool executable_;
@@ -3115,7 +3105,7 @@
/*out*/std::string* error_msg) {
TimingLogger logger(__PRETTY_FUNCTION__, /*precise=*/ true, VLOG_IS_ON(image));
- BootImageLayout layout(image_location_, boot_class_path_, boot_class_path_locations_);
+ BootImageLayout layout(image_locations_, boot_class_path_, boot_class_path_locations_);
if (!layout.LoadFromSystem(image_isa_, error_msg)) {
return false;
}
@@ -3140,7 +3130,7 @@
bool ImageSpace::IsBootClassPathOnDisk(InstructionSet image_isa) {
Runtime* runtime = Runtime::Current();
- BootImageLayout layout(runtime->GetImageLocation(),
+ BootImageLayout layout(ArrayRef<const std::string>(runtime->GetImageLocations()),
ArrayRef<const std::string>(runtime->GetBootClassPath()),
ArrayRef<const std::string>(runtime->GetBootClassPathLocations()));
const std::string image_location = layout.GetPrimaryImageLocation();
@@ -3164,7 +3154,7 @@
bool ImageSpace::LoadBootImage(
const std::vector<std::string>& boot_class_path,
const std::vector<std::string>& boot_class_path_locations,
- const std::string& image_location,
+ const std::vector<std::string>& image_locations,
const InstructionSet image_isa,
bool relocate,
bool executable,
@@ -3179,13 +3169,13 @@
DCHECK(extra_reservation != nullptr);
DCHECK_NE(image_isa, InstructionSet::kNone);
- if (image_location.empty()) {
+ if (image_locations.empty()) {
return false;
}
BootImageLoader loader(boot_class_path,
boot_class_path_locations,
- image_location,
+ image_locations,
image_isa,
relocate,
executable);
@@ -3216,8 +3206,9 @@
oss << msg;
}
- LOG(ERROR) << "Could not create image space with image file '" << image_location << "'. "
- << "Attempting to fall back to imageless running. Error was: " << oss.str();
+ LOG(ERROR) << "Could not create image space with image file '"
+ << Join(image_locations, kComponentSeparator) << "'. Attempting to fall back to imageless "
+ << "running. Error was: " << oss.str();
return false;
}
@@ -3426,7 +3417,7 @@
bool ImageSpace::VerifyBootClassPathChecksums(std::string_view oat_checksums,
std::string_view oat_boot_class_path,
- const std::string& image_location,
+ ArrayRef<const std::string> image_locations,
ArrayRef<const std::string> boot_class_path_locations,
ArrayRef<const std::string> boot_class_path,
InstructionSet image_isa,
@@ -3447,7 +3438,7 @@
size_t bcp_pos = 0u;
if (StartsWith(oat_checksums, "i")) {
// Use only the matching part of the BCP for validation.
- BootImageLayout layout(image_location,
+ BootImageLayout layout(image_locations,
boot_class_path.SubArray(/*pos=*/ 0u, bcp_size),
boot_class_path_locations.SubArray(/*pos=*/ 0u, bcp_size));
std::string primary_image_location = layout.GetPrimaryImageLocation();
@@ -3458,7 +3449,7 @@
&system_filename,
&has_system)) {
*error_msg = StringPrintf("Unable to find image file for %s and %s",
- image_location.c_str(),
+ android::base::Join(image_locations, kComponentSeparator).c_str(),
GetInstructionSetString(image_isa));
return false;
}
diff --git a/runtime/gc/space/image_space.h b/runtime/gc/space/image_space.h
index 545f659..aa93247 100644
--- a/runtime/gc/space/image_space.h
+++ b/runtime/gc/space/image_space.h
@@ -124,7 +124,7 @@
static bool LoadBootImage(
const std::vector<std::string>& boot_class_path,
const std::vector<std::string>& boot_class_path_locations,
- const std::string& image_location,
+ const std::vector<std::string>& image_locations,
const InstructionSet image_isa,
bool relocate,
bool executable,
@@ -233,7 +233,7 @@
// The boot image and dex files do not need to be loaded in memory.
static bool VerifyBootClassPathChecksums(std::string_view oat_checksums,
std::string_view oat_boot_class_path,
- const std::string& image_location,
+ ArrayRef<const std::string> image_locations,
ArrayRef<const std::string> boot_class_path_locations,
ArrayRef<const std::string> boot_class_path,
InstructionSet image_isa,
diff --git a/runtime/gc/space/image_space_test.cc b/runtime/gc/space/image_space_test.cc
index 885dd21..1a53efa 100644
--- a/runtime/gc/space/image_space_test.cc
+++ b/runtime/gc/space/image_space_test.cc
@@ -128,7 +128,7 @@
ASSERT_TRUE(success) << error_msg;
}
- std::string full_image_locations;
+ std::vector<std::string> full_image_locations;
std::vector<std::unique_ptr<gc::space::ImageSpace>> boot_image_spaces;
MemMap extra_reservation;
auto load_boot_image = [&]() REQUIRES_SHARED(Locks::mutator_lock_) {
@@ -165,9 +165,9 @@
// Load extensions and test for the presence of the test string.
ScopedObjectAccess soa(Thread::Current());
ASSERT_EQ(2u, extension_image_locations.size());
- full_image_locations = base_image_location +
- ImageSpace::kComponentSeparator + extension_image_locations[0] +
- ImageSpace::kComponentSeparator + extension_image_locations[1];
+ full_image_locations = {
+ base_image_location, extension_image_locations[0], extension_image_locations[1]
+ };
bool success = load_boot_image();
ASSERT_TRUE(success);
ASSERT_EQ(bcp.size(), boot_image_spaces.size());
@@ -178,9 +178,9 @@
// Reload extensions in reverse order and test for the presence of the test string.
std::swap(bcp[bcp.size() - 2u], bcp[bcp.size() - 1u]);
std::swap(bcp_locations[bcp_locations.size() - 2u], bcp_locations[bcp_locations.size() - 1u]);
- full_image_locations = base_image_location +
- ImageSpace::kComponentSeparator + extension_image_locations[1] +
- ImageSpace::kComponentSeparator + extension_image_locations[0];
+ full_image_locations = {
+ base_image_location, extension_image_locations[1], extension_image_locations[0]
+ };
success = load_boot_image();
ASSERT_TRUE(success);
ASSERT_EQ(bcp.size(), boot_image_spaces.size());
@@ -191,8 +191,7 @@
// Reload the image without the second extension.
bcp.erase(bcp.end() - 2u);
bcp_locations.erase(bcp_locations.end() - 2u);
- full_image_locations =
- base_image_location + ImageSpace::kComponentSeparator + extension_image_locations[0];
+ full_image_locations = {base_image_location, extension_image_locations[0]};
success = load_boot_image();
ASSERT_TRUE(success);
ASSERT_EQ(bcp.size(), boot_image_spaces.size());
@@ -334,7 +333,7 @@
return gc::space::ImageSpace::VerifyBootClassPathChecksums(
checksums,
android::base::Join(bcp_locations, ':'),
- runtime->GetImageLocation(),
+ ArrayRef<const std::string>(runtime->GetImageLocations()),
ArrayRef<const std::string>(bcp_locations),
ArrayRef<const std::string>(bcp),
kRuntimeISA,
diff --git a/runtime/oat_file_assistant.cc b/runtime/oat_file_assistant.cc
index c74f19b..a5ccb69 100644
--- a/runtime/oat_file_assistant.cc
+++ b/runtime/oat_file_assistant.cc
@@ -649,7 +649,7 @@
bool result = gc::space::ImageSpace::VerifyBootClassPathChecksums(
oat_boot_class_path_checksums_view,
oat_boot_class_path_view,
- runtime->GetImageLocation(),
+ ArrayRef<const std::string>(runtime->GetImageLocations()),
ArrayRef<const std::string>(runtime->GetBootClassPathLocations()),
ArrayRef<const std::string>(runtime->GetBootClassPath()),
isa_,
diff --git a/runtime/parsed_options.cc b/runtime/parsed_options.cc
index f896511..fec2587 100644
--- a/runtime/parsed_options.cc
+++ b/runtime/parsed_options.cc
@@ -166,7 +166,7 @@
.IntoKey(M::JITCompileThreshold)
.SetCategory("ART")
.Define("-Ximage:_")
- .WithType<std::string>()
+ .WithType<ParseStringList<':'>>()
.IntoKey(M::Image)
.Define("-Xprimaryzygote")
.IntoKey(M::PrimaryZygote)
@@ -733,8 +733,8 @@
}
if (!args.Exists(M::CompilerCallbacksPtr) && !args.Exists(M::Image)) {
- std::string image = GetDefaultBootImageLocation(GetAndroidRoot());
- args.Set(M::Image, image);
+ std::string image_locations = GetDefaultBootImageLocation(GetAndroidRoot());
+ args.Set(M::Image, ParseStringList<':'>::Split(image_locations));
}
// 0 means no growth limit, and growth limit should be always <= heap size
diff --git a/runtime/parsed_options_test.cc b/runtime/parsed_options_test.cc
index 8873eb9..b9ca988 100644
--- a/runtime/parsed_options_test.cc
+++ b/runtime/parsed_options_test.cc
@@ -86,7 +86,9 @@
EXPECT_PARSED_EQ_AS_STRING_VECTOR(expected_boot_class_path, Opt::BootClassPath);
EXPECT_PARSED_EQ(class_path, Opt::ClassPath);
- EXPECT_PARSED_EQ(std::string("boot_image"), Opt::Image);
+ std::vector<std::string> boot_images = map.GetOrDefault(Opt::Image);
+ ASSERT_EQ(1U, boot_images.size());
+ EXPECT_EQ(std::string("boot_image"), boot_images[0]);
EXPECT_PARSED_EXISTS(Opt::CheckJni);
EXPECT_PARSED_EQ(2048U, Opt::MemoryInitialSize);
EXPECT_PARSED_EQ(4 * KB, Opt::MemoryMaximumSize);
diff --git a/runtime/runtime.cc b/runtime/runtime.cc
index 38f8ce1..97c1d6d 100644
--- a/runtime/runtime.cc
+++ b/runtime/runtime.cc
@@ -1342,7 +1342,7 @@
Monitor::Init(runtime_options.GetOrDefault(Opt::LockProfThreshold),
runtime_options.GetOrDefault(Opt::StackDumpLockProfThreshold));
- image_location_ = runtime_options.GetOrDefault(Opt::Image);
+ image_locations_ = runtime_options.ReleaseOrDefault(Opt::Image);
SetInstructionSet(runtime_options.GetOrDefault(Opt::ImageInstructionSet));
boot_class_path_ = runtime_options.ReleaseOrDefault(Opt::BootClassPath);
@@ -1351,13 +1351,14 @@
boot_class_path_locations_.size() == boot_class_path_.size());
if (boot_class_path_.empty()) {
// Try to extract the boot class path from the system boot image.
- if (image_location_.empty()) {
+ if (image_locations_.empty()) {
LOG(ERROR) << "Empty boot class path, cannot continue without image.";
return false;
}
std::string system_oat_filename = ImageHeader::GetOatLocationFromImageLocation(
- GetSystemImageFilename(image_location_.c_str(), instruction_set_));
- std::string system_oat_location = ImageHeader::GetOatLocationFromImageLocation(image_location_);
+ GetSystemImageFilename(image_locations_[0].c_str(), instruction_set_));
+ std::string system_oat_location = ImageHeader::GetOatLocationFromImageLocation(
+ image_locations_[0]);
std::string error_msg;
std::unique_ptr<OatFile> oat_file(OatFile::Open(/*zip_fd=*/ -1,
system_oat_filename,
@@ -1503,7 +1504,7 @@
runtime_options.GetOrDefault(Opt::NonMovingSpaceCapacity),
GetBootClassPath(),
GetBootClassPathLocations(),
- image_location_,
+ image_locations_,
instruction_set_,
// Override the collector type to CC if the read barrier config.
kUseReadBarrier ? gc::kCollectorTypeCC : xgc_option.collector_type_,
diff --git a/runtime/runtime.h b/runtime/runtime.h
index 88f7bc0..9664977 100644
--- a/runtime/runtime.h
+++ b/runtime/runtime.h
@@ -212,8 +212,8 @@
return image_compiler_options_;
}
- const std::string& GetImageLocation() const {
- return image_location_;
+ const std::vector<std::string>& GetImageLocations() const {
+ return image_locations_;
}
// Starts a runtime, which may cause threads to be started and code to run.
@@ -1089,7 +1089,7 @@
std::string compiler_executable_;
std::vector<std::string> compiler_options_;
std::vector<std::string> image_compiler_options_;
- std::string image_location_;
+ std::vector<std::string> image_locations_;
std::vector<std::string> boot_class_path_;
std::vector<std::string> boot_class_path_locations_;
diff --git a/runtime/runtime_options.def b/runtime/runtime_options.def
index 3f0e3cc..9667484 100644
--- a/runtime/runtime_options.def
+++ b/runtime/runtime_options.def
@@ -41,7 +41,7 @@
RUNTIME_OPTIONS_KEY (ParseStringList<':'>,BootClassPath) // std::vector<std::string>
RUNTIME_OPTIONS_KEY (ParseStringList<':'>,BootClassPathLocations) // std::vector<std::string>
RUNTIME_OPTIONS_KEY (std::string, ClassPath)
-RUNTIME_OPTIONS_KEY (std::string, Image)
+RUNTIME_OPTIONS_KEY (ParseStringList<':'>,Image)
RUNTIME_OPTIONS_KEY (Unit, CheckJni)
RUNTIME_OPTIONS_KEY (Unit, JniOptsForceCopy)
RUNTIME_OPTIONS_KEY (std::string, JdwpOptions, "suspend=n,server=y")
