Fix JIT Zygote in-memory compilation for primary boot image.
Before this change, if multiple named boot images are provided, JIT
Zygote will attempt to compile the primary boot image and the extension
separately and then crash because the in-memory primary boot image
cannot be accessed when it compiles the extension. This change fixed
the problem by generating a single image for everything. Alternatively,
we could pass the primary boot image by FDs, but invoking dex2oat
multiple times creates a lot of overhead.
Bug: 211973309
Bug: 215665835
Test: atest art_standalone_runtime_tests
Test: manual -
1. Patch aosp/1947128 (to remove the boot image from the APEX).
2. Build test_jitzygote_com.android.art and install it.
3. See dex2oat completes successfully without crashes.
Logs: https://paste.googleplex.com/6530152554037248
Change-Id: Id8899832fe22e33613d42397fce9ebf2a28520af
diff --git a/runtime/gc/space/image_space.cc b/runtime/gc/space/image_space.cc
index c7247cc..952b528 100644
--- a/runtime/gc/space/image_space.cc
+++ b/runtime/gc/space/image_space.cc
@@ -20,7 +20,9 @@
#include <sys/types.h>
#include <unistd.h>
+#include <memory>
#include <random>
+#include <string>
#include "android-base/stringprintf.h"
#include "android-base/strings.h"
@@ -146,15 +148,15 @@
const char* file_description,
/*out*/ImageHeader* image_header,
/*out*/std::string* error_msg) {
- if (!image_file->ReadFully(image_header, sizeof(ImageHeader))) {
- *error_msg = StringPrintf("Unable to read image header from \"%s\"", file_description);
- return false;
- }
- if (!image_header->IsValid()) {
- *error_msg = StringPrintf("Image header from \"%s\" is invalid", file_description);
- return false;
- }
- return true;
+ if (!image_file->PreadFully(image_header, sizeof(ImageHeader), /*offset=*/ 0)) {
+ *error_msg = StringPrintf("Unable to read image header from \"%s\"", file_description);
+ return false;
+ }
+ if (!image_header->IsValid()) {
+ *error_msg = StringPrintf("Image header from \"%s\" is invalid", file_description);
+ return false;
+ }
+ return true;
}
static bool ReadSpecificImageHeader(const char* filename,
@@ -2243,31 +2245,44 @@
if (validate) {
return false;
}
- VLOG(image) << "Error reading named image component header for " << base_location
- << ", error: " << local_error_msg;
+ LOG(ERROR) << "Error reading named image component header for " << base_location
+ << ", error: " << local_error_msg;
+ // If the primary boot image is invalid, we generate a single full image. This is faster than
+ // generating the primary boot image and the extension separately.
+ if (bcp_index == 0) {
+ // We must at least have profiles for the core libraries.
+ if (profile_filenames.empty()) {
+ *error_msg = "Full boot image cannot be compiled because no profile is provided.";
+ return false;
+ }
+ std::vector<std::string> all_profiles;
+ for (const NamedComponentLocation& named_component_location : named_component_locations) {
+ const std::vector<std::string>& profiles = named_component_location.profile_filenames;
+ all_profiles.insert(all_profiles.end(), profiles.begin(), profiles.end());
+ }
+ if (!CompileBootclasspathElements(base_location,
+ base_filename,
+ /*bcp_index=*/ 0,
+ all_profiles,
+ /*dependencies=*/ ArrayRef<const std::string>{},
+ &local_error_msg)) {
+ *error_msg =
+ StringPrintf("Full boot image cannot be compiled: %s", local_error_msg.c_str());
+ return false;
+ }
+ // No extensions are needed.
+ return true;
+ }
if (profile_filenames.empty() ||
!CompileBootclasspathElements(base_location,
base_filename,
bcp_index,
profile_filenames,
- /*dependencies=*/ bcp_index == 0 ?
- ArrayRef<const std::string>{} :
- components.SubArray(/*pos=*/ 0, /*length=*/ 1),
+ components.SubArray(/*pos=*/ 0, /*length=*/ 1),
&local_error_msg)) {
if (!profile_filenames.empty()) {
- VLOG(image) << "Error compiling bootclasspath for " << boot_class_path_[bcp_index]
- << " error: " << local_error_msg;
- // We cannot continue without the primary boot image because other boot images use it as
- // a dependency.
- if (bcp_index == 0) {
- LOG(ERROR) << "Primary boot image cannot be compiled: " << local_error_msg;
- return false;
- }
- } else {
- if (bcp_index == 0) {
- LOG(ERROR) << "Primary boot image cannot be compiled because no profile is provided.";
- return false;
- }
+ LOG(ERROR) << "Error compiling boot image extension for " << boot_class_path_[bcp_index]
+ << ", error: " << local_error_msg;
}
bcp_pos = bcp_index + 1u; // Skip at least this component.
DCHECK_GT(bcp_pos, GetNextBcpIndex());
@@ -2488,8 +2503,12 @@
}
// Update `max_image_space_dependencies` if all previous BCP components
// were covered and loading the current chunk succeeded.
+ size_t total_component_count = 0;
+ for (const std::unique_ptr<ImageSpace>& space : spaces) {
+ total_component_count += space->GetComponentCount();
+ }
if (max_image_space_dependencies == chunk.start_index &&
- spaces.size() == chunk.start_index + chunk.component_count) {
+ total_component_count == chunk.start_index + chunk.component_count) {
max_image_space_dependencies = chunk.start_index + chunk.component_count;
}
}
@@ -3804,9 +3823,9 @@
std::string(oat_checksums).c_str());
return false;
}
- if (image_pos != oat_bcp_size) {
+ if (bcp_pos != oat_bcp_size) {
*error_msg = StringPrintf("Component count mismatch between checksums (%zu) and BCP (%zu)",
- image_pos,
+ bcp_pos,
oat_bcp_size);
return false;
}
diff --git a/runtime/gc/space/image_space_test.cc b/runtime/gc/space/image_space_test.cc
index ce984f9..86048bd 100644
--- a/runtime/gc/space/image_space_test.cc
+++ b/runtime/gc/space/image_space_test.cc
@@ -371,30 +371,12 @@
}
}
-template <bool kImage, bool kRelocate, bool kProfile>
+template <bool kImage, bool kRelocate>
class ImageSpaceLoadingTest : public CommonRuntimeTest {
protected:
void SetUpRuntimeOptions(RuntimeOptions* options) override {
- std::string image_location = GetCoreArtLocation();
- if (!kImage) {
- missing_image_base_ = std::make_unique<ScratchFile>();
- image_location = missing_image_base_->GetFilename() + ".art";
- }
- // Compiling the primary boot image into a single image is not allowed on host.
- if (kProfile && kIsTargetBuild) {
- std::vector<std::string> dex_files(GetLibCoreDexFileNames());
- profile1_ = std::make_unique<ScratchFile>();
- GenerateBootProfile(ArrayRef<const std::string>(dex_files),
- profile1_->GetFile(),
- /*method_frequency=*/6,
- /*type_frequency=*/6);
- profile2_ = std::make_unique<ScratchFile>();
- GenerateBootProfile(ArrayRef<const std::string>(dex_files),
- profile2_->GetFile(),
- /*method_frequency=*/8,
- /*type_frequency=*/8);
- image_location += "!" + profile1_->GetFilename() + "!" + profile2_->GetFilename();
- }
+ missing_image_base_ = std::make_unique<ScratchFile>();
+ std::string image_location = PrepareImageLocation();
options->emplace_back(android::base::StringPrintf("-Ximage:%s", image_location.c_str()),
nullptr);
options->emplace_back(kRelocate ? "-Xrelocate" : "-Xnorelocate", nullptr);
@@ -423,47 +405,142 @@
missing_image_base_.reset();
}
- private:
+ virtual std::string PrepareImageLocation() {
+ std::string image_location = GetCoreArtLocation();
+ if (!kImage) {
+ image_location = missing_image_base_->GetFilename() + ".art";
+ }
+ return image_location;
+ }
+
+ void CheckImageSpaceAndOatFile(size_t space_count) {
+ const std::vector<ImageSpace*>& image_spaces =
+ Runtime::Current()->GetHeap()->GetBootImageSpaces();
+ ASSERT_EQ(image_spaces.size(), space_count);
+
+ for (size_t i = 0; i < space_count; i++) {
+ // This test does not support multi-image compilation.
+ ASSERT_NE(image_spaces[i]->GetImageHeader().GetImageReservationSize(), 0u);
+
+ const OatFile* oat_file = image_spaces[i]->GetOatFile();
+ ASSERT_TRUE(oat_file != nullptr);
+
+ // Compiled by JIT Zygote.
+ EXPECT_EQ(oat_file->GetCompilerFilter(), CompilerFilter::Filter::kVerify);
+ }
+ }
+
std::unique_ptr<ScratchFile> missing_image_base_;
- std::unique_ptr<ScratchFile> profile1_;
- std::unique_ptr<ScratchFile> profile2_;
+
+ private:
UniqueCPtr<const char[]> old_dex2oat_bcp_;
};
-using ImageSpaceNoDex2oatTest =
- ImageSpaceLoadingTest</*kImage=*/true, /*kRelocate=*/true, /*kProfile=*/false>;
+using ImageSpaceNoDex2oatTest = ImageSpaceLoadingTest</*kImage=*/true, /*kRelocate=*/true>;
TEST_F(ImageSpaceNoDex2oatTest, Test) {
EXPECT_FALSE(Runtime::Current()->GetHeap()->GetBootImageSpaces().empty());
}
using ImageSpaceNoRelocateNoDex2oatTest =
- ImageSpaceLoadingTest</*kImage=*/true, /*kRelocate=*/false, /*kProfile=*/false>;
+ ImageSpaceLoadingTest</*kImage=*/true, /*kRelocate=*/false>;
TEST_F(ImageSpaceNoRelocateNoDex2oatTest, Test) {
EXPECT_FALSE(Runtime::Current()->GetHeap()->GetBootImageSpaces().empty());
}
-using ImageSpaceNoImageNoProfileTest =
- ImageSpaceLoadingTest</*kImage=*/false, /*kRelocate=*/true, /*kProfile=*/false>;
+using ImageSpaceNoImageNoProfileTest = ImageSpaceLoadingTest</*kImage=*/false, /*kRelocate=*/true>;
TEST_F(ImageSpaceNoImageNoProfileTest, Test) {
// Imageless mode.
EXPECT_TRUE(Runtime::Current()->GetHeap()->GetBootImageSpaces().empty());
}
-using ImageSpaceNoImageTest =
- ImageSpaceLoadingTest</*kImage=*/false, /*kRelocate=*/true, /*kProfile=*/true>;
-TEST_F(ImageSpaceNoImageTest, Test) {
+class ImageSpaceLoadingSingleComponentWithProfilesTest
+ : public ImageSpaceLoadingTest</*kImage=*/false, /*kRelocate=*/true> {
+ protected:
+ std::string PrepareImageLocation() override {
+ std::string image_location = missing_image_base_->GetFilename() + ".art";
+ // Compiling the primary boot image into a single image is not allowed on host.
+ if (kIsTargetBuild) {
+ std::vector<std::string> dex_files(GetLibCoreDexFileNames());
+ profile1_ = std::make_unique<ScratchFile>();
+ GenerateBootProfile(ArrayRef<const std::string>(dex_files),
+ profile1_->GetFile(),
+ /*method_frequency=*/6,
+ /*type_frequency=*/6);
+ profile2_ = std::make_unique<ScratchFile>();
+ GenerateBootProfile(ArrayRef<const std::string>(dex_files),
+ profile2_->GetFile(),
+ /*method_frequency=*/8,
+ /*type_frequency=*/8);
+ image_location += "!" + profile1_->GetFilename() + "!" + profile2_->GetFilename();
+ }
+ // "/path/to/image.art!/path/to/profile1!/path/to/profile2"
+ return image_location;
+ }
+
+ private:
+ std::unique_ptr<ScratchFile> profile1_;
+ std::unique_ptr<ScratchFile> profile2_;
+};
+
+TEST_F(ImageSpaceLoadingSingleComponentWithProfilesTest, Test) {
// Compiling the primary boot image into a single image is not allowed on host.
TEST_DISABLED_FOR_HOST();
- const std::vector<ImageSpace*>& image_spaces =
- Runtime::Current()->GetHeap()->GetBootImageSpaces();
- ASSERT_FALSE(image_spaces.empty());
+ CheckImageSpaceAndOatFile(/*space_count=*/1);
+}
- const OatFile* oat_file = image_spaces[0]->GetOatFile();
- ASSERT_TRUE(oat_file != nullptr);
+class ImageSpaceLoadingMultipleComponentsWithProfilesTest
+ : public ImageSpaceLoadingTest</*kImage=*/false, /*kRelocate=*/true> {
+ protected:
+ std::string PrepareImageLocation() override {
+ std::vector<std::string> dex_files(GetLibCoreDexFileNames());
+ CHECK_GE(dex_files.size(), 2);
+ std::string image_location_1 = missing_image_base_->GetFilename() + ".art";
+ std::string image_location_2 =
+ missing_image_base_->GetFilename() + "-" + Stem(dex_files[dex_files.size() - 1]) + ".art";
+ // Compiling the primary boot image into a single image is not allowed on host.
+ if (kIsTargetBuild) {
+ profile1_ = std::make_unique<ScratchFile>();
+ GenerateBootProfile(ArrayRef<const std::string>(dex_files).SubArray(
+ /*pos=*/0, /*length=*/dex_files.size() - 1),
+ profile1_->GetFile(),
+ /*method_frequency=*/6,
+ /*type_frequency=*/6);
+ image_location_1 += "!" + profile1_->GetFilename();
+ profile2_ = std::make_unique<ScratchFile>();
+ GenerateBootProfile(ArrayRef<const std::string>(dex_files).SubArray(
+ /*pos=*/dex_files.size() - 1, /*length=*/1),
+ profile2_->GetFile(),
+ /*method_frequency=*/8,
+ /*type_frequency=*/8);
+ image_location_2 += "!" + profile2_->GetFilename();
+ }
+ // "/path/to/image.art!/path/to/profile1:/path/to/image-lastdex.art!/path/to/profile2"
+ return image_location_1 + ":" + image_location_2;
+ }
- // Compiled by JIT Zygote.
- EXPECT_EQ(oat_file->GetCompilerFilter(), CompilerFilter::Filter::kVerify);
+ std::string Stem(std::string filename) {
+ size_t last_slash = filename.rfind('/');
+ if (last_slash != std::string::npos) {
+ filename = filename.substr(last_slash + 1);
+ }
+ size_t last_dot = filename.rfind('.');
+ if (last_dot != std::string::npos) {
+ filename.resize(last_dot);
+ }
+ return filename;
+ }
+
+ private:
+ std::unique_ptr<ScratchFile> profile1_;
+ std::unique_ptr<ScratchFile> profile2_;
+};
+
+TEST_F(ImageSpaceLoadingMultipleComponentsWithProfilesTest, Test) {
+ // Compiling the primary boot image into a single image is not allowed on host.
+ TEST_DISABLED_FOR_HOST();
+
+ CheckImageSpaceAndOatFile(/*space_count=*/1);
}
} // namespace space
diff --git a/runtime/runtime.cc b/runtime/runtime.cc
index cd8f936..5f52010 100644
--- a/runtime/runtime.cc
+++ b/runtime/runtime.cc
@@ -1426,11 +1426,11 @@
boot_class_path_vdex_fds_ = runtime_options.ReleaseOrDefault(Opt::BootClassPathVdexFds);
boot_class_path_oat_fds_ = runtime_options.ReleaseOrDefault(Opt::BootClassPathOatFds);
CHECK(boot_class_path_image_fds_.empty() ||
- boot_class_path_image_fds_.size() == boot_class_path_fds_.size());
+ boot_class_path_image_fds_.size() == boot_class_path_.size());
CHECK(boot_class_path_vdex_fds_.empty() ||
- boot_class_path_vdex_fds_.size() == boot_class_path_fds_.size());
+ boot_class_path_vdex_fds_.size() == boot_class_path_.size());
CHECK(boot_class_path_oat_fds_.empty() ||
- boot_class_path_oat_fds_.size() == boot_class_path_fds_.size());
+ boot_class_path_oat_fds_.size() == boot_class_path_.size());
class_path_string_ = runtime_options.ReleaseOrDefault(Opt::ClassPath);
properties_ = runtime_options.ReleaseOrDefault(Opt::PropertiesList);