arc: Reduce overhead for capturing sys metrics.

Stress test shows that reading file from scratch is 1.6x slower than
rewinding the existing file descriptor and reading again. This adopts
this finding by re-using file descriptor for reading system stats.

TEST=Locally on eve, minnie, functionality works, no error is reported.
     Additionally did perf inverstigation of ReadSystemStatOnBackgroundThread
                10%/50%/90%  mcs per invocation
     eve:
        before: 444/1049/1509
	after:  305/688/721
     minnie:
        before: 309/534/1441
	after:  176/385/1033

BUG=b:140939504

Change-Id: I58ebb08299c43b58687eaaa6c9ad225872b2e70d
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1823722
Commit-Queue: Yury Khmel <khmel@chromium.org>
Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#699947}

chrome/browser/chromeos/arc/tracing/arc_system_stat_collector.h

@@ -14,7 +14,6 @@
 #include "base/timer/timer.h"
 
 namespace base {
-class FilePath;
 class TimeDelta;
 class SequencedTaskRunner;
 }  // namespace base
@@ -85,6 +84,7 @@ class ArcSystemStatCollector {
 
  private:
   struct Sample;
+  struct SystemReadersContext;
 
   struct RuntimeFrame {
     // read, written sectors and total time in milliseconds.
@@ -94,22 +94,31 @@ class ArcSystemStatCollector {
     // objects, used bytes.
     int64_t gem_info[base::size(kGemInfoColumns) - 1] = {0};
     // Temperature of CPU, Core 0.
-    int64_t cpu_temperature_ = std::numeric_limits<int>::min();
+    int64_t cpu_temperature = std::numeric_limits<int>::min();
   };
 
-  // Schedule reading System stat files in |ReadSystemStatOnBackgroundThread| on
-  // background thread. Once ready result is passed to
+  // Schedules reading System stat files in |ReadSystemStatOnBackgroundThread|
+  // on background thread. Once ready result is passed to
   // |UpdateSystemStatOnUiThread|
   void ScheduleSystemStatUpdate();
-  static RuntimeFrame ReadSystemStatOnBackgroundThread();
-  void UpdateSystemStatOnUiThread(RuntimeFrame current_frame);
+
+  // Frees |context_| if it exists.
+  void FreeSystemReadersContext();
+
+  // Called when |SystemReadersContext| is initialized.
+  void OnInitOnUiThread(std::unique_ptr<SystemReadersContext> context);
+
+  // Reads system stat files on background thread using |context|.
+  static std::unique_ptr<SystemReadersContext> ReadSystemStatOnBackgroundThread(
+      std::unique_ptr<SystemReadersContext> context);
+  // Processes filled |current_frame_| on UI thread.
+  void UpdateSystemStatOnUiThread(
+      std::unique_ptr<SystemReadersContext> context);
 
   // To schedule updates of system stat.
   base::RepeatingTimer timer_;
   // Performs reading kernel stat files on backgrond thread.
   scoped_refptr<base::SequencedTaskRunner> background_task_runner_;
-  // Use to prevent double scheduling.
-  bool request_scheduled_ = false;
   // Used to limit the number of warnings printed in case System stat update is
   // dropped due to previous update is in progress.
   int missed_update_warning_left_ = 0;
@@ -121,6 +130,8 @@ class ArcSystemStatCollector {
   // Used to calculate delta.
   RuntimeFrame previous_frame_;
 
+  std::unique_ptr<SystemReadersContext> context_;
+
   base::WeakPtrFactory<ArcSystemStatCollector> weak_ptr_factory_{this};
 
   DISALLOW_COPY_AND_ASSIGN(ArcSystemStatCollector);
@@ -128,12 +139,10 @@ class ArcSystemStatCollector {
 
 // Helper that reads and parses stat file containing decimal number separated by
 // whitespace and text fields. It does not have any dynamic memory allocation.
-// |path| specifies the file to read and parse. |columns| contains index of
-// column to parse, end of sequence is specified by terminator -1. |output|
-// receives parsed value. Must be the size as |columns| size - 1.
-bool ParseStatFile(const base::FilePath& path,
-                   const int* columns,
-                   int64_t* output);
+// |fd| specifies the file descriptor to read and parse. |columns| contains
+// index of column to parse, end of sequence is specified by terminator -1.
+// |output| receives parsed value. Must be the size as |columns| size - 1.
+bool ParseStatFile(int fd, const int* columns, int64_t* output);
 
 }  // namespace arc
 

chrome/browser/chromeos/arc/tracing/arc_system_stat_collector_unittest.cc

@@ -4,7 +4,12 @@
 
 #include "chrome/browser/chromeos/arc/tracing/arc_system_stat_collector.h"
 
+#include <fcntl.h>
+#include <sys/types.h>
+#include <unistd.h>
+
 #include "base/files/file_path.h"
+#include "base/files/scoped_file.h"
 #include "base/path_service.h"
 #include "chrome/common/chrome_paths.h"
 #include "testing/gtest/include/gtest/gtest.h"
@@ -15,17 +20,21 @@ using ArcSystemStatCollectorTest = testing::Test;
 
 namespace {
 
-base::FilePath GetPath(const std::string& name) {
+base::ScopedFD OpenTestStatFile(const std::string& name) {
   base::FilePath base_path;
   base::PathService::Get(chrome::DIR_TEST_DATA, &base_path);
-  return base_path.Append("arc_graphics_tracing").Append(name);
+  const base::FilePath path =
+      base_path.Append("arc_graphics_tracing").Append(name);
+  base::ScopedFD result(open(path.value().c_str(), O_RDONLY));
+  DCHECK_GE(result.get(), 0);
+  return result;
 }
 
 }  // namespace
 
 TEST_F(ArcSystemStatCollectorTest, Parse) {
   int64_t zram_values[3];
-  EXPECT_TRUE(ParseStatFile(GetPath("zram_stat"),
+  EXPECT_TRUE(ParseStatFile(OpenTestStatFile("zram_stat").get(),
                             ArcSystemStatCollector::kZramStatColumns,
                             zram_values));
   EXPECT_EQ(2384, zram_values[0]);
@@ -33,21 +42,21 @@ TEST_F(ArcSystemStatCollectorTest, Parse) {
   EXPECT_EQ(79, zram_values[2]);
 
   int64_t mem_values[2];
-  EXPECT_TRUE(ParseStatFile(GetPath("proc_meminfo"),
+  EXPECT_TRUE(ParseStatFile(OpenTestStatFile("proc_meminfo").get(),
                             ArcSystemStatCollector::kMemInfoColumns,
                             mem_values));
   EXPECT_EQ(8058940, mem_values[0]);
   EXPECT_EQ(2714260, mem_values[1]);
 
   int64_t gem_values[2];
-  EXPECT_TRUE(ParseStatFile(GetPath("gem_objects"),
+  EXPECT_TRUE(ParseStatFile(OpenTestStatFile("gem_objects").get(),
                             ArcSystemStatCollector::kGemInfoColumns,
                             gem_values));
   EXPECT_EQ(853, gem_values[0]);
   EXPECT_EQ(458256384, gem_values[1]);
 
   int64_t cpu_temp;
-  EXPECT_TRUE(ParseStatFile(GetPath("temp1_input"),
+  EXPECT_TRUE(ParseStatFile(OpenTestStatFile("temp1_input").get(),
                             ArcSystemStatCollector::kCpuTempInfoColumns,
                             &cpu_temp));
   EXPECT_EQ(52000, cpu_temp);