[pull] master from git:master - #246
Merged
Merged
Conversation
Add a "quiet" parameter to bisect_reset() that passes "--quiet" to the checkout restoring the original HEAD, suppressing its progress and branch-status output. No caller sets the flag yet, so behavior is unchanged. Signed-off-by: Harald Nordgren <haraldnordgren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
When a bisection finishes, "git bisect" reports the first bad commit but leaves the session active until "git bisect reset" is run by hand. Add a "--reset-when-found[=<where>]" option, accepted by both "git bisect start" and "git bisect run", that resets as soon as the first bad commit is found. The "original" value returns to the commit checked out before "git bisect start", while "found" leaves the first bad commit checked out; omitting the value defaults to "original". Persist the selected target in a BISECT_RESET_WHEN_FOUND state file and perform the reset quietly. Let the internal first-bad result propagate to cmd_bisect(), which performs the reset using the existing bad bisect ref after the subcommand has returned. For "git bisect run", this means BISECT_RUN has been printed and closed before cleanup, which also works on systems that cannot unlink an open file. Reject this option together with "--no-checkout", since that mode must not check out either target. Signed-off-by: Harald Nordgren <haraldnordgren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
I have to scroll down at least three screens in man(1) from the `migrate` description in order to see the “known limitations” for it. This is important information since the text says that concurrent writes can lead to an inconsistent migrated state. Let’s move that text up to the command description and put it inside a Warning admonition. This section made sense when it was added in 25a0023 (builtin/refs: new command to migrate ref storage formats, 2024-06-06); `migrate` was the only subcommand, and this section was visible from the command description. A one-page man page. But that is not the case anymore now that the command has nine subcommands to describe. Acked-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Kristoffer Haugsbakk <code@khaugsbakk.name> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Acked-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Kristoffer Haugsbakk <code@khaugsbakk.name> Signed-off-by: Junio C Hamano <gitster@pobox.com>
In a subsequent commit we're going to add the first caller to writev(3p). Introduce a compatibility wrapper for this syscall that we can use on systems that don't have this syscall. The syscall exists on modern Unixes like Linux and macOS, and seemingly even for NonStop according to [1]. It doesn't seem to exist on Windows though. [1]: http://nonstoptools.com/manuals/OSS-SystemCalls.pdf [2]: https://www.gnu.org/software/gnulib/manual/html_node/writev.html Helped-by: Johannes Schindelin <Johannes.Schindelin@gmx.de> Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
In the preceding commit we have added a compatibility wrapper for the writev(3p) syscall. Introduce some generic wrappers for this function that we nowadays take for granted in the Git codebase. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Some systems like NonStop set a comparatively small `MAX_IO_SIZE`, which
limits the maximum number of bytes we're allowed to write in a single
call. We already handle this limit properly in `xwrite()`, but we have
recently introduced wrappers for writev(3p) where we don't. This will
cause the syscall to return EINVAL in case somebody passes an iovec
entry to writev(3p) that is larger than `MAX_IO_SIZE`.
Introduce a new function `xwritev()` that is similar to `xwrite()` in
that it handles such platform-specific nuances:
- We only pass the leading iovec entries to writev(3p) that fit into
`MAX_IO_SIZE`, pretending that the underlying syscall performed a
short write. This mirrors how `xwrite()` chomps overly large
requests before handing them to write(3p). As a consequence, callers
will never see writev(3p)'s EINVAL error for requests whose summed
length would overflow an ssize_t, but observe a short write instead.
- If already the first iovec entry exceeds the limit we instead punt
to `xwrite()`, which knows to handle this case for us.
- We restart the underlying syscall on EINTR and EAGAIN, just like
`xwrite()` does for write(3p).
Adapt `writev_in_full()` to use this new wrapper. With the retry logic
now living in `xwritev()`, the calling loop becomes the exact mirror
image of `write_in_full()`, which also retains the responsibility of
translating a zero-length write into ENOSPC.
Reported-by: Randall Becker <randall.becker@nexbridge.ca>
Helped-by: Jeff King <peff@peff.net>
Helped-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Every pktline that we send out via `send_sideband()` currently requires two syscalls: one to write the pktline's length, and one to send its data. This typically isn't all that much of a problem, but under extreme load the syscalls may cause contention in the kernel. Refactor the code to instead use the newly introduced writev(3p) infra so that we can send out the data with a single syscall. This reduces the number of syscalls from around 133,000 calls to write(3p) to around 67,000 calls to writev(3p). This change leads to a performance improvement for git-upload-pack(1), but we have to cheat a bit to really make it measurable. Usually, the time is strongly dominated by generating the packfile itself. But if we precompute the pack and serve it via the pack-objects hook then we can essentially eliminate that overhead. The following setup is executed in the Git repository: $ cat >request <<-EOF 0048want 5ce91c0 side-band no-progress 00000009done EOF $ echo 5ce91c0 | git pack-objects --revs --stdout >pack $ cat >hook <<-EOF #!/bin/sh cat >/dev/null cat "$(pwd)"/pack EOF $ chmod u+x hook $ git -c uploadpack.packObjectsHook="$(pwd)"/hook upload-pack . <request Benchmarking the last command leads to the following results: Benchmark 1: HEAD~ Time (mean ± σ): 192.9 ms ± 0.6 ms [User: 106.5 ms, System: 95.3 ms] Range (min … max): 191.7 ms … 194.1 ms 50 runs Benchmark 2: HEAD Time (mean ± σ): 141.1 ms ± 0.7 ms [User: 63.2 ms, System: 86.6 ms] Range (min … max): 139.8 ms … 142.7 ms 50 runs Summary HEAD ran 1.37 ± 0.01 times faster than HEAD~ This might not be impressive in absolute numbers when you also take into account the time it takes to generate the packfile itself. But GitLab (and supposedly other forges) have caching mechanisms in place that work exactly like the above setup, where repeated incoming requests can be served from the same cached packfile. And in those cases, the impact is sizeable. More importantly though, as hinted at above, GitLab has observed in the past that with enough cache hits we eventually start to saturate a semaphore in the Linux kernel itself in the pipe write path. This bottleneck is being moved a bit by having to do less syscalls. Suggested-by: Jeff King <peff@peff.net> Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
When answering a `cat-blob` command, `cat_blob()` issues three separate
calls to write(3p) on the cat-blob fd: one for the header line, one for
the full blob payload, and one for the trailing newline. Frontends like
git-filter-repo issue these commands in bulk, once per rewritten blob,
so the syscall overhead adds up.
Use `writev_in_full()` to send all three parts with a single syscall.
This can be benchmarked with the following setup:
$ git cat-file --unordered --filter=object:type=blob
--batch-check='cat-blob %(objectname)' --batch-all-objects >request
$ git fast-import --cat-blob-fd=3 <request
Executing this with 100,000 objects in linux.git:
Benchmark 1: HEAD~
Time (mean ± σ): 1.320 s ± 0.003 s [User: 1.154 s, System: 0.161 s]
Range (min … max): 1.314 s … 1.324 s 10 runs
Benchmark 2: HEAD
Time (mean ± σ): 1.270 s ± 0.022 s [User: 1.133 s, System: 0.132 s]
Range (min … max): 1.209 s … 1.282 s 10 runs
Summary
HEAD ran
1.04 ± 0.02 times faster than HEAD~
Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The known limitations of the ref format migration in 'git refs' have been moved to be displayed as a warning admonition directly under the description of the 'migrate' subcommand, improving visibility. A reference to 'git-maintenance' has also been corrected to use the 'linkgit' macro. * kh/doc-refs-migrate-limitations: doc: refs: linkgit to git-maintenance(1) doc: refs: put ref migration warning under the command
A compatibility wrapper for writev(3p) has been reintroduced, including fixes for CMake build and 'MAX_IO_SIZE' limits on NonStop. Calls to write(3p) in send_sideband() and cat_blob() have been refactored to use writev(3p) wrappers to reduce syscall overhead. * ps/writev: fast-import: use writev(3p) to send cat-blob responses sideband: use writev(3p) to send pktlines wrapper: properly handle MAX_IO_SIZE in writev(3p) wrapper: introduce writev(3p) wrappers compat/posix: introduce writev(3p) wrapper
The 'git bisect' command has been taught a '--reset-when-found[=<where>]' option that tells the command to automatically run 'git bisect reset' to jump back to the original state or to the found culprit. * hn/bisect-reset-when-found: bisect: add --reset-when-found to leave when done bisect: let bisect_reset() optionally check out quietly
Signed-off-by: Junio C Hamano <gitster@pobox.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
See Commits and Changes for more details.
Created by
pull[bot] (v2.0.0-alpha.4)
Can you help keep this open source service alive? 💖 Please sponsor : )