Bug 1744264 - Improve the precision of CPU data shown on about:processes on Linux, r=gerald.
☠☠ backed out by b39067ed9d0a ☠ ☠
authorFlorian Quèze <florian@queze.net>
Tue, 07 Dec 2021 07:48:27 +0000
changeset 601197 2f350df166e2f16d0f6bbec7ae9a0930b3fd4d70
parent 601196 3dfd3c94a105e095aada0b356f1106370de222d3
child 601198 3f5db9defbd6f902ea5e3dfe0d7b4fa3451d470c
push id154241
push userfqueze@mozilla.com
push dateTue, 07 Dec 2021 07:50:49 +0000
treeherderautoland@2f350df166e2 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersgerald
bugs1744264
milestone97.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1744264 - Improve the precision of CPU data shown on about:processes on Linux, r=gerald. Differential Revision: https://phabricator.services.mozilla.com/D132804
toolkit/components/aboutprocesses/content/aboutProcesses.js
toolkit/components/processtools/ProcInfo_linux.cpp
--- a/toolkit/components/aboutprocesses/content/aboutProcesses.js
+++ b/toolkit/components/aboutprocesses/content/aboutProcesses.js
@@ -388,19 +388,23 @@ var View = {
   displayCpu(data, cpuCell) {
     if (data.slopeCpu == null) {
       this._fillCell(cpuCell, {
         fluentName: "about-processes-cpu-user-and-kernel-not-ready",
         classes: ["cpu"],
       });
     } else {
       let { duration, unit } = this._getDuration(data.totalCpu);
-      if (data.totalCpu == 0 && AppConstants.platform == "win") {
-        // The minimum non zero CPU time we can get on Windows is 16ms
-        // so avoid displaying '0ns'.
+      if (data.totalCpu == 0) {
+        // A thread having used exactly 0ns of CPU time is not possible.
+        // When we get 0 it means the thread used less than the precision of
+        // the measurement, and it makes more sense to show '0ms' than '0ns'.
+        // This is useful on Linux where the minimum non-zero CPU time value
+        // for threads of child processes is 10ms, and on Windows ARM64 where
+        // the minimum non-zero value is 16ms.
         unit = "ms";
       }
       let localizedUnit = gLocalizedUnits.duration[unit];
       if (data.slopeCpu == 0) {
         let fluentName = data.active
           ? "about-processes-cpu-almost-idle"
           : "about-processes-cpu-fully-idle";
         this._fillCell(cpuCell, {
--- a/toolkit/components/processtools/ProcInfo_linux.cpp
+++ b/toolkit/components/processtools/ProcInfo_linux.cpp
@@ -14,16 +14,31 @@
 
 #include <cstdio>
 #include <cstring>
 #include <unistd.h>
 #include <dirent.h>
 
 #define NANOPERSEC 1000000000.
 
+#ifndef CPUCLOCK_SCHED
+#  define CPUCLOCK_SCHED 2
+#endif
+#ifndef CPUCLOCK_PERTHREAD_MASK
+#  define CPUCLOCK_PERTHREAD_MASK 4
+#endif
+#ifndef MAKE_PROCESS_CPUCLOCK
+#  define MAKE_PROCESS_CPUCLOCK(pid, clock) \
+    ((int)(~(unsigned)(pid) << 3) | (int)(clock))
+#endif
+#ifndef MAKE_THREAD_CPUCLOCK
+#  define MAKE_THREAD_CPUCLOCK(tid, clock) \
+    MAKE_PROCESS_CPUCLOCK(tid, (clock) | CPUCLOCK_PERTHREAD_MASK)
+#endif
+
 namespace mozilla {
 
 int GetCycleTimeFrequencyMHz() { return 0; }
 
 // StatReader can parse and tokenize a POSIX stat file.
 // see http://man7.org/linux/man-pages/man5/proc.5.html
 //
 // Its usage is quite simple:
@@ -125,17 +140,17 @@ class StatReader {
 
  private:
   // Reads the stat file and puts its content in a nsString.
   nsresult ReadFile(nsAutoString& aFileContent) {
     if (mFilepath.IsEmpty()) {
       if (mPid == 0) {
         mFilepath.AssignLiteral("/proc/self/stat");
       } else {
-        mFilepath.AppendPrintf("/proc/%u/stat", mPid);
+        mFilepath.AppendPrintf("/proc/%u/stat", unsigned(mPid));
       }
     }
     FILE* fstat = fopen(mFilepath.get(), "r");
     if (!fstat) {
       return NS_ERROR_FAILURE;
     }
     // /proc is a virtual file system and all files are
     // of size 0, so GetFileSize() and related functions will
@@ -160,37 +175,42 @@ class StatReader {
   int64_t mTicksPerSec;
 };
 
 // Threads have the same stat file. The only difference is its path
 // and we're getting less info in the ThreadInfo structure.
 class ThreadInfoReader final : public StatReader {
  public:
   ThreadInfoReader(const base::ProcessId aPid, const base::ProcessId aTid)
-      : StatReader(aPid), mTid(aTid) {
-    mFilepath.AppendPrintf("/proc/%u/task/%u/stat", aPid, mTid);
+      : StatReader(aPid) {
+    mFilepath.AppendPrintf("/proc/%u/task/%u/stat", unsigned(aPid),
+                           unsigned(aTid));
   }
 
   nsresult ParseThread(ThreadInfo& aInfo) {
     ProcInfo info;
     nsresult rv = StatReader::ParseProc(info);
     NS_ENSURE_SUCCESS(rv, rv);
 
-    aInfo.tid = mTid;
     // Copying over the data we got from StatReader::ParseProc()
     aInfo.cpuTime = info.cpuTime;
     aInfo.name.Assign(mName);
     return NS_OK;
   }
-
- private:
-  base::ProcessId mTid;
 };
 
 nsresult GetCpuTimeSinceProcessStartInMs(uint64_t* aResult) {
+  timespec t;
+  if (clock_gettime(CLOCK_PROCESS_CPUTIME_ID, &t) == 0) {
+    uint64_t cpuTime =
+        uint64_t(t.tv_sec) * 1'000'000'000u + uint64_t(t.tv_nsec);
+    *aResult = cpuTime / PR_NSEC_PER_MSEC;
+    return NS_OK;
+  }
+
   StatReader reader(0);
   ProcInfo info;
   nsresult rv = reader.ParseProc(info);
   if (NS_FAILED(rv)) {
     return rv;
   }
 
   *aResult = info.cpuTime / PR_NSEC_PER_MSEC;
@@ -206,25 +226,32 @@ ProcInfoPromise::ResolveOrRejectValue Ge
   ProcInfoPromise::ResolveOrRejectValue result;
 
   HashMap<base::ProcessId, ProcInfo> gathered;
   if (!gathered.reserve(aRequests.Length())) {
     result.SetReject(NS_ERROR_OUT_OF_MEMORY);
     return result;
   }
   for (const auto& request : aRequests) {
-    // opening the stat file and reading its content
-    StatReader reader(request.pid);
     ProcInfo info;
-    nsresult rv = reader.ParseProc(info);
-    if (NS_FAILED(rv)) {
-      // Can't read data for this proc.
-      // Probably either a sandboxing issue or a race condition, e.g.
-      // the process has been just been killed. Regardless, skip process.
-      continue;
+
+    timespec t;
+    clockid_t clockid = MAKE_PROCESS_CPUCLOCK(request.pid, CPUCLOCK_SCHED);
+    if (clock_gettime(clockid, &t) == 0) {
+      info.cpuTime = uint64_t(t.tv_sec) * 1'000'000'000u + uint64_t(t.tv_nsec);
+    } else {
+      // Fallback to parsing /proc/<pid>/stat
+      StatReader reader(request.pid);
+      nsresult rv = reader.ParseProc(info);
+      if (NS_FAILED(rv)) {
+        // Can't read data for this proc.
+        // Probably either a sandboxing issue or a race condition, e.g.
+        // the process has been just been killed. Regardless, skip process.
+        continue;
+      }
     }
 
     // The 'Memory' value displayed in the system monitor is resident -
     // shared. statm contains more fields, but we're only interested in
     // the first three.
     static const int MAX_FIELD = 3;
     size_t VmSize, resident, shared;
     info.memory = 0;
@@ -241,41 +268,83 @@ ProcInfoPromise::ResolveOrRejectValue Ge
     info.pid = request.pid;
     info.childId = request.childId;
     info.type = request.processType;
     info.origin = request.origin;
     info.windows = std::move(request.windowInfo);
 
     // Let's look at the threads
     nsCString taskPath;
-    taskPath.AppendPrintf("/proc/%u/task", request.pid);
+    taskPath.AppendPrintf("/proc/%u/task", unsigned(request.pid));
     DIR* dirHandle = opendir(taskPath.get());
     if (!dirHandle) {
       // For some reason, we have no data on the threads for this process.
       // Most likely reason is that we have just lost a race condition and
       // the process is dead.
       // Let's stop here and ignore the entire process.
       continue;
     }
     auto cleanup = mozilla::MakeScopeExit([&] { closedir(dirHandle); });
 
     // If we can't read some thread info, we ignore that thread.
     dirent* entry;
     while ((entry = readdir(dirHandle)) != nullptr) {
       if (entry->d_name[0] == '.') {
         continue;
       }
-      // Threads have a stat file, like processes.
       nsAutoCString entryName(entry->d_name);
+      nsresult rv;
       int32_t tid = entryName.ToInteger(&rv);
       if (NS_FAILED(rv)) {
         continue;
       }
+
+      ThreadInfo threadInfo;
+      threadInfo.tid = tid;
+
+      timespec ts;
+      if (clock_gettime(MAKE_THREAD_CPUCLOCK(tid, CPUCLOCK_SCHED), &ts) == 0) {
+        threadInfo.cpuTime =
+            uint64_t(ts.tv_sec) * 1'000'000'000u + uint64_t(ts.tv_nsec);
+
+        nsCString path;
+        path.AppendPrintf("/proc/%u/task/%u/comm", unsigned(request.pid),
+                          unsigned(tid));
+        FILE* fstat = fopen(path.get(), "r");
+        if (fstat) {
+          // /proc is a virtual file system and all files are
+          // of size 0, so GetFileSize() and related functions will
+          // return 0 - so the way to read the file is to fill a buffer
+          // of an arbitrary big size and look for the end of line char.
+          // The size of the buffer to use for thread names is TASK_COMM_LEN,
+          // which is part of the kernel, but not available in any system
+          // header.
+          constexpr int TASK_COMM_LEN = 16;
+          char buffer[TASK_COMM_LEN];
+          char* start = fgets(buffer, TASK_COMM_LEN, fstat);
+          fclose(fstat);
+          if (start) {
+            // If the thread name uses the entire buffer, we will have
+            // 15 characters followed by the null terminator.
+            // If the thread name is shorter, we will have a newline
+            // character after the thread name, that we don't want to
+            // copy. strchrnul will return a pointer to the null byte
+            // at the end if the newline character is not found.
+            char* end = strchrnul(buffer, '\n');
+            threadInfo.name.AssignASCII(buffer, size_t(end - start));
+            info.threads.AppendElement(threadInfo);
+            continue;
+          }
+        }
+      }
+
+      // Fallback to parsing /proc/<pid>/task/<tid>/stat
+      // This is needed for child processes, as access to the per-thread
+      // CPU clock is restricted to the process owning the thread.
       ThreadInfoReader reader(request.pid, tid);
-      ThreadInfo threadInfo;
       rv = reader.ParseThread(threadInfo);
       if (NS_FAILED(rv)) {
         continue;
       }
       info.threads.AppendElement(threadInfo);
     }
 
     if (!gathered.put(request.pid, std::move(info))) {