Fix the legacy version of `GetDexOptNeeded`.
- when `profile_changed` is true but the target compiler filter is worse
than the current:
- Before: returns kDex2OatForFilter
- After: returns kNoDexOptNeeded
- when `downgrade` is true but there is no usable odex/vdex files:
- Before: returns kDex2OatFromScratch
- After: return kNoDexOptNeeded
- when `profile_changed` and `downgrade` are both true:
- Before: kDex2OatForFilter
- After: the same as `profile_changed` is false and `downgrade` is
true.
- when the usable vdex file is in the DM file:
- Before: returns kDex2OatForFilter with an arbitrary sign
- After: returns kDex2OatFromScratch
Bug: 229268202
Test: m test-art-host-gtest-art_dex2oat_tests
Change-Id: I752d860adb8c026c762e8878ac060211df22670d
Merged-In: I752d860adb8c026c762e8878ac060211df22670d
(cherry picked from commit d0b1aed2dd4067dcdba7c449ff0b08da2109af09)
diff --git a/runtime/oat_file_assistant.cc b/runtime/oat_file_assistant.cc
index e111b27..454ee79 100644
--- a/runtime/oat_file_assistant.cc
+++ b/runtime/oat_file_assistant.cc
@@ -296,36 +296,21 @@
}
OatFileAssistant::DexOptTrigger OatFileAssistant::GetDexOptTrigger(
- CompilerFilter::Filter target_compiler_filter [[gnu::unused]],
- bool profile_changed,
- bool downgrade) {
- OatFileInfo& info = GetBestInfo();
-
- if (!info.IsUseable()) {
- // Essentially always recompile.
- return OatFileAssistant::DexOptTrigger{.targetFilterIsBetter = true};
- }
-
- const OatFile* file = info.GetFile();
- DCHECK(file != nullptr);
-
- CompilerFilter::Filter current = file->GetCompilerFilter();
- if (profile_changed && CompilerFilter::DependsOnProfile(current)) {
- // Since the profile has been changed, we should re-compile even if the compilation does not
- // make the compiler filter better.
- return OatFileAssistant::DexOptTrigger{
- .targetFilterIsBetter = true, .targetFilterIsSame = true, .targetFilterIsWorse = true};
- }
-
+ CompilerFilter::Filter target_compiler_filter, bool profile_changed, bool downgrade) {
if (downgrade) {
// The caller's intention is to downgrade the compiler filter. We should only re-compile if the
// target compiler filter is worse than the current one.
- return OatFileAssistant::DexOptTrigger{.targetFilterIsWorse = true};
+ return DexOptTrigger{.targetFilterIsWorse = true};
}
// This is the usual case. The caller's intention is to see if a better oat file can be generated.
- return OatFileAssistant::DexOptTrigger{.targetFilterIsBetter = true,
- .primaryBootImageBecomesUsable = true};
+ DexOptTrigger dexopt_trigger{.targetFilterIsBetter = true, .primaryBootImageBecomesUsable = true};
+ if (profile_changed && CompilerFilter::DependsOnProfile(target_compiler_filter)) {
+ // Since the profile has been changed, we should re-compile even if the compilation does not
+ // make the compiler filter better.
+ dexopt_trigger.targetFilterIsSame = true;
+ }
+ return dexopt_trigger;
}
int OatFileAssistant::GetDexOptNeeded(CompilerFilter::Filter target_compiler_filter,
@@ -334,6 +319,12 @@
OatFileInfo& info = GetBestInfo();
DexOptNeeded dexopt_needed = info.GetDexOptNeeded(
target_compiler_filter, GetDexOptTrigger(target_compiler_filter, profile_changed, downgrade));
+ if (dexopt_needed != kNoDexOptNeeded && (&info == &dm_for_oat_ || &info == &dm_for_odex_)) {
+ // The usable vdex file is in the DM file. This information cannot be encoded in the integer.
+ // Return kDex2OatFromScratch so that neither the vdex in the "oat" location nor the vdex in the
+ // "odex" location will be picked by installd.
+ return kDex2OatFromScratch;
+ }
if (info.IsOatLocation() || dexopt_needed == kDex2OatFromScratch) {
return dexopt_needed;
}
diff --git a/runtime/oat_file_assistant_test.cc b/runtime/oat_file_assistant_test.cc
index e6ab9ff..a70ec8a 100644
--- a/runtime/oat_file_assistant_test.cc
+++ b/runtime/oat_file_assistant_test.cc
@@ -20,7 +20,6 @@
#include <gtest/gtest.h>
#include <sys/param.h>
-#include <cmath>
#include <functional>
#include <memory>
#include <string>
@@ -933,9 +932,7 @@
/*expected_dexopt_needed=*/false,
/*expected_is_vdex_usable=*/true,
/*expected_location=*/OatFileAssistant::kLocationOat);
- // The legacy implementation is wrong.
- // TODO(jiakaiz): Fix it.
- EXPECT_EQ(OatFileAssistant::kDex2OatForFilter,
+ EXPECT_EQ(OatFileAssistant::kNoDexOptNeeded,
oat_file_assistant.GetDexOptNeeded(CompilerFilter::kVerify, /*profile_changed=*/true));
EXPECT_FALSE(oat_file_assistant.IsInBootClassPath());
@@ -1919,8 +1916,6 @@
// Case: We have a DEX file but we don't have an ODEX file for it. The caller's intention is to
// downgrade the compiler filter.
// Expect: Dexopt should never be performed regardless of the target compiler filter.
-// The legacy implementation is wrong.
-// TODO(jiakaiz): Fix it.
TEST_P(OatFileAssistantTest, DowngradeNoOdex) {
std::string dex_location = GetScratchDir() + "/TestDex.jar";
Copy(GetDexSrc1(), dex_location);
@@ -1936,7 +1931,7 @@
/*expected_dexopt_needed=*/false,
/*expected_is_vdex_usable=*/false,
/*expected_location=*/OatFileAssistant::kLocationNoneOrError);
- EXPECT_EQ(OatFileAssistant::kDex2OatFromScratch,
+ EXPECT_EQ(OatFileAssistant::kNoDexOptNeeded,
oat_file_assistant.GetDexOptNeeded(
CompilerFilter::kSpeed, /*profile_changed=*/false, /*downgrade=*/true));
@@ -1946,7 +1941,7 @@
/*expected_dexopt_needed=*/false,
/*expected_is_vdex_usable=*/false,
/*expected_location=*/OatFileAssistant::kLocationNoneOrError);
- EXPECT_EQ(OatFileAssistant::kDex2OatFromScratch,
+ EXPECT_EQ(OatFileAssistant::kNoDexOptNeeded,
oat_file_assistant.GetDexOptNeeded(
CompilerFilter::kSpeedProfile, /*profile_changed=*/false, /*downgrade=*/true));
@@ -1956,7 +1951,7 @@
/*expected_dexopt_needed=*/false,
/*expected_is_vdex_usable=*/false,
/*expected_location=*/OatFileAssistant::kLocationNoneOrError);
- EXPECT_EQ(OatFileAssistant::kDex2OatFromScratch,
+ EXPECT_EQ(OatFileAssistant::kNoDexOptNeeded,
oat_file_assistant.GetDexOptNeeded(
CompilerFilter::kVerify, /*profile_changed=*/false, /*downgrade=*/true));
}
@@ -1965,8 +1960,6 @@
// both `profile_changed` and `downgrade` being true. This won't happen in the real case. Just to be
// complete.
// Expect: The behavior should be as `profile_changed` is false and `downgrade` is true.
-// The legacy implementation is wrong.
-// TODO(jiakaiz): Fix it.
TEST_P(OatFileAssistantTest, ProfileChangedDowngrade) {
std::string dex_location = GetScratchDir() + "/TestDex.jar";
std::string odex_location = GetOdexDir() + "/TestDex.odex";
@@ -1977,11 +1970,11 @@
OatFileAssistant oat_file_assistant = CreateOatFileAssistant(dex_location.c_str());
- EXPECT_EQ(-OatFileAssistant::kDex2OatForFilter,
+ EXPECT_EQ(-OatFileAssistant::kNoDexOptNeeded,
oat_file_assistant.GetDexOptNeeded(
CompilerFilter::kSpeed, /*profile_changed=*/true, /*downgrade=*/true));
- EXPECT_EQ(-OatFileAssistant::kDex2OatForFilter,
+ EXPECT_EQ(-OatFileAssistant::kNoDexOptNeeded,
oat_file_assistant.GetDexOptNeeded(
CompilerFilter::kSpeedProfile, /*profile_changed=*/true, /*downgrade=*/true));
@@ -2080,9 +2073,8 @@
// Expect: Dexopt should be performed if the compiler filter is better than "verify". The location
// should be kLocationDm.
//
-// The legacy version is wrong this case. It returns kDex2OatForFilter with an arbitrary sign if the
-// target compiler filter is better than "verify".
-// TODO(jiakaiz): Fix it.
+// The legacy version should return kDex2OatFromScratch if the target compiler filter is better than
+// "verify".
TEST_P(OatFileAssistantTest, DmUpToDate) {
std::string dex_location = GetScratchDir() + "/TestDex.jar";
std::string dm_location = GetScratchDir() + "/TestDex.dm";
@@ -2110,8 +2102,8 @@
/*expected_dexopt_needed=*/true,
/*expected_is_vdex_usable=*/true,
/*expected_location=*/OatFileAssistant::kLocationDm);
- EXPECT_EQ(OatFileAssistant::kDex2OatForFilter,
- abs(oat_file_assistant.GetDexOptNeeded(CompilerFilter::kSpeed)));
+ EXPECT_EQ(OatFileAssistant::kDex2OatFromScratch,
+ oat_file_assistant.GetDexOptNeeded(CompilerFilter::kSpeed));
VerifyGetDexOptNeeded(&oat_file_assistant,
CompilerFilter::kSpeedProfile,
@@ -2119,8 +2111,8 @@
/*expected_dexopt_needed=*/true,
/*expected_is_vdex_usable=*/true,
/*expected_location=*/OatFileAssistant::kLocationDm);
- EXPECT_EQ(OatFileAssistant::kDex2OatForFilter,
- abs(oat_file_assistant.GetDexOptNeeded(CompilerFilter::kSpeedProfile)));
+ EXPECT_EQ(OatFileAssistant::kDex2OatFromScratch,
+ oat_file_assistant.GetDexOptNeeded(CompilerFilter::kSpeedProfile));
VerifyGetDexOptNeeded(&oat_file_assistant,
CompilerFilter::kVerify,
@@ -2129,7 +2121,7 @@
/*expected_is_vdex_usable=*/true,
/*expected_location=*/OatFileAssistant::kLocationDm);
EXPECT_EQ(OatFileAssistant::kNoDexOptNeeded,
- abs(oat_file_assistant.GetDexOptNeeded(CompilerFilter::kVerify)));
+ oat_file_assistant.GetDexOptNeeded(CompilerFilter::kVerify));
}
// Test that GetLocation of a dex file is the same whether the dex
