Merge "Refine OatFileAssistant.MakeUpToDate failure result."
diff --git a/runtime/oat_file_assistant.cc b/runtime/oat_file_assistant.cc
index bb90d46..ce892f3 100644
--- a/runtime/oat_file_assistant.cc
+++ b/runtime/oat_file_assistant.cc
@@ -179,9 +179,10 @@
return HasOriginalDexFiles() ? kDex2OatNeeded : kNoDexOptNeeded;
}
-bool OatFileAssistant::MakeUpToDate(CompilerFilter::Filter target, std::string* error_msg) {
+OatFileAssistant::ResultOfAttemptToUpdate
+OatFileAssistant::MakeUpToDate(CompilerFilter::Filter target, std::string* error_msg) {
switch (GetDexOptNeeded(target)) {
- case kNoDexOptNeeded: return true;
+ case kNoDexOptNeeded: return kUpdateSucceeded;
case kDex2OatNeeded: return GenerateOatFile(target, error_msg);
case kPatchOatNeeded: return RelocateOatFile(OdexFileName(), error_msg);
case kSelfPatchOatNeeded: return RelocateOatFile(OatFileName(), error_msg);
@@ -569,21 +570,21 @@
return true;
}
-bool OatFileAssistant::RelocateOatFile(const std::string* input_file,
- std::string* error_msg) {
+OatFileAssistant::ResultOfAttemptToUpdate
+OatFileAssistant::RelocateOatFile(const std::string* input_file, std::string* error_msg) {
CHECK(error_msg != nullptr);
if (input_file == nullptr) {
*error_msg = "Patching of oat file for dex location " + dex_location_
+ " not attempted because the input file name could not be determined.";
- return false;
+ return kUpdateNotAttempted;
}
const std::string& input_file_name = *input_file;
if (OatFileName() == nullptr) {
*error_msg = "Patching of oat file for dex location " + dex_location_
+ " not attempted because the oat file name could not be determined.";
- return false;
+ return kUpdateNotAttempted;
}
const std::string& oat_file_name = *OatFileName();
@@ -592,13 +593,13 @@
if (image_info == nullptr) {
*error_msg = "Patching of oat file " + oat_file_name
+ " not attempted because no image location was found.";
- return false;
+ return kUpdateNotAttempted;
}
if (!runtime->IsDex2OatEnabled()) {
*error_msg = "Patching of oat file " + oat_file_name
+ " not attempted because dex2oat is disabled";
- return false;
+ return kUpdateNotAttempted;
}
std::vector<std::string> argv;
@@ -613,28 +614,29 @@
// Manually delete the file. This ensures there is no garbage left over if
// the process unexpectedly died.
TEMP_FAILURE_RETRY(unlink(oat_file_name.c_str()));
- return false;
+ return kUpdateFailed;
}
// Mark that the oat file has changed and we should try to reload.
ClearOatFileCache();
- return true;
+ return kUpdateSucceeded;
}
-bool OatFileAssistant::GenerateOatFile(CompilerFilter::Filter target, std::string* error_msg) {
+OatFileAssistant::ResultOfAttemptToUpdate
+OatFileAssistant::GenerateOatFile(CompilerFilter::Filter target, std::string* error_msg) {
CHECK(error_msg != nullptr);
Runtime* runtime = Runtime::Current();
if (!runtime->IsDex2OatEnabled()) {
*error_msg = "Generation of oat file for dex location " + dex_location_
+ " not attempted because dex2oat is disabled.";
- return false;
+ return kUpdateNotAttempted;
}
if (OatFileName() == nullptr) {
*error_msg = "Generation of oat file for dex location " + dex_location_
+ " not attempted because the oat file name could not be determined.";
- return false;
+ return kUpdateNotAttempted;
}
const std::string& oat_file_name = *OatFileName();
@@ -643,7 +645,7 @@
// TODO: Why does dex2oat behave that way?
if (!OS::FileExists(dex_location_.c_str())) {
*error_msg = "Dex location " + dex_location_ + " does not exists.";
- return false;
+ return kUpdateNotAttempted;
}
std::unique_ptr<File> oat_file;
@@ -651,14 +653,14 @@
if (oat_file.get() == nullptr) {
*error_msg = "Generation of oat file " + oat_file_name
+ " not attempted because the oat file could not be created.";
- return false;
+ return kUpdateNotAttempted;
}
if (fchmod(oat_file->Fd(), 0644) != 0) {
*error_msg = "Generation of oat file " + oat_file_name
+ " not attempted because the oat file could not be made world readable.";
oat_file->Erase();
- return false;
+ return kUpdateNotAttempted;
}
std::vector<std::string> args;
@@ -672,18 +674,18 @@
// the process unexpectedly died.
oat_file->Erase();
TEMP_FAILURE_RETRY(unlink(oat_file_name.c_str()));
- return false;
+ return kUpdateFailed;
}
if (oat_file->FlushCloseOrErase() != 0) {
*error_msg = "Unable to close oat file " + oat_file_name;
TEMP_FAILURE_RETRY(unlink(oat_file_name.c_str()));
- return false;
+ return kUpdateFailed;
}
// Mark that the oat file has changed and we should try to reload.
ClearOatFileCache();
- return true;
+ return kUpdateSucceeded;
}
bool OatFileAssistant::Dex2Oat(const std::vector<std::string>& args,
diff --git a/runtime/oat_file_assistant.h b/runtime/oat_file_assistant.h
index db754b9..17f72fe 100644
--- a/runtime/oat_file_assistant.h
+++ b/runtime/oat_file_assistant.h
@@ -148,14 +148,26 @@
// given compiler filter.
DexOptNeeded GetDexOptNeeded(CompilerFilter::Filter target_compiler_filter);
+ // Return code used when attempting to generate updated code.
+ enum ResultOfAttemptToUpdate {
+ kUpdateFailed, // We tried making the code up to date, but
+ // encountered an unexpected failure.
+ kUpdateNotAttempted, // We wanted to update the code, but determined we
+ // should not make the attempt.
+ kUpdateSucceeded // We successfully made the code up to date
+ // (possibly by doing nothing).
+ };
+
// Attempts to generate or relocate the oat file as needed to make it up to
// date with in a way that is at least as good as an oat file generated with
// the given compiler filter.
- // Returns true on success.
+ // Returns the result of attempting to update the code.
//
- // If there is a failure, the value of error_msg will be set to a string
- // describing why there was failure. error_msg must not be null.
- bool MakeUpToDate(CompilerFilter::Filter target_compiler_filter, std::string* error_msg);
+ // If the result is not kUpdateSucceeded, the value of error_msg will be set
+ // to a string describing why there was a failure or the update was not
+ // attempted. error_msg must not be null.
+ ResultOfAttemptToUpdate MakeUpToDate(CompilerFilter::Filter target_compiler_filter,
+ std::string* error_msg);
// Returns an oat file that can be used for loading dex files.
// Returns null if no suitable oat file was found.
@@ -232,22 +244,20 @@
// Generates the oat file by relocation from the named input file.
// This does not check the current status before attempting to relocate the
// oat file.
- // Returns true on success.
- // This will fail if dex2oat is not enabled in the current runtime.
//
- // If there is a failure, the value of error_msg will be set to a string
- // describing why there was failure. error_msg must not be null.
- bool RelocateOatFile(const std::string* input_file, std::string* error_msg);
+ // If the result is not kUpdateSucceeded, the value of error_msg will be set
+ // to a string describing why there was a failure or the update was not
+ // attempted. error_msg must not be null.
+ ResultOfAttemptToUpdate RelocateOatFile(const std::string* input_file, std::string* error_msg);
// Generate the oat file from the dex file using the given compiler filter.
// This does not check the current status before attempting to generate the
// oat file.
- // Returns true on success.
- // This will fail if dex2oat is not enabled in the current runtime.
//
- // If there is a failure, the value of error_msg will be set to a string
- // describing why there was failure. error_msg must not be null.
- bool GenerateOatFile(CompilerFilter::Filter filter, std::string* error_msg);
+ // If the result is not kUpdateSucceeded, the value of error_msg will be set
+ // to a string describing why there was a failure or the update was not
+ // attempted. error_msg must not be null.
+ ResultOfAttemptToUpdate GenerateOatFile(CompilerFilter::Filter filter, std::string* error_msg);
// Executes dex2oat using the current runtime configuration overridden with
// the given arguments. This does not check to see if dex2oat is enabled in
diff --git a/runtime/oat_file_assistant_test.cc b/runtime/oat_file_assistant_test.cc
index c247812..bddfa4f 100644
--- a/runtime/oat_file_assistant_test.cc
+++ b/runtime/oat_file_assistant_test.cc
@@ -452,7 +452,8 @@
// Trying to make the oat file up to date should not fail or crash.
std::string error_msg;
- EXPECT_TRUE(oat_file_assistant.MakeUpToDate(CompilerFilter::kSpeed, &error_msg));
+ EXPECT_EQ(OatFileAssistant::kUpdateSucceeded,
+ oat_file_assistant.MakeUpToDate(CompilerFilter::kSpeed, &error_msg));
// Trying to get the best oat file should fail, but not crash.
std::unique_ptr<OatFile> oat_file = oat_file_assistant.GetBestOatFile();
@@ -703,7 +704,8 @@
// Make the oat file up to date.
std::string error_msg;
- ASSERT_TRUE(oat_file_assistant.MakeUpToDate(CompilerFilter::kSpeed, &error_msg)) << error_msg;
+ ASSERT_EQ(OatFileAssistant::kUpdateSucceeded,
+ oat_file_assistant.MakeUpToDate(CompilerFilter::kSpeed, &error_msg)) << error_msg;
EXPECT_EQ(OatFileAssistant::kNoDexOptNeeded,
oat_file_assistant.GetDexOptNeeded(CompilerFilter::kSpeed));
@@ -765,7 +767,8 @@
// Make the oat file up to date.
std::string error_msg;
- ASSERT_TRUE(oat_file_assistant.MakeUpToDate(CompilerFilter::kSpeed, &error_msg)) << error_msg;
+ ASSERT_EQ(OatFileAssistant::kUpdateSucceeded,
+ oat_file_assistant.MakeUpToDate(CompilerFilter::kSpeed, &error_msg)) << error_msg;
EXPECT_EQ(OatFileAssistant::kNoDexOptNeeded,
oat_file_assistant.GetDexOptNeeded(CompilerFilter::kSpeed));
@@ -821,7 +824,8 @@
// Make the oat file up to date. This should have no effect.
std::string error_msg;
- EXPECT_TRUE(oat_file_assistant.MakeUpToDate(CompilerFilter::kSpeed, &error_msg)) << error_msg;
+ EXPECT_EQ(OatFileAssistant::kUpdateSucceeded,
+ oat_file_assistant.MakeUpToDate(CompilerFilter::kSpeed, &error_msg)) << error_msg;
EXPECT_EQ(OatFileAssistant::kNoDexOptNeeded,
oat_file_assistant.GetDexOptNeeded(CompilerFilter::kSpeed));
@@ -871,7 +875,8 @@
// Make the oat file up to date.
std::string error_msg;
- ASSERT_TRUE(oat_file_assistant.MakeUpToDate(CompilerFilter::kSpeed, &error_msg)) << error_msg;
+ ASSERT_EQ(OatFileAssistant::kUpdateSucceeded,
+ oat_file_assistant.MakeUpToDate(CompilerFilter::kSpeed, &error_msg)) << error_msg;
EXPECT_EQ(OatFileAssistant::kNoDexOptNeeded,
oat_file_assistant.GetDexOptNeeded(CompilerFilter::kSpeed));
@@ -914,7 +919,8 @@
// Make the oat file up to date.
std::string error_msg;
- ASSERT_TRUE(oat_file_assistant.MakeUpToDate(CompilerFilter::kSpeed, &error_msg)) << error_msg;
+ ASSERT_EQ(OatFileAssistant::kUpdateSucceeded,
+ oat_file_assistant.MakeUpToDate(CompilerFilter::kSpeed, &error_msg)) << error_msg;
EXPECT_EQ(OatFileAssistant::kNoDexOptNeeded,
oat_file_assistant.GetDexOptNeeded(CompilerFilter::kSpeed));
@@ -1093,7 +1099,8 @@
OatFileAssistant oat_file_assistant(
dex_location.c_str(), oat_location.c_str(), kRuntimeISA, false, true);
std::string error_msg;
- ASSERT_TRUE(oat_file_assistant.MakeUpToDate(CompilerFilter::kSpeed, &error_msg)) << error_msg;
+ ASSERT_EQ(OatFileAssistant::kUpdateSucceeded,
+ oat_file_assistant.MakeUpToDate(CompilerFilter::kSpeed, &error_msg)) << error_msg;
std::unique_ptr<OatFile> oat_file = oat_file_assistant.GetBestOatFile();
ASSERT_TRUE(oat_file.get() != nullptr);
@@ -1123,7 +1130,8 @@
OatFileAssistant oat_file_assistant(
dex_location.c_str(), oat_location.c_str(), kRuntimeISA, false, true);
std::string error_msg;
- ASSERT_FALSE(oat_file_assistant.MakeUpToDate(CompilerFilter::kSpeed, &error_msg));
+ ASSERT_EQ(OatFileAssistant::kUpdateNotAttempted,
+ oat_file_assistant.MakeUpToDate(CompilerFilter::kSpeed, &error_msg));
std::unique_ptr<OatFile> oat_file = oat_file_assistant.GetBestOatFile();
ASSERT_TRUE(oat_file.get() == nullptr);
@@ -1138,7 +1146,8 @@
OatFileAssistant oat_file_assistant(
dex_location.c_str(), oat_location.c_str(), kRuntimeISA, false, true);
std::string error_msg;
- ASSERT_FALSE(oat_file_assistant.GenerateOatFile(CompilerFilter::kSpeed, &error_msg));
+ EXPECT_EQ(OatFileAssistant::kUpdateNotAttempted,
+ oat_file_assistant.GenerateOatFile(CompilerFilter::kSpeed, &error_msg));
}
// Turn an absolute path into a path relative to the current working
@@ -1217,7 +1226,8 @@
// Trying to make it up to date should have no effect.
std::string error_msg;
- EXPECT_TRUE(oat_file_assistant.MakeUpToDate(CompilerFilter::kSpeed, &error_msg));
+ EXPECT_EQ(OatFileAssistant::kUpdateSucceeded,
+ oat_file_assistant.MakeUpToDate(CompilerFilter::kSpeed, &error_msg));
EXPECT_TRUE(error_msg.empty());
}
diff --git a/runtime/oat_file_manager.cc b/runtime/oat_file_manager.cc
index 2f13f55..94f6345 100644
--- a/runtime/oat_file_manager.cc
+++ b/runtime/oat_file_manager.cc
@@ -329,8 +329,20 @@
// Update the oat file on disk if we can. This may fail, but that's okay.
// Best effort is all that matters here.
- if (!oat_file_assistant.MakeUpToDate(filter_, /*out*/ &error_msg)) {
- LOG(INFO) << error_msg;
+ switch (oat_file_assistant.MakeUpToDate(filter_, /*out*/ &error_msg)) {
+ case OatFileAssistant::kUpdateFailed:
+ LOG(WARNING) << error_msg;
+ break;
+
+ case OatFileAssistant::kUpdateNotAttempted:
+ // Avoid spamming the logs if we decided not to attempt making the oat
+ // file up to date.
+ VLOG(oat) << error_msg;
+ break;
+
+ case OatFileAssistant::kUpdateSucceeded:
+ // Nothing to do.
+ break;
}
// Get the oat file on disk.
