SystemMonitor: Only read process command line once

Process command line reading took up 50% (!) of all of SystemMonitor's
work. However, a process's command line per definition never changes, so
we can read it once and carry it over. Also, if we couldn't read a
process's command line once, it is close to impossible that we'll ever
be able to read it in the future. Therefore, skip reading such command
lines again as well.

This commit also converts the command line itself to use String, while
we're at it.

Userland/Applications/SystemMonitor/ProcessModel.cpp

@@ -226,7 +226,7 @@ GUI::Variant ProcessModel::data(GUI::ModelIndex const& index, GUI::ModelRole rol
         case Column::Name:
             return thread.current_state.name;
         case Column::Command:
-            return thread.current_state.command;
+            return thread.current_state.command.visit([](String const& cmdline) { return cmdline; }, [](auto const&) { return ""_short_string; });
         case Column::Syscalls:
             return thread.current_state.syscall_count;
         case Column::InodeFaults:
@@ -296,7 +296,7 @@ GUI::Variant ProcessModel::data(GUI::ModelIndex const& index, GUI::ModelRole rol
                 return DeprecatedString::formatted("{} (*)", thread.current_state.name);
             return thread.current_state.name;
         case Column::Command:
-            return thread.current_state.command;
+            return thread.current_state.command.visit([](String const& cmdline) { return cmdline; }, [](auto const&) { return ""_short_string; });
         case Column::Syscalls:
             return thread.current_state.syscall_count;
         case Column::InodeFaults:
@@ -422,25 +422,16 @@ Vector<GUI::ModelIndex> ProcessModel::matches(StringView searching, unsigned fla
     return found_indices;
 }
 
-static ErrorOr<DeprecatedString> try_read_command_line(pid_t pid)
+ErrorOr<String> ProcessModel::read_command_line(pid_t pid)
 {
-    auto file = TRY(Core::File::open(DeprecatedString::formatted("/proc/{}/cmdline", pid), Core::File::OpenMode::Read));
+    auto file = TRY(Core::File::open(TRY(String::formatted("/proc/{}/cmdline", pid)), Core::File::OpenMode::Read));
     auto data = TRY(file->read_until_eof());
     auto json = TRY(JsonValue::from_string(StringView { data.bytes() }));
     auto array = json.as_array().values();
-    return DeprecatedString::join(" "sv, array);
+    return String::join(" "sv, array);
 }
 
-static DeprecatedString read_command_line(pid_t pid)
-{
-    auto string_or_error = try_read_command_line(pid);
-    if (string_or_error.is_error()) {
-        return "";
-    }
-    return string_or_error.release_value();
-}
-
-ErrorOr<void> ProcessModel::initialize_process_statistics_file()
+ErrorOr<void> ProcessModel::ensure_process_statistics_file()
 {
     if (!m_process_statistics_file || !m_process_statistics_file->is_open())
         m_process_statistics_file = TRY(Core::File::open("/sys/kernel/processes"sv, Core::File::OpenMode::Read));
@@ -450,7 +441,7 @@ ErrorOr<void> ProcessModel::initialize_process_statistics_file()
 
 void ProcessModel::update()
 {
-    auto result = initialize_process_statistics_file();
+    auto result = ensure_process_statistics_file();
     if (result.is_error()) {
         dbgln("Process model couldn't be updated: {}", result.release_error());
         return;
@@ -489,6 +480,8 @@ void ProcessModel::update()
                 auto thread_data = m_threads.ensure(tid, [&] { return make_ref_counted<Thread>(process_state); });
                 thread_data->previous_state = move(thread_data->current_state);
                 thread_data->current_state = move(state);
+                thread_data->read_command_line_if_necessary();
+
                 if (auto maybe_thread_index = process_state.threads.find_first_index(thread_data); maybe_thread_index.has_value()) {
                     process_state.threads[maybe_thread_index.value()] = thread_data;
                 } else {
@@ -511,7 +504,6 @@ void ProcessModel::update()
                     state.kernel = process.kernel;
                     state.executable = process.executable;
                     state.name = thread.name;
-                    state.command = read_command_line(process.pid);
                     state.uid = process.uid;
                     state.state = thread.state;
                     state.user = process.username;
@@ -554,7 +546,6 @@ void ProcessModel::update()
                 state.kernel = process.kernel;
                 state.executable = process.executable;
                 state.name = process.name;
-                state.command = read_command_line(process.pid);
                 state.uid = process.uid;
                 state.state = "Zombie";
                 state.user = process.username;

Userland/Applications/SystemMonitor/ProcessModel.h

@@ -55,6 +55,8 @@ public:
         __Count
     };
 
+    static ErrorOr<String> read_command_line(pid_t pid);
+
     static ProcessModel& the();
 
     static NonnullRefPtr<ProcessModel> create() { return adopt_ref(*new ProcessModel); }
@@ -94,6 +96,11 @@ private:
 
     struct Process;
 
+    enum class EmptyCommand : u8 {
+        NotInitialized,
+        PermissionError,
+    };
+
     struct ThreadState {
         pid_t tid { 0 };
         pid_t pid { 0 };
@@ -105,7 +112,7 @@ private:
         bool kernel { false };
         DeprecatedString executable { "" };
         DeprecatedString name { "" };
-        DeprecatedString command { "" };
+        Variant<String, EmptyCommand> command { EmptyCommand::NotInitialized };
         uid_t uid { 0 };
         DeprecatedString state { "" };
         DeprecatedString user { "" };
@@ -202,6 +209,26 @@ private:
         {
             return current_state.tid == current_state.process.pid;
         }
+
+        void read_command_line_if_necessary()
+        {
+            // Most likely the previous state still has a command line which we can copy over.
+            // Or, reading the command line was not allowed, e.g. on a Kernel process.
+            if ((previous_state.command.has<String>() && !current_state.command.has<String>())
+                || (previous_state.command.has<EmptyCommand>() && previous_state.command.get<EmptyCommand>() == EmptyCommand::PermissionError)) {
+                current_state.command = previous_state.command;
+                return;
+            }
+
+            auto maybe_command_line = read_command_line(current_state.pid);
+            if (!maybe_command_line.is_error()) {
+                current_state.command = maybe_command_line.value();
+                previous_state.command = maybe_command_line.release_value();
+            } else if (maybe_command_line.error().code() == EPERM || maybe_command_line.error().code() == EACCES) {
+                current_state.command = EmptyCommand::PermissionError;
+                previous_state.command = EmptyCommand::PermissionError;
+            }
+        }
     };
 
     struct Process {
@@ -240,7 +267,7 @@ private:
 
     int thread_model_row(Thread const& thread) const;
 
-    ErrorOr<void> initialize_process_statistics_file();
+    ErrorOr<void> ensure_process_statistics_file();
 
     OwnPtr<Core::File> m_process_statistics_file;