ProcessMetrics::GetCPUUsage() is Slow

base/process/process_metrics_linux.cc

 // Copyright (c) 2013 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "base/process/process_metrics.h"
 
 #include <dirent.h>
 #include <fcntl.h>
 #include <sys/stat.h>
 #include <sys/time.h>
@@ skipping 381 matching lines @@
   return ret;
 }
 
 size_t GetSystemCommitCharge() {
   SystemMemoryInfoKB meminfo;
   if (!GetSystemMemoryInfo(&meminfo))
     return 0;
   return meminfo.total - meminfo.free - meminfo.buffers - meminfo.cached;
 }
 
-// Exposed for testing.
 int ParseProcStatCPU(const std::string& input) {
-  std::vector<std::string> proc_stats;
-  if (!internal::ParseProcStats(input, &proc_stats))
+  if (input.empty())
     return -1;
 
-  if (proc_stats.size() <= internal::VM_STIME)
+  int j = 0;
+  int last_space_index = 0;
+  int utime_start_index = 0;
+  bool prev_was_space = false;
+  for (auto& ch : input) {
+    if (ch == ' ') {
+      if (!prev_was_space)
+        ++last_space_index;
+
+      prev_was_space = true;
+    } else {
+      if (ch == ')') {
+        // Restart the counter, we're looking to count from the last ')' we

[danakj, 2015/02/03 20:35:51]

Can you use input.rfind to get the last ')'?

This custom string searching code is really hard to read and to review, if we
can make use of some string methods to make this more obvious, or something,
that would really help.

Or, can we do this with a regular expression?

[afakhry, 2015/02/03 21:21:59]

I understand the need for readability. But this is a critical call and the
intent is to make it as efficient as possible.
The last ')' is likely to be a few characters away from the beginning, and more
than a couple of hundred characters away from the end. It would be a waste
scanning and comparing from the end looking for the last ')' just to rescan and
compare again from its position looking for the space just before utime's value.
That's why we're doing both on the way while scanning once from the beginning.

+        // encounter.
+        last_space_index = 0;
+      } else if (prev_was_space && last_space_index == internal::VM_UTIME - 2) {
+        utime_start_index = j + 1;
+        break;
+      }
+      prev_was_space = false;
+    }
+    ++j;
+  }
+
+  if (last_space_index < internal::VM_UTIME - 2)
     return -1;
-  int utime = GetProcStatsFieldAsInt64(proc_stats, internal::VM_UTIME);
-  int stime = GetProcStatsFieldAsInt64(proc_stats, internal::VM_STIME);
+
+  int utime = 0;
+  int stime = 0;
+  if (sscanf(&input.data()[utime_start_index], "%d %d", &utime, &stime) != 2)
+    return -1;
+
   return utime + stime;
 }
 
 const char kProcSelfExe[] = "/proc/self/exe";
 
 int GetNumberOfThreads(ProcessHandle process) {
   return internal::ReadProcStatsAndGetFieldAsInt64(process,
                                                    internal::VM_NUMTHREADS);
 }
 
@@ skipping 466 matching lines @@
 #if defined(OS_LINUX)
 int ProcessMetrics::GetIdleWakeupsPerSecond() {
   uint64 wake_ups;
   const char kWakeupStat[] = "se.statistics.nr_wakeups";
   return ReadProcSchedAndGetFieldAsUint64(process_, kWakeupStat, &wake_ups) ?
       CalculateIdleWakeupsPerSecond(wake_ups) : 0;
 }
 #endif  // defined(OS_LINUX)
 
 }  // namespace base