| Age | Commit message (Collapse) | Author | Files | Lines |
|---|
|
The type_cas lock lost all of its callers in f08cbf60fe (index-pack:
make quantum of work smaller, 2020-09-08), so we can safely delete it.
The compiler didn't alert us that the variable became unused, because we
still call pthread_mutex_init() and pthread_mutex_destroy() on it.
It's worth considering also whether that commit was in error to remove
the use of the lock. Why don't we need it now, if we did before, as
described in ab791dd138 (index-pack: fix race condition with duplicate
bases, 2014-08-29)? I think the answer is that we now look at and assign
the child_obj->real_type field in the main thread while holding the
work_lock(). So we don't have to worry about racing with the worker
threads.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
Commit f08cbf60fe (index-pack: make quantum of work smaller, 2020-09-08)
refactored the main loop in threaded_second_pass(), but also deleted the
call to display_progress() at the top of the loop. This means that users
typically see no progress at all during the delta resolution phase (and
for large repositories, Git appears to hang).
This looks like an accident that was unrelated to the intended change of
that commit, since we continue to update nr_resolved_deltas in
resolve_delta(). Let's restore the call to get that progress back.
We'll also add a test that confirms we generate the expected progress.
This isn't perfect, as it wouldn't catch a bug where progress was
delayed to the end. That was probably possible to trigger when receiving
a thin pack, because we'd eventually call display_progress() from
fix_unresolved_deltas(), but only once after doing all the work.
However, since our test case generates a complete pack, it reliably
demonstrates this particular bug and its fix. And we can't do better
without making the test racy.
Signed-off-by: Jeff King <peff@peff.net>
Acked-by: Jonathan Tan <jonathantanmy@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
Currently, when index-pack resolves deltas, it does not split up delta
trees into threads: each delta base root (an object that is not a
REF_DELTA or OFS_DELTA) can go into its own thread, but all deltas on
that root (direct or indirect) are processed in the same thread.
This is a problem when a repository contains a large text file (thus,
delta-able) that is modified many times - delta resolution time during
fetching is dominated by processing the deltas corresponding to that
text file.
This patch contains a solution to that. When cloning using
git -c core.deltabasecachelimit=1g clone \
https://fuchsia.googlesource.com/third_party/vulkan-cts
on my laptop, clone time improved from 3m2s to 2m5s (using 3 threads,
which is the default).
The solution is to have a global work stack. This stack contains delta
bases (objects, whether appearing directly in the packfile or generated
by delta resolution, that themselves have delta children) that need to
be processed; whenever a thread needs work, it peeks at the top of the
stack and processes its next unprocessed child. If a thread finds the
stack empty, it will look for more delta base roots to push on the stack
instead.
The main weakness of having a global work stack is that more time is
spent in the mutex, but profiling has shown that most time is spent in
the resolution of the deltas themselves, so this shouldn't be an issue
in practice. In any case, experimentation (as described in the clone
command above) shows that this patch is a net improvement.
Signed-off-by: Jonathan Tan <jonathantanmy@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
A subsequent commit will make the quantum of work smaller, necessitating
more locking. This commit allows resolve_delta() to be called outside
the lock.
Signed-off-by: Jonathan Tan <jonathantanmy@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
This is refactoring 2 of 2 to simplify struct base_data.
Whenever we make a struct base_data, immediately calculate its delta
children. This eliminates confusion as to when the
{ref,ofs}_{first,last} fields are initialized.
Before this patch, the delta children were calculated at the last
possible moment. This allowed the members of struct base_data to be
populated in any order, superficially useful when we have the object
contents before the struct object_entry. But this makes reasoning about
the state of struct base_data more complicated, hence this patch.
Signed-off-by: Jonathan Tan <jonathantanmy@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
This is refactoring 1 of 2 to simplify struct base_data.
In index-pack, each thread maintains a doubly-linked list of the delta
chain that it is currently processing (the "base" and "child" pointers
in struct base_data). When a thread exceeds the delta base cache limit
and needs to reclaim memory, it uses the "child" pointers to traverse
the lineage, reclaiming the memory of the eldest delta bases first.
A subsequent patch will perform memory reclaiming in a different way and
will thus no longer need the "child" pointer. Because the "child"
pointer is redundant even now, remove it so that the aforementioned
subsequent patch will be clearer. In the meantime, reclaim memory in the
reverse order of the "base" pointers.
Signed-off-by: Jonathan Tan <jonathantanmy@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
Signed-off-by: Jonathan Tan <jonathantanmy@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
find_{ref,ofs}_delta_{,children} take an enum object_type parameter, but
the object type is already present in the name of the function. Remove
that parameter from these functions.
Signed-off-by: Jonathan Tan <jonathantanmy@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
Clarify that core.deltaBaseCacheLimit is per-thread, as can be seen from
the fact that cache usage (base_cache_used in struct thread_local in
builtin/index-pack.c) is tracked individually for each thread and
compared against delta_base_cache_limit.
Signed-off-by: Jonathan Tan <jonathantanmy@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
"unlink" emulation on MinGW has been optimized.
* jh/mingw-unlink:
mingw: improve performance of mingw_unlink()
|
|
midx and commit-graph files now use the byte defined in their file
format specification for identifying the hash function used for
object names.
* ds/sha256-leftover-bits:
multi-pack-index: use hash version byte
commit-graph: use the "hash version" byte
t/README: document GIT_TEST_DEFAULT_HASH
|
|
Further update of docs to adjust to the recent SHA-256 work.
* ma/sha-256-docs:
shallow.txt: document SHA-256 shallow format
protocol-capabilities.txt: clarify "allow-x-sha1-in-want" re SHA-256
index-format.txt: document SHA-256 index format
http-protocol.txt: document SHA-256 "want"/"have" format
|
|
A few end-user facing messages have been updated to be
hash-algorithm agnostic.
* jc/object-names-are-not-sha-1:
messages: avoid SHA-1 in end-user facing messages
|
|
Further update of docs to adjust to the recent SHA-256 work.
* bc/sha-256-doc-updates:
docs: fix step in transition plan
docs: document SHA-256 pack and indices
|
|
Doc fix.
* pb/set-url-docfix:
fetch, pull doc: correct description of '--set-upstream'
|
|
The regexp to identify the function boundary for FORTRAN programs
has been updated.
* pb/userdiff-fortran-update:
userdiff: improve Fortran xfuncname regex
userdiff: add tests for Fortran xfuncname regex
|
|
Docfix.
* jb/commit-graph-doc-fix:
docs: commit-graph: fix some whitespace in the diagram
|
|
When given more than one target line ranges, "git blame -La,b
-Lc,d" was over-eager to coalesce groups of original lines and
showed incorrect results, which has been corrected.
* jk/blame-coalesce-fix:
blame: only coalesce lines that are adjacent in result
t8003: factor setup out of coalesce test
t8003: check output of coalesced blame
|
|
Ring buffer with size 4 used for bin-hex translation resulted in a
wrong object name in the sequencer's todo output, which has been
corrected.
* ak/sequencer-fix-find-uniq-abbrev:
rebase -i: fix possibly wrong onto hash in todo
|
|
The commit labels used to explain each side of conflicted hunks
placed by the sequencer machinery have been made more readable by
humans.
* en/sequencer-merge-labels:
sequencer: avoid garbled merge machinery messages due to commit labels
|
|
Code clean-up.
* rs/preserve-merges-unused-code-removal:
rebase: remove unused function reschedule_last_action
|
|
Code clean-up.
* rs/upload-pack-sigchain-fix:
upload-pack: remove superfluous sigchain_pop() call
|
|
"git diff [<tree-ish>] $path" for a $path that is marked with i-t-a
bit was not showing the mode bits from the working tree.
* rp/ita-diff-modefix:
diff-lib: use worktree mode in diffs from i-t-a entries
|
|
Updates to "git merge" tests, in preparation for a new merge
strategy backend.
* en/merge-tests:
t6425: be more flexible with rename/delete conflict messages
t642[23]: be more flexible for add/add conflicts involving pair renames
t6422, t6426: be more flexible for add/add conflicts involving renames
t6423: add an explanation about why one of the tests does not pass
t6416, t6423: clarify some comments and fix some typos
t6422: fix multiple errors with the mod6 test expectations
t6423: fix test setup for a couple tests
t6416, t6422: fix incorrect untracked file count
t6422: fix bad check against missing file
t6418: tighten delete/normalize conflict testcase
Collect merge-related tests to t64xx
|
|
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
Earlier, to countermand the implicit "-m" option when the
"--first-parent" option is used with "git log", we added the
"--[no-]diff-merges" option in the jk/log-fp-implies-m topic. To
leave the door open to allow the "--diff-merges" option to take
values that instructs how patches for merge commits should be
computed (e.g. "cc"? "-p against first parent?"), redefine
"--diff-merges" to take non-optional value, and implement "off"
that means the same thing as "--no-diff-merges".
* so/log-diff-merges-opt:
t/t4013: add test for --diff-merges=off
doc/git-log: describe --diff-merges=off
revision: change "--diff-merges" option to require parameter
|
|
"git log --first-parent -p" showed patches only for single-parent
commits on the first-parent chain; the "--first-parent" option has
been made to imply "-m". Use "--no-diff-merges" to restore the
previous behaviour to omit patches for merge commits.
* jk/log-fp-implies-m:
doc/git-log: clarify handling of merge commit diffs
doc/git-log: move "-t" into diff-options list
doc/git-log: drop "-r" diff option
doc/git-log: move "Diff Formatting" from rev-list-options
log: enable "-m" automatically with "--first-parent"
revision: add "--no-diff-merges" option to counteract "-m"
log: drop "--cc implies -m" logic
|
|
NULL dereference fix.
* ma/stop-progress-null-fix:
progress: don't dereference before checking for NULL
|
|
Test framework update.
* es/test-cmp-typocatcher:
test_cmp: diagnose incorrect arguments
|
|
Recent versions of "git diff-files" shows a diff between the index
and the working tree for "intent-to-add" paths as a "new file"
patch; "git apply --cached" should be able to take "git diff-files"
and should act as an equivalent to "git add" for the path, but the
command failed to do so for such a path.
* rp/apply-cached-with-i-t-a:
t4140: test apply with i-t-a paths
apply: make i-t-a entries never match worktree
apply: allow "new file" patches on i-t-a entries
|
|
"git bisect" learns the "--first-parent" option to find the first
breakage along the first-parent chain.
* al/bisect-first-parent:
bisect: combine args passed to find_bisection()
bisect: introduce first-parent flag
cmd_bisect__helper: defer parsing no-checkout flag
rev-list: allow bisect and first-parent flags
t6030: modernize "git bisect run" tests
|
|
Mark error message for i18n.
* jk/sideband-error-l10n:
sideband: mark "remote error:" prefix for translation
|
|
A no-op replacement function implemented as a C preprocessor macro
does not perform as good a job as one implemented as a "static
inline" function in catching errors in parameters; replace the
former with the latter in <git-compat-util.h> header.
* jc/noop-with-static-inline:
compat-util: type-check parameters of no-op replacement functions
|
|
The existing backends for "git mergetool" based on variants of vim
have been refactored and then support for "nvim" has been added.
* pd/mergetool-nvimdiff:
mergetools: add support for nvimdiff (neovim) family
mergetool--lib: improve support for vimdiff-style tool variants
|
|
Further preliminary change to refs API.
* hn/reftable-prep-part-2:
Make HEAD a PSEUDOREF rather than PER_WORKTREE.
Modify pseudo refs through ref backend storage
t1400: use git rev-parse for testing PSEUDOREF existence
|
|
Stop when "sendmail.*" configuration variables are defined, which
could be a mistaken attempt to define "sendemail.*" variables.
* dd/send-email-config:
git-send-email: die if sendmail.* config is set
|
|
The logic to find the ref transaction hook script attempted to
cache the path to the found hook without realizing that it needed
to keep a copied value, as the API it used returned a transitory
buffer space. This has been corrected.
* ps/ref-transaction-hook:
t1416: avoid hard-coded sha1 ids
refs: fix interleaving hook calls with reference-transaction hook
|
|
Similar to the commit-graph format, the multi-pack-index format has a
byte in the header intended to track the hash version used to write the
file. This allows one to interpret the hash length without having the
context of the repository config specifying the hash length. This was
not modified as part of the SHA-256 work because the hash length was
automatically up-shifted due to that config.
Since we have this byte available, we can make the file formats more
obviously incompatible instead of relying on other context from the
repository.
Add a new oid_version() method in midx.c similar to the one in
commit-graph.c. This is specifically made separate from that
implementation to avoid artificially linking the formats.
The test impact requires a few more things than the corresponding change
in the commit-graph format. Specifically, 'test-tool read-midx' was not
writing anything about this header value to output. Since the value
available in 'struct multi_pack_index' is hash_len instead of a version
value, we output "20" or "32" instead of "1" or "2".
Since we want a user to not have their Git commands fail if their
multi-pack-index has the incorrect hash version compared to the
repository's hash version, we relax the die() to an error() in
load_multi_pack_index(). This has some effect on 'git multi-pack-index
verify' as we need to check that a failed parse of a file that exists is
actually a verify error. For that test that checks the hash version
matches, we change the corrupted byte from "2" to "3" to ensure the test
fails for both hash algorithms.
Helped-by: brian m. carlson <sandals@crustytoothpaste.net>
Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
Reviewed-by: brian m. carlson <sandals@crustytoothpaste.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
The commit-graph format reserved a byte among the header of the file to
store a "hash version". During the SHA-256 work, this was not modified
because file formats are not necessarily intended to work across hash
versions. If a repository has SHA-256 as its hash algorithm, it
automatically up-shifts the lengths of object names in all necessary
formats.
However, since we have this byte available for adjusting the version, we
can make the file formats more obviously incompatible instead of relying
on other context from the repository.
Update the oid_version() method in commit-graph.c to add a new value, 2,
for sha-256. This automatically writes the new value in a SHA-256
repository _and_ verifies the value is correct. This is a breaking
change relative to the current 'master' branch since 092b677 (Merge
branch 'bc/sha-256-cvs-svn-updates', 2020-08-13) but it is not breaking
relative to any released version of Git.
The test impact is relatively minor: the output of 'test-tool
read-graph' lists the header information, so those instances of '1' need
to be replaced with a variable determined by GIT_TEST_DEFAULT_HASH. A
more careful test is added that specifically creates a repository of
each type then swaps the commit-graph files. The important value here is
that the "git log" command succeeds while writing a message to stderr.
Helped-by: brian m. carlson <sandals@crustytoothpaste.net>
Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
Reviewed-by: brian m. carlson <sandals@crustytoothpaste.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
Helped-by: Eric Sunshine <sunshine@sunshineco.com>
Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
Reviewed-by: brian m. carlson <sandals@crustytoothpaste.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
Update mingw_unlink() to first try to delete the file with existing
permissions before trying to force it.
Windows throws an error when trying to delete a read-only file. The
mingw_unlink() compatibility wrapper always tries to _wchmod(666) the
file before calling _wunlink() to avoid that error. However, since
most files in the worktree are already writable, this is usually
wasted effort.
Update mingw_unlink() to just call DeleteFileW() directly and if that
succeeds return. If that fails, fall back into the existing code path
to update the permissions and use _wunlink() to get the existing
error code mapping.
Signed-off-by: Jeff Hostetler <jeffhost@microsoft.com>
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
Similar to recent commits, document that we list object names rather
than SHA-1s.
Signed-off-by: Martin Ågren <martin.agren@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
Two of our capabilities contain "sha1" in their names, but that's
historical. Clarify that object names are still to be given using
whatever object format has been negotiated using the "object-format"
capability.
Signed-off-by: Martin Ågren <martin.agren@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
Document that in SHA-1 repositories, we use SHA-1 and in SHA-256
repositories, we use SHA-256, then replace all other uses of "SHA-1"
with something more neutral. Avoid referring to "160-bit" hash values.
Signed-off-by: Martin Ågren <martin.agren@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
Document that rather than always naming objects using SHA-1, we should
use whatever has been negotiated using the object-format capability.
Signed-off-by: Martin Ågren <martin.agren@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
sequencer's get_message() exists to provide good labels on conflict
hunks; see commits
d68565402a ("revert: clarify label on conflict hunks", 2010-03-20)
bf975d379d ("cherry-pick, revert: add a label for ancestor", 2010-03-20)
043a4492b3 ("sequencer: factor code out of revert builtin", 2012-01-11).
for background on this function. These labels are of the form
<commitID>... <commit summary>
or
parent of <commitID>... <commit summary>
These labels are then passed as branch names to the merge machinery.
However, these labels, as formatted, often also serve to confuse. For
example, if we have a rename involved in a content merge, then it
results in text such as the following:
<<<<<<<< HEAD:foo.c
int j;
========
int counter;
>>>>>>>> b01dface... Removed unnecessary stuff:bar.c
Or in various conflict messages, it can make it very difficult to read:
CONFLICT (rename/delete): foo.c deleted in b01dface... Removed
unnecessary stuff and renamed in HEAD. Version HEAD of foo.c left
in tree.
CONFLICT (file location): dir1/foo.c added in b01dface... Removed
unnecessary stuff inside a directory that was renamed in HEAD,
suggesting it should perhaps be moved to dir2/foo.c.
Make a minor change to remove the ellipses and add parentheses around
the commit summary; this makes all three examples much easier to read:
<<<<<<<< HEAD:foo.c
int j;
========
int counter;
>>>>>>>> b01dface (Removed unnecessary stuff):bar.c
CONFLICT (rename/delete): foo.c deleted in b01dface (Removed
unnecessary stuff) and renamed in HEAD. Version HEAD of foo.c left
in tree.
CONFLICT (file location): dir1/foo.c added in b01dface (Removed
unnecessary stuff) inside a directory that was renamed in HEAD,
suggesting it should perhaps be moved to dir2/foo.c.
Signed-off-by: Elijah Newren <newren@gmail.com>
Reviewed-by: Taylor Blau <me@ttaylorr.com>
Acked-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
There are still a handful mentions of SHA-1 when we meant the
(hexadecimal) object names in end-user facing messages. Rewrite
them.
I was hoping that this can mostly be s/SHA-1/object name/, but
a few messages needed rephrasing to keep the result readable.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
One of the required steps for the objectFormat extension is to implement
the loose object index. However, without support for
compatObjectFormat, we don't even know if the loose object index is
needed, so it makes sense to move that step to the compatObjectFormat
section. Do so.
Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
Now that we have SHA-256 support for packs and indices, let's document
that in SHA-256 repositories, we use SHA-256 instead of SHA-1 for object
names and checksums. Instead of duplicating this information throughout
the document, let's just document that in SHA-1 repositories, we use
SHA-1 for these purposes, and in SHA-256 repositories, we use SHA-256.
Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
Jeff King
Jonathan Tan
Junio C Hamano
Derrick Stolee
Jeff Hostetler
Martin Ågren
Elijah Newren
brian m. carlson