Re-compile on userfaultfd phenotype flag change.
After this change, odrefresh re-compiles everything when the phenotype
flag `runtime_native_boot.enable_uffd_gc` changes. It writes the value
of the flag to cache-info.xml for change detection.
According to go/platform-experiment-namespaces#namespace-types, the
phenotype flag is back by a persistent system property. Therefore, we
can directly read the flag from the system property instead of depending
on the `server_configurable_flags` library. This behavior is consistent
with the existing ART code (particularly, `art::Flag`), which reads
other phenotype flags.
Bug: 231298279
Test: atest odsign_e2e_tests_full
Change-Id: I02b70569ecd96d4ded6d2d3be22c34b2c6a4c5b4
Merged-In: I02b70569ecd96d4ded6d2d3be22c34b2c6a4c5b4
(cherry picked from commit 45d0881f7444307955b51db5be88c5f890647c5e)
diff --git a/odrefresh/CacheInfo.xsd b/odrefresh/CacheInfo.xsd
index 1cbef8a..196caf7 100644
--- a/odrefresh/CacheInfo.xsd
+++ b/odrefresh/CacheInfo.xsd
@@ -27,6 +27,7 @@
<!-- True if the cache info is generated in the Compilation OS. -->
<xs:attribute name="compilationOsMode" type="xs:boolean" />
<xs:sequence>
+ <xs:element name="systemProperties" minOccurs="1" maxOccurs="1" type="t:keyValuePairList" />
<xs:element name="artModuleInfo" minOccurs="1" maxOccurs="1" type="t:moduleInfo" />
<xs:element name="moduleInfoList" minOccurs="1" maxOccurs="1" type="t:moduleInfoList" />
<xs:element name="bootClasspath" minOccurs="1" maxOccurs="1" type="t:classpath" />
@@ -36,6 +37,19 @@
</xs:complexType>
</xs:element>
+ <!-- List of key-value pairs. -->
+ <xs:complexType name="keyValuePairList">
+ <xs:sequence>
+ <xs:element name="item" type="t:keyValuePair" />
+ </xs:sequence>
+ </xs:complexType>
+
+ <!-- A key-value pair. -->
+ <xs:complexType name="keyValuePair">
+ <xs:attribute name="k" type="xs:string" use="required" />
+ <xs:attribute name="v" type="xs:string" use="required" />
+ </xs:complexType>
+
<!-- List of modules. -->
<xs:complexType name="moduleInfoList">
<xs:sequence>
diff --git a/odrefresh/odr_config.h b/odrefresh/odr_config.h
index d95ab96..6a4646a 100644
--- a/odrefresh/odr_config.h
+++ b/odrefresh/odr_config.h
@@ -19,9 +19,11 @@
#include <optional>
#include <string>
+#include <unordered_map>
#include <vector>
#include "android-base/file.h"
+#include "android-base/no_destructor.h"
#include "arch/instruction_set.h"
#include "base/file_utils.h"
#include "base/globals.h"
@@ -32,6 +34,17 @@
namespace art {
namespace odrefresh {
+struct SystemPropertyConfig {
+ const char* name;
+ const char* default_value;
+};
+
+// The system properties that odrefresh keeps track of. Odrefresh will recompile everything if any
+// property changes.
+const android::base::NoDestructor<std::vector<SystemPropertyConfig>> kSystemProperties{
+ {SystemPropertyConfig{.name = "persist.device_config.runtime_native_boot.enable_uffd_gc",
+ .default_value = "false"}}};
+
// An enumeration of the possible zygote configurations on Android.
enum class ZygoteKind : uint8_t {
// 32-bit primary zygote, no secondary zygote.
@@ -66,6 +79,9 @@
bool compilation_os_mode_ = false;
bool minimal_ = false;
+ // The current values of system properties listed in `kSystemProperties`.
+ std::unordered_map<std::string, std::string> system_properties_;
+
// Staging directory for artifacts. The directory must exist and will be automatically removed
// after compilation. If empty, use the default directory.
std::string staging_dir_;
@@ -145,6 +161,9 @@
}
bool GetCompilationOsMode() const { return compilation_os_mode_; }
bool GetMinimal() const { return minimal_; }
+ const std::unordered_map<std::string, std::string>& GetSystemProperties() const {
+ return system_properties_;
+ }
void SetApexInfoListFile(const std::string& file_path) { apex_info_list_file_ = file_path; }
void SetArtBinDir(const std::string& art_bin_dir) { art_bin_dir_ = art_bin_dir; }
@@ -196,6 +215,10 @@
void SetMinimal(bool value) { minimal_ = value; }
+ std::unordered_map<std::string, std::string>* MutableSystemProperties() {
+ return &system_properties_;
+ }
+
private:
// Returns a pair for the possible instruction sets for the configured instruction set
// architecture. The first item is the 32-bit architecture and the second item is the 64-bit
diff --git a/odrefresh/odrefresh.cc b/odrefresh/odrefresh.cc
index 1d12b94..70cf48e 100644
--- a/odrefresh/odrefresh.cc
+++ b/odrefresh/odrefresh.cc
@@ -648,6 +648,11 @@
return Errorf("Could not create directory {}", QuotePath(dir_name));
}
+ std::vector<art_apex::KeyValuePair> system_properties;
+ for (const auto& [key, value] : config_.GetSystemProperties()) {
+ system_properties.emplace_back(key, value);
+ }
+
std::optional<std::vector<apex::ApexInfo>> apex_info_list = GetApexInfoList();
if (!apex_info_list.has_value()) {
return Errorf("Could not update {}: no APEX info", QuotePath(cache_info_filename_));
@@ -685,15 +690,16 @@
return Errorf("Cannot open {} for writing.", QuotePath(cache_info_filename_));
}
- art_apex::CacheInfo info(
+ std::unique_ptr<art_apex::CacheInfo> info(new art_apex::CacheInfo(
+ {art_apex::KeyValuePairList(system_properties)},
{art_module_info},
{art_apex::ModuleInfoList(module_info_list)},
{art_apex::Classpath(bcp_components.value())},
{art_apex::Classpath(bcp_compilable_components.value())},
{art_apex::SystemServerComponents(system_server_components.value())},
- config_.GetCompilationOsMode() ? std::make_optional(true) : std::nullopt);
+ config_.GetCompilationOsMode() ? std::make_optional(true) : std::nullopt));
- art_apex::write(out, info);
+ art_apex::write(out, *info);
out.close();
if (out.fail()) {
return Errorf("Cannot write to {}", QuotePath(cache_info_filename_));
@@ -836,16 +842,106 @@
return jars_missing_artifacts->empty();
}
+WARN_UNUSED bool OnDeviceRefresh::CheckSystemPropertiesAreDefault() const {
+ const std::unordered_map<std::string, std::string>& system_properties =
+ config_.GetSystemProperties();
+
+ for (const SystemPropertyConfig& system_property_config : *kSystemProperties.get()) {
+ auto property = system_properties.find(system_property_config.name);
+ DCHECK(property != system_properties.end());
+
+ if (property->second != system_property_config.default_value) {
+ LOG(INFO) << "System property " << system_property_config.name << " has a non-default value ("
+ << property->second << ").";
+ return false;
+ }
+ }
+
+ return true;
+}
+
+WARN_UNUSED bool OnDeviceRefresh::CheckSystemPropertiesHaveNotChanged(
+ const art_apex::CacheInfo& cache_info) const {
+ std::unordered_map<std::string, std::string> cached_system_properties;
+ const art_apex::KeyValuePairList* list = cache_info.getFirstSystemProperties();
+ if (list == nullptr) {
+ // This should never happen. We have already checked the ART module version, and the cache
+ // info is generated by the latest version of the ART module if it exists.
+ LOG(ERROR) << "Missing system properties from cache-info.";
+ return false;
+ }
+
+ for (const art_apex::KeyValuePair& pair : list->getItem()) {
+ cached_system_properties[pair.getK()] = pair.getV();
+ }
+
+ const std::unordered_map<std::string, std::string>& system_properties =
+ config_.GetSystemProperties();
+
+ for (const SystemPropertyConfig& system_property_config : *kSystemProperties.get()) {
+ auto property = system_properties.find(system_property_config.name);
+ DCHECK(property != system_properties.end());
+
+ auto cached_property = cached_system_properties.find(system_property_config.name);
+ if (cached_property == cached_system_properties.end()) {
+ // This should never happen. We have already checked the ART module version, and the cache
+ // info is generated by the latest version of the ART module if it exists.
+ LOG(ERROR) << "Missing system property from cache-info (" << system_property_config.name
+ << ")";
+ return false;
+ }
+
+ if (property->second != cached_property->second) {
+ LOG(INFO) << "System property " << system_property_config.name
+ << " value changed (before: " << cached_property->second
+ << ", now: " << property->second << ").";
+ return false;
+ }
+ }
+
+ return true;
+}
+
+WARN_UNUSED bool OnDeviceRefresh::BootClasspathArtifactsOnSystemUsable(
+ const apex::ApexInfo& art_apex_info) const {
+ if (!art_apex_info.getIsFactory()) {
+ return false;
+ }
+ LOG(INFO) << "Factory ART APEX mounted.";
+
+ if (!CheckSystemPropertiesAreDefault()) {
+ return false;
+ }
+ LOG(INFO) << "System properties are set to default values.";
+
+ return true;
+}
+
+WARN_UNUSED bool OnDeviceRefresh::SystemServerArtifactsOnSystemUsable(
+ const std::vector<apex::ApexInfo>& apex_info_list) const {
+ if (std::any_of(apex_info_list.begin(),
+ apex_info_list.end(),
+ [](const apex::ApexInfo& apex_info) { return !apex_info.getIsFactory(); })) {
+ return false;
+ }
+ LOG(INFO) << "Factory APEXes mounted.";
+
+ if (!CheckSystemPropertiesAreDefault()) {
+ return false;
+ }
+ LOG(INFO) << "System properties are set to default values.";
+
+ return true;
+}
+
WARN_UNUSED bool OnDeviceRefresh::CheckBootClasspathArtifactsAreUpToDate(
OdrMetrics& metrics,
const InstructionSet isa,
const apex::ApexInfo& art_apex_info,
const std::optional<art_apex::CacheInfo>& cache_info,
/*out*/ std::vector<std::string>* checked_artifacts) const {
- if (art_apex_info.getIsFactory()) {
- LOG(INFO) << "Factory ART APEX mounted.";
-
- // ART is not updated, so we can use the artifacts on /system. Check if they exist.
+ if (BootClasspathArtifactsOnSystemUsable(art_apex_info)) {
+ // We can use the artifacts on /system. Check if they exist.
std::string error_msg;
if (BootClasspathArtifactsExist(/*on_system=*/true, /*minimal=*/false, isa, &error_msg)) {
return true;
@@ -900,6 +996,14 @@
return false;
}
+ if (!CheckSystemPropertiesHaveNotChanged(cache_info.value())) {
+ // We don't have a trigger kind for system property changes. For now, we reuse
+ // `kApexVersionMismatch` as it implies the expected behavior: re-compile regardless of the last
+ // compilation attempt.
+ metrics.SetTrigger(OdrMetrics::Trigger::kApexVersionMismatch);
+ return false;
+ }
+
// Check boot class components.
//
// This checks the size and checksums of odrefresh compilable files on the DEX2OATBOOTCLASSPATH
@@ -961,12 +1065,8 @@
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(),
- [](const apex::ApexInfo& apex_info) { return apex_info.getIsFactory(); })) {
- LOG(INFO) << "Factory APEXes mounted.";
-
- // APEXes are not updated, so we can use the artifacts on /system. Check if they exist.
+ if (SystemServerArtifactsOnSystemUsable(apex_info_list)) {
+ // We can use the artifacts on /system. Check if they exist.
std::string error_msg;
if (SystemServerArtifactsExist(
/*on_system=*/true, &error_msg, &jars_missing_artifacts_on_system)) {
@@ -1052,6 +1152,14 @@
}
}
+ if (!CheckSystemPropertiesHaveNotChanged(cache_info.value())) {
+ // We don't have a trigger kind for system property changes. For now, we reuse
+ // `kApexVersionMismatch` as it implies the expected behavior: re-compile regardless of the last
+ // compilation attempt.
+ metrics.SetTrigger(OdrMetrics::Trigger::kApexVersionMismatch);
+ return false;
+ }
+
// Check system server components.
//
// This checks the size and checksums of odrefresh compilable files on the
diff --git a/odrefresh/odrefresh.h b/odrefresh/odrefresh.h
index 58da57a..9dd4009 100644
--- a/odrefresh/odrefresh.h
+++ b/odrefresh/odrefresh.h
@@ -140,6 +140,25 @@
/*out*/ std::set<std::string>* jars_missing_artifacts,
/*out*/ std::vector<std::string>* checked_artifacts = nullptr) const;
+ // Returns true if all of the system properties listed in `kSystemProperties` are set to the
+ // default values.
+ WARN_UNUSED bool CheckSystemPropertiesAreDefault() const;
+
+ // Returns true if none of the system properties listed in `kSystemProperties` has changed since
+ // the last compilation.
+ WARN_UNUSED bool CheckSystemPropertiesHaveNotChanged(
+ const com::android::art::CacheInfo& cache_info) const;
+
+ // Returns true if boot classpath artifacts on /system are usable if they exist. Note that this
+ // function does not check file existence.
+ WARN_UNUSED bool BootClasspathArtifactsOnSystemUsable(
+ const com::android::apex::ApexInfo& art_apex_info) const;
+
+ // Returns true if system_server artifacts on /system are usable if they exist. Note that this
+ // function does not check file existence.
+ WARN_UNUSED bool SystemServerArtifactsOnSystemUsable(
+ const std::vector<com::android::apex::ApexInfo>& apex_info_list) const;
+
// Checks whether all boot classpath artifacts are up to date. Returns true if all are present,
// false otherwise.
// If `checked_artifacts` is present, adds checked artifacts to `checked_artifacts`.
diff --git a/odrefresh/odrefresh_main.cc b/odrefresh/odrefresh_main.cc
index c2298f0..01299e0 100644
--- a/odrefresh/odrefresh_main.cc
+++ b/odrefresh/odrefresh_main.cc
@@ -18,6 +18,7 @@
#include <string>
#include <string_view>
+#include <unordered_map>
#include "android-base/properties.h"
#include "android-base/stringprintf.h"
@@ -34,14 +35,17 @@
namespace {
+using ::android::base::GetProperty;
using ::art::odrefresh::CompilationOptions;
using ::art::odrefresh::ExitCode;
+using ::art::odrefresh::kSystemProperties;
using ::art::odrefresh::OdrCompilationLog;
using ::art::odrefresh::OdrConfig;
using ::art::odrefresh::OdrMetrics;
using ::art::odrefresh::OnDeviceRefresh;
using ::art::odrefresh::QuotePath;
using ::art::odrefresh::ShouldDisableRefresh;
+using ::art::odrefresh::SystemPropertyConfig;
using ::art::odrefresh::ZygoteKind;
void UsageMsgV(const char* fmt, va_list ap) {
@@ -155,7 +159,7 @@
if (zygote.empty()) {
// Use ro.zygote by default, if not overridden by --zygote-arch flag.
- zygote = android::base::GetProperty("ro.zygote", {});
+ zygote = GetProperty("ro.zygote", {});
}
ZygoteKind zygote_kind;
if (!ParseZygoteKind(zygote.c_str(), &zygote_kind)) {
@@ -164,19 +168,24 @@
config->SetZygoteKind(zygote_kind);
if (config->GetSystemServerCompilerFilter().empty()) {
- std::string filter =
- android::base::GetProperty("dalvik.vm.systemservercompilerfilter", "speed");
+ std::string filter = GetProperty("dalvik.vm.systemservercompilerfilter", "speed");
config->SetSystemServerCompilerFilter(filter);
}
- if (ShouldDisableRefresh(
- android::base::GetProperty("ro.build.version.sdk", /*default_value=*/""))) {
+ if (ShouldDisableRefresh(GetProperty("ro.build.version.sdk", /*default_value=*/""))) {
config->SetRefresh(false);
}
return n;
}
+void GetSystemProperties(std::unordered_map<std::string, std::string>* system_properties) {
+ for (const SystemPropertyConfig& system_property_config : *kSystemProperties.get()) {
+ (*system_properties)[system_property_config.name] =
+ GetProperty(system_property_config.name, system_property_config.default_value);
+ }
+}
+
NO_RETURN void UsageHelp(const char* argv0) {
std::string name(android::base::Basename(argv0));
UsageMsg("Usage: %s [OPTION...] ACTION", name.c_str());
@@ -229,6 +238,7 @@
if (argc != 1) {
ArgumentError("Expected 1 argument, but have %d.", argc);
}
+ GetSystemProperties(config.MutableSystemProperties());
OdrMetrics metrics(config.GetArtifactDirectory());
OnDeviceRefresh odr(config);
@@ -268,6 +278,6 @@
} else if (action == "--help") {
UsageHelp(argv[0]);
} else {
- ArgumentError("Unknown argument: ", action);
+ ArgumentError("Unknown argument: %s", action.data());
}
}
diff --git a/odrefresh/schema/current.txt b/odrefresh/schema/current.txt
index 958f2a0..c45870b 100644
--- a/odrefresh/schema/current.txt
+++ b/odrefresh/schema/current.txt
@@ -8,12 +8,14 @@
method public boolean getCompilationOsMode();
method public com.android.art.Classpath getDex2oatBootClasspath();
method public com.android.art.ModuleInfoList getModuleInfoList();
+ method public com.android.art.KeyValuePairList getSystemProperties();
method public com.android.art.SystemServerComponents getSystemServerComponents();
method public void setArtModuleInfo(com.android.art.ModuleInfo);
method public void setBootClasspath(com.android.art.Classpath);
method public void setCompilationOsMode(boolean);
method public void setDex2oatBootClasspath(com.android.art.Classpath);
method public void setModuleInfoList(com.android.art.ModuleInfoList);
+ method public void setSystemProperties(com.android.art.KeyValuePairList);
method public void setSystemServerComponents(com.android.art.SystemServerComponents);
}
@@ -33,6 +35,20 @@
method public void setSize(java.math.BigInteger);
}
+ public class KeyValuePair {
+ ctor public KeyValuePair();
+ method public String getK();
+ method public String getV();
+ method public void setK(String);
+ method public void setV(String);
+ }
+
+ public class KeyValuePairList {
+ ctor public KeyValuePairList();
+ method public com.android.art.KeyValuePair getItem();
+ method public void setItem(com.android.art.KeyValuePair);
+ }
+
public class ModuleInfo {
ctor public ModuleInfo();
method public long getLastUpdateMillis();
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 d27902f..1b852d7 100644
--- a/test/odsign/test-src/com/android/tests/odsign/OdrefreshHostTest.java
+++ b/test/odsign/test-src/com/android/tests/odsign/OdrefreshHostTest.java
@@ -144,6 +144,60 @@
}
@Test
+ public void verifyEnableUffdGcChangeTriggersCompilation() throws Exception {
+ try {
+ // Disable phenotype flag syncing.
+ getDevice().executeShellV2Command(
+ "device_config set_sync_disabled_for_tests until_reboot");
+
+ // Simulate that the phenotype flag is set to the default value.
+ getDevice().executeShellV2Command(
+ "device_config put runtime_native_boot enable_uffd_gc false");
+
+ long timeMs = mTestUtils.getCurrentTimeMs();
+ getDevice().executeShellV2Command(ODREFRESH_COMMAND);
+
+ // Artifacts should not be re-compiled.
+ assertArtifactsNotModifiedAfter(getZygoteArtifacts(), timeMs);
+ assertArtifactsNotModifiedAfter(getSystemServerArtifacts(), timeMs);
+
+ // Simulate that the phenotype flag is set to true.
+ getDevice().executeShellV2Command(
+ "device_config put runtime_native_boot enable_uffd_gc true");
+
+ timeMs = mTestUtils.getCurrentTimeMs();
+ getDevice().executeShellV2Command(ODREFRESH_COMMAND);
+
+ // Artifacts should be re-compiled.
+ assertArtifactsModifiedAfter(getZygoteArtifacts(), timeMs);
+ assertArtifactsModifiedAfter(getSystemServerArtifacts(), timeMs);
+
+ // Run odrefresh again with the flag unchanged.
+ timeMs = mTestUtils.getCurrentTimeMs();
+ getDevice().executeShellV2Command(ODREFRESH_COMMAND);
+
+ // Artifacts should not be re-compiled.
+ assertArtifactsNotModifiedAfter(getZygoteArtifacts(), timeMs);
+ assertArtifactsNotModifiedAfter(getSystemServerArtifacts(), timeMs);
+
+ // Simulate that the phenotype flag is set to false.
+ getDevice().executeShellV2Command(
+ "device_config put runtime_native_boot enable_uffd_gc false");
+
+ timeMs = mTestUtils.getCurrentTimeMs();
+ getDevice().executeShellV2Command(ODREFRESH_COMMAND);
+
+ // Artifacts should be re-compiled.
+ assertArtifactsModifiedAfter(getZygoteArtifacts(), timeMs);
+ assertArtifactsModifiedAfter(getSystemServerArtifacts(), timeMs);
+ } finally {
+ getDevice().executeShellV2Command("device_config set_sync_disabled_for_tests none");
+ getDevice().executeShellV2Command(
+ "device_config delete runtime_native_boot enable_uffd_gc");
+ }
+ }
+
+ @Test
public void verifyNoCompilationWhenCacheIsGood() throws Exception {
mTestUtils.removeCompilationLogToAvoidBackoff();
long timeMs = mTestUtils.getCurrentTimeMs();