Consider the size of the reference profile when assessing IsFirstSave
This makes the detection logic for the first profile save much more
precise. It prevents the case where we would return true if
the data was already moved from the current profile to the reference
profile.
At the same time, rework part of the logic for early save. The checks
needed to be moved earlier to encompass the startup class resolution.
Test: run-test & manual
adb shell stop;
adb shell setprop dalvik.vm.ps-min-save-period-ms 300;
adb shell setprop dalvik.vm.extra-opts -verbose:profiler;
adb shell start;
adb shell cmd package compile com.android.systemui -r bg-dexopt
restart & check
Bug: 185979271
Merged-In: Icfec30c6f49c8e03b03ff4bcbc2b869393fdcbfe
Change-Id: Icfec30c6f49c8e03b03ff4bcbc2b869393fdcbfe
(cherry picked from commit beb9f2012587035a5c30d0eca8af458b2ad659b7)
diff --git a/runtime/jit/jit.cc b/runtime/jit/jit.cc
index 0af9aa3..5ee8871 100644
--- a/runtime/jit/jit.cc
+++ b/runtime/jit/jit.cc
@@ -390,10 +390,15 @@
}
}
-void Jit::StartProfileSaver(const std::string& filename,
- const std::vector<std::string>& code_paths) {
+void Jit::StartProfileSaver(const std::string& profile_filename,
+ const std::vector<std::string>& code_paths,
+ const std::string& ref_profile_filename) {
if (options_->GetSaveProfilingInfo()) {
- ProfileSaver::Start(options_->GetProfileSaverOptions(), filename, code_cache_, code_paths);
+ ProfileSaver::Start(options_->GetProfileSaverOptions(),
+ profile_filename,
+ code_cache_,
+ code_paths,
+ ref_profile_filename);
}
}
diff --git a/runtime/jit/jit.h b/runtime/jit/jit.h
index 59ad17c..a6e484f 100644
--- a/runtime/jit/jit.h
+++ b/runtime/jit/jit.h
@@ -335,10 +335,15 @@
}
// Starts the profile saver if the config options allow profile recording.
- // The profile will be stored in the specified `filename` and will contain
+ // The profile will be stored in the specified `profile_filename` and will contain
// information collected from the given `code_paths` (a set of dex locations).
- void StartProfileSaver(const std::string& filename,
- const std::vector<std::string>& code_paths);
+ //
+ // The `ref_profile_filename` denotes the path to the reference profile which
+ // might be queried to determine if an initial save should be done earlier.
+ // It can be empty indicating there is no reference profile.
+ void StartProfileSaver(const std::string& profile_filename,
+ const std::vector<std::string>& code_paths,
+ const std::string& ref_profile_filename);
void StopProfileSaver();
void DumpForSigQuit(std::ostream& os) REQUIRES(!lock_);
diff --git a/runtime/jit/profile_saver.cc b/runtime/jit/profile_saver.cc
index cb1ab77..425eadc 100644
--- a/runtime/jit/profile_saver.cc
+++ b/runtime/jit/profile_saver.cc
@@ -81,10 +81,7 @@
#endif
}
-ProfileSaver::ProfileSaver(const ProfileSaverOptions& options,
- const std::string& output_filename,
- jit::JitCodeCache* jit_code_cache,
- const std::vector<std::string>& code_paths)
+ProfileSaver::ProfileSaver(const ProfileSaverOptions& options, jit::JitCodeCache* jit_code_cache)
: jit_code_cache_(jit_code_cache),
shutting_down_(false),
last_time_ns_saver_woke_up_(0),
@@ -102,7 +99,6 @@
total_number_of_wake_ups_(0),
options_(options) {
DCHECK(options_.IsEnabled());
- AddTrackedLocations(output_filename, code_paths);
}
ProfileSaver::~ProfileSaver() {
@@ -128,6 +124,10 @@
// under mutex, but should drop it.
Locks::profiler_lock_->ExclusiveUnlock(self);
+ bool check_for_first_save =
+ options_.GetMinFirstSaveMs() != ProfileSaverOptions::kMinFirstSaveMsNotSet;
+ bool force_early_first_save = check_for_first_save && IsFirstSave();
+
// Fetch the resolved classes for the app images after sleeping for
// options_.GetSaveResolvedClassesDelayMs().
// TODO(calin) This only considers the case of the primary profile file.
@@ -135,7 +135,10 @@
// classes save (unless they started before the initial saving was done).
{
MutexLock mu(self, wait_lock_);
- const uint64_t end_time = NanoTime() + MsToNs(options_.GetSaveResolvedClassesDelayMs());
+
+ const uint64_t end_time = NanoTime() + MsToNs(force_early_first_save
+ ? options_.GetMinFirstSaveMs()
+ : options_.GetSaveResolvedClassesDelayMs());
while (!Runtime::Current()->GetStartupCompleted()) {
const uint64_t current_time = NanoTime();
if (current_time >= end_time) {
@@ -155,10 +158,16 @@
// exponential back off policy bounded by max_wait_without_jit.
uint32_t max_wait_without_jit = options_.GetMinSavePeriodMs() * 16;
uint64_t cur_wait_without_jit = options_.GetMinSavePeriodMs();
+
// Loop for the profiled methods.
while (!ShuttingDown(self)) {
- uint64_t sleep_start = NanoTime();
- {
+ // Sleep only if we don't have to force an early first save configured
+ // with GetMinFirstSaveMs().
+ // If we do have to save early, move directly to the processing part
+ // since we already slept before fetching and resolving the startup
+ // classes.
+ if (!force_early_first_save) {
+ uint64_t sleep_start = NanoTime();
uint64_t sleep_time = 0;
{
MutexLock mu(self, wait_lock_);
@@ -180,11 +189,7 @@
// We might have been woken up by a huge number of notifications to guarantee saving.
// If we didn't meet the minimum saving period go back to sleep (only if missed by
// a reasonable margin).
- bool check_for_first_save = options_.GetMinFirstSaveMs() !=
- ProfileSaverOptions::kMinFirstSaveMsNotSet;
- uint64_t min_save_period_ns = MsToNs(check_for_first_save && IsFirstSave()
- ? options_.GetMinFirstSaveMs()
- : options_.GetMinSavePeriodMs());
+ uint64_t min_save_period_ns = options_.GetMinSavePeriodMs();
while (min_save_period_ns * 0.9 > sleep_time) {
{
MutexLock mu(self, wait_lock_);
@@ -197,8 +202,8 @@
}
total_number_of_wake_ups_++;
}
+ total_ms_of_sleep_ += NsToMs(NanoTime() - sleep_start);
}
- total_ms_of_sleep_ += NsToMs(NanoTime() - sleep_start);
if (ShuttingDown(self)) {
break;
@@ -206,7 +211,16 @@
uint16_t number_of_new_methods = 0;
uint64_t start_work = NanoTime();
- bool profile_saved_to_disk = ProcessProfilingInfo(/*force_save=*/false, &number_of_new_methods);
+ // If we force an early_first_save do not run FetchAndCacheResolvedClassesAndMethods
+ // again. We just did it. So pass true to skip_class_and_method_fetching.
+ bool profile_saved_to_disk = ProcessProfilingInfo(
+ /*force_save=*/ false,
+ /*skip_class_and_method_fetching=*/ force_early_first_save,
+ &number_of_new_methods);
+
+ // Reset the flag, so we can continue on the normal schedule.
+ force_early_first_save = false;
+
// Update the notification counter based on result. Note that there might be contention on this
// but we don't care about to be 100% precise.
if (!profile_saved_to_disk) {
@@ -219,40 +233,45 @@
}
}
-// TODO(b/185979271): include reference profiles in the test.
-// The current profiles are cleared after bg-dexopt so this test will currently
-// return True after every bg-dexopt call.
+// Checks if the profile file is empty.
+// Return true if the size of the profile file is 0 or if there were errors when
+// trying to open the file.
+static bool IsProfileEmpty(const std::string& location) {
+ if (location.empty()) {
+ return true;
+ }
+
+ struct stat stat_buffer;
+ if (stat(location.c_str(), &stat_buffer) != 0) {
+ if (VLOG_IS_ON(profiler)) {
+ PLOG(WARNING) << "Failed to stat profile location for IsFirstUse: " << location;
+ }
+ return true;
+ }
+
+ VLOG(profiler) << "Profile " << location << " size=" << stat_buffer.st_size;
+ return stat_buffer.st_size == 0;
+}
+
bool ProfileSaver::IsFirstSave() {
- // Resolve any new registered locations.
- ResolveTrackedLocations();
Thread* self = Thread::Current();
- SafeMap<std::string, std::set<std::string>> tracked_locations;
+ SafeMap<std::string, std::string> tracked_locations;
{
// Make a copy so that we don't hold the lock while doing I/O.
MutexLock mu(self, *Locks::profiler_lock_);
- tracked_locations = tracked_dex_base_locations_;
+ tracked_locations = tracked_profiles_;
}
for (const auto& it : tracked_locations) {
if (ShuttingDown(self)) {
return false;
}
- const std::set<std::string>& locations = it.second;
+ const std::string& cur_profile = it.first;
+ const std::string& ref_profile = it.second;
// Check if any profile is non empty. If so, then this is not the first save.
- for (const auto& location : locations) {
- struct stat stat_buffer;
- if (stat(location.c_str(), &stat_buffer) != 0) {
- if (VLOG_IS_ON(profiler)) {
- PLOG(WARNING) << "Failed to stat profile location for IsFirstUse: " << location;
- }
- continue;
- }
- if (stat_buffer.st_size > 0) {
- return false;
- } else {
- VLOG(profiler) << "Profile location is empty: " << location;
- }
+ if (!IsProfileEmpty(cur_profile) || !IsProfileEmpty(ref_profile)) {
+ return false;
}
}
@@ -801,7 +820,10 @@
<< " in " << PrettyDuration(NanoTime() - start_time);
}
-bool ProfileSaver::ProcessProfilingInfo(bool force_save, /*out*/uint16_t* number_of_new_methods) {
+bool ProfileSaver::ProcessProfilingInfo(
+ bool force_save,
+ bool skip_class_and_method_fetching,
+ /*out*/uint16_t* number_of_new_methods) {
ScopedTrace trace(__PRETTY_FUNCTION__);
// Resolve any new registered locations.
@@ -819,9 +841,11 @@
*number_of_new_methods = 0;
}
- // We only need to do this once, not once per dex location.
- // TODO: Figure out a way to only do it when stuff has changed? It takes 30-50ms.
- FetchAndCacheResolvedClassesAndMethods(/*startup=*/ false);
+ if (!skip_class_and_method_fetching) {
+ // We only need to do this once, not once per dex location.
+ // TODO: Figure out a way to only do it when stuff has changed? It takes 30-50ms.
+ FetchAndCacheResolvedClassesAndMethods(/*startup=*/ false);
+ }
for (const auto& it : tracked_locations) {
if (!force_save && ShuttingDown(Thread::Current())) {
@@ -990,9 +1014,10 @@
}
void ProfileSaver::Start(const ProfileSaverOptions& options,
- const std::string& output_filename,
- jit::JitCodeCache* jit_code_cache,
- const std::vector<std::string>& code_paths) {
+ const std::string& output_filename,
+ jit::JitCodeCache* jit_code_cache,
+ const std::vector<std::string>& code_paths,
+ const std::string& ref_profile_filename) {
Runtime* const runtime = Runtime::Current();
DCHECK(options.IsEnabled());
DCHECK(runtime->GetJit() != nullptr);
@@ -1045,17 +1070,16 @@
// apps which share the same runtime).
DCHECK_EQ(instance_->jit_code_cache_, jit_code_cache);
// Add the code_paths to the tracked locations.
- instance_->AddTrackedLocations(output_filename, code_paths_to_profile);
+ instance_->AddTrackedLocations(output_filename, code_paths_to_profile, ref_profile_filename);
return;
}
VLOG(profiler) << "Starting profile saver using output file: " << output_filename
- << ". Tracking: " << android::base::Join(code_paths_to_profile, ':');
+ << ". Tracking: " << android::base::Join(code_paths_to_profile, ':')
+ << ". With reference profile: " << ref_profile_filename;
- instance_ = new ProfileSaver(options,
- output_filename,
- jit_code_cache,
- code_paths_to_profile);
+ instance_ = new ProfileSaver(options, jit_code_cache);
+ instance_->AddTrackedLocations(output_filename, code_paths_to_profile, ref_profile_filename);
// Create a new thread which does the saving.
CHECK_PTHREAD_CALL(
@@ -1094,7 +1118,10 @@
// Force save everything before destroying the thread since we want profiler_pthread_ to remain
// valid.
- profile_saver->ProcessProfilingInfo(/*force_save=*/true, /*number_of_new_methods=*/nullptr);
+ profile_saver->ProcessProfilingInfo(
+ /*force_ save=*/ true,
+ /*skip_class_and_method_fetching=*/ false,
+ /*number_of_new_methods=*/ nullptr);
// Wait for the saver thread to stop.
CHECK_PTHREAD_CALL(pthread_join, (profiler_pthread, nullptr), "profile saver thread shutdown");
@@ -1159,7 +1186,14 @@
}
void ProfileSaver::AddTrackedLocations(const std::string& output_filename,
- const std::vector<std::string>& code_paths) {
+ const std::vector<std::string>& code_paths,
+ const std::string& ref_profile_filename) {
+ // Register the output profile and its reference profile.
+ auto it = tracked_profiles_.find(output_filename);
+ if (it == tracked_profiles_.end()) {
+ tracked_profiles_.Put(output_filename, ref_profile_filename);
+ }
+
// Add the code paths to the list of tracked location.
AddTrackedLocationsToMap(output_filename, code_paths, &tracked_dex_base_locations_);
// The code paths may contain symlinks which could fool the profiler.
@@ -1206,7 +1240,10 @@
// but we only use this in testing when we now this won't happen.
// Refactor the way we handle the instance so that we don't end up in this situation.
if (saver != nullptr) {
- saver->ProcessProfilingInfo(/*force_save=*/true, /*number_of_new_methods=*/nullptr);
+ saver->ProcessProfilingInfo(
+ /*force_save=*/ true,
+ /*skip_class_and_method_fetching=*/ false,
+ /*number_of_new_methods=*/ nullptr);
}
}
diff --git a/runtime/jit/profile_saver.h b/runtime/jit/profile_saver.h
index ff6c169..b5fb1e6 100644
--- a/runtime/jit/profile_saver.h
+++ b/runtime/jit/profile_saver.h
@@ -30,10 +30,15 @@
public:
// Starts the profile saver thread if not already started.
// If the saver is already running it adds (output_filename, code_paths) to its tracked locations.
+ //
+ // The `ref_profile_filename` denotes the path to the reference profile which
+ // might be queried to determine if an initial save should be done earlier.
+ // It can be empty indicating there is no reference profile.
static void Start(const ProfileSaverOptions& options,
const std::string& output_filename,
jit::JitCodeCache* jit_code_cache,
- const std::vector<std::string>& code_paths)
+ const std::vector<std::string>& code_paths,
+ const std::string& ref_profile_filename)
REQUIRES(!Locks::profiler_lock_, !instance_->wait_lock_);
// Stops the profile saver thread.
@@ -58,10 +63,7 @@
class GetClassesAndMethodsHelper;
class ScopedDefaultPriority;
- ProfileSaver(const ProfileSaverOptions& options,
- const std::string& output_filename,
- jit::JitCodeCache* jit_code_cache,
- const std::vector<std::string>& code_paths);
+ ProfileSaver(const ProfileSaverOptions& options, jit::JitCodeCache* jit_code_cache);
~ProfileSaver();
static void* RunProfileSaverThread(void* arg)
@@ -78,7 +80,10 @@
// written to disk.
// If force_save is true, the saver will ignore any constraints which limit IO (e.g. will write
// the profile to disk even if it's just one new method).
- bool ProcessProfilingInfo(bool force_save, /*out*/uint16_t* number_of_new_methods)
+ bool ProcessProfilingInfo(
+ bool force_save,
+ bool skip_class_and_method_fetching,
+ /*out*/uint16_t* number_of_new_methods)
REQUIRES(!Locks::profiler_lock_)
REQUIRES(!Locks::mutator_lock_);
@@ -89,7 +94,8 @@
bool ShuttingDown(Thread* self) REQUIRES(!Locks::profiler_lock_);
void AddTrackedLocations(const std::string& output_filename,
- const std::vector<std::string>& code_paths)
+ const std::vector<std::string>& code_paths,
+ const std::string& ref_profile_filename)
REQUIRES(Locks::profiler_lock_);
// Fetches the current resolved classes and methods from the ClassLinker and stores them in the
@@ -130,6 +136,13 @@
SafeMap<std::string, std::set<std::string>> tracked_dex_base_locations_to_be_resolved_
GUARDED_BY(Locks::profiler_lock_);
+ // Collection of output profiles that the profile tracks.
+ // It maps output profile locations to reference profiles, and is used
+ // to determine if any profile is non-empty at the start of the ProfileSaver.
+ // This influences the time of the first ever save.
+ SafeMap<std::string, std::string> tracked_profiles_
+ GUARDED_BY(Locks::profiler_lock_);
+
bool shutting_down_ GUARDED_BY(Locks::profiler_lock_);
uint64_t last_time_ns_saver_woke_up_ GUARDED_BY(wait_lock_);
uint32_t jit_activity_notifications_;
diff --git a/runtime/jit/profile_saver_test.cc b/runtime/jit/profile_saver_test.cc
index 9a866a3..e737b7c 100644
--- a/runtime/jit/profile_saver_test.cc
+++ b/runtime/jit/profile_saver_test.cc
@@ -40,13 +40,9 @@
void PostRuntimeCreate() override {
// Create a profile saver.
Runtime* runtime = Runtime::Current();
- const std::vector<std::string> code_paths;
- const std::string fake_file = "fake_file";
profile_saver_ = new ProfileSaver(
runtime->GetJITOptions()->GetProfileSaverOptions(),
- fake_file,
- runtime->GetJitCodeCache(),
- code_paths);
+ runtime->GetJitCodeCache());
}
~ProfileSaverTest() {
diff --git a/runtime/native/dalvik_system_VMRuntime.cc b/runtime/native/dalvik_system_VMRuntime.cc
index 1590c9c..d46a36f 100644
--- a/runtime/native/dalvik_system_VMRuntime.cc
+++ b/runtime/native/dalvik_system_VMRuntime.cc
@@ -372,7 +372,7 @@
jclass clazz ATTRIBUTE_UNUSED,
jstring package_name ATTRIBUTE_UNUSED,
jstring cur_profile_file,
- jstring ref_profile_file ATTRIBUTE_UNUSED,
+ jstring ref_profile_file,
jobjectArray code_paths,
jint code_path_type ATTRIBUTE_UNUSED) {
std::vector<std::string> code_paths_vec;
@@ -388,7 +388,11 @@
std::string cur_profile_file_str(raw_cur_profile_file);
env->ReleaseStringUTFChars(cur_profile_file, raw_cur_profile_file);
- Runtime::Current()->RegisterAppInfo(code_paths_vec, cur_profile_file_str);
+ const char* raw_ref_profile_file = env->GetStringUTFChars(ref_profile_file, nullptr);
+ std::string ref_profile_file_str(raw_ref_profile_file);
+ env->ReleaseStringUTFChars(ref_profile_file, raw_ref_profile_file);
+
+ Runtime::Current()->RegisterAppInfo(code_paths_vec, cur_profile_file_str, ref_profile_file_str);
}
static jboolean VMRuntime_isBootClassPathOnDisk(JNIEnv* env, jclass, jstring java_instruction_set) {
diff --git a/runtime/runtime.cc b/runtime/runtime.cc
index fbb8508..74a563e 100644
--- a/runtime/runtime.cc
+++ b/runtime/runtime.cc
@@ -1004,7 +1004,12 @@
!jit_options_->GetProfileSaverOptions().GetProfilePath().empty()) {
std::vector<std::string> dex_filenames;
Split(class_path_string_, ':', &dex_filenames);
- RegisterAppInfo(dex_filenames, jit_options_->GetProfileSaverOptions().GetProfilePath());
+ // It's ok to pass "" to the ref profile filename. It indicates we don't have
+ // a reference profile.
+ RegisterAppInfo(
+ dex_filenames,
+ jit_options_->GetProfileSaverOptions().GetProfilePath(),
+ /*ref_profile_filename=*/ "");
}
return true;
@@ -2548,7 +2553,8 @@
}
void Runtime::RegisterAppInfo(const std::vector<std::string>& code_paths,
- const std::string& profile_output_filename) {
+ const std::string& profile_output_filename,
+ const std::string& ref_profile_filename) {
if (jit_.get() == nullptr) {
// We are not JITing. Nothing to do.
return;
@@ -2556,6 +2562,7 @@
VLOG(profiler) << "Register app with " << profile_output_filename
<< " " << android::base::Join(code_paths, ':');
+ VLOG(profiler) << "Reference profile is: " << ref_profile_filename;
if (profile_output_filename.empty()) {
LOG(WARNING) << "JIT profile information will not be recorded: profile filename is empty.";
@@ -2570,7 +2577,7 @@
return;
}
- jit_->StartProfileSaver(profile_output_filename, code_paths);
+ jit_->StartProfileSaver(profile_output_filename, code_paths, ref_profile_filename);
}
// Transaction support.
diff --git a/runtime/runtime.h b/runtime/runtime.h
index 517212e..9026e3d 100644
--- a/runtime/runtime.h
+++ b/runtime/runtime.h
@@ -525,7 +525,8 @@
}
void RegisterAppInfo(const std::vector<std::string>& code_paths,
- const std::string& profile_output_filename);
+ const std::string& profile_output_filename,
+ const std::string& ref_profile_filename);
// Transaction support.
bool IsActiveTransaction() const;