Perfprofd: Generalize post-collection handling

Add a handler function to process to encoded protobuf. For now, just write
it to file, as before.

Test: mmma system/extras/perfprofd
Test: perfprofd_test
Change-Id: Ife55d6f62408fd1fd2eac5af3ad4cd319b985a69

6 files changed, 162 insertions, 73 deletions

diff --git a/perfprofd/binder_interface/perfprofd_binder.cc b/perfprofd/binder_interface/perfprofd_binder.cc
index 11d17ffb..f124d412 100644
--- a/perfprofd/binder_interface/perfprofd_binder.cc
+++ b/perfprofd/binder_interface/perfprofd_binder.cc
@@ -25,6 +25,7 @@
 #include <mutex>
 #include <string>
 #include <thread>
+#include <functional>
 
 #include <inttypes.h>
 #include <unistd.h>
@@ -41,6 +42,7 @@
 
 #include "android/os/BnPerfProfd.h"
 #include "perfprofd_config.pb.h"
+#include "perf_profile.pb.h"
 
 #include "config.h"
 #include "configreader.h"
@@ -110,6 +112,12 @@ class PerfProfdNativeService : public BinderService<PerfProfdNativeService>,
                       uint32_t _aidl_flags = 0) override;
 
  private:
+  // Handler for ProfilingLoop.
+  bool BinderHandler(wireless_android_play_playlog::AndroidPerfProfile* encodedProfile,
+                     Config* config);
+  // Helper for the handler.
+  HandlerFn GetBinderHandler();
+
   status_t shellCommand(int /*in*/, int out, int err, Vector<String16>& args);
 
   template <typename ConfigFn> Status StartProfiling(ConfigFn fn);
@@ -119,8 +127,34 @@ class PerfProfdNativeService : public BinderService<PerfProfdNativeService>,
   std::mutex lock_;
 
   BinderConfig cur_config_;
+
+  int seq_ = 0;
 };
 
+bool PerfProfdNativeService::BinderHandler(
+    wireless_android_play_playlog::AndroidPerfProfile* encodedProfile, Config* config) {
+  if (encodedProfile == nullptr) {
+    return false;
+  }
+  std::string data_file_path(config->destination_directory);
+  data_file_path += "/perf.data";
+  std::string path = android::base::StringPrintf("%s.encoded.%d", data_file_path.c_str(), seq_);
+  PROFILE_RESULT result = SerializeProtobuf(encodedProfile, path.c_str());
+  if (result != PROFILE_RESULT::OK_PROFILE_COLLECTION) {
+    return false;
+  }
+
+  seq_++;
+  return true;
+}
+
+HandlerFn PerfProfdNativeService::GetBinderHandler() {
+  return HandlerFn(std::bind(&PerfProfdNativeService::BinderHandler,
+                             this,
+                             std::placeholders::_1,
+                             std::placeholders::_2));
+}
+
 status_t PerfProfdNativeService::start() {
   IPCThreadState::self()->disableBackgroundScheduling(true);
   status_t ret = BinderService<PerfProfdNativeService>::publish();
@@ -172,8 +206,9 @@ Status PerfProfdNativeService::StartProfiling(ConfigFn fn) {
 
   fn(cur_config_);
 
-  auto profile_runner = [](PerfProfdNativeService* service) {
-    ProfilingLoop(service->cur_config_);
+  HandlerFn handler = GetBinderHandler();
+  auto profile_runner = [handler](PerfProfdNativeService* service) {
+    ProfilingLoop(service->cur_config_, handler);
 
     // This thread is done.
     std::lock_guard<std::mutex> unset_guard(service->lock_);
diff --git a/perfprofd/perf_data_converter.cc b/perfprofd/perf_data_converter.cc
index 244386bc..f9a245d6 100644
--- a/perfprofd/perf_data_converter.cc
+++ b/perfprofd/perf_data_converter.cc
@@ -8,6 +8,8 @@
 
 #include <android-base/strings.h>
 
+#include "perf_profile.pb.h"
+
 #include "quipper/perf_parser.h"
 #include "symbolizer.h"
 
@@ -72,14 +74,15 @@ struct BinaryProfile {
   map<const callchain *, uint64, callchain_lt> callchain_count_map;
 };
 
-wireless_android_play_playlog::AndroidPerfProfile
+wireless_android_play_playlog::AndroidPerfProfile*
 RawPerfDataToAndroidPerfProfile(const string &perf_file,
                                 ::perfprofd::Symbolizer* symbolizer) {
-  wireless_android_play_playlog::AndroidPerfProfile ret;
   quipper::PerfParser parser;
   if (!parser.ReadFile(perf_file) || !parser.ParseRawEvents()) {
-    return ret;
+    return nullptr;
   }
+  std::unique_ptr<wireless_android_play_playlog::AndroidPerfProfile> ret(
+      new wireless_android_play_playlog::AndroidPerfProfile());
 
   typedef map<string, BinaryProfile> ModuleProfileMap;
   typedef map<string, ModuleProfileMap> ProgramProfileMap;
@@ -184,10 +187,10 @@ RawPerfDataToAndroidPerfProfile(const string &perf_file,
 
   map<string, string> name_buildid_map;
   parser.GetFilenamesToBuildIDs(&name_buildid_map);
-  ret.set_total_samples(total_samples);
+  ret->set_total_samples(total_samples);
 
   for (auto& name_data : name_data_map) {
-    auto load_module = ret.add_load_modules();
+    auto load_module = ret->add_load_modules();
     load_module->set_name(name_data.first);
     auto nbmi = name_buildid_map.find(name_data.first);
     bool has_build_id = nbmi != name_buildid_map.end();
@@ -240,7 +243,7 @@ RawPerfDataToAndroidPerfProfile(const string &perf_file,
   };
 
   for (const auto &program_profile : name_profile_map) {
-    auto program = ret.add_programs();
+    auto program = ret->add_programs();
     program->set_name(program_profile.first);
     for (const auto &module_profile : program_profile.second) {
       ModuleData& module_data = name_data_map[module_profile.first];
@@ -286,7 +289,7 @@ RawPerfDataToAndroidPerfProfile(const string &perf_file,
     }
   }
 
-  return ret;
+  return ret.release();
 }
 
 }  // namespace wireless_android_logging_awp
diff --git a/perfprofd/perf_data_converter.h b/perfprofd/perf_data_converter.h
index 8a409dcc..5941a31e 100644
--- a/perfprofd/perf_data_converter.h
+++ b/perfprofd/perf_data_converter.h
@@ -3,15 +3,17 @@
 
 #include <string>
 
-#include "perf_profile.pb.h"
-
 namespace perfprofd {
 struct Symbolizer;
-}
+}  // namespace perfprofd
+
+namespace wireless_android_play_playlog {
+class AndroidPerfProfile;
+}  // namespace wireless_android_play_playlog
 
 namespace wireless_android_logging_awp {
 
-wireless_android_play_playlog::AndroidPerfProfile
+wireless_android_play_playlog::AndroidPerfProfile*
 RawPerfDataToAndroidPerfProfile(const std::string &perf_file,
                                 ::perfprofd::Symbolizer* symbolizer);
 
diff --git a/perfprofd/perfprofdcore.cc b/perfprofd/perfprofdcore.cc
index 6de7a71f..7a7ef339 100644
--- a/perfprofd/perfprofdcore.cc
+++ b/perfprofd/perfprofdcore.cc
@@ -41,6 +41,9 @@
 #include <android-base/macros.h>
 #include <android-base/properties.h>
 #include <android-base/stringprintf.h>
+#include <android-base/unique_fd.h>
+
+#include "perf_profile.pb.h"
 
 #include "perfprofdcore.h"
 #include "perf_data_converter.h"
@@ -465,64 +468,75 @@ static void annotate_encoded_perf_profile(wireless_android_play_playlog::Android
   }
 }
 
-inline char* string_as_array(std::string* str) {
-  return str->empty() ? NULL : &*str->begin();
-}
-
-PROFILE_RESULT encode_to_proto(const std::string &data_file_path,
-                               const char *encoded_file_path,
-                               const Config& config,
-                               unsigned cpu_utilization,
-                               perfprofd::Symbolizer* symbolizer)
-{
+using ProtoUniquePtr = std::unique_ptr<wireless_android_play_playlog::AndroidPerfProfile>;
+static ProtoUniquePtr encode_to_proto(const std::string &data_file_path,
+                                      const Config& config,
+                                      unsigned cpu_utilization,
+                                      perfprofd::Symbolizer* symbolizer) {
   //
   // Open and read perf.data file
   //
-  const wireless_android_play_playlog::AndroidPerfProfile &encodedProfile =
-      wireless_android_logging_awp::RawPerfDataToAndroidPerfProfile(data_file_path, symbolizer);
-
-  //
-  // Issue error if no samples
-  //
-  if (encodedProfile.programs().size() == 0) {
-    return ERR_PERF_ENCODE_FAILED;
+  ProtoUniquePtr encodedProfile(
+      wireless_android_logging_awp::RawPerfDataToAndroidPerfProfile(data_file_path, symbolizer));
+  if (encodedProfile == nullptr) {
+    return nullptr;
   }
 
   // All of the info in 'encodedProfile' is derived from the perf.data file;
   // here we tack display status, cpu utilization, system load, etc.
-  wireless_android_play_playlog::AndroidPerfProfile &prof =
-      const_cast<wireless_android_play_playlog::AndroidPerfProfile&>
-      (encodedProfile);
-  annotate_encoded_perf_profile(&prof, config, cpu_utilization);
+  annotate_encoded_perf_profile(encodedProfile.get(), config, cpu_utilization);
 
+  return encodedProfile;
+}
+
+PROFILE_RESULT SerializeProtobuf(wireless_android_play_playlog::AndroidPerfProfile* encodedProfile,
+                                 const char* encoded_file_path) {
   //
   // Serialize protobuf to array
   //
-  int size = encodedProfile.ByteSize();
-  std::string data;
-  data.resize(size);
-  ::google::protobuf::uint8* dtarget =
-        reinterpret_cast<::google::protobuf::uint8*>(string_as_array(&data));
-  encodedProfile.SerializeWithCachedSizesToArray(dtarget);
+  size_t size = encodedProfile->ByteSize();
+  std::unique_ptr<uint8_t[]> data(new uint8_t[size]);
+  encodedProfile->SerializeWithCachedSizesToArray(data.get());
 
   //
   // Open file and write encoded data to it
   //
-  FILE *fp = fopen(encoded_file_path, "w");
-  if (!fp) {
+  unlink(encoded_file_path);  // Attempt to unlink for a clean slate.
+  constexpr int kFlags = O_CREAT | O_WRONLY | O_TRUNC | O_NOFOLLOW | O_CLOEXEC;
+  android::base::unique_fd fd(open(encoded_file_path, kFlags, 0664));
+  if (fd.get() == -1) {
+    PLOG(WARNING) << "Could not open " << encoded_file_path << " for serialization";
     return ERR_OPEN_ENCODED_FILE_FAILED;
   }
-  size_t fsiz = size;
-  if (fwrite(dtarget, fsiz, 1, fp) != 1) {
-    fclose(fp);
+  if (!android::base::WriteFully(fd.get(), data.get(), size)) {
+    PLOG(WARNING) << "Could not write to " << encoded_file_path;
     return ERR_WRITE_ENCODED_FILE_FAILED;
   }
-  fclose(fp);
-  chmod(encoded_file_path, S_IRUSR | S_IWUSR | S_IRGRP | S_IWGRP | S_IROTH);
 
   return OK_PROFILE_COLLECTION;
 }
 
+PROFILE_RESULT encode_to_proto(const std::string &data_file_path,
+                               const char *encoded_file_path,
+                               const Config& config,
+                               unsigned cpu_utilization,
+                               perfprofd::Symbolizer* symbolizer)
+{
+  ProtoUniquePtr encodedProfile = encode_to_proto(data_file_path,
+                                                  config,
+                                                  cpu_utilization,
+                                                  symbolizer);
+
+  //
+  // Issue error if no samples
+  //
+  if (encodedProfile == nullptr || encodedProfile->programs().size() == 0) {
+    return ERR_PERF_ENCODE_FAILED;
+  }
+
+  return SerializeProtobuf(encodedProfile.get(), encoded_file_path);
+}
+
 //
 // Invoke "perf record". Return value is OK_PROFILE_COLLECTION for
 // success, or some other error code if something went wrong.
@@ -729,7 +743,7 @@ static bool post_process(const Config& config, int current_seq)
 // - kick off 'perf record'
 // - read perf.data, convert to protocol buf
 //
-static PROFILE_RESULT collect_profile(Config& config, int seq)
+static ProtoUniquePtr collect_profile(Config& config)
 {
   //
   // Collect cpu utilization if enabled
@@ -794,20 +808,18 @@ static PROFILE_RESULT collect_profile(Config& config, int seq)
                                    data_file_path,
                                    perf_stderr_path);
   if (ret != OK_PROFILE_COLLECTION) {
-    return ret;
+    return nullptr;
   }
 
   //
   // Read the resulting perf.data file, encode into protocol buffer, then write
   // the result to the file perf.data.encoded
   //
-  std::string path = android::base::StringPrintf(
-      "%s.encoded.%d", data_file_path.c_str(), seq);
   std::unique_ptr<perfprofd::Symbolizer> symbolizer;
   if (config.use_elf_symbolizer) {
     symbolizer = perfprofd::CreateELFSymbolizer();
   }
-  return encode_to_proto(data_file_path, path.c_str(), config, cpu_utilization, symbolizer.get());
+  return encode_to_proto(data_file_path, config, cpu_utilization, symbolizer.get());
 }
 
 //
@@ -891,9 +903,8 @@ static void init(ConfigReader &config)
 }
 
 template <typename ConfigFn, typename UpdateFn>
-static void ProfilingLoopImpl(ConfigFn config, UpdateFn update) {
+static void ProfilingLoopImpl(ConfigFn config, UpdateFn update, HandlerFn handler) {
   unsigned iterations = 0;
-  int seq = 0;
   while(config()->main_loop_iterations == 0 ||
       iterations < config()->main_loop_iterations) {
     if (config()->ShouldStopProfiling()) {
@@ -922,14 +933,17 @@ static void ProfilingLoopImpl(ConfigFn config, UpdateFn update) {
     } else {
       // Kick off the profiling run...
       LOG(INFO) << "initiating profile collection";
-      PROFILE_RESULT result = collect_profile(*config(), seq);
-      if (result != OK_PROFILE_COLLECTION) {
-        LOG(INFO) << "profile collection failed (" << profile_result_to_string(result) << ")";
-      } else {
-        if (post_process(*config(), seq)) {
-          seq++;
-        }
+      ProtoUniquePtr proto = collect_profile(*config());
+      if (proto == nullptr) {
+        LOG(WARNING) << "profile collection failed";
+      }
+
+      // Always report, even a null result.
+      bool handle_result = handler(proto.get(), config());
+      if (handle_result) {
         LOG(INFO) << "profile collection complete";
+      } else if (proto != nullptr) {
+        LOG(WARNING) << "profile handling failed";
       }
     }
 
@@ -942,7 +956,7 @@ static void ProfilingLoopImpl(ConfigFn config, UpdateFn update) {
   }
 }
 
-void ProfilingLoop(Config& config) {
+void ProfilingLoop(Config& config, HandlerFn handler) {
   init(config);
 
   auto config_fn = [&config]() {
@@ -950,7 +964,7 @@ void ProfilingLoop(Config& config) {
   };
   auto do_nothing = []() {
   };
-  ProfilingLoopImpl(config_fn, do_nothing);
+  ProfilingLoopImpl(config_fn, do_nothing, handler);
 }
 
 //
@@ -993,7 +1007,28 @@ int perfprofd_main(int argc, char** argv, Config* config)
     config_reader.readFile();
     config_reader.FillConfig(config);
   };
-  ProfilingLoopImpl(config_fn, reread_config);
+  int seq = 0;
+  auto handler = [&seq](wireless_android_play_playlog::AndroidPerfProfile* proto,
+                        Config* handler_config) {
+    if (proto == nullptr) {
+      return false;
+    }
+    std::string data_file_path(handler_config->destination_directory);
+    data_file_path += "/";
+    data_file_path += PERF_OUTPUT;
+    std::string path = android::base::StringPrintf("%s.encoded.%d", data_file_path.c_str(), seq);
+    PROFILE_RESULT result = SerializeProtobuf(proto, path.c_str());
+    if (result != PROFILE_RESULT::OK_PROFILE_COLLECTION) {
+      return false;
+    }
+
+    if (!post_process(*handler_config, seq)) {
+      return false;
+    }
+    seq++;
+    return true;
+  };
+  ProfilingLoopImpl(config_fn, reread_config, handler);
 
   LOG(INFO) << "finishing Android Wide Profiling daemon";
   return 0;
diff --git a/perfprofd/perfprofdcore.h b/perfprofd/perfprofdcore.h
index 1e1e80ab..6ea24ab1 100644
--- a/perfprofd/perfprofdcore.h
+++ b/perfprofd/perfprofdcore.h
@@ -18,8 +18,15 @@
 #ifndef SYSTEM_EXTRAS_PERFPROFD_PERFPROFDCORE_H_
 #define SYSTEM_EXTRAS_PERFPROFD_PERFPROFDCORE_H_
 
+#include <functional>
+#include <memory>
+
 struct Config;
 
+namespace wireless_android_play_playlog {
+class AndroidPerfProfile;
+}
+
 namespace perfprofd {
 struct Symbolizer;
 }
@@ -77,7 +84,13 @@ PROFILE_RESULT encode_to_proto(const std::string &data_file_path,
                                unsigned cpu_utilization,
                                perfprofd::Symbolizer* symbolizer);
 
-void ProfilingLoop(Config& config);
+PROFILE_RESULT SerializeProtobuf(wireless_android_play_playlog::AndroidPerfProfile* encodedProfile,
+                                 const char* encoded_file_path);
+
+using HandlerFn = std::function<bool(wireless_android_play_playlog::AndroidPerfProfile* proto,
+                                     Config* config)>;
+
+void ProfilingLoop(Config& config, HandlerFn handler);
 
 //
 // Exposed for unit testing
diff --git a/perfprofd/tests/perfprofd_test.cc b/perfprofd/tests/perfprofd_test.cc
index 802f4d17..1ee1f777 100644
--- a/perfprofd/tests/perfprofd_test.cc
+++ b/perfprofd/tests/perfprofd_test.cc
@@ -534,7 +534,8 @@ TEST_F(PerfProfdTest, BadPerfRun)
 
   // Verify log contents
   const std::string expected = RAW_RESULT(
-      I: profile collection failed (perf record returned bad exit status)
+      W: perf bad exit status 1
+      W: profile collection failed
                                           );
 
   // check to make sure log excerpt matches
@@ -615,7 +616,7 @@ TEST_F(PerfProfdTest, BasicRunWithCannedPerf)
   // Kick off encoder and check return code
   PROFILE_RESULT result =
       encode_to_proto(input_perf_data, encoded_file_path(dest_dir, 0).c_str(), config, 0, nullptr);
-  EXPECT_EQ(OK_PROFILE_COLLECTION, result);
+  ASSERT_EQ(OK_PROFILE_COLLECTION, result) << JoinTestLog(" ");
 
   // Read and decode the resulting perf.data.encoded file
   wireless_android_play_playlog::AndroidPerfProfile encodedProfile;
@@ -703,7 +704,7 @@ TEST_F(PerfProfdTest, BasicRunWithCannedPerfWithSymbolizer)
                       config,
                       0,
                       &test_symbolizer);
-  EXPECT_EQ(OK_PROFILE_COLLECTION, result);
+  ASSERT_EQ(OK_PROFILE_COLLECTION, result);
 
   // Read and decode the resulting perf.data.encoded file
   wireless_android_play_playlog::AndroidPerfProfile encodedProfile;
@@ -780,7 +781,7 @@ TEST_F(PerfProfdTest, CallchainRunWithCannedPerf)
   // Kick off encoder and check return code
   PROFILE_RESULT result =
       encode_to_proto(input_perf_data, encoded_file_path(dest_dir, 0).c_str(), config, 0, nullptr);
-  EXPECT_EQ(OK_PROFILE_COLLECTION, result);
+  ASSERT_EQ(OK_PROFILE_COLLECTION, result);
 
   // Read and decode the resulting perf.data.encoded file
   wireless_android_play_playlog::AndroidPerfProfile encodedProfile;
@@ -864,7 +865,7 @@ TEST_F(PerfProfdTest, BasicRunWithLivePerf)
   int daemon_main_return_code = runner.invoke();
 
   // Check return code from daemon
-  EXPECT_EQ(0, daemon_main_return_code);
+  ASSERT_EQ(0, daemon_main_return_code);
 
   // Read and decode the resulting perf.data.encoded file
   wireless_android_play_playlog::AndroidPerfProfile encodedProfile;
@@ -916,7 +917,7 @@ TEST_F(PerfProfdTest, MultipleRunWithLivePerf)
   int daemon_main_return_code = runner.invoke();
 
   // Check return code from daemon
-  EXPECT_EQ(0, daemon_main_return_code);
+  ASSERT_EQ(0, daemon_main_return_code);
 
   // Read and decode the resulting perf.data.encoded file
   wireless_android_play_playlog::AndroidPerfProfile encodedProfile;
@@ -983,7 +984,7 @@ TEST_F(PerfProfdTest, CallChainRunWithLivePerf)
   int daemon_main_return_code = runner.invoke();
 
   // Check return code from daemon
-  EXPECT_EQ(0, daemon_main_return_code);
+  ASSERT_EQ(0, daemon_main_return_code);
 
   // Read and decode the resulting perf.data.encoded file
   wireless_android_play_playlog::AndroidPerfProfile encodedProfile;