From 0d7d1fccff412c745d4e718fa0eec8cf63cc9709 Mon Sep 17 00:00:00 2001 From: Augustin Cavalier Date: Wed, 28 Aug 2019 00:53:35 -0400 Subject: [PATCH] libroot: Implement the general case of posix_spawn using load_image. The goal here is to avoid potentially expensive fork()ing. The time for a fork() is (for a process with no real heap usage and thus few areas) 300-400us on my system. load_image() takes 3000us (3ms) or so, but this of course includes exec() time. Overall, for compiling HaikuDepot (with a tweaked jam to use posix_spawn on Haiku, not just on Linux) there is a slight decrease in time: before: real 1m21.727s user 1m2.131s sys 0m43.029s after: real 1m19.472s user 1m1.752s sys 0m41.740s Which is probably within the realm of "noise", so more benchmarks are needed. Likely if we tweak our jam usage to not need as many shells when running commands, this would be a much more noticeable change. Change-Id: I217f2476b1ed9aa18322b3c2bc8986571d89549a --- headers/private/libroot/libroot_private.h | 2 + src/system/libroot/os/image.cpp | 16 +++++-- src/system/libroot/posix/spawn.cpp | 53 +++++++++++++++++++++-- 3 files changed, 64 insertions(+), 7 deletions(-) diff --git a/headers/private/libroot/libroot_private.h b/headers/private/libroot/libroot_private.h index 20bc2ade99..5055848b03 100644 --- a/headers/private/libroot/libroot_private.h +++ b/headers/private/libroot/libroot_private.h @@ -33,6 +33,8 @@ status_t __test_executable(const char *path, char *invoker); status_t __flatten_process_args(const char* const* args, int32 argCount, const char* const* env, int32* envCount, const char* executablePath, char*** _flatArgs, size_t* _flatSize); +thread_id __load_image_at_path(const char* path, int32 argCount, + const char **args, const char **environ); void _call_atexit_hooks_for_range(addr_t start, addr_t size); void __init_env(const struct user_space_program_args *args); void __init_env_post_heap(void); diff --git a/src/system/libroot/os/image.cpp b/src/system/libroot/os/image.cpp index e672ce8b8b..caf4adb873 100644 --- a/src/system/libroot/os/image.cpp +++ b/src/system/libroot/os/image.cpp @@ -201,7 +201,8 @@ private: thread_id -load_image(int32 argCount, const char **args, const char **environ) +__load_image_at_path(const char* path, int32 argCount, const char **args, + const char **environ) { char invoker[B_FILE_NAME_LENGTH]; char **newArgs = NULL; @@ -213,13 +214,13 @@ load_image(int32 argCount, const char **args, const char **environ) // test validity of executable + support for scripts { - status_t status = __test_executable(args[0], invoker); + status_t status = __test_executable(path, invoker); if (status < B_OK) return status; if (invoker[0]) { status = __parse_invoke_line(invoker, &newArgs, - (char * const **)&args, &argCount, args[0]); + (char * const **)&args, &argCount, path); if (status < B_OK) return status; } @@ -232,7 +233,7 @@ load_image(int32 argCount, const char **args, const char **environ) char** flatArgs = NULL; size_t flatArgsSize; status_t status = __flatten_process_args(args, argCount, environ, - &envCount, args[0], &flatArgs, &flatArgsSize); + &envCount, path, &flatArgs, &flatArgsSize); if (status == B_OK) { thread = _kern_load_image(flatArgs, flatArgsSize, argCount, envCount, @@ -247,6 +248,13 @@ load_image(int32 argCount, const char **args, const char **environ) } +thread_id +load_image(int32 argCount, const char **args, const char **environ) +{ + return __load_image_at_path(args[0], argCount, args, environ); +} + + image_id load_add_on(char const *name) { diff --git a/src/system/libroot/posix/spawn.cpp b/src/system/libroot/posix/spawn.cpp index bb5ddfaefc..091720bef0 100644 --- a/src/system/libroot/posix/spawn.cpp +++ b/src/system/libroot/posix/spawn.cpp @@ -1,6 +1,6 @@ /* - * Copyright 2017-2019, Jérôme Duval, jerome.Duval@gmail.com - * Distributed under the terms of the MIT license. + * Copyright 2017-2019, Jérôme Duval, jerome.duval@gmail.com + * Distributed under the terms of the MIT License. */ @@ -13,6 +13,7 @@ #include #include +#include #include #include @@ -495,7 +496,7 @@ process_file_actions(const posix_spawn_file_actions_t *_actions, int *errfd) static int -do_posix_spawn(pid_t *_pid, const char *path, +spawn_using_fork(pid_t *_pid, const char *path, const posix_spawn_file_actions_t *actions, const posix_spawnattr_t *attrp, char *const argv[], char *const envp[], bool envpath) @@ -555,6 +556,52 @@ fail: } +static int +spawn_using_load_image(pid_t *_pid, const char *_path, + char *const argv[], char *const envp[], bool envpath) +{ + const char* path; + // if envpath is specified but the path contains '/', don't search PATH + if (!envpath || strchr(_path, '/') != NULL) { + path = _path; + } else { + char* buffer = (char*)alloca(B_PATH_NAME_LENGTH); + status_t status = __look_up_in_path(_path, buffer); + if (status != B_OK) + return status; + path = buffer; + } + + // count arguments + int32 argCount = 0; + while (argv[argCount] != NULL) + argCount++; + + thread_id thread = __load_image_at_path(path, argCount, (const char**)argv, + (const char**)(envp != NULL ? envp : environ)); + if (thread < 0) + return thread; + + *_pid = thread; + return resume_thread(thread); +} + + +static int +do_posix_spawn(pid_t *_pid, const char *path, + const posix_spawn_file_actions_t *actions, + const posix_spawnattr_t *attrp, char *const argv[], char *const envp[], + bool envpath) +{ + if (actions == NULL && attrp == NULL) { + return spawn_using_load_image(_pid, path, argv, envp, envpath); + } else { + return spawn_using_fork(_pid, path, actions, attrp, argv, envp, + envpath); + } +} + + int posix_spawn(pid_t *pid, const char *path, const posix_spawn_file_actions_t *file_actions,