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
LineageOS-OP2 · Nov 17, 2021 · 5a65d03 · 5a65d03
1 parent 2498d85
commit 5a65d03
Show file tree

Hide file tree

Showing 6 changed files with 58 additions and 216 deletions.
diff --git a/odrefresh/odr_compilation_log.cc b/odrefresh/odr_compilation_log.cc
@@ -179,27 +179,15 @@ void OdrCompilationLog::Log(int64_t apex_version,
   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
@@ -65,10 +65,7 @@ class OdrCompilationLog {
   ~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
@@ -103,24 +103,16 @@ TEST(OdrCompilationLogEntry, ReadMultiple) {
 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,84 +121,51 @@ TEST(OdrCompilationLog, BackOffNoHistory) {
 
   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,
           /*last_update_millis=*/0,
           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,
           /*last_update_millis=*/0,
           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,
           /*last_update_millis=*/0,
           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,
           /*last_update_millis=*/0,
           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,38 +180,21 @@ TEST(OdrCompilationLog, BackOffHappyHistory) {
           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,
           /*last_update_millis=*/0,
           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 @@ TEST_F(OdrCompilationLogTest, BackoffBasedOnLog) {
   {
     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 @@ TEST_F(OdrCompilationLogTest, BackoffBasedOnLog) {
 
   {
     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 @@ TEST_F(OdrCompilationLogTest, BackoffBasedOnLog) {
   {
     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 @@ TEST_F(OdrCompilationLogTest, BackoffBasedOnLog) {
 
   {
     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 @@ TEST_F(OdrCompilationLogTest, BackoffBasedOnLog) {
 
   {
     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 @@ TEST_F(OdrCompilationLogTest, NewLogVersionTriggersCompilation) {
               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 @@ TEST_F(OdrCompilationLogTest, NewLogVersionTriggersCompilation) {
   // 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());
   }
 }