Support compiling partial system server jars.
Before this change, odrefresh compiles system server jars atomically.
After this change, odrefresh can compile only the system server jars
whose artifacts are missing. This can benefit users in couple ways:
1. When a previous odrefresh run failed due to low storage space,
subsequent runs can continue from the failure point instead of starting
over.
2. When we update odrefresh to compile more things before a change has
been made to the build system to preopt those things at build-time
(e.g., b/203198541), this change can avoid too much regression on the
first boot time by not compiling things that are already preopted.
3. When some artifacts are missing from the system image for unknown
reasons, this change can avoid too much regression on the first boot
time. (This should rarely happen because artifacts' existence is checked
at built-time.)
This CL also includes some minor changes:
1. Introduce a struct `CompilationOptions` to represent what to compile
in a more structured way.
2. Update the comments about compilation space estimation based on the
latest observation.
3. Improve the boot animation progress reporting to reflect the real
number of compilations.
4. Fix the exit code returned on low storage space. (Was `kOkay`, which
was wrong because odsign does not sign artifacts on `kOkay`)
Bug: 203198541
Test: atest art_standalone_odrefresh_tests
Test: odsign_e2e_tests
Change-Id: Id13fa26fa2327a82daff8d8d6fdefe11cd0928a2
diff --git a/odrefresh/odrefresh.cc b/odrefresh/odrefresh.cc
index e92e60c..2bfc49a 100644
--- a/odrefresh/odrefresh.cc
+++ b/odrefresh/odrefresh.cc
@@ -34,6 +34,7 @@
#include <cstdlib>
#include <filesystem>
#include <fstream>
+#include <functional>
#include <initializer_list>
#include <iosfwd>
#include <iostream>
@@ -41,6 +42,7 @@
#include <memory>
#include <optional>
#include <ostream>
+#include <set>
#include <sstream>
#include <string>
#include <string_view>
@@ -424,10 +426,10 @@
// the users data partition.
//
// We do not have a good way of pre-computing the required space for a compilation step, but
- // typically observe 16MB as the largest size of an AOT artifact. Since there are three
- // AOT artifacts per compilation step - an image file, executable file, and a verification
- // data file - the threshold is three times 16MB.
- static constexpr uint64_t kMinimumSpaceForCompilation = 3 * 16 * 1024 * 1024;
+ // typically observe no more than 48MiB as the largest total size of AOT artifacts for a single
+ // dex2oat invocation, which includes an image file, an executable file, and a verification data
+ // file.
+ static constexpr uint64_t kMinimumSpaceForCompilation = 48 * 1024 * 1024;
uint64_t bytes_available;
const std::string& art_apex_data_path = GetArtApexData();
@@ -479,7 +481,7 @@
: OnDeviceRefresh(config,
Concatenate({config.GetArtifactDirectory(), "/", kCacheInfoFile}),
std::make_unique<ExecUtils>(),
- std::move(OdrDexopt::Create(config, std::make_unique<ExecUtils>()))) {}
+ OdrDexopt::Create(config, std::make_unique<ExecUtils>())) {}
OnDeviceRefresh::OnDeviceRefresh(const OdrConfig& config,
const std::string& cache_info_filename,
@@ -595,11 +597,10 @@
art_apex::write(out, info);
}
-void OnDeviceRefresh::ReportNextBootAnimationProgress(uint32_t current_compilation) const {
- uint32_t number_of_compilations =
- config_.GetBootExtensionIsas().size() + systemserver_compilable_jars_.size();
- // We arbitrarily show progress until 90%, expecting that our compilations
- // take a large chunk of boot time.
+static void ReportNextBootAnimationProgress(uint32_t current_compilation,
+ uint32_t number_of_compilations) {
+ // We arbitrarily show progress until 90%, expecting that our compilations take a large chunk of
+ // boot time.
uint32_t value = (90 * current_compilation) / number_of_compilations;
android::base::SetProperty("service.bootanim.progress", std::to_string(value));
}
@@ -670,17 +671,20 @@
WARN_UNUSED bool OnDeviceRefresh::SystemServerArtifactsExist(
bool on_system,
/*out*/ std::string* error_msg,
+ /*out*/ std::set<std::string>* jars_missing_artifacts,
/*out*/ std::vector<std::string>* checked_artifacts) const {
for (const std::string& jar_path : systemserver_compilable_jars_) {
const std::string image_location = GetSystemServerImagePath(on_system, jar_path);
const OdrArtifacts artifacts = OdrArtifacts::ForSystemServer(image_location);
// .art files are optional and are not generated for all jars by the build system.
const bool check_art_file = !on_system;
- if (!ArtifactsExist(artifacts, check_art_file, error_msg, checked_artifacts)) {
- return false;
+ std::string error_msg_tmp;
+ if (!ArtifactsExist(artifacts, check_art_file, &error_msg_tmp, checked_artifacts)) {
+ jars_missing_artifacts->insert(jar_path);
+ *error_msg = error_msg->empty() ? error_msg_tmp : *error_msg + "\n" + error_msg_tmp;
}
}
- return true;
+ return jars_missing_artifacts->empty();
}
WARN_UNUSED bool OnDeviceRefresh::CheckBootExtensionArtifactsAreUpToDate(
@@ -782,12 +786,20 @@
return true;
}
-WARN_UNUSED bool OnDeviceRefresh::CheckSystemServerArtifactsAreUpToDate(
+bool OnDeviceRefresh::CheckSystemServerArtifactsAreUpToDate(
OdrMetrics& metrics,
const std::vector<apex::ApexInfo>& apex_info_list,
const std::optional<art_apex::CacheInfo>& cache_info,
+ /*out*/ std::set<std::string>* jars_to_compile,
/*out*/ std::vector<std::string>* checked_artifacts) const {
+ auto compile_all = [&, this]() {
+ *jars_to_compile = AllSystemServerJars();
+ return false;
+ };
+
std::string error_msg;
+ std::set<std::string> jars_missing_artifacts_on_system;
+ bool artifacts_on_system_up_to_date = false;
if (std::all_of(apex_info_list.begin(),
apex_info_list.end(),
@@ -795,12 +807,14 @@
LOG(INFO) << "Factory APEXes mounted.";
// APEXes are not updated, so we can use the artifacts on /system. Check if they exist.
- if (SystemServerArtifactsExist(/*on_system=*/true, &error_msg)) {
+ if (SystemServerArtifactsExist(
+ /*on_system=*/true, &error_msg, &jars_missing_artifacts_on_system)) {
return true;
}
LOG(INFO) << "Incomplete system server artifacts on /system. " << error_msg;
LOG(INFO) << "Checking cache.";
+ artifacts_on_system_up_to_date = true;
}
if (!cache_info.has_value()) {
@@ -808,7 +822,11 @@
// before.
PLOG(INFO) << "No prior cache-info file: " << QuotePath(cache_info_filename_);
metrics.SetTrigger(OdrMetrics::Trigger::kMissingArtifacts);
- return false;
+ if (artifacts_on_system_up_to_date) {
+ *jars_to_compile = jars_missing_artifacts_on_system;
+ return false;
+ }
+ return compile_all();
}
// Check whether the current cached module info differs from the current module info.
@@ -817,7 +835,7 @@
if (cached_module_info_list == nullptr) {
LOG(INFO) << "Missing APEX info list from cache-info.";
metrics.SetTrigger(OdrMetrics::Trigger::kApexVersionMismatch);
- return false;
+ return compile_all();
}
std::unordered_map<std::string, const art_apex::ModuleInfo*> cached_module_info_map;
@@ -825,7 +843,7 @@
if (!module_info.hasName()) {
LOG(INFO) << "Unexpected module info from cache-info. Missing module name.";
metrics.SetTrigger(OdrMetrics::Trigger::kApexVersionMismatch);
- return false;
+ return compile_all();
}
cached_module_info_map[module_info.getName()] = &module_info;
}
@@ -836,7 +854,7 @@
LOG(INFO) << "Missing APEX info from cache-info (" << current_apex_info.getModuleName()
<< ").";
metrics.SetTrigger(OdrMetrics::Trigger::kApexVersionMismatch);
- return false;
+ return compile_all();
}
const art_apex::ModuleInfo* cached_module_info = it->second;
@@ -846,7 +864,7 @@
<< cached_module_info->getVersionCode()
<< " != " << current_apex_info.getVersionCode() << ").";
metrics.SetTrigger(OdrMetrics::Trigger::kApexVersionMismatch);
- return false;
+ return compile_all();
}
if (cached_module_info->getVersionName() != current_apex_info.getVersionName()) {
@@ -854,7 +872,7 @@
<< cached_module_info->getVersionName()
<< " != " << current_apex_info.getVersionName() << ").";
metrics.SetTrigger(OdrMetrics::Trigger::kApexVersionMismatch);
- return false;
+ return compile_all();
}
if (!cached_module_info->hasLastUpdateMillis() ||
@@ -863,7 +881,7 @@
<< cached_module_info->getLastUpdateMillis()
<< " != " << current_apex_info.getLastUpdateMillis() << ").";
metrics.SetTrigger(OdrMetrics::Trigger::kApexVersionMismatch);
- return false;
+ return compile_all();
}
}
@@ -883,7 +901,7 @@
!cache_info->getFirstSystemServerClasspath()->hasComponent())) {
LOG(INFO) << "Missing SystemServerClasspath components.";
metrics.SetTrigger(OdrMetrics::Trigger::kDexFilesChanged);
- return false;
+ return compile_all();
}
const std::vector<art_apex::Component>& system_server_components =
@@ -891,7 +909,7 @@
if (!CheckComponents(expected_system_server_components, system_server_components, &error_msg)) {
LOG(INFO) << "SystemServerClasspath components mismatch: " << error_msg;
metrics.SetTrigger(OdrMetrics::Trigger::kDexFilesChanged);
- return false;
+ return compile_all();
}
const std::vector<art_apex::Component> expected_bcp_components =
@@ -910,16 +928,32 @@
metrics.SetTrigger(OdrMetrics::Trigger::kDexFilesChanged);
// Boot classpath components can be dependencies of system_server components, so system_server
// components need to be recompiled if boot classpath components are changed.
- return false;
+ return compile_all();
}
- if (!SystemServerArtifactsExist(/*on_system=*/false, &error_msg, checked_artifacts)) {
+ std::set<std::string> jars_missing_artifacts_on_data;
+ if (!SystemServerArtifactsExist(
+ /*on_system=*/false, &error_msg, &jars_missing_artifacts_on_data, checked_artifacts)) {
+ if (artifacts_on_system_up_to_date) {
+ // Check if the remaining system_server artifacts are on /data.
+ std::set_intersection(jars_missing_artifacts_on_system.begin(),
+ jars_missing_artifacts_on_system.end(),
+ jars_missing_artifacts_on_data.begin(),
+ jars_missing_artifacts_on_data.end(),
+ std::inserter(*jars_to_compile, jars_to_compile->end()));
+ if (!jars_to_compile->empty()) {
+ LOG(INFO) << "Incomplete system_server artifacts on /data. " << error_msg;
+ metrics.SetTrigger(OdrMetrics::Trigger::kMissingArtifacts);
+ return false;
+ }
+
+ LOG(INFO) << "Found the remaining system_server artifacts on /data.";
+ return true;
+ }
+
LOG(INFO) << "Incomplete system_server artifacts. " << error_msg;
- // No clean-up is required here: we have boot extension artifacts. The method
- // `SystemServerArtifactsExistOnData()` checks in compilation order so it is possible some of
- // the artifacts are here. We likely ran out of space compiling the system_server artifacts.
- // Any artifacts present are usable.
metrics.SetTrigger(OdrMetrics::Trigger::kMissingArtifacts);
+ *jars_to_compile = jars_missing_artifacts_on_data;
return false;
}
@@ -990,16 +1024,15 @@
return {};
}
-WARN_UNUSED ExitCode OnDeviceRefresh::CheckArtifactsAreUpToDate(
- OdrMetrics& metrics,
- /*out*/ std::vector<InstructionSet>* compile_boot_extensions,
- /*out*/ bool* compile_system_server) const {
+WARN_UNUSED ExitCode
+OnDeviceRefresh::CheckArtifactsAreUpToDate(OdrMetrics& metrics,
+ /*out*/ CompilationOptions* compilation_options) const {
metrics.SetStage(OdrMetrics::Stage::kCheck);
// Clean-up helper used to simplify clean-ups and handling failures there.
auto cleanup_and_compile_all = [&, this]() {
- *compile_boot_extensions = config_.GetBootExtensionIsas();
- *compile_system_server = true;
+ compilation_options->compile_boot_extensions_for_isas = config_.GetBootExtensionIsas();
+ compilation_options->system_server_jars_to_compile = AllSystemServerJars();
return RemoveArtifactsDirectory() ? ExitCode::kCompilationRequired : ExitCode::kCleanupFailed;
};
@@ -1045,21 +1078,24 @@
for (const InstructionSet isa : config_.GetBootExtensionIsas()) {
if (!CheckBootExtensionArtifactsAreUpToDate(
metrics, isa, art_apex_info.value(), cache_info, &checked_artifacts)) {
- compile_boot_extensions->push_back(isa);
+ compilation_options->compile_boot_extensions_for_isas.push_back(isa);
// system_server artifacts are invalid without valid boot extension artifacts.
if (isa == system_server_isa) {
- *compile_system_server = true;
+ compilation_options->system_server_jars_to_compile = AllSystemServerJars();
}
}
}
- if (!*compile_system_server &&
- !CheckSystemServerArtifactsAreUpToDate(
- metrics, apex_info_list.value(), cache_info, &checked_artifacts)) {
- *compile_system_server = true;
+ if (compilation_options->system_server_jars_to_compile.empty()) {
+ CheckSystemServerArtifactsAreUpToDate(metrics,
+ apex_info_list.value(),
+ cache_info,
+ &compilation_options->system_server_jars_to_compile,
+ &checked_artifacts);
}
- bool compilation_required = !compile_boot_extensions->empty() || *compile_system_server;
+ bool compilation_required = (!compilation_options->compile_boot_extensions_for_isas.empty() ||
+ !compilation_options->system_server_jars_to_compile.empty());
if (compilation_required) {
if (!config_.GetPartialCompilation()) {
@@ -1075,11 +1111,12 @@
return compilation_required ? ExitCode::kCompilationRequired : ExitCode::kOkay;
}
-WARN_UNUSED bool OnDeviceRefresh::CompileBootExtensionArtifacts(const InstructionSet isa,
- const std::string& staging_dir,
- OdrMetrics& metrics,
- uint32_t* dex2oat_invocation_count,
- std::string* error_msg) const {
+WARN_UNUSED bool OnDeviceRefresh::CompileBootExtensionArtifacts(
+ const InstructionSet isa,
+ const std::string& staging_dir,
+ OdrMetrics& metrics,
+ const std::function<void()>& on_dex2oat_success,
+ std::string* error_msg) const {
ScopedOdrCompilationTimer compilation_timer(metrics);
DexoptBcpExtArgs dexopt_args;
@@ -1186,22 +1223,29 @@
return false;
}
- *dex2oat_invocation_count = *dex2oat_invocation_count + 1;
- ReportNextBootAnimationProgress(*dex2oat_invocation_count);
-
+ on_dex2oat_success();
return true;
}
-WARN_UNUSED bool OnDeviceRefresh::CompileSystemServerArtifacts(const std::string& staging_dir,
- OdrMetrics& metrics,
- uint32_t* dex2oat_invocation_count,
- std::string* error_msg) const {
+WARN_UNUSED bool OnDeviceRefresh::CompileSystemServerArtifacts(
+ const std::string& staging_dir,
+ OdrMetrics& metrics,
+ const std::set<std::string>& system_server_jars_to_compile,
+ const std::function<void()>& on_dex2oat_success,
+ std::string* error_msg) const {
ScopedOdrCompilationTimer compilation_timer(metrics);
std::vector<std::string> classloader_context;
const std::string dex2oat = config_.GetDex2Oat();
const InstructionSet isa = config_.GetSystemServerIsa();
for (const std::string& jar : systemserver_compilable_jars_) {
+ auto scope_guard =
+ android::base::make_scope_guard([&]() { classloader_context.emplace_back(jar); });
+
+ if (!ContainsElement(system_server_jars_to_compile, jar)) {
+ continue;
+ }
+
std::vector<std::unique_ptr<File>> readonly_files_raii;
DexoptSystemServerArgs dexopt_args;
dexopt_args.isa = InstructionSetToAidlIsa(isa);
@@ -1230,12 +1274,9 @@
const std::string image_location = GetSystemServerImagePath(/*on_system=*/false, jar);
const std::string install_location = android::base::Dirname(image_location);
- if (classloader_context.empty()) {
- // All images are in the same directory, we only need to check on the first iteration.
- if (!EnsureDirectoryExists(install_location)) {
- metrics.SetStatus(OdrMetrics::Status::kIoError);
- return false;
- }
+ if (!EnsureDirectoryExists(install_location)) {
+ metrics.SetStatus(OdrMetrics::Status::kIoError);
+ return false;
}
OdrArtifacts artifacts = OdrArtifacts::ForSystemServer(image_location);
@@ -1323,18 +1364,14 @@
return false;
}
- *dex2oat_invocation_count = *dex2oat_invocation_count + 1;
- ReportNextBootAnimationProgress(*dex2oat_invocation_count);
- classloader_context.emplace_back(jar);
+ on_dex2oat_success();
}
return true;
}
-WARN_UNUSED ExitCode
-OnDeviceRefresh::Compile(OdrMetrics& metrics,
- const std::vector<InstructionSet>& compile_boot_extensions,
- bool compile_system_server) const {
+WARN_UNUSED ExitCode OnDeviceRefresh::Compile(OdrMetrics& metrics,
+ const CompilationOptions& compilation_options) const {
const char* staging_dir = nullptr;
metrics.SetStage(OdrMetrics::Stage::kPreparation);
@@ -1354,22 +1391,29 @@
std::string error_msg;
uint32_t dex2oat_invocation_count = 0;
- ReportNextBootAnimationProgress(dex2oat_invocation_count);
+ uint32_t total_dex2oat_invocation_count =
+ compilation_options.compile_boot_extensions_for_isas.size() +
+ compilation_options.system_server_jars_to_compile.size();
+ ReportNextBootAnimationProgress(dex2oat_invocation_count, total_dex2oat_invocation_count);
+ auto advance_animation_progress = [&]() {
+ ReportNextBootAnimationProgress(++dex2oat_invocation_count, total_dex2oat_invocation_count);
+ };
const auto& bcp_instruction_sets = config_.GetBootExtensionIsas();
DCHECK(!bcp_instruction_sets.empty() && bcp_instruction_sets.size() <= 2);
- for (const InstructionSet isa : compile_boot_extensions) {
+ for (const InstructionSet isa : compilation_options.compile_boot_extensions_for_isas) {
auto stage = (isa == bcp_instruction_sets.front()) ? OdrMetrics::Stage::kPrimaryBootClasspath :
OdrMetrics::Stage::kSecondaryBootClasspath;
metrics.SetStage(stage);
if (!CheckCompilationSpace()) {
metrics.SetStatus(OdrMetrics::Status::kNoSpace);
- // Return kOkay so odsign will keep and sign whatever we have been able to compile.
- return ExitCode::kOkay;
+ // Return kCompilationFailed so odsign will keep and sign whatever we have been able to
+ // compile.
+ return ExitCode::kCompilationFailed;
}
if (!CompileBootExtensionArtifacts(
- isa, staging_dir, metrics, &dex2oat_invocation_count, &error_msg)) {
+ isa, staging_dir, metrics, advance_animation_progress, &error_msg)) {
LOG(ERROR) << "Compilation of BCP failed: " << error_msg;
if (!config_.GetDryRun() && !RemoveDirectory(staging_dir)) {
return ExitCode::kCleanupFailed;
@@ -1378,17 +1422,21 @@
}
}
- if (compile_system_server) {
+ if (!compilation_options.system_server_jars_to_compile.empty()) {
metrics.SetStage(OdrMetrics::Stage::kSystemServerClasspath);
if (!CheckCompilationSpace()) {
metrics.SetStatus(OdrMetrics::Status::kNoSpace);
- // Return kOkay so odsign will keep and sign whatever we have been able to compile.
- return ExitCode::kOkay;
+ // Return kCompilationFailed so odsign will keep and sign whatever we have been able to
+ // compile.
+ return ExitCode::kCompilationFailed;
}
- if (!CompileSystemServerArtifacts(
- staging_dir, metrics, &dex2oat_invocation_count, &error_msg)) {
+ if (!CompileSystemServerArtifacts(staging_dir,
+ metrics,
+ compilation_options.system_server_jars_to_compile,
+ advance_animation_progress,
+ &error_msg)) {
LOG(ERROR) << "Compilation of system_server failed: " << error_msg;
if (!config_.GetDryRun() && !RemoveDirectory(staging_dir)) {
return ExitCode::kCleanupFailed;
diff --git a/odrefresh/odrefresh.h b/odrefresh/odrefresh.h
index 54791d6..17003ee 100644
--- a/odrefresh/odrefresh.h
+++ b/odrefresh/odrefresh.h
@@ -18,8 +18,10 @@
#define ART_ODREFRESH_ODREFRESH_H_
#include <ctime>
+#include <functional>
#include <memory>
#include <optional>
+#include <set>
#include <string>
#include <vector>
@@ -36,6 +38,14 @@
namespace art {
namespace odrefresh {
+struct CompilationOptions {
+ // If not empty, compile the bootclasspath extensions for ISAs in the list.
+ std::vector<InstructionSet> compile_boot_extensions_for_isas;
+
+ // If not empty, compile the system server jars in the list.
+ std::set<std::string> system_server_jars_to_compile;
+};
+
class OnDeviceRefresh final {
public:
explicit OnDeviceRefresh(const OdrConfig& config);
@@ -50,15 +60,18 @@
// boolean indicating whether the system server should be compiled.
WARN_UNUSED ExitCode
CheckArtifactsAreUpToDate(OdrMetrics& metrics,
- /*out*/ std::vector<InstructionSet>* compile_boot_extensions,
- /*out*/ bool* compile_system_server) const;
+ /*out*/ CompilationOptions* compilation_options) const;
WARN_UNUSED ExitCode Compile(OdrMetrics& metrics,
- const std::vector<InstructionSet>& compile_boot_extensions,
- bool compile_system_server) const;
+ const CompilationOptions& compilation_options) const;
WARN_UNUSED bool RemoveArtifactsDirectory() const;
+ // Returns a set of all system server jars.
+ std::set<std::string> AllSystemServerJars() const {
+ return {systemserver_compilable_jars_.begin(), systemserver_compilable_jars_.end()};
+ }
+
private:
time_t GetExecutionTimeUsed() const;
@@ -75,8 +88,6 @@
// Write ART APEX cache information to `kOnDeviceRefreshOdrefreshArtifactDirectory`.
void WriteCacheInfo() const;
- void ReportNextBootAnimationProgress(uint32_t current_compilation) const;
-
std::vector<com::android::art::Component> GenerateBootClasspathComponents() const;
std::vector<com::android::art::Component> GenerateBootExtensionCompilableComponents() const;
@@ -106,10 +117,12 @@
// Checks whether all system_server artifacts are present. The artifacts are checked in their
// order of compilation. Returns true if all are present, false otherwise.
+ // Adds the paths to the jars that are missing artifacts in `jars_with_missing_artifacts`.
// If `checked_artifacts` is present, adds checked artifacts to `checked_artifacts`.
WARN_UNUSED bool SystemServerArtifactsExist(
bool on_system,
/*out*/ std::string* error_msg,
+ /*out*/ std::set<std::string>* jars_missing_artifacts,
/*out*/ std::vector<std::string>* checked_artifacts = nullptr) const;
// Checks whether all boot extension artifacts are up to date. Returns true if all are present,
@@ -124,23 +137,27 @@
// Checks whether all system_server artifacts are up to date. The artifacts are checked in their
// order of compilation. Returns true if all are present, false otherwise.
+ // Adds the paths to the jars that needs to be compiled in `jars_to_compile`.
// If `checked_artifacts` is present, adds checked artifacts to `checked_artifacts`.
- WARN_UNUSED bool CheckSystemServerArtifactsAreUpToDate(
+ bool CheckSystemServerArtifactsAreUpToDate(
OdrMetrics& metrics,
const std::vector<com::android::apex::ApexInfo>& apex_info_list,
const std::optional<com::android::art::CacheInfo>& cache_info,
+ /*out*/ std::set<std::string>* jars_to_compile,
/*out*/ std::vector<std::string>* checked_artifacts) const;
WARN_UNUSED bool CompileBootExtensionArtifacts(const InstructionSet isa,
const std::string& staging_dir,
OdrMetrics& metrics,
- uint32_t* dex2oat_invocation_count,
+ const std::function<void()>& on_dex2oat_success,
std::string* error_msg) const;
- WARN_UNUSED bool CompileSystemServerArtifacts(const std::string& staging_dir,
- OdrMetrics& metrics,
- uint32_t* dex2oat_invocation_count,
- std::string* error_msg) const;
+ WARN_UNUSED bool CompileSystemServerArtifacts(
+ const std::string& staging_dir,
+ OdrMetrics& metrics,
+ const std::set<std::string>& system_server_jars_to_compile,
+ const std::function<void()>& on_dex2oat_success,
+ std::string* error_msg) const;
// Configuration to use.
const OdrConfig& config_;
diff --git a/odrefresh/odrefresh_main.cc b/odrefresh/odrefresh_main.cc
index aecabd0..ce7d015 100644
--- a/odrefresh/odrefresh_main.cc
+++ b/odrefresh/odrefresh_main.cc
@@ -35,7 +35,7 @@
namespace {
-using ::art::InstructionSet;
+using ::art::odrefresh::CompilationOptions;
using ::art::odrefresh::Concatenate;
using ::art::odrefresh::ExitCode;
using ::art::odrefresh::OdrCompilationLog;
@@ -301,15 +301,12 @@
OnDeviceRefresh odr(config);
for (int i = 0; i < argc; ++i) {
std::string_view action(argv[i]);
- std::vector<InstructionSet> compile_boot_extensions;
- bool compile_system_server;
+ CompilationOptions compilation_options;
if (action == "--check") {
// Fast determination of whether artifacts are up to date.
- return odr.CheckArtifactsAreUpToDate(
- metrics, &compile_boot_extensions, &compile_system_server);
+ return odr.CheckArtifactsAreUpToDate(metrics, &compilation_options);
} else if (action == "--compile") {
- const ExitCode exit_code =
- odr.CheckArtifactsAreUpToDate(metrics, &compile_boot_extensions, &compile_system_server);
+ const ExitCode exit_code = odr.CheckArtifactsAreUpToDate(metrics, &compilation_options);
if (exit_code != ExitCode::kCompilationRequired) {
return exit_code;
}
@@ -319,8 +316,7 @@
// Artifacts refreshed. Return `kCompilationFailed` so that odsign will sign them again.
return ExitCode::kCompilationFailed;
}
- ExitCode compile_result =
- odr.Compile(metrics, compile_boot_extensions, compile_system_server);
+ ExitCode compile_result = odr.Compile(metrics, compilation_options);
compilation_log.Log(metrics.GetArtApexVersion(),
metrics.GetArtApexLastUpdateMillis(),
metrics.GetTrigger(),
@@ -333,8 +329,10 @@
return ExitCode::kCleanupFailed;
}
return odr.Compile(metrics,
- /*compile_boot_extensions=*/config.GetBootExtensionIsas(),
- /*compile_system_server=*/true);
+ CompilationOptions{
+ .compile_boot_extensions_for_isas = config.GetBootExtensionIsas(),
+ .system_server_jars_to_compile = odr.AllSystemServerJars(),
+ });
} else if (action == "--help") {
UsageHelp(argv[0]);
} else {
diff --git a/odrefresh/odrefresh_test.cc b/odrefresh/odrefresh_test.cc
index 5094843..5882424 100644
--- a/odrefresh/odrefresh_test.cc
+++ b/odrefresh/odrefresh_test.cc
@@ -25,6 +25,10 @@
#include <utility>
#include <vector>
+#include "aidl/com/android/art/CompilerFilter.h"
+#include "aidl/com/android/art/DexoptBcpExtArgs.h"
+#include "aidl/com/android/art/DexoptSystemServerArgs.h"
+#include "aidl/com/android/art/Isa.h"
#include "android-base/parseint.h"
#include "android-base/properties.h"
#include "android-base/scopeguard.h"
@@ -45,24 +49,21 @@
#include "odr_metrics.h"
#include "odrefresh/odrefresh.h"
-#include "aidl/com/android/art/CompilerFilter.h"
-#include "aidl/com/android/art/DexoptBcpExtArgs.h"
-#include "aidl/com/android/art/DexoptSystemServerArgs.h"
-#include "aidl/com/android/art/Isa.h"
-
namespace art {
namespace odrefresh {
+using ::aidl::com::android::art::CompilerFilter;
+using ::aidl::com::android::art::DexoptBcpExtArgs;
+using ::aidl::com::android::art::DexoptSystemServerArgs;
+using ::aidl::com::android::art::Isa;
using ::testing::AllOf;
using ::testing::Contains;
+using ::testing::ElementsAre;
using ::testing::Eq;
using ::testing::Field;
using ::testing::Ge;
+using ::testing::IsEmpty;
using ::testing::Return;
-using aidl::com::android::art::CompilerFilter;
-using aidl::com::android::art::DexoptBcpExtArgs;
-using aidl::com::android::art::DexoptSystemServerArgs;
-using aidl::com::android::art::Isa;
constexpr int kReplace = 1;
@@ -227,6 +228,44 @@
EXPECT_EQ(kOdrefreshArtifactDirectory, GetArtApexData() + "/dalvik-cache");
}
+TEST_F(OdRefreshTest, AllSystemServerJars) {
+ auto [odrefresh, mock_odr_dexopt] = CreateOdRefresh();
+
+ EXPECT_CALL(*mock_odr_dexopt,
+ DoDexoptSystemServer(
+ AllOf(Field(&DexoptSystemServerArgs::dexPath, Eq(location_provider_jar_)),
+ Field(&DexoptSystemServerArgs::classloaderContext, IsEmpty()))))
+ .WillOnce(Return(0));
+ EXPECT_CALL(*mock_odr_dexopt,
+ DoDexoptSystemServer(AllOf(Field(&DexoptSystemServerArgs::dexPath, Eq(services_jar_)),
+ Field(&DexoptSystemServerArgs::classloaderContext,
+ ElementsAre(location_provider_jar_)))))
+ .WillOnce(Return(0));
+
+ EXPECT_EQ(
+ odrefresh->Compile(*metrics_,
+ CompilationOptions{
+ .system_server_jars_to_compile = odrefresh->AllSystemServerJars(),
+ }),
+ ExitCode::kCompilationSuccess);
+}
+
+TEST_F(OdRefreshTest, PartialSystemServerJars) {
+ auto [odrefresh, mock_odr_dexopt] = CreateOdRefresh();
+
+ EXPECT_CALL(*mock_odr_dexopt,
+ DoDexoptSystemServer(AllOf(Field(&DexoptSystemServerArgs::dexPath, Eq(services_jar_)),
+ Field(&DexoptSystemServerArgs::classloaderContext,
+ ElementsAre(location_provider_jar_)))))
+ .WillOnce(Return(0));
+
+ EXPECT_EQ(odrefresh->Compile(*metrics_,
+ CompilationOptions{
+ .system_server_jars_to_compile = {services_jar_},
+ }),
+ ExitCode::kCompilationSuccess);
+}
+
TEST_F(OdRefreshTest, CompileSetsCompilerFilter) {
// This test depends on a system property that doesn't exist on old platforms. Since the whole
// odrefresh program is for S and later, we don't need to run the test on old platforms.
@@ -254,9 +293,12 @@
Field(&DexoptSystemServerArgs::compilerFilter, Eq(CompilerFilter::SPEED)))))
.WillOnce(Return(0))
.RetiresOnSaturation();
- EXPECT_EQ(odrefresh->Compile(
- *metrics_, /*compile_boot_extensions=*/{}, /*compile_system_server=*/true),
- ExitCode::kCompilationSuccess);
+ EXPECT_EQ(
+ odrefresh->Compile(*metrics_,
+ CompilationOptions{
+ .system_server_jars_to_compile = odrefresh->AllSystemServerJars(),
+ }),
+ ExitCode::kCompilationSuccess);
}
{
@@ -282,9 +324,12 @@
Field(&DexoptSystemServerArgs::compilerFilter, Eq(CompilerFilter::SPEED)))))
.WillOnce(Return(0))
.RetiresOnSaturation();
- EXPECT_EQ(odrefresh->Compile(
- *metrics_, /*compile_boot_extensions=*/{}, /*compile_system_server=*/true),
- ExitCode::kCompilationSuccess);
+ EXPECT_EQ(
+ odrefresh->Compile(*metrics_,
+ CompilationOptions{
+ .system_server_jars_to_compile = odrefresh->AllSystemServerJars(),
+ }),
+ ExitCode::kCompilationSuccess);
}
{
@@ -307,9 +352,12 @@
Field(&DexoptSystemServerArgs::compilerFilter, Eq(CompilerFilter::VERIFY)))))
.WillOnce(Return(0))
.RetiresOnSaturation();
- EXPECT_EQ(odrefresh->Compile(
- *metrics_, /*compile_boot_extensions=*/{}, /*compile_system_server=*/true),
- ExitCode::kCompilationSuccess);
+ EXPECT_EQ(
+ odrefresh->Compile(*metrics_,
+ CompilationOptions{
+ .system_server_jars_to_compile = odrefresh->AllSystemServerJars(),
+ }),
+ ExitCode::kCompilationSuccess);
}
}
@@ -325,20 +373,21 @@
Field(&DexoptBcpExtArgs::oatFd, Ge(0)))))
.WillOnce(Return(0));
- EXPECT_CALL(
- *mock_odr_dexopt,
- DoDexoptSystemServer(AllOf(
- Field(&DexoptSystemServerArgs::isa, Eq(Isa::X86_64)),
- Field(&DexoptSystemServerArgs::imageFd, Ge(0)),
- Field(&DexoptSystemServerArgs::vdexFd, Ge(0)),
- Field(&DexoptSystemServerArgs::oatFd, Ge(0)))))
- .Times(2)
+ EXPECT_CALL(*mock_odr_dexopt,
+ DoDexoptSystemServer(AllOf(Field(&DexoptSystemServerArgs::isa, Eq(Isa::X86_64)),
+ Field(&DexoptSystemServerArgs::imageFd, Ge(0)),
+ Field(&DexoptSystemServerArgs::vdexFd, Ge(0)),
+ Field(&DexoptSystemServerArgs::oatFd, Ge(0)))))
+ .Times(odrefresh->AllSystemServerJars().size())
.WillRepeatedly(Return(0));
- EXPECT_EQ(odrefresh->Compile(*metrics_,
- /*compile_boot_extensions=*/{InstructionSet::kX86_64},
- /*compile_system_server=*/true),
- ExitCode::kCompilationSuccess);
+ EXPECT_EQ(
+ odrefresh->Compile(*metrics_,
+ CompilationOptions{
+ .compile_boot_extensions_for_isas = {InstructionSet::kX86_64},
+ .system_server_jars_to_compile = odrefresh->AllSystemServerJars(),
+ }),
+ ExitCode::kCompilationSuccess);
}
TEST_F(OdRefreshTest, CompileChoosesBootImage) {
@@ -354,19 +403,21 @@
EXPECT_CALL(
*mock_odr_dexopt,
- DoDexoptSystemServer(AllOf(
- Field(&DexoptSystemServerArgs::isBootImageOnSystem, Eq(false)),
- Field(&DexoptSystemServerArgs::bootClasspathImageFds,
- Contains(FdOf(artifacts.ImagePath()))),
- Field(&DexoptSystemServerArgs::bootClasspathVdexFds,
- Contains(FdOf(artifacts.VdexPath()))),
- Field(&DexoptSystemServerArgs::bootClasspathOatFds,
- Contains(FdOf(artifacts.OatPath()))))))
- .Times(2)
+ DoDexoptSystemServer(AllOf(Field(&DexoptSystemServerArgs::isBootImageOnSystem, Eq(false)),
+ Field(&DexoptSystemServerArgs::bootClasspathImageFds,
+ Contains(FdOf(artifacts.ImagePath()))),
+ Field(&DexoptSystemServerArgs::bootClasspathVdexFds,
+ Contains(FdOf(artifacts.VdexPath()))),
+ Field(&DexoptSystemServerArgs::bootClasspathOatFds,
+ Contains(FdOf(artifacts.OatPath()))))))
+ .Times(odrefresh->AllSystemServerJars().size())
.WillRepeatedly(Return(0));
- EXPECT_EQ(odrefresh->Compile(
- *metrics_, /*compile_boot_extensions=*/{}, /*compile_system_server=*/true),
- ExitCode::kCompilationSuccess);
+ EXPECT_EQ(
+ odrefresh->Compile(*metrics_,
+ CompilationOptions{
+ .system_server_jars_to_compile = odrefresh->AllSystemServerJars(),
+ }),
+ ExitCode::kCompilationSuccess);
}
{
@@ -381,19 +432,21 @@
EXPECT_CALL(
*mock_odr_dexopt,
- DoDexoptSystemServer(AllOf(
- Field(&DexoptSystemServerArgs::isBootImageOnSystem, Eq(true)),
- Field(&DexoptSystemServerArgs::bootClasspathImageFds,
- Contains(FdOf(artifacts.ImagePath()))),
- Field(&DexoptSystemServerArgs::bootClasspathVdexFds,
- Contains(FdOf(artifacts.VdexPath()))),
- Field(&DexoptSystemServerArgs::bootClasspathOatFds,
- Contains(FdOf(artifacts.OatPath()))))))
- .Times(2)
+ DoDexoptSystemServer(AllOf(Field(&DexoptSystemServerArgs::isBootImageOnSystem, Eq(true)),
+ Field(&DexoptSystemServerArgs::bootClasspathImageFds,
+ Contains(FdOf(artifacts.ImagePath()))),
+ Field(&DexoptSystemServerArgs::bootClasspathVdexFds,
+ Contains(FdOf(artifacts.VdexPath()))),
+ Field(&DexoptSystemServerArgs::bootClasspathOatFds,
+ Contains(FdOf(artifacts.OatPath()))))))
+ .Times(odrefresh->AllSystemServerJars().size())
.WillRepeatedly(Return(0));
- EXPECT_EQ(odrefresh->Compile(
- *metrics_, /*compile_boot_extensions=*/{}, /*compile_system_server=*/true),
- ExitCode::kCompilationSuccess);
+ EXPECT_EQ(
+ odrefresh->Compile(*metrics_,
+ CompilationOptions{
+ .system_server_jars_to_compile = odrefresh->AllSystemServerJars(),
+ }),
+ ExitCode::kCompilationSuccess);
}
}
diff --git a/test/odsign/test-src/com/android/tests/odsign/OdrefreshHostTest.java b/test/odsign/test-src/com/android/tests/odsign/OdrefreshHostTest.java
index 22fbde7..94d5a19 100644
--- a/test/odsign/test-src/com/android/tests/odsign/OdrefreshHostTest.java
+++ b/test/odsign/test-src/com/android/tests/odsign/OdrefreshHostTest.java
@@ -109,6 +109,22 @@
}
@Test
+ public void verifyMissingArtifactTriggersCompilation() throws Exception {
+ Set<String> missingArtifacts = simulateMissingArtifacts();
+ Set<String> remainingArtifacts = new HashSet<>();
+ remainingArtifacts.addAll(sZygoteArtifacts);
+ remainingArtifacts.addAll(sSystemServerArtifacts);
+ remainingArtifacts.removeAll(missingArtifacts);
+
+ sTestUtils.removeCompilationLogToAvoidBackoff();
+ long timeMs = getCurrentTimeMs();
+ getDevice().executeShellV2Command(ODREFRESH_COMMAND);
+
+ assertArtifactsNotModifiedAfter(remainingArtifacts, timeMs);
+ assertArtifactsModifiedAfter(missingArtifacts, timeMs);
+ }
+
+ @Test
public void verifyNoCompilationWhenCacheIsGood() throws Exception {
sTestUtils.removeCompilationLogToAvoidBackoff();
long timeMs = getCurrentTimeMs();
@@ -190,6 +206,17 @@
getDevice().pushString(apexInfo, CACHE_INFO_FILE);
}
+ private Set<String> simulateMissingArtifacts() throws Exception {
+ Set<String> missingArtifacts = new HashSet<>();
+ String sample = sSystemServerArtifacts.iterator().next();
+ for (String extension : OdsignTestUtils.APP_ARTIFACT_EXTENSIONS) {
+ String artifact = replaceExtension(sample, extension);
+ getDevice().deleteFile(artifact);
+ missingArtifacts.add(artifact);
+ }
+ return missingArtifacts;
+ }
+
private long parseFormattedDateTime(String dateTimeStr) throws Exception {
DateTimeFormatter formatter = DateTimeFormatter.ofPattern(
"yyyy-MM-dd HH:mm:ss.nnnnnnnnn Z");
@@ -243,4 +270,10 @@
modifiedTime < timeMs);
}
}
+
+ private String replaceExtension(String filename, String extension) throws Exception {
+ int index = filename.lastIndexOf(".");
+ assertTrue("Extension not found in filename: " + filename, index != -1);
+ return filename.substring(0, index) + extension;
+ }
}
diff --git a/test/odsign/test-src/com/android/tests/odsign/OdsignTestUtils.java b/test/odsign/test-src/com/android/tests/odsign/OdsignTestUtils.java
index c51b0c6..101e5e2 100644
--- a/test/odsign/test-src/com/android/tests/odsign/OdsignTestUtils.java
+++ b/test/odsign/test-src/com/android/tests/odsign/OdsignTestUtils.java
@@ -40,6 +40,9 @@
public static final List<String> ZYGOTE_NAMES = List.of("zygote", "zygote64");
+ public static final List<String> APP_ARTIFACT_EXTENSIONS = List.of(".art", ".odex", ".vdex");
+ public static final List<String> BCP_ARTIFACT_EXTENSIONS = List.of(".art", ".oat", ".vdex");
+
private static final String APEX_FILENAME = "test_com.android.art.apex";
private static final String ODREFRESH_COMPILATION_LOG =
diff --git a/test/odsign/test-src/com/android/tests/odsign/OnDeviceSigningHostTest.java b/test/odsign/test-src/com/android/tests/odsign/OnDeviceSigningHostTest.java
index 867c849..53f6b21 100644
--- a/test/odsign/test-src/com/android/tests/odsign/OnDeviceSigningHostTest.java
+++ b/test/odsign/test-src/com/android/tests/odsign/OnDeviceSigningHostTest.java
@@ -38,10 +38,6 @@
*/
@RunWith(DeviceJUnit4ClassRunner.class)
public class OnDeviceSigningHostTest extends BaseHostJUnit4Test {
- private static final List<String> APP_ARTIFACT_EXTENSIONS = List.of(".art", ".odex", ".vdex");
-
- private static final List<String> BCP_ARTIFACT_EXTENSIONS = List.of(".art", ".oat", ".vdex");
-
private static final String TEST_APP_PACKAGE_NAME = "com.android.tests.odsign";
private static final String TEST_APP_APK = "odsign_e2e_test_app.apk";
@@ -139,9 +135,9 @@
// Check components in the system_server classpath have mapped artifacts.
for (String element : classpathElements) {
String escapedPath = element.substring(1).replace('/', '@');
- for (String extension : APP_ARTIFACT_EXTENSIONS) {
+ for (String extension : OdsignTestUtils.APP_ARTIFACT_EXTENSIONS) {
final String fullArtifactPath =
- String.format("%s/%s@classes%s", isaCacheDirectory, escapedPath, extension);
+ String.format("%s/%s@classes%s", isaCacheDirectory, escapedPath, extension);
assertTrue("Missing " + fullArtifactPath, mappedArtifacts.contains(fullArtifactPath));
}
}
@@ -149,7 +145,8 @@
for (String mappedArtifact : mappedArtifacts) {
// Check the mapped artifact has a .art, .odex or .vdex extension.
final boolean knownArtifactKind =
- APP_ARTIFACT_EXTENSIONS.stream().anyMatch(e -> mappedArtifact.endsWith(e));
+ OdsignTestUtils.APP_ARTIFACT_EXTENSIONS.stream().anyMatch(
+ e -> mappedArtifact.endsWith(e));
assertTrue("Unknown artifact kind: " + mappedArtifact, knownArtifactKind);
}
}
@@ -161,7 +158,7 @@
assertTrue("Expect 3 boot-framework artifacts", mappedArtifacts.size() == 3);
String allArtifacts = mappedArtifacts.stream().collect(Collectors.joining(","));
- for (String extension : BCP_ARTIFACT_EXTENSIONS) {
+ for (String extension : OdsignTestUtils.BCP_ARTIFACT_EXTENSIONS) {
final String artifact = bootExtensionName + extension;
final boolean found = mappedArtifacts.stream().anyMatch(a -> a.endsWith(artifact));
assertTrue(zygoteName + " " + artifact + " not found: '" + allArtifacts + "'", found);