Let's start with some background about oe_delta_size() and
oe_set_delta_size(). If you already know, skip the next paragraph.
These two are added in 0aca34e826 (pack-objects: shrink delta_size
field in struct object_entry - 2018-04-14) to help reduce 'struct
object_entry' size. The delta size field in this struct is reduced to
only contain max 1MB. So if any new delta is produced and larger than
1MB, it's dropped because we can't really save such a large size
anywhere. Fallback is provided in case existing packfiles already have
large deltas, then we can retrieve it from the pack.
While this should help small machines repacking large repos without
large deltas (i.e. less memory pressure), dropping large deltas during
the delta selection process could end up with worse pack files. And if
existing packfiles already have >1MB delta and pack-objects is
instructed to not reuse deltas, all of them will be dropped on the
floor, and the resulting pack would be definitely bigger.
There is also a regression in terms of CPU/IO if we have large on-disk
deltas because fallback code needs to parse the pack every time the
delta size is needed and just access to the mmap'd pack data is enough
for extra page faults when memory is under pressure.
Both of these issues were reported on the mailing list. Here's some
numbers for comparison.
Version Pack (MB) MaxRSS(kB) Time (s)
------- --------- ---------- --------
2.17.0 5498 43513628 2494.85
2.18.0 10531 40449596 4168.94
This patch provides a better fallback that is
- cheaper in terms of cpu and io because we won't have to read
existing pack files as much
- better in terms of pack size because the pack heuristics is back to
2.17.0 time, we do not drop large deltas at all
If we encounter any delta (on-disk or created during try_delta phase)
that is larger than the 1MB limit, we stop using delta_size_ field for
this because it can't contain such size anyway. A new array of delta
size is dynamically allocated and can hold all the deltas that 2.17.0
can. This array only contains delta sizes that delta_size_ can't
contain.
With this, we do not have to drop deltas in try_delta() anymore. Of
course the downside is we use slightly more memory, even compared to
2.17.0. But since this is considered an uncommon case, a bit more
memory consumption should not be a problem.
Delta size limit is also raised from 1MB to 16MB to better cover
common case and avoid that extra memory consumption (99.999% deltas in
this reported repo are under 12MB; Jeff noted binary artifacts topped
out at about 3MB in some other private repos). Other fields are
shuffled around to keep this struct packed tight. We don't use more
memory in common case even with this limit update.
A note about thread synchronization. Since this code can be run in
parallel during delta searching phase, we need a mutex. The realloc
part in packlist_alloc() is not protected because it only happens
during the object counting phase, which is always single-threaded.
Access to e->delta_size_ (and by extension
pack->delta_size[e - pack->objects]) is unprotected as before, the
thread scheduler in pack-objects must make sure "e" is never updated
by two different threads.
The area under the new lock is as small as possible, avoiding locking
at all in common case, since lock contention with high thread count
could be expensive (most blobs are small enough that delta compute
time is short and we end up taking the lock very often). The previous
attempt to always hold a lock in oe_delta_size() and
oe_set_delta_size() increases execution time by 33% when repacking
linux.git with with 40 threads.
Reported-by: Elijah Newren <newren@gmail.com>
Helped-by: Elijah Newren <newren@gmail.com>
Helped-by: Jeff King <peff@peff.net>
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
It's very very rare that an uncompressed object is larger than 4GB
(partly because Git does not handle those large files very well to
begin with). Let's optimize it for the common case where object size
is smaller than this limit.
Shrink size field down to 31 bits and one overflow bit. If the size is
too large, we read it back from disk. As noted in the previous patch,
we need to return the delta size instead of canonical size when the
to-be-reused object entry type is a delta instead of a canonical one.
Add two compare helpers that can take advantage of the overflow
bit (e.g. if the file is 4GB+, chances are it's already larger than
core.bigFileThreshold and there's no point in comparing the actual
value).
Another note about oe_get_size_slow(). This function MUST be thread
safe because SIZE() macro is used inside try_delta() which may run in
parallel. Outside parallel code, no-contention locking should be dirt
cheap (or insignificant compared to i/o access anyway). To exercise
this code, it's best to run the test suite with something like
make test GIT_TEST_OE_SIZE=4
which forces this code on all objects larger than 3 bytes.
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Instead of using 8 bytes (on 64 bit arch) to store a pointer to a
pack. Use an index instead since the number of packs should be
relatively small.
This limits the number of packs we can handle to 1k. Since we can't be
sure people can never run into the situation where they have more than
1k pack files. Provide a fall back route for it.
If we find out they have too many packs, the new in_pack_by_idx[]
array (which has at most 1k elements) will not be used. Instead we
allocate in_pack[] array that holds nr_objects elements. This is
similar to how the optional in_pack_pos field is handled.
The new simple test is just to make sure the too-many-packs code path
is at least executed. The true test is running
make test GIT_TEST_FULL_IN_PACK_ARRAY=1
to take advantage of other special case tests.
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
While at there, document about this special mode when running the test
suite.
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
* 'svn/authors-prog-2' of git://bogomips.org/git-svn:
git-svn: allow empty email-address using authors-prog and authors-file
git-svn: search --authors-prog in PATH too
This reverts commit e4bb62fa1e, reversing
changes made to 468165c1d8.
The topic appears to inflict severe regression in renaming merges,
even though the promise of it was that it would improve them.
We do not yet know which exact change in the topic was wrong, but in
the meantime, let's play it safe and revert it out of 'master'
before real Git-using projects are harmed.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
"git submodule status" misbehaved on a submodule that has been
removed from the working tree.
* rs/status-with-removed-submodule:
submodule: check for NULL return of get_submodule_ref_store()
Small test-helper programs have been consolidated into a single
binary.
* nd/combined-test-helper: (36 commits)
t/helper: merge test-write-cache into test-tool
t/helper: merge test-wildmatch into test-tool
t/helper: merge test-urlmatch-normalization into test-tool
t/helper: merge test-subprocess into test-tool
t/helper: merge test-submodule-config into test-tool
t/helper: merge test-string-list into test-tool
t/helper: merge test-strcmp-offset into test-tool
t/helper: merge test-sigchain into test-tool
t/helper: merge test-sha1-array into test-tool
t/helper: merge test-scrap-cache-tree into test-tool
t/helper: merge test-run-command into test-tool
t/helper: merge test-revision-walking into test-tool
t/helper: merge test-regex into test-tool
t/helper: merge test-ref-store into test-tool
t/helper: merge test-read-cache into test-tool
t/helper: merge test-prio-queue into test-tool
t/helper: merge test-path-utils into test-tool
t/helper: merge test-online-cpus into test-tool
t/helper: merge test-mktemp into test-tool
t/helper: merge (unused) test-mergesort into test-tool
...
Refactoring the internal global data structure to make it possible
to open multiple repositories, work with and then close them.
Rerolled by Duy on top of a separate preliminary clean-up topic.
The resulting structure of the topics looked very sensible.
* sb/object-store: (27 commits)
sha1_file: allow sha1_loose_object_info to handle arbitrary repositories
sha1_file: allow map_sha1_file to handle arbitrary repositories
sha1_file: allow map_sha1_file_1 to handle arbitrary repositories
sha1_file: allow open_sha1_file to handle arbitrary repositories
sha1_file: allow stat_sha1_file to handle arbitrary repositories
sha1_file: allow sha1_file_name to handle arbitrary repositories
sha1_file: add repository argument to sha1_loose_object_info
sha1_file: add repository argument to map_sha1_file
sha1_file: add repository argument to map_sha1_file_1
sha1_file: add repository argument to open_sha1_file
sha1_file: add repository argument to stat_sha1_file
sha1_file: add repository argument to sha1_file_name
sha1_file: allow prepare_alt_odb to handle arbitrary repositories
sha1_file: allow link_alt_odb_entries to handle arbitrary repositories
sha1_file: add repository argument to prepare_alt_odb
sha1_file: add repository argument to link_alt_odb_entries
sha1_file: add repository argument to read_info_alternates
sha1_file: add repository argument to link_alt_odb_entry
sha1_file: add raw_object_store argument to alt_odb_usable
pack: move approximate object count to object store
...
when refs that do not point at committish are given, "git
filter-branch" gave a misleading error messages. This has been
corrected.
* yk/filter-branch-non-committish-refs:
filter-branch: fix errors caused by refs that point at non-committish
The mechanism to use parse-options API to automate the command line
completion continues to get extended and polished.
* nd/parseopt-completion-more:
completion: use __gitcomp_builtin in _git_cherry
completion: use __gitcomp_builtin in _git_ls_tree
completion: delete option-only completion commands
completion: add --option completion for most builtin commands
completion: factor out _git_xxx calling code
completion: mention the oldest version we need to support
git.c: add hidden option --list-parseopt-builtins
git.c: move cmd_struct declaration up
"git tag --contains no-such-commit" gave a full list of options
after giving an error message.
* ps/contains-id-error-message:
parse-options: do not show usage upon invalid option value
"git stash push -u -- <pathspec>" gave an unnecessary and confusing
error message when there was no tracked files that match the
<pathspec>, which has been fixed.
* tg/stash-untracked-with-pathspec-fix:
stash: drop superfluos pathspec parameter
stash push -u: don't create empty stash
stash push: avoid printing errors
stash: fix nonsense pipeline
The way "git worktree prune" worked internally has been simplified,
by assuming how "git worktree move" moves an existing worktree to a
different place.
* nd/worktree-prune:
worktree prune: improve prune logic when worktree is moved
worktree: delete dead code
gc.txt: more details about what gc does
"git shortlog cruft" aborted with a BUG message when run outside a
Git repository. The command has been taught to complain about
extra and unwanted arguments on its command line instead in such a
case.
* ma/shortlog-revparse:
shortlog: disallow left-over arguments outside repo
shortlog: add usage-string for stdin-reading
git-shortlog.txt: reorder usages
Rename detection logic in "diff" family that is used in "merge" has
learned to guess when all of x/a, x/b and x/c have moved to z/a,
z/b and z/c, it is likely that x/d added in the meantime would also
want to move to z/d by taking the hint that the entire directory
'x' moved to 'z'. A bug causing dirty files involved in a rename
to be overwritten during merge has also been fixed as part of this
work.
* en/rename-directory-detection: (29 commits)
merge-recursive: ensure we write updates for directory-renamed file
merge-recursive: avoid spurious rename/rename conflict from dir renames
directory rename detection: new testcases showcasing a pair of bugs
merge-recursive: fix remaining directory rename + dirty overwrite cases
merge-recursive: fix overwriting dirty files involved in renames
merge-recursive: avoid clobbering untracked files with directory renames
merge-recursive: apply necessary modifications for directory renames
merge-recursive: when comparing files, don't include trees
merge-recursive: check for file level conflicts then get new name
merge-recursive: add computation of collisions due to dir rename & merging
merge-recursive: check for directory level conflicts
merge-recursive: add get_directory_renames()
merge-recursive: make a helper function for cleanup for handle_renames
merge-recursive: split out code for determining diff_filepairs
merge-recursive: make !o->detect_rename codepath more obvious
merge-recursive: fix leaks of allocated renames and diff_filepairs
merge-recursive: introduce new functions to handle rename logic
merge-recursive: move the get_renames() function
directory rename detection: tests for handling overwriting dirty files
directory rename detection: tests for handling overwriting untracked files
...
The email address in --authors-file and --authors-prog can be empty but
git-svn translated it into a fictional email address in the form
jondoe <jondoe@6aafaa21e0fb4338a68ab372a049893d>
containing the SVN repository UUID. Now git-svn behaves like git-commit:
If the email is *explicitly* set to the empty string using '<>', the
commit does not contain an email address, only the name:
jondoe <>
Allowing to remove the email address *intentionally* prevents automatic
systems from sending emails to those fictional addresses and avoids
cluttering the log output with unnecessary stuff.
Signed-off-by: Andreas Heiduk <asheiduk@gmail.com>
Signed-off-by: Eric Wong <e@80x24.org>
If we can't find a ref store for a submodule then assume the latter
is not initialized (or was removed). Print a status line accordingly
instead of causing a segmentation fault by passing NULL as the first
parameter of refs_head_ref().
Reported-by: Jeremy Feusi <jeremy@feusi.co>
Reviewed-by: Stefan Beller <sbeller@google.com>
Initial-Test-By: Stefan Beller <sbeller@google.com>
Helped-by: Eric Sunshine <sunshine@sunshineco.com>
Signed-off-by: Rene Scharfe <l.s.r@web.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Hotfix for recently graduated topic that give help to completion
scripts from the Git subcommands that are being completed
* nd/parseopt-completion:
t9902: disable test on the list of merge-strategies under GETTEXT_POISON
completion: clear cached --options when sourcing the completion script
Avoid using pipes downstream of Git commands since the exit codes
of commands upstream of pipes get swallowed, thus potentially
hiding failure of those commands. Instead, capture Git command
output to a file and apply the downstream command(s) to that file.
Signed-off-by: Pratik Karki <predatoramigo@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Commit 11395a3b4b (test_must_be_empty: make sure the file exists, not
just empty, 2018-02-27) basically duplicated the 'test_path_is_file'
helper function in 'test_must_be_empty'.
Just call 'test_path_is_file' to avoid this code duplication.
Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
One of the most interesting thing one can be interested in when
looking at performance test results is possible performance
regressions.
This new option makes it easy to spot such possible regressions.
This new option is named '--sort-by=regression' to make it
possible and easy to add other ways to sort the results, like for
example '--sort-by=utime'.
If we would like to sort according to how much the stime regressed
we could also add a new option called '--sort-by=regression:stime'.
Then '--sort-by=regression' could become a synonym for
'--sort-by=regression:rtime'.
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
This new helper function will be reused in a subsequent
commit.
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
