Everywhere: Improve CPU usage calculation

As threads come and go, we can't simply account for how many time
slices the threads at any given point may have been using. We need to
also account for threads that have since disappeared. This means we
also need to track how many time slices we have expired globally.

However, because this doesn't account for context switches outside of
the system timer tick values may still be under-reported. To solve this
we will need to track more accurate time information on each context
switch.

This also fixes top's cpu usage calculation which was still based on
the number of context switches.

Fixes #6473

Kernel/GlobalProcessExposed.cpp

@@ -406,10 +406,10 @@ private:
     ProcFSOverallProcesses();
     virtual bool output(KBufferBuilder& builder) override
     {
-        JsonArraySerializer array { builder };
+        JsonObjectSerializer<KBufferBuilder> json { builder };
 
         // Keep this in sync with CProcessStatistics.
-        auto build_process = [&](const Process& process) {
+        auto build_process = [&](JsonArraySerializer<KBufferBuilder>& array, const Process& process) {
             auto process_object = array.add_object();
 
             if (process.is_user_process()) {
@@ -488,11 +488,19 @@ private:
         };
 
         ScopedSpinLock lock(g_scheduler_lock);
+        {
+            {
+                auto array = json.add_array("processes");
                 auto processes = Process::all_processes();
-        build_process(*Scheduler::colonel());
+                build_process(array, *Scheduler::colonel());
                 for (auto& process : processes)
-            build_process(process);
+                    build_process(array, process);
-        array.finish();
+            }
+
+            auto total_ticks_scheduled = Scheduler::get_total_ticks_scheduled();
+            json.add("total_ticks", total_ticks_scheduled.total);
+            json.add("total_ticks_kernel", total_ticks_scheduled.total_kernel);
+        }
         return true;
     }
 };

Kernel/Scheduler.cpp

@@ -51,6 +51,7 @@ struct ThreadReadyQueue {
 };
 static SpinLock<u8> g_ready_queues_lock;
 static u32 g_ready_queues_mask;
+static TotalTicksScheduled g_total_ticks_scheduled;
 static constexpr u32 g_ready_queue_buckets = sizeof(g_ready_queues_mask) * 8;
 READONLY_AFTER_INIT static ThreadReadyQueue* g_ready_queues; // g_ready_queue_buckets entries
 static void dump_thread_list(bool = false);
@@ -446,18 +447,22 @@ void Scheduler::timer_tick(const RegisterState& regs)
         return; // TODO: This prevents scheduling on other CPUs!
 #endif
 
-    if (current_thread->previous_mode() == Thread::PreviousMode::UserMode && current_thread->should_die() && !current_thread->is_blocked()) {
-        dbgln_if(SCHEDULER_DEBUG, "Scheduler[{}]: Terminating user mode thread {}", Processor::id(), *current_thread);
     {
         ScopedSpinLock scheduler_lock(g_scheduler_lock);
+        if (current_thread->previous_mode() == Thread::PreviousMode::UserMode && current_thread->should_die() && !current_thread->is_blocked()) {
+            dbgln_if(SCHEDULER_DEBUG, "Scheduler[{}]: Terminating user mode thread {}", Processor::id(), *current_thread);
             current_thread->set_state(Thread::Dying);
-        }
-        VERIFY(!Processor::current().in_critical());
             Processor::current().invoke_scheduler_async();
             return;
         }
+
+        g_total_ticks_scheduled.total++;
+        if (current_thread->previous_mode() == Thread::PreviousMode::KernelMode)
+            g_total_ticks_scheduled.total_kernel++;
+
         if (current_thread->tick())
             return;
+    }
 
     if (!current_thread->is_idle_thread() && !peek_next_runnable_thread()) {
         // If no other thread is ready to be scheduled we don't need to
@@ -540,6 +545,12 @@ bool Scheduler::is_initialized()
     return Processor::idle_thread() != nullptr;
 }
 
+TotalTicksScheduled Scheduler::get_total_ticks_scheduled()
+{
+    ScopedSpinLock scheduler_lock(g_scheduler_lock);
+    return g_total_ticks_scheduled;
+}
+
 void dump_thread_list(bool with_stack_traces)
 {
     dbgln("Scheduler thread list for processor {}:", Processor::id());

Kernel/Scheduler.h

@@ -24,6 +24,11 @@ extern WaitQueue* g_finalizer_wait_queue;
 extern Atomic<bool> g_finalizer_has_work;
 extern RecursiveSpinLock g_scheduler_lock;
 
+struct TotalTicksScheduled {
+    u64 total { 0 };
+    u64 total_kernel { 0 };
+};
+
 class Scheduler {
 public:
     static void initialize();
@@ -49,6 +54,7 @@ public:
     static void queue_runnable_thread(Thread&);
     static void dump_scheduler_state(bool = false);
     static bool is_initialized();
+    static TotalTicksScheduled get_total_ticks_scheduled();
 };
 
 }

Userland/Applets/ResourceGraph/main.cpp

@@ -45,14 +45,12 @@ private:
     {
         switch (m_graph_type) {
         case GraphType::CPU: {
-            unsigned busy;
+            u64 busy, idle, scheduled_diff;
-            unsigned idle;
+            if (get_cpu_usage(busy, idle, scheduled_diff)) {
-            if (get_cpu_usage(busy, idle)) {
+                auto busy_diff = busy - m_last_cpu_busy;
-                unsigned busy_diff = busy - m_last_cpu_busy;
-                unsigned idle_diff = idle - m_last_cpu_idle;
                 m_last_cpu_busy = busy;
                 m_last_cpu_idle = idle;
-                float cpu = (float)busy_diff / (float)(busy_diff + idle_diff);
+                float cpu = scheduled_diff > 0 ? (float)busy_diff / (float)scheduled_diff : 0;
                 m_history.enqueue(cpu);
                 m_tooltip = String::formatted("CPU usage: {:.1}%", 100 * cpu);
             } else {
@@ -120,16 +118,21 @@ private:
         }
     }
 
-    bool get_cpu_usage(unsigned& busy, unsigned& idle)
+    bool get_cpu_usage(u64& busy, u64& idle, u64& scheduled_diff)
     {
         busy = 0;
         idle = 0;
+        scheduled_diff = 0;
 
         auto all_processes = Core::ProcessStatisticsReader::get_all(m_proc_all);
-        if (!all_processes.has_value() || all_processes.value().is_empty())
+        if (!all_processes.has_value() || all_processes.value().processes.is_empty())
             return false;
 
-        for (auto& it : all_processes.value()) {
+        if (m_last_total_sum.has_value())
+            scheduled_diff = all_processes->total_ticks_scheduled - m_last_total_sum.value();
+        m_last_total_sum = all_processes->total_ticks_scheduled;
+
+        for (auto& it : all_processes.value().processes) {
             for (auto& jt : it.threads) {
                 if (it.pid == 0)
                     idle += jt.ticks_user + jt.ticks_kernel;
@@ -174,8 +177,9 @@ private:
     Gfx::Color m_graph_color;
     Gfx::Color m_graph_error_color;
     CircularQueue<float, history_size> m_history;
-    unsigned m_last_cpu_busy { 0 };
+    u64 m_last_cpu_busy { 0 };
-    unsigned m_last_cpu_idle { 0 };
+    u64 m_last_cpu_idle { 0 };
+    Optional<u64> m_last_total_sum;
     String m_tooltip;
     RefPtr<Core::File> m_proc_all;
     RefPtr<Core::File> m_proc_mem;

Userland/Applications/SystemMonitor/ProcessModel.cpp

@@ -318,17 +318,18 @@ void ProcessModel::update()
     auto previous_tid_count = m_tids.size();
     auto all_processes = Core::ProcessStatisticsReader::get_all(m_proc_all);
 
-    u64 last_sum_ticks_scheduled = 0, last_sum_ticks_scheduled_kernel = 0;
-    for (auto& it : m_threads) {
-        auto& current_state = it.value->current_state;
-        last_sum_ticks_scheduled += current_state.ticks_user + current_state.ticks_kernel;
-        last_sum_ticks_scheduled_kernel += current_state.ticks_kernel;
-    }
-
     HashTable<int> live_tids;
     u64 sum_ticks_scheduled = 0, sum_ticks_scheduled_kernel = 0;
+    u64 total_ticks_scheduled_diff = 0;
     if (all_processes.has_value()) {
-        for (auto& process : all_processes.value()) {
+        if (m_has_total_ticks)
+            total_ticks_scheduled_diff = all_processes->total_ticks_scheduled - m_total_ticks_scheduled;
+
+        m_total_ticks_scheduled = all_processes->total_ticks_scheduled;
+        m_total_ticks_scheduled_kernel = all_processes->total_ticks_scheduled_kernel;
+        m_has_total_ticks = true;
+
+        for (auto& process : all_processes.value().processes) {
             for (auto& thread : process.threads) {
                 ThreadState state;
                 state.kernel = process.kernel;
@@ -388,6 +389,7 @@ void ProcessModel::update()
         c.total_cpu_percent = 0.0;
         c.total_cpu_percent_kernel = 0.0;
     }
+
     Vector<int, 16> tids_to_remove;
     for (auto& it : m_threads) {
         if (!live_tids.contains(it.key)) {
@@ -398,8 +400,8 @@ void ProcessModel::update()
         u32 ticks_scheduled_diff = (thread.current_state.ticks_user + thread.current_state.ticks_kernel)
             - (thread.previous_state.ticks_user + thread.previous_state.ticks_kernel);
         u32 ticks_scheduled_diff_kernel = thread.current_state.ticks_kernel - thread.previous_state.ticks_kernel;
-        thread.current_state.cpu_percent = ((float)ticks_scheduled_diff * 100) / (float)(sum_ticks_scheduled - last_sum_ticks_scheduled);
+        thread.current_state.cpu_percent = total_ticks_scheduled_diff > 0 ? ((float)ticks_scheduled_diff * 100) / (float)total_ticks_scheduled_diff : 0;
-        thread.current_state.cpu_percent_kernel = ((float)ticks_scheduled_diff_kernel * 100) / (float)(sum_ticks_scheduled - last_sum_ticks_scheduled);
+        thread.current_state.cpu_percent_kernel = total_ticks_scheduled_diff > 0 ? ((float)ticks_scheduled_diff_kernel * 100) / (float)total_ticks_scheduled_diff : 0;
         if (it.value->current_state.pid != 0) {
             auto& cpu_info = m_cpus[thread.current_state.cpu];
             cpu_info.total_cpu_percent += thread.current_state.cpu_percent;
@@ -407,6 +409,7 @@ void ProcessModel::update()
             m_tids.append(it.key);
         }
     }
+
     for (auto tid : tids_to_remove)
         m_threads.remove(tid);
 
@@ -414,7 +417,7 @@ void ProcessModel::update()
         on_cpu_info_change(m_cpus);
 
     if (on_state_update)
-        on_state_update(all_processes->size(), m_threads.size());
+        on_state_update(all_processes.has_value() ? all_processes->processes.size() : 0, m_threads.size());
 
     // FIXME: This is a rather hackish way of invalidating indices.
     //        It would be good if GUI::Model had a way to orchestrate removal/insertion while preserving indices.

Userland/Applications/SystemMonitor/ProcessModel.h

@@ -129,4 +129,7 @@ private:
     Vector<int> m_tids;
     RefPtr<Core::File> m_proc_all;
     GUI::Icon m_kernel_process_icon;
+    u64 m_total_ticks_scheduled { 0 };
+    u64 m_total_ticks_scheduled_kernel { 0 };
+    bool m_has_total_ticks { false };
 };

Userland/DevTools/Profiler/main.cpp

@@ -273,7 +273,8 @@ bool generate_profile(pid_t& pid)
 
     auto all_processes = Core::ProcessStatisticsReader::get_all();
     if (all_processes.has_value()) {
-        if (auto it = all_processes.value().find_if([&](auto& entry) { return entry.pid == pid; }); it != all_processes.value().end())
+        auto& processes = all_processes->processes;
+        if (auto it = processes.find_if([&](auto& entry) { return entry.pid == pid; }); it != processes.end())
             process_name = it->name;
         else
             process_name = "(unknown)";

Userland/Libraries/LibCore/ProcessStatisticsReader.cpp

@@ -16,7 +16,7 @@ namespace Core {
 
 HashMap<uid_t, String> ProcessStatisticsReader::s_usernames;
 
-Optional<Vector<Core::ProcessStatistics>> ProcessStatisticsReader::get_all(RefPtr<Core::File>& proc_all_file)
+Optional<AllProcessesStatistics> ProcessStatisticsReader::get_all(RefPtr<Core::File>& proc_all_file)
 {
     if (proc_all_file) {
         if (!proc_all_file->seek(0, Core::SeekMode::SetPosition)) {
@@ -31,13 +31,14 @@ Optional<Vector<Core::ProcessStatistics>> ProcessStatisticsReader::get_all(RefPt
         }
     }
 
-    Vector<Core::ProcessStatistics> processes;
+    AllProcessesStatistics all_processes_statistics;
 
     auto file_contents = proc_all_file->read_all();
     auto json = JsonValue::from_string(file_contents);
     if (!json.has_value())
         return {};
-    json.value().as_array().for_each([&](auto& value) {
+    auto& json_obj = json.value().as_object();
+    json_obj.get("processes").as_array().for_each([&](auto& value) {
         const JsonObject& process_object = value.as_object();
         Core::ProcessStatistics process;
 
@@ -92,13 +93,15 @@ Optional<Vector<Core::ProcessStatistics>> ProcessStatisticsReader::get_all(RefPt
 
         // and synthetic data last
         process.username = username_from_uid(process.uid);
-        processes.append(move(process));
+        all_processes_statistics.processes.append(move(process));
     });
 
-    return processes;
+    all_processes_statistics.total_ticks_scheduled = json_obj.get("total_ticks").to_u64();
+    all_processes_statistics.total_ticks_scheduled_kernel = json_obj.get("total_ticks_kernel").to_u64();
+    return all_processes_statistics;
 }
 
-Optional<Vector<Core::ProcessStatistics>> ProcessStatisticsReader::get_all()
+Optional<AllProcessesStatistics> ProcessStatisticsReader::get_all()
 {
     RefPtr<Core::File> proc_all_file;
     return get_all(proc_all_file);

Userland/Libraries/LibCore/ProcessStatisticsReader.h

@@ -64,10 +64,16 @@ struct ProcessStatistics {
     String username;
 };
 
+struct AllProcessesStatistics {
+    Vector<ProcessStatistics> processes;
+    u64 total_ticks_scheduled;
+    u64 total_ticks_scheduled_kernel;
+};
+
 class ProcessStatisticsReader {
 public:
-    static Optional<Vector<Core::ProcessStatistics>> get_all(RefPtr<Core::File>&);
+    static Optional<AllProcessesStatistics> get_all(RefPtr<Core::File>&);
-    static Optional<Vector<Core::ProcessStatistics>> get_all();
+    static Optional<AllProcessesStatistics> get_all();
 
 private:
     static String username_from_uid(uid_t);

Userland/Libraries/LibGUI/RunningProcessesModel.cpp

@@ -27,9 +27,9 @@ void RunningProcessesModel::update()
 {
     m_processes.clear();
 
-    auto processes = Core::ProcessStatisticsReader::get_all();
+    auto all_processes = Core::ProcessStatisticsReader::get_all();
-    if (processes.has_value()) {
+    if (all_processes.has_value()) {
-        for (auto& it : processes.value()) {
+        for (auto& it : all_processes.value().processes) {
             Process process;
             process.pid = it.pid;
             process.uid = it.uid;

Userland/Utilities/killall.cpp

@@ -19,11 +19,11 @@ static void print_usage_and_exit()
 
 static int kill_all(const String& process_name, const unsigned signum)
 {
-    auto processes = Core::ProcessStatisticsReader::get_all();
+    auto all_processes = Core::ProcessStatisticsReader::get_all();
-    if (!processes.has_value())
+    if (!all_processes.has_value())
         return 1;
 
-    for (auto& process : processes.value()) {
+    for (auto& process : all_processes.value().processes) {
         if (process.name == process_name) {
             int ret = kill(process.pid, signum);
             if (ret < 0)

Userland/Utilities/lsof.cpp

@@ -146,11 +146,11 @@ int main(int argc, char* argv[])
     }
 
     outln("{:28} {:>4} {:>4} {:10} {:>4} {}", "COMMAND", "PID", "PGID", "USER", "FD", "NAME");
-    auto processes = Core::ProcessStatisticsReader::get_all();
+    auto all_processes = Core::ProcessStatisticsReader::get_all();
-    if (!processes.has_value())
+    if (!all_processes.has_value())
         return 1;
     if (arg_pid == -1) {
-        for (auto& process : processes.value()) {
+        for (auto& process : all_processes.value().processes) {
             if (process.pid == 0)
                 continue;
             auto open_files = get_open_files_by_pid(process.pid);
@@ -175,7 +175,7 @@ int main(int argc, char* argv[])
             return 0;
 
         for (auto& file : open_files) {
-            display_entry(file, *processes->find_if([&](auto& entry) { return entry.pid == arg_pid; }));
+            display_entry(file, *all_processes->processes.find_if([&](auto& entry) { return entry.pid == arg_pid; }));
         }
     }
 

Userland/Utilities/pgrep.cpp

@@ -36,12 +36,12 @@ int main(int argc, char** argv)
         return 1;
     }
 
-    auto processes = Core::ProcessStatisticsReader::get_all();
+    auto all_processes = Core::ProcessStatisticsReader::get_all();
-    if (!processes.has_value())
+    if (!all_processes.has_value())
         return 1;
 
     Vector<pid_t> matches;
-    for (auto& it : processes.value()) {
+    for (auto& it : all_processes.value().processes) {
         auto result = re.match(it.name, PosixFlags::Global);
         if (result.success ^ invert_match) {
             matches.append(it.pid);

Userland/Utilities/pidof.cpp

@@ -16,11 +16,11 @@ static int pid_of(const String& process_name, bool single_shot, bool omit_pid, p
 {
     bool displayed_at_least_one = false;
 
-    auto processes = Core::ProcessStatisticsReader::get_all();
+    auto all_processes = Core::ProcessStatisticsReader::get_all();
-    if (!processes.has_value())
+    if (!all_processes.has_value())
         return 1;
 
-    for (auto& it : processes.value()) {
+    for (auto& it : all_processes.value().processes) {
         if (it.name == process_name) {
             if (!omit_pid || it.pid != pid) {
                 out(displayed_at_least_one ? " {}" : "{}", it.pid);

Userland/Utilities/ps.cpp

@@ -83,14 +83,15 @@ int main(int argc, char** argv)
         cmd_column = add_column("CMD", Alignment::Left);
     }
 
-    auto processes = Core::ProcessStatisticsReader::get_all();
+    auto all_processes = Core::ProcessStatisticsReader::get_all();
-    if (!processes.has_value())
+    if (!all_processes.has_value())
         return 1;
 
-    quick_sort(processes.value(), [](auto& a, auto& b) { return a.pid < b.pid; });
+    auto& processes = all_processes.value().processes;
+    quick_sort(processes, [](auto& a, auto& b) { return a.pid < b.pid; });
 
     Vector<Vector<String>> rows;
-    rows.ensure_capacity(1 + processes.value().size());
+    rows.ensure_capacity(1 + processes.size());
 
     Vector<String> header;
     header.ensure_capacity(columns.size());
@@ -98,7 +99,7 @@ int main(int argc, char** argv)
         header.append(column.title);
     rows.append(move(header));
 
-    for (auto const& process : processes.value()) {
+    for (auto const& process : processes) {
         auto tty = process.tty;
 
         if (!every_process_flag && tty != this_tty)

Userland/Utilities/top.cpp

@@ -54,9 +54,9 @@ struct ThreadData {
     unsigned inode_faults;
     unsigned zero_faults;
     unsigned cow_faults;
-    unsigned times_scheduled;
+    u64 ticks_scheduled;
 
-    unsigned times_scheduled_since_prev { 0 };
+    u64 ticks_scheduled_since_prev { 0 };
     unsigned cpu_percent { 0 };
     unsigned cpu_percent_decimal { 0 };
 
@@ -83,7 +83,8 @@ struct Traits<PidAndTid> : public GenericTraits<PidAndTid> {
 
 struct Snapshot {
     HashMap<PidAndTid, ThreadData> map;
-    u32 sum_times_scheduled { 0 };
+    u64 total_ticks_scheduled { 0 };
+    u64 total_ticks_scheduled_kernel { 0 };
 };
 
 static Snapshot get_snapshot()
@@ -93,9 +94,8 @@ static Snapshot get_snapshot()
         return {};
 
     Snapshot snapshot;
-    for (auto& process : all_processes.value()) {
+    for (auto& process : all_processes.value().processes) {
         for (auto& thread : process.threads) {
-            snapshot.sum_times_scheduled += thread.times_scheduled;
             ThreadData thread_data;
             thread_data.tid = thread.tid;
             thread_data.pid = process.pid;
@@ -115,7 +115,7 @@ static Snapshot get_snapshot()
             thread_data.inode_faults = thread.inode_faults;
             thread_data.zero_faults = thread.zero_faults;
             thread_data.cow_faults = thread.cow_faults;
-            thread_data.times_scheduled = thread.times_scheduled;
+            thread_data.ticks_scheduled = (u64)thread.ticks_user + (u64)thread.ticks_kernel;
             thread_data.priority = thread.priority;
             thread_data.state = thread.state;
             thread_data.username = process.username;
@@ -124,6 +124,9 @@ static Snapshot get_snapshot()
         }
     }
 
+    snapshot.total_ticks_scheduled = all_processes->total_ticks_scheduled;
+    snapshot.total_ticks_scheduled_kernel = all_processes->total_ticks_scheduled_kernel;
+
     return snapshot;
 }
 
@@ -217,7 +220,7 @@ int main(int argc, char** argv)
         }
 
         auto current = get_snapshot();
-        auto sum_diff = current.sum_times_scheduled - prev.sum_times_scheduled;
+        auto total_scheduled_diff = current.total_ticks_scheduled - prev.total_ticks_scheduled;
 
         printf("\033[3J\033[H\033[2J");
         printf("\033[47;30m%6s %3s %3s  %-9s  %-13s  %6s  %6s  %4s  %s\033[K\033[0m\n",
@@ -234,15 +237,14 @@ int main(int argc, char** argv)
             auto pid_and_tid = it.key;
             if (pid_and_tid.pid == 0)
                 continue;
-            u32 times_scheduled_now = it.value.times_scheduled;
             auto jt = prev.map.find(pid_and_tid);
             if (jt == prev.map.end())
                 continue;
-            u32 times_scheduled_before = (*jt).value.times_scheduled;
+            auto ticks_scheduled_before = (*jt).value.ticks_scheduled;
-            u32 times_scheduled_diff = times_scheduled_now - times_scheduled_before;
+            auto ticks_scheduled_diff = it.value.ticks_scheduled - ticks_scheduled_before;
-            it.value.times_scheduled_since_prev = times_scheduled_diff;
+            it.value.ticks_scheduled_since_prev = ticks_scheduled_diff;
-            it.value.cpu_percent = sum_diff > 0 ? ((times_scheduled_diff * 100) / sum_diff) : 0;
+            it.value.cpu_percent = total_scheduled_diff > 0 ? ((ticks_scheduled_diff * 100) / total_scheduled_diff) : 0;
-            it.value.cpu_percent_decimal = sum_diff > 0 ? (((times_scheduled_diff * 1000) / sum_diff) % 10) : 0;
+            it.value.cpu_percent_decimal = total_scheduled_diff > 0 ? (((ticks_scheduled_diff * 1000) / total_scheduled_diff) % 10) : 0;
             threads.append(&it.value);
         }
 
@@ -265,8 +267,9 @@ int main(int argc, char** argv)
             case TopOption::SortBy::Name:
                 return p2->name > p1->name;
             case TopOption::SortBy::Cpu:
+                return p2->cpu_percent * 10 + p2->cpu_percent_decimal < p1->cpu_percent * 10 + p1->cpu_percent_decimal;
             default:
-                return p2->times_scheduled_since_prev < p1->times_scheduled_since_prev;
+                return p2->ticks_scheduled_since_prev < p1->ticks_scheduled_since_prev;
             }
         });
 

Userland/Utilities/w.cpp

@@ -93,7 +93,7 @@ int main()
 
         String what = "n/a";
 
-        for (auto& process : process_statistics.value()) {
+        for (auto& process : process_statistics.value().processes) {
             if (process.tty == tty && process.pid == process.pgid)
                 what = process.name;
         }