diff --git a/Documentation/config/checkout.adoc b/Documentation/config/checkout.adoc index e35d21296978fe..45951bf38a5e3c 100644 --- a/Documentation/config/checkout.adoc +++ b/Documentation/config/checkout.adoc @@ -30,6 +30,11 @@ commands or functionality in the future. all commands that perform checkout. E.g. checkout, clone, reset, sparse-checkout, etc. + +On Windows the number of workers is capped at 62, because the `poll()` +emulation cannot wait on more worker pipes than that. A higher configured +value, including the logical core count on a machine with many cores, is +silently reduced to the cap. ++ NOTE: Parallel checkout usually delivers better performance for repositories located on SSDs or over NFS. For repositories on spinning disks and/or machines with a small number of cores, the default sequential checkout often performs diff --git a/compat/poll/poll.c b/compat/poll/poll.c index ea362b4a8e2340..ab895fc91ca309 100644 --- a/compat/poll/poll.c +++ b/compat/poll/poll.c @@ -303,6 +303,40 @@ compute_revents (int fd, int sought, fd_set *rfds, fd_set *wfds, fd_set *efds) } #endif /* !MinGW */ +#ifdef WIN32_NATIVE +/* POLL_MAX_DESCRIPTORS descriptors, plus hEvent and the QS_ALLINPUT message + queue, must fit in one MsgWaitForMultipleObjects call, and the collected + handles plus the NULL sentinel must fit in handle_array. */ +#if POLL_MAX_DESCRIPTORS + 2 > MAXIMUM_WAIT_OBJECTS +#error POLL_MAX_DESCRIPTORS exceeds MAXIMUM_WAIT_OBJECTS +#endif +#if POLL_MAX_DESCRIPTORS + 2 > FD_SETSIZE + 2 +#error POLL_MAX_DESCRIPTORS does not fit in handle_array +#endif + +/* Undo the WSAEventSelect() calls made for the first NFD descriptors. */ +static void +reset_socket_events (struct pollfd *pfd, nfds_t nfd) +{ + nfds_t i; + + for (i = 0; i < nfd; i++) + { + HANDLE h; + + if (pfd[i].fd < 0) + continue; + + h = (HANDLE) _get_osfhandle (pfd[i].fd); + if (h == NULL || h == INVALID_HANDLE_VALUE) + continue; + + if (IsSocketHandle (h)) + WSAEventSelect ((SOCKET) h, NULL, 0); + } +} +#endif + int poll (struct pollfd *pfd, nfds_t nfd, int timeout) { @@ -504,7 +538,16 @@ poll (struct pollfd *pfd, nfds_t nfd, int timeout) bits for the "wrong" direction. */ pfd[i].revents = win32_compute_revents (h, &sought); if (sought) - handle_array[nhandles++] = h; + { + /* hEvent occupies handle_array[0]. See POLL_MAX_DESCRIPTORS. */ + if (nhandles > POLL_MAX_DESCRIPTORS) + { + reset_socket_events (pfd, i); + errno = EINVAL; + return -1; + } + handle_array[nhandles++] = h; + } if (pfd[i].revents) timeout = 0; } diff --git a/compat/poll/poll.h b/compat/poll/poll.h index 1e1597360f4485..d7977806c18e96 100644 --- a/compat/poll/poll.h +++ b/compat/poll/poll.h @@ -59,6 +59,20 @@ typedef unsigned long nfds_t; extern int poll (struct pollfd *pfd, nfds_t nfd, int timeout); +/* + * This poll() is emulated with MsgWaitForMultipleObjects(), which waits on at + * most MAXIMUM_WAIT_OBJECTS (64) objects. Two of those are never available for + * polled descriptors: poll() waits on its own event object, and QS_ALLINPUT + * adds the thread message queue. Sockets do not count, because they are all + * multiplexed onto that one event object; every other descriptor takes a wait + * slot of its own. + * + * Callers that poll one or more descriptors per child must keep the number of + * simultaneously live descriptors within this limit. Exceeding it fails with + * EINVAL. + */ +#define POLL_MAX_DESCRIPTORS 62 + /* Define INFTIM only if doing so conforms to POSIX. */ #if !defined (_POSIX_C_SOURCE) && !defined (_XOPEN_SOURCE) #define INFTIM (-1) diff --git a/compat/posix.h b/compat/posix.h index e2e794cad7d419..1a77b198aa5bc7 100644 --- a/compat/posix.h +++ b/compat/posix.h @@ -133,6 +133,16 @@ /* Pull the compat stuff */ #include #endif + +/* + * compat/poll defines POLL_MAX_DESCRIPTORS to the largest number of + * descriptors its poll() emulation can wait on. A native poll() has no such + * limit, so callers that fan out one descriptor per child can clamp against + * this unconditionally. + */ +#ifndef POLL_MAX_DESCRIPTORS +#define POLL_MAX_DESCRIPTORS INT_MAX +#endif #ifdef HAVE_BSD_SYSCTL #include #endif diff --git a/parallel-checkout.c b/parallel-checkout.c index 1eb277a0fc0a55..4595cf4e8d9250 100644 --- a/parallel-checkout.c +++ b/parallel-checkout.c @@ -671,6 +671,13 @@ int run_parallel_checkout(struct checkout *state, int num_workers, int threshold if (parallel_checkout.nr < num_workers) num_workers = parallel_checkout.nr; + /* + * gather_results_from_workers() polls one pipe per worker, so the + * worker count must stay within what poll() can wait on. + */ + if (num_workers > POLL_MAX_DESCRIPTORS) + num_workers = POLL_MAX_DESCRIPTORS; + if (num_workers <= 1 || parallel_checkout.nr < threshold) { write_items_sequentially(state); } else { diff --git a/run-command.c b/run-command.c index ceb33119655de9..e8fcaaa7194026 100644 --- a/run-command.c +++ b/run-command.c @@ -1894,6 +1894,7 @@ void run_processes_parallel(const struct run_process_parallel_opts *opts) int i, code; int timeout = 100; int spawn_cap = 4; + size_t max_live; struct parallel_processes_for_signal pp_sig; struct parallel_processes pp = { .buffered_output = STRBUF_INIT, @@ -1903,6 +1904,18 @@ void run_processes_parallel(const struct run_process_parallel_opts *opts) const char *tr2_label = opts->tr2_label; const int do_trace2 = tr2_category && tr2_label; + /* + * Unless the caller handles its own output, pp_buffer_io() polls one + * pipe for each child that is sending output and a second one for each + * child that is being fed on stdin. Limit how many children run at once + * so that the worst case stays within what poll() can wait on. Only + * concurrency is limited; the configured maximum is still honoured for + * the number of tasks that are run in total. + */ + max_live = opts->processes; + if (!opts->ungroup && max_live > POLL_MAX_DESCRIPTORS / 2) + max_live = POLL_MAX_DESCRIPTORS / 2; + if (do_trace2) trace2_region_enter_printf(tr2_category, tr2_label, NULL, "max:%"PRIuMAX, @@ -1924,7 +1937,7 @@ void run_processes_parallel(const struct run_process_parallel_opts *opts) while (1) { for (i = 0; i < spawn_cap && !pp.shutdown && - pp.nr_processes < opts->processes; + pp.nr_processes < max_live; i++) { code = pp_start_one(&pp, opts); if (!code) diff --git a/t/t2080-parallel-checkout-basics.sh b/t/t2080-parallel-checkout-basics.sh index 7ad96cd5cd24a3..94d1f4bf1e7718 100755 --- a/t/t2080-parallel-checkout-basics.sh +++ b/t/t2080-parallel-checkout-basics.sh @@ -319,5 +319,41 @@ test_expect_success MINGW 'parallel checkout with fscache does not fail on new d test_cmp expect2 sub/deep/dir/file2.txt ) ' +# Windows has no native poll(). compat/poll emulates it with +# MsgWaitForMultipleObjects(), which cannot wait on more than +# MAXIMUM_WAIT_OBJECTS objects, so run_parallel_checkout() caps the worker +# count at MAXIMUM_WAIT_OBJECTS - 2. Without that cap, compat/poll collected +# one wait handle per polled worker pipe in a fixed-size stack array and +# smashed the stack. +# +# MAXIMUM_WAIT_OBJECTS is 64, hence the expected 62 below. The test is +# MINGW-only because the cap only exists there; on other platforms the +# requested 200 workers are used as-is. +test_expect_success MINGW 'checkout caps workers at the poll limit' ' + test_when_finished "rm -rf many-workers" && + git init many-workers && + ( + cd many-workers && + mkdir dir && + for i in $(test_seq 1 200) + do + echo "content $i" >dir/file$i || return 1 + done && + git add -A && + git commit -q -m base && + + git checkout -q -b other && + for i in $(test_seq 1 200) + do + echo "changed $i" >dir/file$i || return 1 + done && + git commit -q -a -m changed && + git checkout -q - + ) && + + set_checkout_config 200 1 && + test_checkout_workers 62 git -C many-workers checkout other && + verify_checkout many-workers +' test_done