Fix the logic for skipping compilation based on backoff.
Before this change, `OdrCompilationLog::ShouldAttemptCompile` checked
the ART module version and last update time to decide whether
compilation should always be performed or not. However, since we now
need to compile after any module update, the ART module is no longer a
special case, and the decision should be made based on any APEX version
mismatch.
Bug: 205276874
Test: atest odsign_e2e_tests
Test: atest art_standalone_odrefresh_tests
Change-Id: Ie3bfa17c2c8aed838334f1096679602632a1dbcb
diff --git a/odrefresh/odr_compilation_log.cc b/odrefresh/odr_compilation_log.cc
index 37804a2..9c50817 100644
--- a/odrefresh/odr_compilation_log.cc
+++ b/odrefresh/odr_compilation_log.cc
@@ -179,27 +179,15 @@
Truncate();
}
-bool OdrCompilationLog::ShouldAttemptCompile(int64_t apex_version,
- int64_t last_update_millis,
- OdrMetrics::Trigger trigger,
- time_t now) const {
+bool OdrCompilationLog::ShouldAttemptCompile(OdrMetrics::Trigger trigger, time_t now) const {
if (entries_.size() == 0) {
// We have no history, try to compile.
return true;
}
- if (apex_version != entries_.back().apex_version) {
- // There is a new ART APEX, we should compile right away.
- return true;
- }
-
- if (last_update_millis != entries_.back().last_update_millis) {
- // There is a samegrade ART APEX update, we should compile right away.
- return true;
- }
-
- if (trigger == OdrMetrics::Trigger::kDexFilesChanged) {
- // The DEX files in the classpaths have changed, possibly an OTA has updated them.
+ if (trigger == OdrMetrics::Trigger::kApexVersionMismatch ||
+ trigger == OdrMetrics::Trigger::kDexFilesChanged) {
+ // Things have changed since the last run.
return true;
}
diff --git a/odrefresh/odr_compilation_log.h b/odrefresh/odr_compilation_log.h
index 894079c..48d8c68 100644
--- a/odrefresh/odr_compilation_log.h
+++ b/odrefresh/odr_compilation_log.h
@@ -65,10 +65,7 @@
~OdrCompilationLog();
// Applies policy to compilation log to determine whether to recompile.
- bool ShouldAttemptCompile(int64_t apex_version,
- int64_t last_update_millis,
- OdrMetrics::Trigger trigger,
- time_t now = 0) const;
+ bool ShouldAttemptCompile(OdrMetrics::Trigger trigger, time_t now = 0) const;
// Returns the number of entries in the log. The log never exceeds `kMaxLoggedEntries`.
size_t NumberOfEntries() const;
diff --git a/odrefresh/odr_compilation_log_test.cc b/odrefresh/odr_compilation_log_test.cc
index d99b4d9..46cea79 100644
--- a/odrefresh/odr_compilation_log_test.cc
+++ b/odrefresh/odr_compilation_log_test.cc
@@ -103,24 +103,16 @@
TEST(OdrCompilationLog, ShouldAttemptCompile) {
OdrCompilationLog ocl(/*compilation_log_path=*/nullptr);
- ASSERT_TRUE(ocl.ShouldAttemptCompile(
- /*apex_version=*/1, /*last_update_millis=*/762, OdrMetrics::Trigger::kMissingArtifacts, 0));
+ ASSERT_TRUE(ocl.ShouldAttemptCompile(OdrMetrics::Trigger::kUnknown, 0));
ocl.Log(
/*apex_version=*/1,
/*last_update_millis=*/762,
OdrMetrics::Trigger::kApexVersionMismatch,
ExitCode::kCompilationSuccess);
- ASSERT_TRUE(ocl.ShouldAttemptCompile(
- /*apex_version=*/2, /*last_update_millis=*/762, OdrMetrics::Trigger::kApexVersionMismatch));
- ASSERT_TRUE(ocl.ShouldAttemptCompile(
- /*apex_version=*/1, /*last_update_millis=*/10000, OdrMetrics::Trigger::kApexVersionMismatch));
- ASSERT_FALSE(ocl.ShouldAttemptCompile(
- /*apex_version=*/1, /*last_update_millis=*/762, OdrMetrics::Trigger::kApexVersionMismatch));
- ASSERT_TRUE(ocl.ShouldAttemptCompile(
- /*apex_version=*/1, /*last_update_millis=*/762, OdrMetrics::Trigger::kDexFilesChanged));
- ASSERT_FALSE(ocl.ShouldAttemptCompile(
- /*apex_version=*/1, /*last_update_millis=*/762, OdrMetrics::Trigger::kUnknown));
+ ASSERT_TRUE(ocl.ShouldAttemptCompile(OdrMetrics::Trigger::kApexVersionMismatch));
+ ASSERT_TRUE(ocl.ShouldAttemptCompile(OdrMetrics::Trigger::kDexFilesChanged));
+ ASSERT_FALSE(ocl.ShouldAttemptCompile(OdrMetrics::Trigger::kUnknown));
}
TEST(OdrCompilationLog, BackOffNoHistory) {
@@ -129,11 +121,7 @@
OdrCompilationLog ocl(/*compilation_log_path=*/nullptr);
- ASSERT_TRUE(ocl.ShouldAttemptCompile(
- /*apex_version=*/1,
- /*last_update_millis=*/0,
- OdrMetrics::Trigger::kApexVersionMismatch,
- start_time));
+ ASSERT_TRUE(ocl.ShouldAttemptCompile(OdrMetrics::Trigger::kUnknown, start_time));
// Start log
ocl.Log(/*apex_version=*/1,
@@ -141,21 +129,10 @@
OdrMetrics::Trigger::kApexVersionMismatch,
start_time,
ExitCode::kCompilationFailed);
- ASSERT_FALSE(ocl.ShouldAttemptCompile(
- /*apex_version=*/1,
- /*last_update_millis=*/0,
- OdrMetrics::Trigger::kApexVersionMismatch,
- start_time));
- ASSERT_FALSE(ocl.ShouldAttemptCompile(
- /*apex_version=*/1,
- /*last_update_millis=*/0,
- OdrMetrics::Trigger::kApexVersionMismatch,
- start_time + kSecondsPerDay / 2));
- ASSERT_TRUE(ocl.ShouldAttemptCompile(
- /*apex_version=*/1,
- /*last_update_millis=*/0,
- OdrMetrics::Trigger::kApexVersionMismatch,
- start_time + kSecondsPerDay));
+ ASSERT_FALSE(ocl.ShouldAttemptCompile(OdrMetrics::Trigger::kUnknown, start_time));
+ ASSERT_FALSE(
+ ocl.ShouldAttemptCompile(OdrMetrics::Trigger::kUnknown, start_time + kSecondsPerDay / 2));
+ ASSERT_TRUE(ocl.ShouldAttemptCompile(OdrMetrics::Trigger::kUnknown, start_time + kSecondsPerDay));
// Add one more log entry
ocl.Log(/*apex_version=*/1,
@@ -163,16 +140,10 @@
OdrMetrics::Trigger::kApexVersionMismatch,
start_time,
ExitCode::kCompilationFailed);
- ASSERT_FALSE(ocl.ShouldAttemptCompile(
- /*apex_version=*/1,
- /*last_update_millis=*/0,
- OdrMetrics::Trigger::kApexVersionMismatch,
- start_time + kSecondsPerDay));
- ASSERT_TRUE(ocl.ShouldAttemptCompile(
- /*apex_version=*/1,
- /*last_update_millis=*/0,
- OdrMetrics::Trigger::kApexVersionMismatch,
- start_time + 2 * kSecondsPerDay));
+ ASSERT_FALSE(
+ ocl.ShouldAttemptCompile(OdrMetrics::Trigger::kUnknown, start_time + kSecondsPerDay));
+ ASSERT_TRUE(
+ ocl.ShouldAttemptCompile(OdrMetrics::Trigger::kUnknown, start_time + 2 * kSecondsPerDay));
// One more.
ocl.Log(/*apex_version=*/1,
@@ -180,16 +151,10 @@
OdrMetrics::Trigger::kApexVersionMismatch,
start_time,
ExitCode::kCompilationFailed);
- ASSERT_FALSE(ocl.ShouldAttemptCompile(
- /*apex_version=*/1,
- /*last_update_millis=*/0,
- OdrMetrics::Trigger::kApexVersionMismatch,
- start_time + 3 * kSecondsPerDay));
- ASSERT_TRUE(ocl.ShouldAttemptCompile(
- /*apex_version=*/1,
- /*last_update_millis=*/0,
- OdrMetrics::Trigger::kApexVersionMismatch,
- start_time + 4 * kSecondsPerDay));
+ ASSERT_FALSE(
+ ocl.ShouldAttemptCompile(OdrMetrics::Trigger::kUnknown, start_time + 3 * kSecondsPerDay));
+ ASSERT_TRUE(
+ ocl.ShouldAttemptCompile(OdrMetrics::Trigger::kUnknown, start_time + 4 * kSecondsPerDay));
// And one for the road.
ocl.Log(/*apex_version=*/1,
@@ -197,16 +162,10 @@
OdrMetrics::Trigger::kApexVersionMismatch,
start_time,
ExitCode::kCompilationFailed);
- ASSERT_FALSE(ocl.ShouldAttemptCompile(
- /*apex_version=*/1,
- /*last_update_millis=*/0,
- OdrMetrics::Trigger::kApexVersionMismatch,
- start_time + 7 * kSecondsPerDay));
- ASSERT_TRUE(ocl.ShouldAttemptCompile(
- /*apex_version=*/1,
- /*last_update_millis=*/0,
- OdrMetrics::Trigger::kApexVersionMismatch,
- start_time + 8 * kSecondsPerDay));
+ ASSERT_FALSE(
+ ocl.ShouldAttemptCompile(OdrMetrics::Trigger::kUnknown, start_time + 7 * kSecondsPerDay));
+ ASSERT_TRUE(
+ ocl.ShouldAttemptCompile(OdrMetrics::Trigger::kUnknown, start_time + 8 * kSecondsPerDay));
}
TEST(OdrCompilationLog, BackOffHappyHistory) {
@@ -221,21 +180,11 @@
OdrMetrics::Trigger::kApexVersionMismatch,
start_time,
ExitCode::kCompilationSuccess);
- ASSERT_FALSE(ocl.ShouldAttemptCompile(
- /*apex_version=*/1,
- /*last_update_millis=*/0,
- OdrMetrics::Trigger::kApexVersionMismatch,
- start_time));
- ASSERT_FALSE(ocl.ShouldAttemptCompile(
- /*apex_version=*/1,
- /*last_update_millis=*/0,
- OdrMetrics::Trigger::kApexVersionMismatch,
- start_time + kSecondsPerDay / 4));
- ASSERT_TRUE(ocl.ShouldAttemptCompile(
- /*apex_version=*/1,
- /*last_update_millis=*/0,
- OdrMetrics::Trigger::kApexVersionMismatch,
- start_time + kSecondsPerDay / 2));
+ ASSERT_FALSE(ocl.ShouldAttemptCompile(OdrMetrics::Trigger::kUnknown, start_time));
+ ASSERT_FALSE(
+ ocl.ShouldAttemptCompile(OdrMetrics::Trigger::kUnknown, start_time + kSecondsPerDay / 4));
+ ASSERT_TRUE(
+ ocl.ShouldAttemptCompile(OdrMetrics::Trigger::kUnknown, start_time + kSecondsPerDay / 2));
// Add a log entry for a failed compilation.
ocl.Log(/*apex_version=*/1,
@@ -243,16 +192,9 @@
OdrMetrics::Trigger::kApexVersionMismatch,
start_time,
ExitCode::kCompilationFailed);
- ASSERT_FALSE(ocl.ShouldAttemptCompile(
- /*apex_version=*/1,
- /*last_update_millis=*/0,
- OdrMetrics::Trigger::kApexVersionMismatch,
- start_time + kSecondsPerDay / 2));
- ASSERT_TRUE(ocl.ShouldAttemptCompile(
- /*apex_version=*/1,
- /*last_update_millis=*/0,
- OdrMetrics::Trigger::kApexVersionMismatch,
- start_time + kSecondsPerDay));
+ ASSERT_FALSE(
+ ocl.ShouldAttemptCompile(OdrMetrics::Trigger::kUnknown, start_time + kSecondsPerDay / 2));
+ ASSERT_TRUE(ocl.ShouldAttemptCompile(OdrMetrics::Trigger::kUnknown, start_time + kSecondsPerDay));
}
TEST_F(OdrCompilationLogTest, LogNumberOfEntriesAndPeek) {
@@ -344,11 +286,7 @@
{
OdrCompilationLog ocl(log_path);
- ASSERT_TRUE(ocl.ShouldAttemptCompile(
- /*apex_version=*/1,
- /*last_update_millis=*/0,
- OdrMetrics::Trigger::kApexVersionMismatch,
- start_time));
+ ASSERT_TRUE(ocl.ShouldAttemptCompile(OdrMetrics::Trigger::kUnknown, start_time));
}
{
@@ -364,21 +302,11 @@
{
OdrCompilationLog ocl(log_path);
- ASSERT_FALSE(ocl.ShouldAttemptCompile(
- /*apex_version=*/1,
- /*last_update_millis=*/0,
- OdrMetrics::Trigger::kApexVersionMismatch,
- start_time));
- ASSERT_FALSE(ocl.ShouldAttemptCompile(
- /*apex_version=*/1,
- /*last_update_millis=*/0,
- OdrMetrics::Trigger::kApexVersionMismatch,
- start_time + kSecondsPerDay / 2));
- ASSERT_TRUE(ocl.ShouldAttemptCompile(
- /*apex_version=*/1,
- /*last_update_millis=*/0,
- OdrMetrics::Trigger::kApexVersionMismatch,
- start_time + kSecondsPerDay));
+ ASSERT_FALSE(ocl.ShouldAttemptCompile(OdrMetrics::Trigger::kUnknown, start_time));
+ ASSERT_FALSE(
+ ocl.ShouldAttemptCompile(OdrMetrics::Trigger::kUnknown, start_time + kSecondsPerDay / 2));
+ ASSERT_TRUE(
+ ocl.ShouldAttemptCompile(OdrMetrics::Trigger::kUnknown, start_time + kSecondsPerDay));
}
{
@@ -394,16 +322,10 @@
{
OdrCompilationLog ocl(log_path);
- ASSERT_FALSE(ocl.ShouldAttemptCompile(
- /*apex_version=*/1,
- /*last_update_millis=*/0,
- OdrMetrics::Trigger::kApexVersionMismatch,
- start_time + kSecondsPerDay));
- ASSERT_TRUE(ocl.ShouldAttemptCompile(
- /*apex_version=*/1,
- /*last_update_millis=*/0,
- OdrMetrics::Trigger::kApexVersionMismatch,
- start_time + 2 * kSecondsPerDay));
+ ASSERT_FALSE(
+ ocl.ShouldAttemptCompile(OdrMetrics::Trigger::kUnknown, start_time + kSecondsPerDay));
+ ASSERT_TRUE(
+ ocl.ShouldAttemptCompile(OdrMetrics::Trigger::kUnknown, start_time + 2 * kSecondsPerDay));
}
{
@@ -418,16 +340,10 @@
{
OdrCompilationLog ocl(log_path);
- ASSERT_FALSE(ocl.ShouldAttemptCompile(
- /*apex_version=*/1,
- /*last_update_millis=*/0,
- OdrMetrics::Trigger::kApexVersionMismatch,
- start_time + 3 * kSecondsPerDay));
- ASSERT_TRUE(ocl.ShouldAttemptCompile(
- /*apex_version=*/1,
- /*last_update_millis=*/0,
- OdrMetrics::Trigger::kApexVersionMismatch,
- start_time + 4 * kSecondsPerDay));
+ ASSERT_FALSE(
+ ocl.ShouldAttemptCompile(OdrMetrics::Trigger::kUnknown, start_time + 3 * kSecondsPerDay));
+ ASSERT_TRUE(
+ ocl.ShouldAttemptCompile(OdrMetrics::Trigger::kUnknown, start_time + 4 * kSecondsPerDay));
}
{
@@ -442,62 +358,10 @@
{
OdrCompilationLog ocl(log_path);
- ASSERT_FALSE(ocl.ShouldAttemptCompile(
- /*apex_version=*/1,
- /*last_update_millis=*/0,
- OdrMetrics::Trigger::kApexVersionMismatch,
- start_time + 7 * kSecondsPerDay));
- ASSERT_TRUE(ocl.ShouldAttemptCompile(
- /*apex_version=*/1,
- /*last_update_millis=*/0,
- OdrMetrics::Trigger::kApexVersionMismatch,
- start_time + 8 * kSecondsPerDay));
- }
-}
-
-TEST(OdrCompilationLog, LastUpdateMillisChangeTriggersCompilation) {
- time_t start_time;
- time(&start_time);
-
- OdrCompilationLog ocl(/*compilation_log_path=*/nullptr);
-
- for (int64_t last_update_millis = 0; last_update_millis < 10000; last_update_millis += 1000) {
- static const int64_t kApexVersion = 19999;
- ASSERT_TRUE(ocl.ShouldAttemptCompile(
- kApexVersion, last_update_millis, OdrMetrics::Trigger::kApexVersionMismatch, start_time));
- ocl.Log(kApexVersion,
- last_update_millis,
- OdrMetrics::Trigger::kApexVersionMismatch,
- start_time,
- ExitCode::kCompilationSuccess);
- ASSERT_FALSE(ocl.ShouldAttemptCompile(kApexVersion,
- last_update_millis,
- OdrMetrics::Trigger::kApexVersionMismatch,
- start_time + 1));
- }
-}
-
-TEST(OdrCompilationLog, ApexVersionChangeTriggersCompilation) {
- time_t start_time;
- time(&start_time);
-
- OdrCompilationLog ocl(/*compilation_log_path=*/nullptr);
-
- for (int64_t apex_version = 0; apex_version < 10000; apex_version += 1000) {
- static const int64_t kLastUpdateMillis = 777;
- ASSERT_TRUE(ocl.ShouldAttemptCompile(apex_version,
- kLastUpdateMillis,
- OdrMetrics::Trigger::kApexVersionMismatch,
- start_time + 8 * kSecondsPerDay));
- ocl.Log(apex_version,
- kLastUpdateMillis,
- OdrMetrics::Trigger::kApexVersionMismatch,
- start_time,
- ExitCode::kCompilationSuccess);
- ASSERT_FALSE(ocl.ShouldAttemptCompile(apex_version,
- kLastUpdateMillis,
- OdrMetrics::Trigger::kApexVersionMismatch,
- start_time + 1));
+ ASSERT_FALSE(
+ ocl.ShouldAttemptCompile(OdrMetrics::Trigger::kUnknown, start_time + 7 * kSecondsPerDay));
+ ASSERT_TRUE(
+ ocl.ShouldAttemptCompile(OdrMetrics::Trigger::kUnknown, start_time + 8 * kSecondsPerDay));
}
}
@@ -519,8 +383,7 @@
OdrMetrics::Trigger::kApexVersionMismatch,
start_time,
ExitCode::kCompilationSuccess);
- ASSERT_FALSE(ocl.ShouldAttemptCompile(
- kApexVersion, kLastUpdateMillis, OdrMetrics::Trigger::kApexVersionMismatch, start_time));
+ ASSERT_FALSE(ocl.ShouldAttemptCompile(OdrMetrics::Trigger::kUnknown, start_time));
}
}
@@ -534,8 +397,7 @@
// Read log with updated version entry, check it is treated as out-of-date.
{
OdrCompilationLog ocl(scratch_file.GetFilename().c_str());
- ASSERT_TRUE(ocl.ShouldAttemptCompile(
- kApexVersion, kLastUpdateMillis, OdrMetrics::Trigger::kApexVersionMismatch, start_time));
+ ASSERT_TRUE(ocl.ShouldAttemptCompile(OdrMetrics::Trigger::kUnknown, start_time));
ASSERT_EQ(0u, ocl.NumberOfEntries());
}
}
diff --git a/odrefresh/odrefresh.cc b/odrefresh/odrefresh.cc
index a1985ad..c0b9e1f 100644
--- a/odrefresh/odrefresh.cc
+++ b/odrefresh/odrefresh.cc
@@ -865,7 +865,7 @@
for (const art_apex::ModuleInfo& module_info : cached_module_info_list->getModuleInfo()) {
if (!module_info.hasName()) {
LOG(INFO) << "Unexpected module info from cache-info. Missing module name.";
- metrics.SetTrigger(OdrMetrics::Trigger::kUnknown);
+ metrics.SetTrigger(OdrMetrics::Trigger::kApexVersionMismatch);
*cleanup_required = true;
return false;
}
@@ -1022,7 +1022,7 @@
// compatible with an old cache-info file. Further up-to-date checks are not possible if it
// does.
PLOG(ERROR) << "Failed to parse cache-info file: " << QuotePath(cache_info_filename_);
- metrics.SetTrigger(OdrMetrics::Trigger::kUnknown);
+ metrics.SetTrigger(OdrMetrics::Trigger::kApexVersionMismatch);
return cleanup_and_compile_all();
}
diff --git a/odrefresh/odrefresh_main.cc b/odrefresh/odrefresh_main.cc
index 4aca36f..a740252 100644
--- a/odrefresh/odrefresh_main.cc
+++ b/odrefresh/odrefresh_main.cc
@@ -305,9 +305,8 @@
return exit_code;
}
OdrCompilationLog compilation_log;
- if (!compilation_log.ShouldAttemptCompile(metrics.GetArtApexVersion(),
- metrics.GetArtApexLastUpdateMillis(),
- metrics.GetTrigger())) {
+ if (!compilation_log.ShouldAttemptCompile(metrics.GetTrigger())) {
+ LOG(INFO) << "Compilation skipped because it was attempted recently";
return ExitCode::kOkay;
}
ExitCode compile_result =
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 a6e9e73..3f6be8a 100644
--- a/test/odsign/test-src/com/android/tests/odsign/OdrefreshHostTest.java
+++ b/test/odsign/test-src/com/android/tests/odsign/OdrefreshHostTest.java
@@ -69,7 +69,6 @@
@Test
public void verifyArtSamegradeUpdateTriggersCompilation() throws Exception {
simulateArtApexUpgrade();
- sTestUtils.removeCompilationLogToAvoidBackoff();
long timeMs = getCurrentTimeMs();
getDevice().executeShellV2Command("odrefresh --compile");
@@ -80,7 +79,6 @@
@Test
public void verifyOtherApexSamegradeUpdateTriggersCompilation() throws Exception {
simulateApexUpgrade();
- sTestUtils.removeCompilationLogToAvoidBackoff();
long timeMs = getCurrentTimeMs();
getDevice().executeShellV2Command("odrefresh --compile");
@@ -91,7 +89,6 @@
@Test
public void verifyBootClasspathOtaTriggersCompilation() throws Exception {
simulateBootClasspathOta();
- sTestUtils.removeCompilationLogToAvoidBackoff();
long timeMs = getCurrentTimeMs();
getDevice().executeShellV2Command("odrefresh --compile");
@@ -102,7 +99,6 @@
@Test
public void verifySystemServerOtaTriggersCompilation() throws Exception {
simulateSystemServerOta();
- sTestUtils.removeCompilationLogToAvoidBackoff();
long timeMs = getCurrentTimeMs();
getDevice().executeShellV2Command("odrefresh --compile");