From: Bruno Haible Date: Sat, 10 Sep 2022 00:31:20 +0000 (+0200) Subject: posix_spawn-internal: Optimize DuplicateHandle calls on native Windows. X-Git-Tag: v1.0~2048 X-Git-Url: https://gitweb.git.savannah.gnu.org/gitweb/?a=commitdiff_plain;h=71b603702b8cf7977dedd5f6b71ea0ffc1669894;p=gnulib.git posix_spawn-internal: Optimize DuplicateHandle calls on native Windows. * lib/windows-spawn.h (DELAYED_DUP2_OLDFD, DELAYED_DUP2_NEWFD): New macros. (struct IHANDLE): Add a linked_fd field. * lib/spawni.c (SPAWN_INTERNAL_OPTIMIZE_DUPLICATEHANDLE): New macro. (do_delayed_dup2, do_remaining_delayed_dup2): New functions. (close_inheritable_handles): Don't close handles in DELAYED_DUP2_NEWFD entries. (do_close): Add a third parameter. Optimize delayed dup2 calls. (do_open): Use do_close. (do_dup2): Likewise. Prepare for optimizing the DuplicateHandle call. (__spawni): Do the remaining delayed dup2 invocations after the loop over the actions. --- diff --git a/ChangeLog b/ChangeLog index 0a11fb895c..47b05390f6 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,5 +1,19 @@ 2022-09-09 Bruno Haible + posix_spawn-internal: Optimize DuplicateHandle calls on native Windows. + * lib/windows-spawn.h (DELAYED_DUP2_OLDFD, DELAYED_DUP2_NEWFD): New + macros. + (struct IHANDLE): Add a linked_fd field. + * lib/spawni.c (SPAWN_INTERNAL_OPTIMIZE_DUPLICATEHANDLE): New macro. + (do_delayed_dup2, do_remaining_delayed_dup2): New functions. + (close_inheritable_handles): Don't close handles in DELAYED_DUP2_NEWFD + entries. + (do_close): Add a third parameter. Optimize delayed dup2 calls. + (do_open): Use do_close. + (do_dup2): Likewise. Prepare for optimizing the DuplicateHandle call. + (__spawni): Do the remaining delayed dup2 invocations after the loop + over the actions. + posix_spawn-internal: Refactor. * lib/windows-spawn.h (struct IHANDLE): New type. (struct inheritable_handles): Combine handles and flags into a single diff --git a/lib/spawni.c b/lib/spawni.c index ca99c5bff1..aa72125289 100644 --- a/lib/spawni.c +++ b/lib/spawni.c @@ -89,6 +89,12 @@ #if defined _WIN32 && ! defined __CYGWIN__ /* Native Windows API. */ +/* Define to 1 to enable DuplicateHandle optimization. + Define to 0 to disable this optimization. */ +# ifndef SPAWN_INTERNAL_OPTIMIZE_DUPLICATEHANDLE +# define SPAWN_INTERNAL_OPTIMIZE_DUPLICATEHANDLE 1 +# endif + /* Get declarations of the native Windows API functions. */ # define WIN32_LEAN_AND_MEAN # include @@ -141,6 +147,57 @@ grow_inheritable_handles (struct inheritable_handles *inh_handles, int newfd) return 0; } +# if SPAWN_INTERNAL_OPTIMIZE_DUPLICATEHANDLE + +/* Assuming inh_handles->ih[newfd].handle != INVALID_HANDLE_VALUE + and (inh_handles->ih[newfd].flags & DELAYED_DUP2_NEWFD) != 0, + actually performs the delayed dup2 (oldfd, newfd). + Returns 0 upon success. In case of failure, -1 is returned, with errno set. + */ +static int +do_delayed_dup2 (int newfd, struct inheritable_handles *inh_handles, + HANDLE curr_process) +{ + int oldfd = inh_handles->ih[newfd].linked_fd; + /* Check invariants. */ + if (!((inh_handles->ih[oldfd].flags & DELAYED_DUP2_OLDFD) != 0 + && newfd == inh_handles->ih[oldfd].linked_fd + && inh_handles->ih[newfd].handle == inh_handles->ih[oldfd].handle)) + abort (); + /* Duplicate the handle now. */ + if (!DuplicateHandle (curr_process, inh_handles->ih[oldfd].handle, + curr_process, &inh_handles->ih[newfd].handle, + 0, TRUE, DUPLICATE_SAME_ACCESS)) + { + errno = EBADF; /* arbitrary */ + return -1; + } + inh_handles->ih[oldfd].flags &= ~DELAYED_DUP2_OLDFD; + inh_handles->ih[newfd].flags = + (unsigned char) inh_handles->ih[oldfd].flags | KEEP_OPEN_IN_CHILD; + return 0; +} + +/* Performs the remaining delayed dup2 (oldfd, newfd). + Returns 0 upon success. In case of failure, -1 is returned, with errno set. + */ +static int +do_remaining_delayed_dup2 (struct inheritable_handles *inh_handles, + HANDLE curr_process) +{ + size_t handles_count = inh_handles->count; + int newfd; + + for (newfd = 0; newfd < handles_count; newfd++) + if (inh_handles->ih[newfd].handle != INVALID_HANDLE_VALUE + && (inh_handles->ih[newfd].flags & DELAYED_DUP2_NEWFD) != 0) + if (do_delayed_dup2 (newfd, inh_handles, curr_process) < 0) + return -1; + return 0; +} + +# endif + /* Closes the handles in inh_handles that are not meant to be preserved in the child process, and reduces inh_handles->count to the minimum needed. */ static void @@ -183,6 +240,7 @@ close_inheritable_handles (struct inheritable_handles *inh_handles) HANDLE handle = ih[fd].handle; if (handle != INVALID_HANDLE_VALUE + && !(ih[fd].flags & DELAYED_DUP2_NEWFD) && !(ih[fd].flags & KEEP_OPEN_IN_PARENT)) CloseHandle (handle); } @@ -207,6 +265,64 @@ sigisempty (const sigset_t *s) return memiszero (s, sizeof (sigset_t)); } +/* Executes a 'close' action. + Returns 0 upon success. In case of failure, -1 is returned, with errno set. + */ +static int +do_close (struct inheritable_handles *inh_handles, int fd, bool ignore_EBADF) +{ + if (!(fd >= 0 && fd < inh_handles->count + && inh_handles->ih[fd].handle != INVALID_HANDLE_VALUE)) + { + if (ignore_EBADF) + return 0; + else + { + errno = EBADF; + return -1; + } + } + +# if SPAWN_INTERNAL_OPTIMIZE_DUPLICATEHANDLE + if ((inh_handles->ih[fd].flags & DELAYED_DUP2_NEWFD) != 0) + { + int dup2_oldfd = inh_handles->ih[fd].linked_fd; + /* Check invariants. */ + if (!((inh_handles->ih[dup2_oldfd].flags & DELAYED_DUP2_OLDFD) != 0 + && fd == inh_handles->ih[dup2_oldfd].linked_fd + && inh_handles->ih[fd].handle == inh_handles->ih[dup2_oldfd].handle)) + abort (); + /* Annihilate a delayed dup2 (..., fd) call. */ + inh_handles->ih[dup2_oldfd].flags &= ~DELAYED_DUP2_OLDFD; + } + else if ((inh_handles->ih[fd].flags & DELAYED_DUP2_OLDFD) != 0) + { + int dup2_newfd = inh_handles->ih[fd].linked_fd; + /* Check invariants. */ + if (!((inh_handles->ih[dup2_newfd].flags & DELAYED_DUP2_NEWFD) != 0 + && fd == inh_handles->ih[dup2_newfd].linked_fd + && inh_handles->ih[fd].handle == inh_handles->ih[dup2_newfd].handle)) + abort (); + /* Optimize a delayed dup2 (fd, ...) call. */ + inh_handles->ih[dup2_newfd].flags = + (inh_handles->ih[fd].flags & ~DELAYED_DUP2_OLDFD) | KEEP_OPEN_IN_CHILD; + } + else +# endif + { + if (!(inh_handles->ih[fd].flags & KEEP_OPEN_IN_PARENT) + && !CloseHandle (inh_handles->ih[fd].handle)) + { + inh_handles->ih[fd].handle = INVALID_HANDLE_VALUE; + errno = EIO; + return -1; + } + } + inh_handles->ih[fd].handle = INVALID_HANDLE_VALUE; + + return 0; +} + /* Opens an inheritable HANDLE to a file. Upon failure, returns INVALID_HANDLE_VALUE with errno set. */ static HANDLE @@ -385,13 +501,8 @@ do_open (struct inheritable_handles *inh_handles, int newfd, } if (grow_inheritable_handles (inh_handles, newfd) < 0) return -1; - if (inh_handles->ih[newfd].handle != INVALID_HANDLE_VALUE - && !(inh_handles->ih[newfd].flags & KEEP_OPEN_IN_PARENT) - && !CloseHandle (inh_handles->ih[newfd].handle)) - { - errno = EIO; - return -1; - } + if (do_close (inh_handles, newfd, true) < 0) + return -1; if (filename == NULL) { errno = EINVAL; @@ -444,15 +555,45 @@ do_dup2 (struct inheritable_handles *inh_handles, int oldfd, int newfd, { if (grow_inheritable_handles (inh_handles, newfd) < 0) return -1; - if (inh_handles->ih[newfd].handle != INVALID_HANDLE_VALUE - && !(inh_handles->ih[newfd].flags & KEEP_OPEN_IN_PARENT) - && !CloseHandle (inh_handles->ih[newfd].handle)) - { - errno = EIO; + if (do_close (inh_handles, newfd, true) < 0) + return -1; + /* We may need to duplicate the handle, so that a forthcoming do_close + action on oldfd has no effect on newfd. */ +# if SPAWN_INTERNAL_OPTIMIZE_DUPLICATEHANDLE + /* But try to not do it now; delay it if possible. In many cases, the + DuplicateHandle call can be optimized away. */ + if ((inh_handles->ih[oldfd].flags & DELAYED_DUP2_NEWFD) != 0) + if (do_delayed_dup2 (oldfd, inh_handles, curr_process) < 0) return -1; + if ((inh_handles->ih[oldfd].flags & DELAYED_DUP2_OLDFD) != 0) + { + /* Check invariants. */ + int dup2_newfd = inh_handles->ih[oldfd].linked_fd; + if (!((inh_handles->ih[dup2_newfd].flags & DELAYED_DUP2_NEWFD) != 0 + && oldfd == inh_handles->ih[dup2_newfd].linked_fd + && inh_handles->ih[oldfd].handle == inh_handles->ih[dup2_newfd].handle)) + abort (); + /* We can't delay two or more dup2 calls from the same oldfd. */ + if (!DuplicateHandle (curr_process, inh_handles->ih[oldfd].handle, + curr_process, &inh_handles->ih[newfd].handle, + 0, TRUE, DUPLICATE_SAME_ACCESS)) + { + errno = EBADF; /* arbitrary */ + return -1; + } + inh_handles->ih[newfd].flags = + (unsigned char) inh_handles->ih[oldfd].flags | KEEP_OPEN_IN_CHILD; } - /* Duplicate the handle, so that a forthcoming do_close action on oldfd - has no effect on newfd. */ + else + { + /* Delay the dup2 (oldfd, newfd) action. */ + inh_handles->ih[oldfd].flags |= DELAYED_DUP2_OLDFD; + inh_handles->ih[oldfd].linked_fd = newfd; + inh_handles->ih[newfd].handle = inh_handles->ih[oldfd].handle; + inh_handles->ih[newfd].flags = DELAYED_DUP2_NEWFD; + inh_handles->ih[newfd].linked_fd = oldfd; + } +# else if (!DuplicateHandle (curr_process, inh_handles->ih[oldfd].handle, curr_process, &inh_handles->ih[newfd].handle, 0, TRUE, DUPLICATE_SAME_ACCESS)) @@ -462,32 +603,11 @@ do_dup2 (struct inheritable_handles *inh_handles, int oldfd, int newfd, } inh_handles->ih[newfd].flags = (unsigned char) inh_handles->ih[oldfd].flags | KEEP_OPEN_IN_CHILD; +# endif } return 0; } -/* Executes a 'close' action. - Returns 0 upon success. In case of failure, -1 is returned, with errno set. - */ -static int -do_close (struct inheritable_handles *inh_handles, int fd) -{ - if (!(fd >= 0 && fd < inh_handles->count - && inh_handles->ih[fd].handle != INVALID_HANDLE_VALUE)) - { - errno = EBADF; - return -1; - } - if (!(inh_handles->ih[fd].flags & KEEP_OPEN_IN_PARENT) - && !CloseHandle (inh_handles->ih[fd].handle)) - { - errno = EIO; - return -1; - } - inh_handles->ih[fd].handle = INVALID_HANDLE_VALUE; - return 0; -} - int __spawni (pid_t *pid, const char *prog_filename, const posix_spawn_file_actions_t *file_actions, @@ -583,7 +703,7 @@ __spawni (pid_t *pid, const char *prog_filename, case spawn_do_close: { int fd = action->action.close_action.fd; - if (do_close (&inh_handles, fd) < 0) + if (do_close (&inh_handles, fd, false) < 0) goto failed_2; } break; @@ -632,6 +752,12 @@ __spawni (pid_t *pid, const char *prog_filename, goto failed_2; } } + +# if SPAWN_INTERNAL_OPTIMIZE_DUPLICATEHANDLE + /* Do the remaining delayed dup2 invocations. */ + if (do_remaining_delayed_dup2 (&inh_handles, curr_process) < 0) + goto failed_2; +# endif } /* Close the handles in inh_handles that are not meant to be preserved in the diff --git a/lib/windows-spawn.h b/lib/windows-spawn.h index 5cc770b59d..1e42d80d21 100644 --- a/lib/windows-spawn.h +++ b/lib/windows-spawn.h @@ -97,10 +97,23 @@ struct IHANDLE child process. - KEEP_OPEN_IN_PARENT if the handle is shared with (and needs to be kept open in) the parent process. - */ + - DELAYED_DUP2_OLDFD if there is a delayed dup2 (oldfd, newfd) and + this IHANDLE is at position oldfd. + - DELAYED_DUP2_NEWFD if there is a delayed dup2 (oldfd, newfd) and + this IHANDLE is at position newfd. + Note that DELAYED_DUP2_OLDFD and DELAYED_DUP2_NEWFD cannot be set in the + same IHANDLE. */ unsigned short flags; #define KEEP_OPEN_IN_CHILD 0x100 #define KEEP_OPEN_IN_PARENT 0x200 + #define DELAYED_DUP2_OLDFD 0x400 + #define DELAYED_DUP2_NEWFD 0x800 + /* Only relevant if handle != INVALID_HANDLE_VALUE and flags contains + either DELAYED_DUP2_OLDFD or DELAYED_DUP2_NEWFD. + It is the other fd of the delayed dup2 (oldfd, newfd), i.e. + - for DELAYED_DUP2_OLDFD, the newfd, + - for DELAYED_DUP2_NEWFD, the oldfd. */ + int linked_fd; }; /* This struct keeps track of which handles to potentially pass to a subprocess,