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(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.