Fix CPU usage percentage calculation as reported by jobs

This rationalizes our types for computing CPU usage percentage and
fixes the computation. Fixes #8919.
This commit is contained in:
ridiculousfish 2022-05-07 15:08:12 -07:00
parent 770a2582de
commit 1f7d4c7441
4 changed files with 29 additions and 13 deletions

View File

@ -3,6 +3,7 @@ fish 3.5.0 (not yet released)
Notable improvements and fixes 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 Deprecations and removed features
--------------------------------- ---------------------------------

View File

@ -26,18 +26,19 @@ enum {
JOBS_PRINT_NOTHING, // print nothing (exit status only) JOBS_PRINT_NOTHING, // print nothing (exit status only)
}; };
/// Calculates the cpu usage (in percent) of the specified job. /// Calculates the cpu usage (as a fraction of 1) of the specified job.
static int cpu_use(const job_t *j) { /// This may exceed 1 if there are multiple CPUs!
static double cpu_use(const job_t *j) {
double u = 0; double u = 0;
for (const process_ptr_t &p : j->processes) { for (const process_ptr_t &p : j->processes) {
timepoint_t now = timef(); 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; double since = now - p->last_time;
if (since > 0 && jiffies > p->last_jiffies) { 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. /// 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); streams.out.append_format(L"%d\t%d\t", j->job_id(), pgid);
if (have_proc_stat()) { 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")); streams.out.append(j->is_stopped() ? _(L"stopped") : _(L"running"));

View File

@ -734,8 +734,16 @@ bool job_reap(parser_t &parser, bool allow_interactive) {
return process_clean_after_marking(parser, 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. /// 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; if (inpid <= 0 || !have_proc_stat()) return 0;
char state; char state;
@ -754,7 +762,6 @@ unsigned long proc_get_jiffies(pid_t inpid) {
int fd = open_cloexec(fn, O_RDONLY); int fd = open_cloexec(fn, O_RDONLY);
if (fd < 0) return 0; 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"); FILE *f = fdopen(fd, "r");
int count = fscanf(f, int count = fscanf(f,
"%9d %1023s %c %9d %9d %9d %9d %9d %9lu " "%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); &sigignore, &sigcatch, &wchan, &nswap, &cnswap, &exit_signal, &processor);
fclose(f); fclose(f);
if (count < 17) return 0; 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. /// Update the CPU time for all jobs.
@ -939,8 +947,7 @@ bool job_t::resume() {
} }
void job_t::continue_job(parser_t &parser) { void job_t::continue_job(parser_t &parser) {
FLOGF(proc_job_run, L"Run job %d (%ls), %ls, %ls", FLOGF(proc_job_run, L"Run job %d (%ls), %ls, %ls", job_id(), command_wcstr(),
job_id(), command_wcstr(),
is_completed() ? L"COMPLETED" : L"UNCOMPLETED", is_completed() ? L"COMPLETED" : L"UNCOMPLETED",
parser.libdata().is_interactive ? L"INTERACTIVE" : L"NON-INTERACTIVE"); parser.libdata().is_interactive ? L"INTERACTIVE" : L"NON-INTERACTIVE");

View File

@ -43,6 +43,13 @@ enum class job_control_t : uint8_t {
none, 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 { namespace ast {
struct statement_t; struct statement_t;
} }
@ -315,7 +322,7 @@ class process_t : noncopyable_t {
timepoint_t last_time{0}; timepoint_t last_time{0};
/// Number of jiffies spent in process at last cpu time check. /// Number of jiffies spent in process at last cpu time check.
unsigned long last_jiffies{0}; clock_ticks_t last_jiffies{0};
private: private:
wcstring_list_t argv_; 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 /// 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. /// 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 /// Update process time usage for all processes by calling the proc_get_jiffies function for every
/// process of every job. /// process of every job.