From 1f7d4c74416489b7cb5dc96a32cd1357ab854fb2 Mon Sep 17 00:00:00 2001
From: ridiculousfish <rf@fishshell.com>
Date: Sat, 7 May 2022 15:08:12 -0700
Subject: [PATCH] Fix CPU usage percentage calculation as reported by jobs

This rationalizes our types for computing CPU usage percentage and
fixes the computation. Fixes #8919.
---
 CHANGELOG.rst         |  1 +
 src/builtins/jobs.cpp | 13 +++++++------
 src/proc.cpp          | 17 ++++++++++++-----
 src/proc.h            | 11 +++++++++--
 4 files changed, 29 insertions(+), 13 deletions(-)

diff --git a/CHANGELOG.rst b/CHANGELOG.rst
index ed84abeae..0d55cf543 100644
--- a/CHANGELOG.rst
+++ b/CHANGELOG.rst
@@ -3,6 +3,7 @@ fish 3.5.0 (not yet released)
 
 Notable improvements and fixes
 ------------------------------
+- ``jobs`` now correctly reports CPU usage as a percentage, instead of as a number of clock ticks (:issue:`8919`).
 
 Deprecations and removed features
 ---------------------------------
diff --git a/src/builtins/jobs.cpp b/src/builtins/jobs.cpp
index b5017f3d4..fb8969c02 100644
--- a/src/builtins/jobs.cpp
+++ b/src/builtins/jobs.cpp
@@ -26,18 +26,19 @@ enum {
     JOBS_PRINT_NOTHING,  // print nothing (exit status only)
 };
 
-/// Calculates the cpu usage (in percent) of the specified job.
-static int cpu_use(const job_t *j) {
+/// Calculates the cpu usage (as a fraction of 1) of the specified job.
+/// This may exceed 1 if there are multiple CPUs!
+static double cpu_use(const job_t *j) {
     double u = 0;
     for (const process_ptr_t &p : j->processes) {
         timepoint_t now = timef();
-        unsigned long jiffies = proc_get_jiffies(p->pid);
+        clock_ticks_t jiffies = proc_get_jiffies(p->pid);
         double since = now - p->last_time;
         if (since > 0 && jiffies > p->last_jiffies) {
-            u += (jiffies - p->last_jiffies) / since;
+            u += clock_ticks_to_seconds(jiffies - p->last_jiffies) / since;
         }
     }
-    return u * 1000000;
+    return u;
 }
 
 /// Print information about the specified job.
@@ -64,7 +65,7 @@ static void builtin_jobs_print(const job_t *j, int mode, int header, io_streams_
             streams.out.append_format(L"%d\t%d\t", j->job_id(), pgid);
 
             if (have_proc_stat()) {
-                streams.out.append_format(L"%d%%\t", cpu_use(j));
+                streams.out.append_format(L"%.0f%%\t", 100. * cpu_use(j));
             }
 
             streams.out.append(j->is_stopped() ? _(L"stopped") : _(L"running"));
diff --git a/src/proc.cpp b/src/proc.cpp
index 575a6634e..4f99675bf 100644
--- a/src/proc.cpp
+++ b/src/proc.cpp
@@ -734,8 +734,16 @@ bool job_reap(parser_t &parser, bool allow_interactive) {
     return process_clean_after_marking(parser, allow_interactive);
 }
 
+double clock_ticks_to_seconds(clock_ticks_t ticks) {
+    long clock_ticks_per_sec = sysconf(_SC_CLK_TCK);
+    if (clock_ticks_per_sec > 0) {
+        return ticks / static_cast<double>(clock_ticks_per_sec);
+    }
+    return 0;
+}
+
 /// Get the CPU time for the specified process.
-unsigned long proc_get_jiffies(pid_t inpid) {
+clock_ticks_t proc_get_jiffies(pid_t inpid) {
     if (inpid <= 0 || !have_proc_stat()) return 0;
 
     char state;
@@ -754,7 +762,6 @@ unsigned long proc_get_jiffies(pid_t inpid) {
     int fd = open_cloexec(fn, O_RDONLY);
     if (fd < 0) return 0;
 
-    // TODO: replace the use of fscanf() as it is brittle and should never be used.
     FILE *f = fdopen(fd, "r");
     int count = fscanf(f,
                        "%9d %1023s %c %9d %9d %9d %9d %9d %9lu "
@@ -769,7 +776,8 @@ unsigned long proc_get_jiffies(pid_t inpid) {
                        &sigignore, &sigcatch, &wchan, &nswap, &cnswap, &exit_signal, &processor);
     fclose(f);
     if (count < 17) return 0;
-    return utime + stime + cutime + cstime;
+    return clock_ticks_t(utime) + clock_ticks_t(stime) + clock_ticks_t(cutime) +
+           clock_ticks_t(cstime);
 }
 
 /// Update the CPU time for all jobs.
@@ -939,8 +947,7 @@ bool job_t::resume() {
 }
 
 void job_t::continue_job(parser_t &parser) {
-    FLOGF(proc_job_run, L"Run job %d (%ls), %ls, %ls",
-          job_id(), command_wcstr(),
+    FLOGF(proc_job_run, L"Run job %d (%ls), %ls, %ls", job_id(), command_wcstr(),
           is_completed() ? L"COMPLETED" : L"UNCOMPLETED",
           parser.libdata().is_interactive ? L"INTERACTIVE" : L"NON-INTERACTIVE");
 
diff --git a/src/proc.h b/src/proc.h
index 5bc48289c..11476dae2 100644
--- a/src/proc.h
+++ b/src/proc.h
@@ -43,6 +43,13 @@ enum class job_control_t : uint8_t {
     none,
 };
 
+/// A number of clock ticks.
+using clock_ticks_t = uint64_t;
+
+/// \return clock ticks in seconds, or 0 on failure.
+/// This uses sysconf(_SC_CLK_TCK) to convert to seconds.
+double clock_ticks_to_seconds(clock_ticks_t ticks);
+
 namespace ast {
 struct statement_t;
 }
@@ -315,7 +322,7 @@ class process_t : noncopyable_t {
     timepoint_t last_time{0};
 
     /// Number of jiffies spent in process at last cpu time check.
-    unsigned long last_jiffies{0};
+    clock_ticks_t last_jiffies{0};
 
    private:
     wcstring_list_t argv_;
@@ -546,7 +553,7 @@ void print_exit_warning_for_jobs(const job_list_t &jobs);
 
 /// Use the procfs filesystem to look up how many jiffies of cpu time was used by a given pid. This
 /// function is only available on systems with the procfs file entry 'stat', i.e. Linux.
-unsigned long proc_get_jiffies(pid_t inpid);
+clock_ticks_t proc_get_jiffies(pid_t inpid);
 
 /// Update process time usage for all processes by calling the proc_get_jiffies function for every
 /// process of every job.