Merge "Allow DexFile#getDexOptNeeded to check case when downgrading is required"

7 files changed, 73 insertions, 25 deletions

diff --git a/dexoptanalyzer/dexoptanalyzer.cc b/dexoptanalyzer/dexoptanalyzer.cc
index e2c159aa5c..fc72bbdb87 100644
--- a/dexoptanalyzer/dexoptanalyzer.cc
+++ b/dexoptanalyzer/dexoptanalyzer.cc
@@ -97,6 +97,9 @@ NO_RETURN static void Usage(const char *fmt, ...) {
   UsageError("  --android-data=<directory>: optional, the directory which should be used as");
   UsageError("       android-data. By default ANDROID_DATA env variable is used.");
   UsageError("");
+  UsageError("  --downgrade: optional, if the purpose of dexopt is to downgrade the dex file");
+  UsageError("       By default, dexopt considers upgrade case.");
+  UsageError("");
   UsageError("Return code:");
   UsageError("  To make it easier to integrate with the internal tools this command will make");
   UsageError("    available its result (dexoptNeeded) as the exit/return code. i.e. it will not");
@@ -121,7 +124,9 @@ NO_RETURN static void Usage(const char *fmt, ...) {
 
 class DexoptAnalyzer FINAL {
  public:
-  DexoptAnalyzer() : assume_profile_changed_(false) {}
+  DexoptAnalyzer() :
+      assume_profile_changed_(false),
+      downgrade_(false) {}
 
   void ParseArgs(int argc, char **argv) {
     original_argc = argc;
@@ -160,9 +165,9 @@ class DexoptAnalyzer FINAL {
         // compute dalvik-cache folder). This is mostly used in tests.
         std::string new_android_data = option.substr(strlen("--android-data=")).ToString();
         setenv("ANDROID_DATA", new_android_data.c_str(), 1);
-      } else {
-        Usage("Unknown argument '%s'", option.data());
-      }
+      } else if (option.starts_with("--downgrade")) {
+        downgrade_ = true;
+      } else { Usage("Unknown argument '%s'", option.data()); }
     }
 
     if (image_.empty()) {
@@ -225,7 +230,7 @@ class DexoptAnalyzer FINAL {
       return kNoDexOptNeeded;
     }
     int dexoptNeeded = oat_file_assistant.GetDexOptNeeded(
-        compiler_filter_, assume_profile_changed_);
+        compiler_filter_, assume_profile_changed_, downgrade_);
 
     // Convert OatFileAssitant codes to dexoptanalyzer codes.
     switch (dexoptNeeded) {
@@ -249,6 +254,7 @@ class DexoptAnalyzer FINAL {
   InstructionSet isa_;
   CompilerFilter::Filter compiler_filter_;
   bool assume_profile_changed_;
+  bool downgrade_;
   std::string image_;
 };
 
diff --git a/dexoptanalyzer/dexoptanalyzer_test.cc b/dexoptanalyzer/dexoptanalyzer_test.cc
index 1703ff4cbc..1cbf5461a4 100644
--- a/dexoptanalyzer/dexoptanalyzer_test.cc
+++ b/dexoptanalyzer/dexoptanalyzer_test.cc
@@ -71,12 +71,13 @@ class DexoptAnalyzerTest : public DexoptTest {
   // as the output of OatFileAssistant::GetDexOptNeeded.
   void Verify(const std::string& dex_file,
               CompilerFilter::Filter compiler_filter,
-              bool assume_profile_changed = false) {
+              bool assume_profile_changed = false,
+              bool downgrade = false) {
     int dexoptanalyzerResult = Analyze(dex_file, compiler_filter, assume_profile_changed);
     dexoptanalyzerResult = DexoptanalyzerToOatFileAssistant(dexoptanalyzerResult);
     OatFileAssistant oat_file_assistant(dex_file.c_str(), kRuntimeISA, /*load_executable*/ false);
     int assistantResult = oat_file_assistant.GetDexOptNeeded(
-        compiler_filter, assume_profile_changed);
+        compiler_filter, assume_profile_changed, downgrade);
     EXPECT_EQ(assistantResult, dexoptanalyzerResult);
   }
 };
@@ -118,6 +119,16 @@ TEST_F(DexoptAnalyzerTest, ProfileOatUpToDate) {
   Verify(dex_location, CompilerFilter::kQuicken, true);
 }
 
+TEST_F(DexoptAnalyzerTest, Downgrade) {
+  std::string dex_location = GetScratchDir() + "/Downgrade.jar";
+  Copy(GetDexSrc1(), dex_location);
+  GenerateOatForTest(dex_location.c_str(), CompilerFilter::kQuicken);
+
+  Verify(dex_location, CompilerFilter::kSpeedProfile, false, true);
+  Verify(dex_location, CompilerFilter::kQuicken, false, true);
+  Verify(dex_location, CompilerFilter::kVerify, false, true);
+}
+
 // Case: We have a MultiDEX file and up-to-date OAT file for it.
 TEST_F(DexoptAnalyzerTest, MultiDexOatUpToDate) {
   std::string dex_location = GetScratchDir() + "/MultiDexOatUpToDate.jar";
diff --git a/runtime/compiler_filter.cc b/runtime/compiler_filter.cc
index 4847f38489..7b2dd05156 100644
--- a/runtime/compiler_filter.cc
+++ b/runtime/compiler_filter.cc
@@ -165,6 +165,10 @@ bool CompilerFilter::IsAsGoodAs(Filter current, Filter target) {
   return current >= target;
 }
 
+bool CompilerFilter::IsBetter(Filter current, Filter target) {
+  return current > target;
+}
+
 std::string CompilerFilter::NameOfFilter(Filter filter) {
   switch (filter) {
     case CompilerFilter::kAssumeVerified: return "assume-verified";
diff --git a/runtime/compiler_filter.h b/runtime/compiler_filter.h
index f802439053..60975b04f7 100644
--- a/runtime/compiler_filter.h
+++ b/runtime/compiler_filter.h
@@ -84,6 +84,11 @@ class CompilerFilter FINAL {
   // not as good as kSpeed.
   static bool IsAsGoodAs(Filter current, Filter target);
 
+  // Returns true if 'current' compiler filter is better than 'target' compiler
+  // filter. Compared to IsAsGoodAs, this returns false if the compiler filters are
+  // equal.
+  static bool IsBetter(Filter current, Filter target);
+
   // Return the flag name of the given filter.
   // For example: given kVerifyAtRuntime, returns "verify-at-runtime".
   // The name returned corresponds to the name accepted by
diff --git a/runtime/native/dalvik_system_DexFile.cc b/runtime/native/dalvik_system_DexFile.cc
index f6a836079e..07dfb65972 100644
--- a/runtime/native/dalvik_system_DexFile.cc
+++ b/runtime/native/dalvik_system_DexFile.cc
@@ -458,7 +458,8 @@ static jint GetDexOptNeeded(JNIEnv* env,
                             const char* filename,
                             const char* instruction_set,
                             const char* compiler_filter_name,
-                            bool profile_changed) {
+                            bool profile_changed,
+                            bool downgrade) {
   if ((filename == nullptr) || !OS::FileExists(filename)) {
     LOG(ERROR) << "DexFile_getDexOptNeeded file '" << filename << "' does not exist";
     ScopedLocalRef<jclass> fnfe(env, env->FindClass("java/io/FileNotFoundException"));
@@ -492,7 +493,7 @@ static jint GetDexOptNeeded(JNIEnv* env,
   if (oat_file_assistant.IsInBootClassPath()) {
     return OatFileAssistant::kNoDexOptNeeded;
   }
-  return oat_file_assistant.GetDexOptNeeded(filter, profile_changed);
+  return oat_file_assistant.GetDexOptNeeded(filter, profile_changed, downgrade);
 }
 
 static jstring DexFile_getDexFileStatus(JNIEnv* env,
@@ -528,7 +529,8 @@ static jint DexFile_getDexOptNeeded(JNIEnv* env,
                                     jstring javaFilename,
                                     jstring javaInstructionSet,
                                     jstring javaTargetCompilerFilter,
-                                    jboolean newProfile) {
+                                    jboolean newProfile,
+                                    jboolean downgrade) {
   ScopedUtfChars filename(env, javaFilename);
   if (env->ExceptionCheck()) {
     return -1;
@@ -548,7 +550,8 @@ static jint DexFile_getDexOptNeeded(JNIEnv* env,
                          filename.c_str(),
                          instruction_set.c_str(),
                          target_compiler_filter.c_str(),
-                         newProfile == JNI_TRUE);
+                         newProfile == JNI_TRUE,
+                         downgrade == JNI_TRUE);
 }
 
 // public API
@@ -725,7 +728,7 @@ static JNINativeMethod gMethods[] = {
   NATIVE_METHOD(DexFile, getClassNameList, "(Ljava/lang/Object;)[Ljava/lang/String;"),
   NATIVE_METHOD(DexFile, isDexOptNeeded, "(Ljava/lang/String;)Z"),
   NATIVE_METHOD(DexFile, getDexOptNeeded,
-                "(Ljava/lang/String;Ljava/lang/String;Ljava/lang/String;Z)I"),
+                "(Ljava/lang/String;Ljava/lang/String;Ljava/lang/String;ZZ)I"),
   NATIVE_METHOD(DexFile, openDexFileNative,
                 "(Ljava/lang/String;"
                 "Ljava/lang/String;"
diff --git a/runtime/oat_file_assistant.cc b/runtime/oat_file_assistant.cc
index c8766578c4..96423c3ac3 100644
--- a/runtime/oat_file_assistant.cc
+++ b/runtime/oat_file_assistant.cc
@@ -187,9 +187,11 @@ bool OatFileAssistant::Lock(std::string* error_msg) {
   return true;
 }
 
-int OatFileAssistant::GetDexOptNeeded(CompilerFilter::Filter target, bool profile_changed) {
+int OatFileAssistant::GetDexOptNeeded(CompilerFilter::Filter target,
+                                      bool profile_changed,
+                                      bool downgrade) {
   OatFileInfo& info = GetBestInfo();
-  DexOptNeeded dexopt_needed = info.GetDexOptNeeded(target, profile_changed);
+  DexOptNeeded dexopt_needed = info.GetDexOptNeeded(target, profile_changed, downgrade);
   if (info.IsOatLocation() || dexopt_needed == kDex2OatFromScratch) {
     return dexopt_needed;
   }
@@ -230,7 +232,7 @@ OatFileAssistant::MakeUpToDate(bool profile_changed, std::string* error_msg) {
   }
 
   OatFileInfo& info = GetBestInfo();
-  switch (info.GetDexOptNeeded(target, profile_changed)) {
+  switch (info.GetDexOptNeeded(target, profile_changed, /*downgrade*/ false)) {
     case kNoDexOptNeeded:
       return kUpdateSucceeded;
 
@@ -1005,9 +1007,9 @@ OatFileAssistant::OatStatus OatFileAssistant::OatFileInfo::Status() {
 }
 
 OatFileAssistant::DexOptNeeded OatFileAssistant::OatFileInfo::GetDexOptNeeded(
-    CompilerFilter::Filter target, bool profile_changed) {
+    CompilerFilter::Filter target, bool profile_changed, bool downgrade) {
   bool compilation_desired = CompilerFilter::IsAotCompilationEnabled(target);
-  bool filter_okay = CompilerFilterIsOkay(target, profile_changed);
+  bool filter_okay = CompilerFilterIsOkay(target, profile_changed, downgrade);
 
   if (filter_okay && Status() == kOatUpToDate) {
     // The oat file is in good shape as is.
@@ -1064,7 +1066,7 @@ const OatFile* OatFileAssistant::OatFileInfo::GetFile() {
 }
 
 bool OatFileAssistant::OatFileInfo::CompilerFilterIsOkay(
-    CompilerFilter::Filter target, bool profile_changed) {
+    CompilerFilter::Filter target, bool profile_changed, bool downgrade) {
   const OatFile* file = GetFile();
   if (file == nullptr) {
     return false;
@@ -1075,7 +1077,8 @@ bool OatFileAssistant::OatFileInfo::CompilerFilterIsOkay(
     VLOG(oat) << "Compiler filter not okay because Profile changed";
     return false;
   }
-  return CompilerFilter::IsAsGoodAs(current, target);
+  return downgrade ? !CompilerFilter::IsBetter(current, target) :
+    CompilerFilter::IsAsGoodAs(current, target);
 }
 
 bool OatFileAssistant::OatFileInfo::IsExecutable() {
diff --git a/runtime/oat_file_assistant.h b/runtime/oat_file_assistant.h
index 92d87eaeae..320aa4f860 100644
--- a/runtime/oat_file_assistant.h
+++ b/runtime/oat_file_assistant.h
@@ -147,13 +147,24 @@ class OatFileAssistant {
   bool Lock(std::string* error_msg);
 
   // Return what action needs to be taken to produce up-to-date code for this
-  // dex location that is at least as good as an oat file generated with the
-  // given compiler filter. profile_changed should be true to indicate the
-  // profile has recently changed for this dex location.
+  // dex location. If "downgrade" is set to false, it verifies if the current
+  // compiler filter is at least as good as an oat file generated with the
+  // given compiler filter otherwise, if its set to true, it checks whether
+  // the oat file generated with the target filter will be downgraded as
+  // compared to the current state. For example, if the current compiler filter is
+  // quicken, and target filter is verify, it will recommend to dexopt, while
+  // if the target filter is speed profile, it will recommend to keep it in its
+  // current state.
+  // profile_changed should be true to indicate the profile has recently changed
+  // for this dex location.
+  // If the purpose of the dexopt is to downgrade the compiler filter,
+  // set downgrade to true.
   // Returns a positive status code if the status refers to the oat file in
   // the oat location. Returns a negative status code if the status refers to
   // the oat file in the odex location.
-  int GetDexOptNeeded(CompilerFilter::Filter target_compiler_filter, bool profile_changed = false);
+  int GetDexOptNeeded(CompilerFilter::Filter target_compiler_filter,
+                      bool profile_changed = false,
+                      bool downgrade = false);
 
   // Returns true if there is up-to-date code for this dex location,
   // irrespective of the compiler filter of the up-to-date code.
@@ -310,8 +321,11 @@ class OatFileAssistant {
     // given target_compilation_filter.
     // profile_changed should be true to indicate the profile has recently
     // changed for this dex location.
+    // downgrade should be true if the purpose of dexopt is to downgrade the
+    // compiler filter.
     DexOptNeeded GetDexOptNeeded(CompilerFilter::Filter target_compiler_filter,
-                                 bool profile_changed);
+                                 bool profile_changed,
+                                 bool downgrade);
 
     // Returns the loaded file.
     // Loads the file if needed. Returns null if the file failed to load.
@@ -344,7 +358,9 @@ class OatFileAssistant {
     // least as good as the given target filter. profile_changed should be
     // true to indicate the profile has recently changed for this dex
     // location.
-    bool CompilerFilterIsOkay(CompilerFilter::Filter target, bool profile_changed);
+    // downgrade should be true if the purpose of dexopt is to downgrade the
+    // compiler filter.
+    bool CompilerFilterIsOkay(CompilerFilter::Filter target, bool profile_changed, bool downgrade);
 
     // Release the loaded oat file.
     // Returns null if the oat file hasn't been loaded.