Perfprofd: various changes related to config parameters.

Details:
- turn "max unprocessed profiles" into a configurable parameter.
- use a longer default collection interval
- reread config file on very iteration through the main loop,
  so as to incorporate new parameters written by the upload service

Bug: http://b/19483574
Change-Id: I1ecacbdeccf26f09ddd8387aef0f2587483eb967
(cherry picked from commit f353d8bf370eab2117e6259630f5540f12b361b0)

diff --git a/perfprofd/perfprofdcore.cc b/perfprofd/perfprofdcore.cc
index b5f1872..1cf08ad 100644 (file)
--- a/perfprofd/perfprofdcore.cc
+++ b/perfprofd/perfprofdcore.cc
@@ -106,11 +106,6 @@ static const char *config_file_path =
     "/data/data/com.google.android.gms/files/perfprofd.conf";
 
 //
-// Set by SIGHUP signal handler
-//
-volatile unsigned please_reread_config_file = 0;
-
-//
 // This table describes the config file syntax in terms of key/value pairs.
 // Values come in two flavors: strings, or unsigned integers. In the latter
 // case the reader sets allowable minimum/maximum for the setting.
@@ -126,7 +121,7 @@ class ConfigReader {
   std::string getStringValue(const char *key) const;
 
   // read the specified config file, applying any settings it contains
-  void readFile();
+  void readFile(bool initial);
 
  private:
   void addUnsignedEntry(const char *key,
@@ -163,7 +158,7 @@ void ConfigReader::addDefaultEntries()
   // set to 100, then over time we want to see a perf profile
   // collected every 100 seconds). The actual time within the interval
   // for the collection is chosen randomly.
-  addUnsignedEntry("collection_interval", 901, 100, UINT32_MAX);
+  addUnsignedEntry("collection_interval", 14400, 100, UINT32_MAX);
 
   // Use the specified fixed seed for random number generation (unit
   // testing)
@@ -205,6 +200,11 @@ void ConfigReader::addDefaultEntries()
   addUnsignedEntry("hardwire_cpus", 1, 0, 1);
   addUnsignedEntry("hardwire_cpus_max_duration", 5, 1, UINT32_MAX);
 
+  // Maximum number of unprocessed profiles we can accumulate in the
+  // destination directory. Once we reach this limit, we continue
+  // to collect, but we just overwrite the most recent profile.
+  addUnsignedEntry("max_unprocessed_profiles", 10, 1, UINT32_MAX);
+
   // If set to 1, pass the -g option when invoking 'perf' (requests
   // stack traces as opposed to flat profile).
   addUnsignedEntry("stack_profile", 0, 0, 1);
@@ -323,11 +323,13 @@ static bool isblank(const std::string &line)
   return true;
 }
 
-void ConfigReader::readFile()
+void ConfigReader::readFile(bool initial)
 {
   FILE *fp = fopen(config_file_path, "r");
   if (!fp) {
-    W_ALOGE("unable to open configuration file %s", config_file_path);
+    if (initial) {
+      W_ALOGE("unable to open configuration file %s", config_file_path);
+    }
     return;
   }
 
@@ -636,6 +638,9 @@ static void cleanup_destination_dir(const ConfigReader &config)
       }
     }
     closedir(dir);
+  } else {
+    W_ALOGW("unable to open destination dir %s for cleanup",
+            dest_dir.c_str());
   }
 }
 
@@ -680,7 +685,8 @@ static bool post_process(const ConfigReader &config, int current_seq)
     fclose(fp);
   }
 
-  if (produced.size() >= MAX_UNPROCESSED_FILE) {
+  unsigned maxLive = config.getUnsignedValue("max_unprocessed_profiles");
+  if (produced.size() >= maxLive) {
     return false;
   }
 
@@ -774,12 +780,11 @@ static PROFILE_RESULT collect_profile(const ConfigReader &config, int seq)
 }
 
 //
-// SIGHUP handler. Sets a flag to indicate that we should reread the
-// config file
+// SIGHUP handler. Sending SIGHUP to the daemon can be used to break it
+// out of a sleep() call so as to trigger a new collection (debugging)
 //
 static void sig_hup(int /* signum */)
 {
-  please_reread_config_file = 1;
 }
 
 //
@@ -828,7 +833,7 @@ static void set_seed(ConfigReader &config)
 //
 static void init(ConfigReader &config)
 {
-  config.readFile();
+  config.readFile(true);
   set_seed(config);
   cleanup_destination_dir(config);
 
@@ -880,11 +885,9 @@ int perfprofd_main(int argc, char** argv)
                            config.getUnsignedValue("collection_interval"));
     perfprofd_sleep(sleep_before_collect);
 
-    // Reread config file if someone sent a SIGHUP
-    if (please_reread_config_file) {
-      config.readFile();
-      please_reread_config_file = 0;
-    }
+    // Reread config file -- the uploader may have rewritten it as a result
+    // of a gservices change
+    config.readFile(false);
 
     // Check for profiling enabled...
     CKPROFILE_RESULT ckresult = check_profiling_enabled(config);

diff --git a/perfprofd/perfprofdcore.h b/perfprofd/perfprofdcore.h
index f3b1717..53695e2 100644 (file)
--- a/perfprofd/perfprofdcore.h
+++ b/perfprofd/perfprofdcore.h
@@ -28,9 +28,6 @@
 // by perfprofd within the destination directory; consumed by GmsCore.
 #define PRODUCED_FILENAME "perfprofd_produced.txt"
 
-// Maximum number of encoded perf.data files stored in destination dir
-#define MAX_UNPROCESSED_FILE 10
-
 // Main routine for perfprofd daemon
 extern int perfprofd_main(int argc, char **argv);
 

diff --git a/perfprofd/tests/perfprofd_test.cc b/perfprofd/tests/perfprofd_test.cc
index 0dd0f73..d13e21e 100644 (file)
--- a/perfprofd/tests/perfprofd_test.cc
+++ b/perfprofd/tests/perfprofd_test.cc
@@ -304,7 +304,7 @@ TEST_F(PerfProfdTest, MissingGMS)
   //
   PerfProfdRunner runner;
   runner.addToConfig("only_debug_build=0");
-  runner.addToConfig("trace_config_read=1");
+  runner.addToConfig("trace_config_read=0");
   runner.addToConfig("config_directory=/does/not/exist");
   runner.addToConfig("main_loop_iterations=1");
   runner.addToConfig("use_fixed_seed=1");
@@ -563,6 +563,7 @@ TEST_F(PerfProfdTest, BasicRunWithLivePerf)
   runner.addToConfig(cfparam);
   runner.addToConfig("main_loop_iterations=1");
   runner.addToConfig("use_fixed_seed=12345678");
+  runner.addToConfig("max_unprocessed_profiles=100");
   runner.addToConfig("collection_interval=9999");
   runner.addToConfig("sample_duration=2");