Browse Source

Ladybird: Restore ability to run WebContent under callgrind

This broke due to the way we now use posix_spawn under the hood. This
moves the handling of the callgrind option to the launcher helper where
we iterate over the candidate process paths, as we need to augment the
way we fork the process for callgrind based on those paths.

This also opens the door for running other processes under callgrind in
the future.
Timothy Flynn 1 year ago
parent
commit
56e0cd0b1a
1 changed files with 37 additions and 28 deletions
  1. 37 28
      Ladybird/HelperProcess.cpp

+ 37 - 28
Ladybird/HelperProcess.cpp

@@ -19,16 +19,31 @@ template<typename ClientType, typename SpawnFunction>
 static ErrorOr<NonnullRefPtr<ClientType>> launch_server_process_impl(
 static ErrorOr<NonnullRefPtr<ClientType>> launch_server_process_impl(
     StringView server_name,
     StringView server_name,
     ReadonlySpan<ByteString> candidate_server_paths,
     ReadonlySpan<ByteString> candidate_server_paths,
-    Vector<ByteString> const& arguments,
+    Vector<ByteString> arguments,
     RegisterWithProcessManager register_with_process_manager,
     RegisterWithProcessManager register_with_process_manager,
+    Ladybird::EnableCallgrindProfiling enable_callgrind_profiling,
     SpawnFunction&& spawn_function)
     SpawnFunction&& spawn_function)
 {
 {
-    for (auto [i, path] : enumerate(candidate_server_paths)) {
-        auto result = spawn_function(Core::ProcessSpawnOptions {
-            .name = server_name,
-            .executable = path,
-            .arguments = arguments,
+    if (enable_callgrind_profiling == Ladybird::EnableCallgrindProfiling::Yes) {
+        arguments.prepend({
+            "--tool=callgrind"sv,
+            "--instr-atstart=no"sv,
+            ""sv, // Placeholder for the process path.
         });
         });
+    }
+
+    for (auto [i, path] : enumerate(candidate_server_paths)) {
+        Core::ProcessSpawnOptions options { .name = server_name, .arguments = arguments };
+
+        if (enable_callgrind_profiling == Ladybird::EnableCallgrindProfiling::Yes) {
+            options.executable = "valgrind"sv;
+            options.search_for_executable_in_path = true;
+            arguments[2] = path;
+        } else {
+            options.executable = path;
+        }
+
+        auto result = spawn_function(options);
 
 
         if (!result.is_error()) {
         if (!result.is_error()) {
             auto process = result.release_value();
             auto process = result.release_value();
@@ -36,6 +51,13 @@ static ErrorOr<NonnullRefPtr<ClientType>> launch_server_process_impl(
             if (register_with_process_manager == RegisterWithProcessManager::Yes)
             if (register_with_process_manager == RegisterWithProcessManager::Yes)
                 WebView::ProcessManager::the().add_process(WebView::process_type_from_name(server_name), process.process.pid());
                 WebView::ProcessManager::the().add_process(WebView::process_type_from_name(server_name), process.process.pid());
 
 
+            if (enable_callgrind_profiling == Ladybird::EnableCallgrindProfiling::Yes) {
+                dbgln();
+                dbgln("\033[1;45mLaunched {} process under callgrind!\033[0m", server_name);
+                dbgln("\033[100mRun `\033[4mcallgrind_control -i on\033[24m` to start instrumentation and `\033[4mcallgrind_control -i off\033[24m` stop it again.\033[0m");
+                dbgln();
+            }
+
             return move(process.client);
             return move(process.client);
         }
         }
 
 
@@ -52,11 +74,12 @@ template<typename ClientType, typename... ClientArguments>
 static ErrorOr<NonnullRefPtr<ClientType>> launch_generic_server_process(
 static ErrorOr<NonnullRefPtr<ClientType>> launch_generic_server_process(
     StringView server_name,
     StringView server_name,
     ReadonlySpan<ByteString> candidate_server_paths,
     ReadonlySpan<ByteString> candidate_server_paths,
-    Vector<ByteString> const& arguments,
+    Vector<ByteString> arguments,
     RegisterWithProcessManager register_with_process_manager,
     RegisterWithProcessManager register_with_process_manager,
+    Ladybird::EnableCallgrindProfiling enable_callgrind_profiling,
     ClientArguments&&... client_arguments)
     ClientArguments&&... client_arguments)
 {
 {
-    return launch_server_process_impl<ClientType>(server_name, candidate_server_paths, arguments, register_with_process_manager, [&](auto options) {
+    return launch_server_process_impl<ClientType>(server_name, candidate_server_paths, move(arguments), register_with_process_manager, enable_callgrind_profiling, [&](auto options) {
         return Core::IPCProcess::spawn<ClientType>(move(options), forward<ClientArguments>(client_arguments)...);
         return Core::IPCProcess::spawn<ClientType>(move(options), forward<ClientArguments>(client_arguments)...);
     });
     });
 }
 }
@@ -65,11 +88,11 @@ template<typename ClientType, typename... ClientArguments>
 static ErrorOr<NonnullRefPtr<ClientType>> launch_singleton_server_process(
 static ErrorOr<NonnullRefPtr<ClientType>> launch_singleton_server_process(
     StringView server_name,
     StringView server_name,
     ReadonlySpan<ByteString> candidate_server_paths,
     ReadonlySpan<ByteString> candidate_server_paths,
-    Vector<ByteString> const& arguments,
+    Vector<ByteString> arguments,
     RegisterWithProcessManager register_with_process_manager,
     RegisterWithProcessManager register_with_process_manager,
     ClientArguments&&... client_arguments)
     ClientArguments&&... client_arguments)
 {
 {
-    return launch_server_process_impl<ClientType>(server_name, candidate_server_paths, arguments, register_with_process_manager, [&](auto options) {
+    return launch_server_process_impl<ClientType>(server_name, candidate_server_paths, move(arguments), register_with_process_manager, Ladybird::EnableCallgrindProfiling::No, [&](auto options) {
         return Core::IPCProcess::spawn_singleton<ClientType>(move(options), forward<ClientArguments>(client_arguments)...);
         return Core::IPCProcess::spawn_singleton<ClientType>(move(options), forward<ClientArguments>(client_arguments)...);
     });
     });
 }
 }
@@ -87,11 +110,6 @@ ErrorOr<NonnullRefPtr<WebView::WebContentClient>> launch_web_content_process(
         web_content_options.executable_path.to_byte_string(),
         web_content_options.executable_path.to_byte_string(),
     };
     };
 
 
-    if (web_content_options.enable_callgrind_profiling == Ladybird::EnableCallgrindProfiling::Yes) {
-        arguments.prepend("--instr-atstart=no"sv);
-        arguments.prepend("--tool=callgrind"sv);
-        arguments.prepend("valgrind"sv);
-    }
     if (web_content_options.is_layout_test_mode == Ladybird::IsLayoutTestMode::Yes)
     if (web_content_options.is_layout_test_mode == Ladybird::IsLayoutTestMode::Yes)
         arguments.append("--layout-test-mode"sv);
         arguments.append("--layout-test-mode"sv);
     if (web_content_options.use_lagom_networking == Ladybird::UseLagomNetworking::Yes)
     if (web_content_options.use_lagom_networking == Ladybird::UseLagomNetworking::Yes)
@@ -115,21 +133,12 @@ ErrorOr<NonnullRefPtr<WebView::WebContentClient>> launch_web_content_process(
         arguments.append(ByteString::number(request_server_socket->fd()));
         arguments.append(ByteString::number(request_server_socket->fd()));
     }
     }
 
 
-    auto web_content_client = TRY(launch_generic_server_process<WebView::WebContentClient>("WebContent"sv, candidate_web_content_paths, arguments, RegisterWithProcessManager::No, view));
-
-    if (web_content_options.enable_callgrind_profiling == Ladybird::EnableCallgrindProfiling::Yes) {
-        dbgln();
-        dbgln("\033[1;45mLaunched WebContent process under callgrind!\033[0m");
-        dbgln("\033[100mRun `\033[4mcallgrind_control -i on\033[24m` to start instrumentation and `\033[4mcallgrind_control -i off\033[24m` stop it again.\033[0m");
-        dbgln();
-    }
-
-    return web_content_client;
+    return launch_generic_server_process<WebView::WebContentClient>("WebContent"sv, candidate_web_content_paths, move(arguments), RegisterWithProcessManager::No, web_content_options.enable_callgrind_profiling, view);
 }
 }
 
 
 ErrorOr<NonnullRefPtr<ImageDecoderClient::Client>> launch_image_decoder_process(ReadonlySpan<ByteString> candidate_image_decoder_paths)
 ErrorOr<NonnullRefPtr<ImageDecoderClient::Client>> launch_image_decoder_process(ReadonlySpan<ByteString> candidate_image_decoder_paths)
 {
 {
-    return launch_generic_server_process<ImageDecoderClient::Client>("ImageDecoder"sv, candidate_image_decoder_paths, {}, RegisterWithProcessManager::Yes);
+    return launch_generic_server_process<ImageDecoderClient::Client>("ImageDecoder"sv, candidate_image_decoder_paths, {}, RegisterWithProcessManager::Yes, Ladybird::EnableCallgrindProfiling::No);
 }
 }
 
 
 ErrorOr<NonnullRefPtr<Web::HTML::WebWorkerClient>> launch_web_worker_process(ReadonlySpan<ByteString> candidate_web_worker_paths, NonnullRefPtr<Protocol::RequestClient> request_client)
 ErrorOr<NonnullRefPtr<Web::HTML::WebWorkerClient>> launch_web_worker_process(ReadonlySpan<ByteString> candidate_web_worker_paths, NonnullRefPtr<Protocol::RequestClient> request_client)
@@ -141,7 +150,7 @@ ErrorOr<NonnullRefPtr<Web::HTML::WebWorkerClient>> launch_web_worker_process(Rea
         ByteString::number(socket.fd()),
         ByteString::number(socket.fd()),
     };
     };
 
 
-    return launch_generic_server_process<Web::HTML::WebWorkerClient>("WebWorker"sv, candidate_web_worker_paths, arguments, RegisterWithProcessManager::Yes);
+    return launch_generic_server_process<Web::HTML::WebWorkerClient>("WebWorker"sv, candidate_web_worker_paths, move(arguments), RegisterWithProcessManager::Yes, Ladybird::EnableCallgrindProfiling::No);
 }
 }
 
 
 ErrorOr<NonnullRefPtr<Protocol::RequestClient>> launch_request_server_process(ReadonlySpan<ByteString> candidate_request_server_paths, StringView serenity_resource_root, Vector<ByteString> const& certificates)
 ErrorOr<NonnullRefPtr<Protocol::RequestClient>> launch_request_server_process(ReadonlySpan<ByteString> candidate_request_server_paths, StringView serenity_resource_root, Vector<ByteString> const& certificates)
@@ -161,7 +170,7 @@ ErrorOr<NonnullRefPtr<Protocol::RequestClient>> launch_request_server_process(Re
         arguments.append(server.value());
         arguments.append(server.value());
     }
     }
 
 
-    return launch_generic_server_process<Protocol::RequestClient>("RequestServer"sv, candidate_request_server_paths, arguments, RegisterWithProcessManager::Yes);
+    return launch_generic_server_process<Protocol::RequestClient>("RequestServer"sv, candidate_request_server_paths, move(arguments), RegisterWithProcessManager::Yes, Ladybird::EnableCallgrindProfiling::No);
 }
 }
 
 
 ErrorOr<NonnullRefPtr<SQL::SQLClient>> launch_sql_server_process(ReadonlySpan<ByteString> candidate_sql_server_paths)
 ErrorOr<NonnullRefPtr<SQL::SQLClient>> launch_sql_server_process(ReadonlySpan<ByteString> candidate_sql_server_paths)