* [PATCH v3 03/14] i18n: apply: mark info messages for translation
From: Vasco Almeida @ 2016-09-15 14:58 UTC (permalink / raw)
To: git
Cc: Vasco Almeida, Jiang Xin, Ævar Arnfjörð Bjarmason,
Jean-Noël AVILA, Junio C Hamano
In-Reply-To: <1473259758-11836-1-git-send-email-vascomalmeida@sapo.pt>
Mark messages for translation printed to stderr.
Signed-off-by: Vasco Almeida <vascomalmeida@sapo.pt>
---
builtin/apply.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/builtin/apply.c b/builtin/apply.c
index ef2c084..43ab7c5 100644
--- a/builtin/apply.c
+++ b/builtin/apply.c
@@ -3525,7 +3525,7 @@ static int try_threeway(struct apply_state *state,
read_blob_object(&buf, pre_sha1, patch->old_mode))
return error(_("repository lacks the necessary blob to fall back on 3-way merge."));
- fprintf(stderr, "Falling back to three-way merge...\n");
+ fprintf(stderr, _("Falling back to three-way merge...\n"));
img = strbuf_detach(&buf, &len);
prepare_image(&tmp_image, img, len, 1);
@@ -3555,7 +3555,7 @@ static int try_threeway(struct apply_state *state,
status = three_way_merge(image, patch->new_name,
pre_sha1, our_sha1, post_sha1);
if (status < 0) {
- fprintf(stderr, "Failed to fall back on three-way merge...\n");
+ fprintf(stderr, _("Failed to fall back on three-way merge...\n"));
return status;
}
@@ -3567,9 +3567,9 @@ static int try_threeway(struct apply_state *state,
hashcpy(patch->threeway_stage[0].hash, pre_sha1);
hashcpy(patch->threeway_stage[1].hash, our_sha1);
hashcpy(patch->threeway_stage[2].hash, post_sha1);
- fprintf(stderr, "Applied patch to '%s' with conflicts.\n", patch->new_name);
+ fprintf(stderr, _("Applied patch to '%s' with conflicts.\n"), patch->new_name);
} else {
- fprintf(stderr, "Applied patch to '%s' cleanly.\n", patch->new_name);
+ fprintf(stderr, _("Applied patch to '%s' cleanly.\n"), patch->new_name);
}
return 0;
}
--
2.7.4
^ permalink raw reply related
* [PATCH v3 02/14] i18n: apply: mark error messages for translation
From: Vasco Almeida @ 2016-09-15 14:58 UTC (permalink / raw)
To: git
Cc: Vasco Almeida, Jiang Xin, Ævar Arnfjörð Bjarmason,
Jean-Noël AVILA, Junio C Hamano
In-Reply-To: <1473259758-11836-1-git-send-email-vascomalmeida@sapo.pt>
Mark error messages for translation passed to error() and die()
functions.
Signed-off-by: Vasco Almeida <vascomalmeida@sapo.pt>
---
builtin/apply.c | 46 +++++++++++++++++++++++-----------------------
1 file changed, 23 insertions(+), 23 deletions(-)
diff --git a/builtin/apply.c b/builtin/apply.c
index ef03c74..ef2c084 100644
--- a/builtin/apply.c
+++ b/builtin/apply.c
@@ -3065,8 +3065,8 @@ static int apply_binary_fragment(struct apply_state *state,
/* Binary patch is irreversible without the optional second hunk */
if (state->apply_in_reverse) {
if (!fragment->next)
- return error("cannot reverse-apply a binary patch "
- "without the reverse hunk to '%s'",
+ return error(_("cannot reverse-apply a binary patch "
+ "without the reverse hunk to '%s'"),
patch->new_name
? patch->new_name : patch->old_name);
fragment = fragment->next;
@@ -3111,8 +3111,8 @@ static int apply_binary(struct apply_state *state,
strlen(patch->new_sha1_prefix) != 40 ||
get_sha1_hex(patch->old_sha1_prefix, sha1) ||
get_sha1_hex(patch->new_sha1_prefix, sha1))
- return error("cannot apply binary patch to '%s' "
- "without full index line", name);
+ return error(_("cannot apply binary patch to '%s' "
+ "without full index line"), name);
if (patch->old_name) {
/*
@@ -3121,16 +3121,16 @@ static int apply_binary(struct apply_state *state,
*/
hash_sha1_file(img->buf, img->len, blob_type, sha1);
if (strcmp(sha1_to_hex(sha1), patch->old_sha1_prefix))
- return error("the patch applies to '%s' (%s), "
- "which does not match the "
- "current contents.",
+ return error(_("the patch applies to '%s' (%s), "
+ "which does not match the "
+ "current contents."),
name, sha1_to_hex(sha1));
}
else {
/* Otherwise, the old one must be empty. */
if (img->len)
- return error("the patch applies to an empty "
- "'%s' but it is not empty", name);
+ return error(_("the patch applies to an empty "
+ "'%s' but it is not empty"), name);
}
get_sha1_hex(patch->new_sha1_prefix, sha1);
@@ -3147,8 +3147,8 @@ static int apply_binary(struct apply_state *state,
result = read_sha1_file(sha1, &type, &size);
if (!result)
- return error("the necessary postimage %s for "
- "'%s' cannot be read",
+ return error(_("the necessary postimage %s for "
+ "'%s' cannot be read"),
patch->new_sha1_prefix, name);
clear_image(img);
img->buf = result;
@@ -3523,7 +3523,7 @@ static int try_threeway(struct apply_state *state,
write_sha1_file("", 0, blob_type, pre_sha1);
else if (get_sha1(patch->old_sha1_prefix, pre_sha1) ||
read_blob_object(&buf, pre_sha1, patch->old_mode))
- return error("repository lacks the necessary blob to fall back on 3-way merge.");
+ return error(_("repository lacks the necessary blob to fall back on 3-way merge."));
fprintf(stderr, "Falling back to three-way merge...\n");
@@ -3541,11 +3541,11 @@ static int try_threeway(struct apply_state *state,
/* our_sha1[] is ours */
if (patch->is_new) {
if (load_current(state, &tmp_image, patch))
- return error("cannot read the current contents of '%s'",
+ return error(_("cannot read the current contents of '%s'"),
patch->new_name);
} else {
if (load_preimage(state, &tmp_image, patch, st, ce))
- return error("cannot read the current contents of '%s'",
+ return error(_("cannot read the current contents of '%s'"),
patch->old_name);
}
write_sha1_file(tmp_image.buf, tmp_image.len, blob_type, our_sha1);
@@ -4020,29 +4020,29 @@ static void build_fake_ancestor(struct patch *list, const char *filename)
if (!preimage_sha1_in_gitlink_patch(patch, sha1))
; /* ok, the textual part looks sane */
else
- die("sha1 information is lacking or useless for submodule %s",
+ die(_("sha1 information is lacking or useless for submodule %s"),
name);
} else if (!get_sha1_blob(patch->old_sha1_prefix, sha1)) {
; /* ok */
} else if (!patch->lines_added && !patch->lines_deleted) {
/* mode-only change: update the current */
if (get_current_sha1(patch->old_name, sha1))
- die("mode change for %s, which is not "
- "in current HEAD", name);
+ die(_("mode change for %s, which is not "
+ "in current HEAD"), name);
} else
- die("sha1 information is lacking or useless "
- "(%s).", name);
+ die(_("sha1 information is lacking or useless "
+ "(%s)."), name);
ce = make_cache_entry(patch->old_mode, sha1, name, 0, 0);
if (!ce)
die(_("make_cache_entry failed for path '%s'"), name);
if (add_index_entry(&result, ce, ADD_CACHE_OK_TO_ADD))
- die ("Could not add %s to temporary index", name);
+ die(_("Could not add %s to temporary index"), name);
}
hold_lock_file_for_update(&lock, filename, LOCK_DIE_ON_ERROR);
if (write_locked_index(&result, &lock, COMMIT_LOCK))
- die ("Could not write temporary index to %s", filename);
+ die(_("Could not write temporary index to %s"), filename);
discard_index(&result);
}
@@ -4693,9 +4693,9 @@ static void check_apply_state(struct apply_state *state, int force_apply)
int is_not_gitdir = !startup_info->have_repository;
if (state->apply_with_reject && state->threeway)
- die("--reject and --3way cannot be used together.");
+ die(_("--reject and --3way cannot be used together."));
if (state->cached && state->threeway)
- die("--cached and --3way cannot be used together.");
+ die(_("--cached and --3way cannot be used together."));
if (state->threeway) {
if (is_not_gitdir)
die(_("--3way outside a repository"));
--
2.7.4
^ permalink raw reply related
* [PATCH v3 01/14] i18n: apply: mark plural string for translation
From: Vasco Almeida @ 2016-09-15 14:58 UTC (permalink / raw)
To: git
Cc: Vasco Almeida, Jiang Xin, Ævar Arnfjörð Bjarmason,
Jean-Noël AVILA, Junio C Hamano
In-Reply-To: <1473259758-11836-1-git-send-email-vascomalmeida@sapo.pt>
Mark plural string for translation using Q_().
Signed-off-by: Vasco Almeida <vascomalmeida@sapo.pt>
---
builtin/apply.c | 10 ++++++----
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/builtin/apply.c b/builtin/apply.c
index 1a488f9..ef03c74 100644
--- a/builtin/apply.c
+++ b/builtin/apply.c
@@ -4768,10 +4768,12 @@ static int apply_all_patches(struct apply_state *state,
state->whitespace_error),
state->whitespace_error);
if (state->applied_after_fixing_ws && state->apply)
- warning("%d line%s applied after"
- " fixing whitespace errors.",
- state->applied_after_fixing_ws,
- state->applied_after_fixing_ws == 1 ? "" : "s");
+ warning(Q_("%d line applied after"
+ " fixing whitespace errors.",
+ "%d lines applied after"
+ " fixing whitespace errors.",
+ state->applied_after_fixing_ws),
+ state->applied_after_fixing_ws);
else if (state->whitespace_error)
warning(Q_("%d line adds whitespace errors.",
"%d lines add whitespace errors.",
--
2.7.4
^ permalink raw reply related
* [wishlist?] make submodule commands robust to having non-submodule Subprojects
From: Yaroslav Halchenko @ 2016-09-15 13:02 UTC (permalink / raw)
To: git
NB echos some questions of mine a few days back on IRC about Subprojects
and submodules
If e.g. you just 'git add' a subdirectory which is a git repository, git
adds it as a subproject but doesn't initiate any entry in .gitmodules
since it is the job done by submodule and git core itself is
agnostic of those beasts.
But having then this "Subproject"s which aren't registered as submodules
(and I haven't found any other use for them besides being a submodule)
brakes "git submodule" commands, e.g.
$> git submodule
cc6a09ac06c13cf06b4f4c8b54cda9a535e4e385 ds000001 (2.0.0+4)
0a9f3b66e06a2137311a537b7377c336f1fb30ad ds000002 (1.0.0-3-g0a9f3b6)
9da7e4f4221699915645ac2003298c6aba2db109 ds000003 (1.1.0+4)
fe16cacb5cb9b4d53c50e498298fab182500e147 ds000005 (2.0.0+3)
6898d99ff3ba26880183ed3672a458a7fcde1737 ds000006 (2.0.0+2)
bbd10f634fe87e9d5853df3a891edbdb18cda7f9 ds000007 (2.0.0+3)
138e6730193c0585a69b8baf5b9d7a4439e83ecc ds000008 (2.0.0+2)
ddf3a4cf7ce51a01a664e6faff4b8334b8414b1f ds000009 (2.0.1+1)
7fa73b4df8166dba950c7dc07c3f8cdd50fca313 ds000011 (1.0.0-5-g7fa73b4)
fatal: no submodule mapping found in .gitmodules for path 'ds000017
which then stops without even looking at other submodules.
I think it would be more logical to make it a 'warning:' not a 'fatal:' and
proceed.
Thank you for consideration
--
Yaroslav O. Halchenko
Center for Open Neuroscience http://centerforopenneuroscience.org
Dartmouth College, 419 Moore Hall, Hinman Box 6207, Hanover, NH 03755
Phone: +1 (603) 646-9834 Fax: +1 (603) 646-1419
WWW: http://www.linkedin.com/in/yarik
^ permalink raw reply
* [PATCH 4/2] use actual start hashes for submodule push check instead of local refs
From: Heiko Voigt @ 2016-09-15 12:18 UTC (permalink / raw)
To: Junio C Hamano
Cc: Jeff King, Stefan Beller, git@vger.kernel.org, Jens Lehmann,
Fredrik Gustafsson, Leandro Lucarella
In-Reply-To: <xmqqwpiep10i.fsf@gitster.mtv.corp.google.com>
Push knows the actual revision range it is actually pushing to a remote.
Let's use the start revisions to reduce the amount of checked revisions
instead of the locally cached remote refs which might be out of date.
This actually changes behavior as it now can also properly handle pushes
with URLs. That is also the reason why we change some tests. When
passing the remote as URL the push is made unconditionally in on-demand.
This is wrong since on-demand should only push the submodule if its SHA1
reference got changed in the superproject history that is getting pushed.
The reason behind that is that the exclusion list for revisions was
reduced by using the parameters "--not --remotes=<remote name>" to
reduce the amount of revisions for the revision walk. In case of an URL
this does not match anything and thus we would always do a full revision
walk until the root commit.
Signed-off-by: Heiko Voigt <hvoigt@hvoigt.net>
---
And here is another one which makes the check more correct. For push
--recurse-submodules=check|on-demand with URLs this is also a
performance improvement since at the moment we iterate over all
revisions unconditionally as explained above.
This patch changes the behavior (now its how its supposed to be) so
there may be fallout from users complaining that their submodules do not
get pushed with on-demand anymore. This is only for users using URL for
pushing though.
Cheers Heiko
BTW: Peff, thanks for the good diagnosis of all these problems.
submodule.c | 12 ++++++------
submodule.h | 6 +++---
t/t5531-deep-submodule-push.sh | 24 ++++++++++++++++++------
transport.c | 40 ++++++++++++++++++++++++++++++----------
4 files changed, 57 insertions(+), 25 deletions(-)
diff --git a/submodule.c b/submodule.c
index 28bb74e..1f82974 100644
--- a/submodule.c
+++ b/submodule.c
@@ -639,8 +639,8 @@ static void free_submodules_sha1s(struct string_list *submodules)
string_list_clear(submodules, 1);
}
-int find_unpushed_submodules(struct sha1_array *hashes,
- const char *remotes_name, struct string_list *needs_pushing)
+int find_unpushed_submodules(struct sha1_array *old_hashes,
+ struct sha1_array *new_hashes, struct string_list *needs_pushing)
{
struct rev_info rev;
struct commit *commit;
@@ -652,9 +652,9 @@ int find_unpushed_submodules(struct sha1_array *hashes,
/* argv.argv[0] will be ignored by setup_revisions */
argv_array_push(&argv, "find_unpushed_submodules");
- sha1_array_for_each_unique(hashes, append_hash_to_argv, &argv);
+ sha1_array_for_each_unique(new_hashes, append_hash_to_argv, &argv);
argv_array_push(&argv, "--not");
- argv_array_pushf(&argv, "--remotes=%s", remotes_name);
+ sha1_array_for_each_unique(old_hashes, append_hash_to_argv, &argv);
setup_revisions(argv.argc, argv.argv, &rev, NULL);
if (prepare_revision_walk(&rev))
@@ -700,12 +700,12 @@ static int push_submodule(const char *path)
return 1;
}
-int push_unpushed_submodules(struct sha1_array *hashes, const char *remotes_name)
+int push_unpushed_submodules(struct sha1_array *old_hashes, struct sha1_array *new_hashes)
{
int i, ret = 1;
struct string_list needs_pushing = STRING_LIST_INIT_DUP;
- if (!find_unpushed_submodules(hashes, remotes_name, &needs_pushing))
+ if (!find_unpushed_submodules(old_hashes, new_hashes, &needs_pushing))
return 1;
for (i = 0; i < needs_pushing.nr; i++) {
diff --git a/submodule.h b/submodule.h
index 065b2f0..55c6c91 100644
--- a/submodule.h
+++ b/submodule.h
@@ -63,9 +63,9 @@ int submodule_uses_gitfile(const char *path);
int ok_to_remove_submodule(const char *path);
int merge_submodule(unsigned char result[20], const char *path, const unsigned char base[20],
const unsigned char a[20], const unsigned char b[20], int search);
-int find_unpushed_submodules(struct sha1_array *hashes, const char *remotes_name,
- struct string_list *needs_pushing);
-int push_unpushed_submodules(struct sha1_array *hashes, const char *remotes_name);
+int find_unpushed_submodules(struct sha1_array *old_hashes,
+ struct sha1_array *new_hashes, struct string_list *needs_pushing);
+int push_unpushed_submodules(struct sha1_array *old_hashes, struct sha1_array *new_hashes);
void connect_work_tree_and_git_dir(const char *work_tree, const char *git_dir);
int parallel_submodules(void);
diff --git a/t/t5531-deep-submodule-push.sh b/t/t5531-deep-submodule-push.sh
index 198ce84..a5b8ef6 100755
--- a/t/t5531-deep-submodule-push.sh
+++ b/t/t5531-deep-submodule-push.sh
@@ -154,6 +154,8 @@ test_expect_success 'push recurse-submodules on command line overrides config' '
git fetch ../pub.git &&
git diff --quiet FETCH_HEAD master &&
(cd gar/bage && git diff --quiet origin/master master^) &&
+ # Since this push was executed reset to previous state
+ (git push -f ../pub.git master^:master) &&
# Ensure that we can override check in the config to
# disable submodule recursion entirely (alternative form)
@@ -161,6 +163,8 @@ test_expect_success 'push recurse-submodules on command line overrides config' '
git fetch ../pub.git &&
git diff --quiet FETCH_HEAD master &&
(cd gar/bage && git diff --quiet origin/master master^) &&
+ # Since this push was executed reset to previous state
+ (git push -f ../pub.git master^:master) &&
# Ensure that we can override check in the config to
# push the submodule too
@@ -198,11 +202,15 @@ test_expect_success 'push recurse-submodules last one wins on command line' '
git diff --quiet FETCH_HEAD master &&
# Check that the submodule commit did not get there
(cd gar/bage && git diff --quiet origin/master master^) &&
+ # Since this push was executed reset to previous state
+ (git push -f ../pub.git master^:master) &&
# should result in "no"
git push --recurse-submodules=on-demand --no-recurse-submodules ../pub.git master &&
# Check that the submodule commit did not get there
(cd gar/bage && git diff --quiet origin/master master^) &&
+ # Since this push was executed reset to previous state
+ (git push -f ../pub.git master^:master) &&
# But the options in the other order should push the submodule
git push --recurse-submodules=check --recurse-submodules=on-demand ../pub.git master &&
@@ -249,9 +257,11 @@ test_expect_success 'push succeeds if submodule commit disabling recursion from
git fetch ../pub.git &&
git diff --quiet FETCH_HEAD master &&
# But that the submodule commit did not
- ( cd gar/bage && git diff --quiet origin/master master^ ) &&
- # Now push it to avoid confusing future tests
- git push --recurse-submodules=on-demand ../pub.git master
+ (cd gar/bage &&
+ git diff --quiet origin/master master^ &&
+ # Now push it to avoid confusing future tests
+ git push
+ )
)
'
@@ -271,9 +281,11 @@ test_expect_success 'push succeeds if submodule commit disabling recursion from
git fetch ../pub.git &&
git diff --quiet FETCH_HEAD master &&
# But that the submodule commit did not
- ( cd gar/bage && git diff --quiet origin/master master^ ) &&
- # Now push it to avoid confusing future tests
- git push --recurse-submodules=on-demand ../pub.git master
+ (cd gar/bage &&
+ git diff --quiet origin/master master^ &&
+ # Now push it to avoid confusing future tests
+ git push
+ )
)
'
diff --git a/transport.c b/transport.c
index 76e1daf..475faaf 100644
--- a/transport.c
+++ b/transport.c
@@ -845,6 +845,28 @@ static int run_pre_push_hook(struct transport *transport,
return ret;
}
+static void collect_new_old_hashes(struct ref *ref, struct sha1_array *new_hashes,
+ struct sha1_array *old_hashes)
+{
+ for (; ref; ref = ref->next) {
+ if (!is_null_oid(&ref->new_oid)) {
+ /* Just need to handle non-null oid's. If there
+ * is no new we are handling a deletion and do
+ * not need to check
+ */
+ sha1_array_append(new_hashes, ref->new_oid.hash);
+
+ /* if the old id is null we are handling an new
+ * ref and can simply skip appending the oid
+ * since they are used to reduce the amount of
+ * checked revisions.
+ */
+ if (!is_null_oid(&ref->old_oid))
+ sha1_array_append(old_hashes, ref->old_oid.hash);
+ }
+ }
+}
+
int transport_push(struct transport *transport,
int refspec_nr, const char **refspec, int flags,
unsigned int *reject_reasons)
@@ -903,13 +925,12 @@ int transport_push(struct transport *transport,
if ((flags & TRANSPORT_RECURSE_SUBMODULES_ON_DEMAND) && !is_bare_repository()) {
struct ref *ref = remote_refs;
- struct sha1_array hashes = SHA1_ARRAY_INIT;
+ struct sha1_array new_hashes = SHA1_ARRAY_INIT;
+ struct sha1_array old_hashes = SHA1_ARRAY_INIT;
- for (; ref; ref = ref->next)
- if (!is_null_oid(&ref->new_oid))
- sha1_array_append(&hashes, ref->new_oid.hash);
+ collect_new_old_hashes(ref, &new_hashes, &old_hashes);
- if (!push_unpushed_submodules(&hashes, transport->remote->name))
+ if (!push_unpushed_submodules(&old_hashes, &new_hashes))
die ("Failed to push all needed submodules!");
}
@@ -917,13 +938,12 @@ int transport_push(struct transport *transport,
TRANSPORT_RECURSE_SUBMODULES_CHECK)) && !is_bare_repository()) {
struct ref *ref = remote_refs;
struct string_list needs_pushing = STRING_LIST_INIT_DUP;
- struct sha1_array hashes = SHA1_ARRAY_INIT;
+ struct sha1_array new_hashes = SHA1_ARRAY_INIT;
+ struct sha1_array old_hashes = SHA1_ARRAY_INIT;
- for (; ref; ref = ref->next)
- if (!is_null_oid(&ref->new_oid))
- sha1_array_append(&hashes, ref->new_oid.hash);
+ collect_new_old_hashes(ref, &new_hashes, &old_hashes);
- if (find_unpushed_submodules(&hashes, transport->remote->name,
+ if (find_unpushed_submodules(&old_hashes, &new_hashes,
&needs_pushing))
die_with_unpushed_submodules(&needs_pushing);
}
--
2.10.0.133.g13017a3
^ permalink raw reply related
* [PATCH 3/2] batch check whether submodule needs pushing into one call
From: Heiko Voigt @ 2016-09-15 12:10 UTC (permalink / raw)
To: Junio C Hamano
Cc: Jeff King, Stefan Beller, git@vger.kernel.org, Jens Lehmann,
Fredrik Gustafsson, Leandro Lucarella
In-Reply-To: <xmqqwpiep10i.fsf@gitster.mtv.corp.google.com>
We run a command for each sha1 change in a submodule. This is
unnecessary since we can simply batch all sha1's we want to check into
one command. Lets do it so we can speedup the check when many submodule
changes are in need of checking.
Signed-off-by: Heiko Voigt <hvoigt@hvoigt.net>
---
On Wed, Sep 14, 2016 at 03:30:53PM -0700, Junio C Hamano wrote:
> Heiko Voigt <hvoigt@hvoigt.net> writes:
>
> > Sorry about the late reply. I was not able to process emails until now.
> > Here are two patches that should help to improve the situation and batch
> > up some processing. This one is for repositories with submodules, so
> > that they do not iterate over the same submodule twice with the same
> > hash.
> >
> > The second one will be the one people without submodules are interested
> > in.
>
> Thanks. Will take a look at later as I'm already deep in today's
> integration cycle. Very much appreciated.
No problem. While I am at it: Here are actually another two patches that
should make life of submodule users easier (push times of big pushes).
In Numbers with the qt5[1] superproject and all submodules initialized.
The same --mirror test as before with the git repository:
# Without patch:
book:qt5 hvoigt (5.6)$
rm -rf ~/Downloads/git-test && mkdir ~/Downloads/git-test &&
(cd ~/Downloads/git-test && git init) &&
time git push --mirror --recurse-submodules=check ~/Downloads/git-test
real 4m0.881s
user 3m30.139s
sys 0m22.329s
Without --recurse-submodules=check
real 0m0.251s
user 0m0.218s
sys 0m0.082s
# With patch:
real 0m1.167s
user 0m0.846s
sys 0m0.262s
real 0m1.110s
user 0m0.815s
sys 0m0.247s
real 0m1.111s
user 0m0.818s
sys 0m0.251s
Without --recurse-submodules=check
real 0m0.294s
user 0m0.221s
sys 0m0.104s
real 0m0.248s
user 0m0.216s
sys 0m0.080s
real 0m0.247s
user 0m0.212s
sys 0m0.082s
[1] git://code.qt.io/qt/qt5.git
submodule.c | 75 ++++++++++++++++++++++++++++++++-----------------------------
1 file changed, 39 insertions(+), 36 deletions(-)
diff --git a/submodule.c b/submodule.c
index a15e346..28bb74e 100644
--- a/submodule.c
+++ b/submodule.c
@@ -522,27 +522,54 @@ static int has_remote(const char *refname, const struct object_id *oid,
return 1;
}
-static int submodule_needs_pushing(const char *path, const unsigned char sha1[20])
+static void append_hash_to_argv(const unsigned char sha1[20], void *data)
{
- if (add_submodule_odb(path) || !lookup_commit_reference(sha1))
+ struct argv_array *argv = (struct argv_array *) data;
+ argv_array_push(argv, sha1_to_hex(sha1));
+}
+
+static void check_has_hash(const unsigned char sha1[20], void *data)
+{
+ int *has_hash = (int *) data;
+
+ if (!lookup_commit_reference(sha1))
+ *has_hash = 0;
+}
+
+static int submodule_has_hashes(const char *path, struct sha1_array *hashes)
+{
+ int has_hash = 1;
+
+ if (add_submodule_odb(path))
+ return 0;
+
+ sha1_array_for_each_unique(hashes, check_has_hash, &has_hash);
+ return has_hash;
+}
+
+static int submodule_needs_pushing(const char *path, struct sha1_array *hashes)
+{
+ if (!submodule_has_hashes(path, hashes))
return 0;
if (for_each_remote_ref_submodule(path, has_remote, NULL) > 0) {
struct child_process cp = CHILD_PROCESS_INIT;
- const char *argv[] = {"rev-list", NULL, "--not", "--remotes", "-n", "1" , NULL};
+
+ argv_array_push(&cp.args, "rev-list");
+ sha1_array_for_each_unique(hashes, append_hash_to_argv, &cp.args);
+ argv_array_pushl(&cp.args, "--not", "--remotes", "-n", "1" , NULL);
+
struct strbuf buf = STRBUF_INIT;
int needs_pushing = 0;
- argv[1] = sha1_to_hex(sha1);
- cp.argv = argv;
prepare_submodule_repo_env(&cp.env_array);
cp.git_cmd = 1;
cp.no_stdin = 1;
cp.out = -1;
cp.dir = path;
if (start_command(&cp))
- die("Could not run 'git rev-list %s --not --remotes -n 1' command in submodule %s",
- sha1_to_hex(sha1), path);
+ die("Could not run 'git rev-list <hashes> --not --remotes -n 1' command in submodule %s",
+ path);
if (strbuf_read(&buf, cp.out, 41))
needs_pushing = 1;
finish_command(&cp);
@@ -601,21 +628,6 @@ static void find_unpushed_submodule_commits(struct commit *commit,
diff_tree_combined_merge(commit, 1, &rev);
}
-struct collect_submodule_from_sha1s_data {
- char *submodule_path;
- struct string_list *needs_pushing;
-};
-
-static void collect_submodules_from_sha1s(const unsigned char sha1[20],
- void *data)
-{
- struct collect_submodule_from_sha1s_data *me =
- (struct collect_submodule_from_sha1s_data *) data;
-
- if (submodule_needs_pushing(me->submodule_path, sha1))
- string_list_insert(me->needs_pushing, me->submodule_path);
-}
-
static void free_submodules_sha1s(struct string_list *submodules)
{
int i;
@@ -627,13 +639,6 @@ static void free_submodules_sha1s(struct string_list *submodules)
string_list_clear(submodules, 1);
}
-static void append_hash_to_argv(const unsigned char sha1[20],
- void *data)
-{
- struct argv_array *argv = (struct argv_array *) data;
- argv_array_push(argv, sha1_to_hex(sha1));
-}
-
int find_unpushed_submodules(struct sha1_array *hashes,
const char *remotes_name, struct string_list *needs_pushing)
{
@@ -662,13 +667,11 @@ int find_unpushed_submodules(struct sha1_array *hashes,
argv_array_clear(&argv);
for (i = 0; i < submodules.nr; i++) {
- struct string_list_item *item = &submodules.items[i];
- struct collect_submodule_from_sha1s_data data;
- data.submodule_path = item->string;
- data.needs_pushing = needs_pushing;
- sha1_array_for_each_unique((struct sha1_array *) item->util,
- collect_submodules_from_sha1s,
- &data);
+ struct string_list_item *submodule = &submodules.items[i];
+ struct sha1_array *hashes = (struct sha1_array *) submodule->util;
+
+ if (submodule_needs_pushing(submodule->string, hashes))
+ string_list_insert(needs_pushing, submodule->string);
}
free_submodules_sha1s(&submodules);
--
2.10.0.133.g13017a3
^ permalink raw reply related
* Re: [RFC] extending pathspec support to submodules
From: Heiko Voigt @ 2016-09-15 11:57 UTC (permalink / raw)
To: Brandon Williams; +Cc: git, pclouds, Jens Lehmann, Stefan Beller
In-Reply-To: <1473897473-154528-1-git-send-email-bmwill@google.com>
Hi,
On Wed, Sep 14, 2016 at 04:57:53PM -0700, Brandon Williams wrote:
> ---
> I've been trying to think through how we could potentially add pathspec support
> for --recurse-submodule options (for builtins like ls-files or grep down the
> line). This is something that could be useful if the user supply's a pathspec
> that could match to a file in a submodule. We could match the submodule to the
> pathspec and then fork the process to recursively run the command on the
> submodule which can be passed a modified pathspec.
>
> For example with a pathspec 'sub/dir/a', where sub is a submodule in the root
> directory of the supermodule's repo, we could match 'sub' to that spec and then
> recursively call the git command with a pathspec of 'dir/a'. The child process
> would then have the responsibility of matching 'dir/a' to files in its repo.
>
> Does this seem like a reasonable feature to add? And if so are how is my
> initial approach at solving the problem?
The problem when you do that is that the child is not aware that it is
actually run as a submodule process. E.g.
git grep --recurse-submodules foobar -- sub/dir/a
would report back matches in 'dir/a' instead of 'sub/dir/a'. From the
users perspective this will be confusing since she started the grep in
the superproject.
So what I would suggest is that we make the childs aware of the fact
that they are called from a superproject by passing a prefix when
calling it. E.g. like this
git grep --submodule-prefix=sub foobar -- sub/dir/a
Whether we pass the full or stripped path is now a matter of taste or
ease of implementation, since we could either preprend or strip it in
the child. But the important difference is that we have information
about the original path.
Another approach could, of course, be to omit passing the prefix and
parse the output of the child and try to substitute, paths but that
seems quite error prone to me.
Cheers Heiko
^ permalink raw reply
* Re: [RFC 0/1] mailinfo: de-quote quoted-pair in header fields
From: Jeff King @ 2016-09-15 7:18 UTC (permalink / raw)
To: Kevin Daudt; +Cc: Junio C Hamano, git
In-Reply-To: <20160915051533.GC26893@ikke.info>
On Thu, Sep 15, 2016 at 07:15:33AM +0200, Kevin Daudt wrote:
> > > Another small thing I am not sure about is if the \ quoting can hide
> > > an embedded newline in the author name. Would we end up turning
> > >
> > > From: "Jeff \
> > > King" <peff@peff.net>
> > >
> > > or somesuch into
> > >
> > > Author: Jeff
> > > King
> > > Email: peff@peff.net
> > >
> > > ;-)
> >
> > Heh, yeah. That is another reason to clean up and sanitize as much as
> > possible before stuffing it into another text format that will be
> > parsed.
>
> A quoted string cannot contain newlines according to the RFC, so I think
> we don't need to care about that.
I wondered how we handled something like:
From: =?UTF-8?q?J=0Aff=20King?= <peff@peff.net>
which sticks a newline into the middle of the buffer. We do decode it
that way, but eventually call cleanup_space() which converts a run of 1
or more isspace() characters into a single space (0x20). So you end up
with:
Author: J ff King
which is probably reasonable.
> Makes sense, the current itteration of my patch already strips exterior
> quotes, no matter where they happen.
>
> I will send a patch soon.
Great. Thanks for working on this.
-Peff
^ permalink raw reply
* Re: [PATCH] object: measure time needed for resolving hash collisions
From: Jeff King @ 2016-09-15 7:01 UTC (permalink / raw)
To: Stefan Beller; +Cc: git
In-Reply-To: <20160915064701.c4ishixuynbzpgwx@sigill.intra.peff.net>
On Wed, Sep 14, 2016 at 11:47:01PM -0700, Jeff King wrote:
> > first = i = hash_obj(sha1, obj_hash_size);
> > + clock_gettime(CLOCK_MONOTONIC, &time1);
> > while ((obj = obj_hash[i]) != NULL) {
> > if (!hashcmp(sha1, obj->oid.hash))
> > break;
> > @@ -98,6 +131,9 @@ struct object *lookup_object(const unsigned char *sha1)
> > if (i == obj_hash_size)
> > i = 0;
> > }
> > + clock_gettime(CLOCK_MONOTONIC, &time2);
> > + diff(&time1, &time2, &t_diff);
> > + add_time_to(&caching, &t_diff);
> > if (obj && i != first) {
>
> I don't think this is actually measuring the time spent on collisions.
> It's measuring the time we spend in hashcmp(), but that includes the
> non-collision case where we find it in the first hashcmp.
>
> Measuring _just_ the collisions is more like the patch below. In my
> measurements it's more like 30ms, compared to 10s for all of the
> hashcmps.
I forgot to send the patch. Which is just as well, because I realized it
was totally buggy.
Here's a patch that I believe is correct (it counts only times when we
move past the first hash slot). It spends about 280ms. Which is still a
lot less than 10s, so I think my other comments stand.
---
diff --git a/object.c b/object.c
index e9e73e0..7a74a1d 100644
--- a/object.c
+++ b/object.c
@@ -123,17 +123,20 @@ struct object *lookup_object(const unsigned char *sha1)
return NULL;
first = i = hash_obj(sha1, obj_hash_size);
- clock_gettime(CLOCK_MONOTONIC, &time1);
while ((obj = obj_hash[i]) != NULL) {
if (!hashcmp(sha1, obj->oid.hash))
break;
+ if (first == i)
+ clock_gettime(CLOCK_MONOTONIC, &time1);
i++;
if (i == obj_hash_size)
i = 0;
}
- clock_gettime(CLOCK_MONOTONIC, &time2);
- diff(&time1, &time2, &t_diff);
- add_time_to(&caching, &t_diff);
+ if (i != first) {
+ clock_gettime(CLOCK_MONOTONIC, &time2);
+ diff(&time1, &time2, &t_diff);
+ add_time_to(&caching, &t_diff);
+ }
if (obj && i != first) {
/*
* Move object to where we started to look for it so
^ permalink raw reply related
* Re: [PATCH] object: measure time needed for resolving hash collisions
From: Jeff King @ 2016-09-15 6:47 UTC (permalink / raw)
To: Stefan Beller; +Cc: git
In-Reply-To: <20160915020141.32000-1-sbeller@google.com>
On Wed, Sep 14, 2016 at 07:01:41PM -0700, Stefan Beller wrote:
> According to Jeff, sending patches that don't get accepted is the new hype!
It is what all the cool kids are doing. Unfortunately, it does not save
you from nitpicky reviews...;)
> first = i = hash_obj(sha1, obj_hash_size);
> + clock_gettime(CLOCK_MONOTONIC, &time1);
> while ((obj = obj_hash[i]) != NULL) {
> if (!hashcmp(sha1, obj->oid.hash))
> break;
> @@ -98,6 +131,9 @@ struct object *lookup_object(const unsigned char *sha1)
> if (i == obj_hash_size)
> i = 0;
> }
> + clock_gettime(CLOCK_MONOTONIC, &time2);
> + diff(&time1, &time2, &t_diff);
> + add_time_to(&caching, &t_diff);
> if (obj && i != first) {
I don't think this is actually measuring the time spent on collisions.
It's measuring the time we spend in hashcmp(), but that includes the
non-collision case where we find it in the first hashcmp.
Measuring _just_ the collisions is more like the patch below. In my
measurements it's more like 30ms, compared to 10s for all of the
hashcmps.
So we really aren't dealing with collisions, but rather just verifying
that our hash landed at the right spot. And _any_ data structure is
going to have to do that. If you want to make it faster, I'd try
optimizing hashcmp (and you can see why the critbit tree was slower; if
we spend so much time just on hashcmp() to make sure we're at the right
key, then making that slower with a bunch of branching is not going to
help).
I notice we still open-code hashcmp. I get a slight speedup by switching
it to memcmp(). About 2.5%, which is similar to what I showed in
http://public-inbox.org/git/20130318073229.GA5551@sigill.intra.peff.net/
a few years ago (though it's more pronounced as a portion of the whole
now, because we've optimized some of the other bits).
The main driver there was memcmp() improvements that went into glibc
2.13 several years ago. It might be time to start assuming that memcmp()
beats our open-coded loop.
It may also be possible to really micro-optimize it on some platforms,
because we know the size in advance (I'd kind of expect the compiler to
do that, but if we're ending up in glibc memcmp then it sounds like it
is not the case).
-Peff
^ permalink raw reply
* Re: [PATCH 2/2] use zstd zlib wrapper
From: Jeff King @ 2016-09-15 6:28 UTC (permalink / raw)
To: Stefan Beller; +Cc: git@vger.kernel.org
In-Reply-To: <CAGZ79kYcB-x40_w1fcWL3NSp8JU9SPrTAEiru-6Jpb7fDM1Y0w@mail.gmail.com>
On Wed, Sep 14, 2016 at 06:22:17PM -0700, Stefan Beller wrote:
> > Disappointingly, the answer seems to be "no".
>
> After having looked at the data, I disagree with the conclusion.
> And for that I think we need to reason about the frequency
> of the operations happening.
I definitely agree that reads outnumber writes, and it's OK to have an
asymmetric tradeoff between the two. zstd5 isn't _too_ bad in that
respect. I guess I was just disappointed that the pack size was still
bigger, as I was really hoping to see some speed tradeoff without
getting a worse pack.
The other thing to weigh against is "if we were designing it today"
versus "is it worth the compatibility headaches now". A 6% improvement
in "rev-list --objects" is not that amazing for a data format change.
Bitmaps were an _easier_ data format change and are more like a 99%
speedup.
They do not apply to every operations, but we may be able to do similar
space/time tradeoffs that are easier to handle in terms of backwards
compatibility, and which yield bigger results.
-Peff
^ permalink raw reply
* Re: [RFC 0/1] mailinfo: de-quote quoted-pair in header fields
From: Kevin Daudt @ 2016-09-15 5:15 UTC (permalink / raw)
To: Jeff King; +Cc: Junio C Hamano, git
In-Reply-To: <20160914193819.ua5ubvbf5wz7tvuj@sigill.intra.peff.net>
On Wed, Sep 14, 2016 at 12:38:20PM -0700, Jeff King wrote:
> On Wed, Sep 14, 2016 at 12:30:06PM -0700, Junio C Hamano wrote:
>
> > Another small thing I am not sure about is if the \ quoting can hide
> > an embedded newline in the author name. Would we end up turning
> >
> > From: "Jeff \
> > King" <peff@peff.net>
> >
> > or somesuch into
> >
> > Author: Jeff
> > King
> > Email: peff@peff.net
> >
> > ;-)
>
> Heh, yeah. That is another reason to clean up and sanitize as much as
> possible before stuffing it into another text format that will be
> parsed.
A quoted string cannot contain newlines according to the RFC, so I think
we don't need to care about that.
>
> > So let's roll the \" -> " into mailinfo.
> >
> > I am not sure if we also should remove the surrounding "", i.e. we
> > currently do not turn this
> >
> > From: "Jeff King" <peff@peff.net>
> >
> > into this:
> >
> > Author: Jeff King
> > Email: peff@peff.net
> >
> > I think we probably should, and remove the one that does so from the
> > reader.
>
> I think you have to, or else you cannot tell the difference between
> surrounding quotes that need to be stripped, and ones that were
> backslash-escaped. Like:
>
> From: "Jeff King" <peff@peff.net>
> From: \"Jeff King\" <peff@peff.net>
>
> which would both become:
>
> Author: "Jeff King"
> Email: peff@peff.net
>
> though I am not sure the latter one is actually valid; you might need to
> be inside syntactic quotes in order to include backslashed quotes. I
> haven't read rfc2822 carefully recently enough to know.
>
> Anyway, I think that:
>
> From: One "Two \"Three\" Four" Five
>
> may also be valid. So the quote-stripping in the reader is not just "at
> the outside", but may need to handle interior syntactic quotes, too. So
> it really makes sense for me to clean and sanitize as much as possible
> in one step, and then make the parser of mailinfo as dumb as possible.
>
Makes sense, the current itteration of my patch already strips exterior
quotes, no matter where they happen.
I will send a patch soon.
^ permalink raw reply
* git add --intent-to-add silently creates empty commits
From: Aviv Eyal @ 2016-09-15 5:01 UTC (permalink / raw)
To: git
Hello,
I think this is an unintended behavior, but I might be wrong:
Using `git add -N` allows creating of empty commits:
git init test && cd test
echo text > file
git add --intent-to-add file
git commit -m 'Empty commit'
echo $? # prints 0
git log -1 -p --format='' HEAD | wc -l # prints 0
git status -s # prints `AM file`
I'd expect `git commit` to error out instead of producing an empty commit.
I've seen this with git 2.8.1 and 2.10.0.129.g35f6318
Thanks,
Aviv
^ permalink raw reply
* [PATCH] object: measure time needed for resolving hash collisions
From: Stefan Beller @ 2016-09-15 2:01 UTC (permalink / raw)
To: peff; +Cc: git, Stefan Beller
$ time ./git rev-list HEAD --count
44271
print_time_spent_object: measure time needed for resolving hash collisions
This shows that we roughly spent a third of the time in resolving
hash collisions:
I am using linux.git to measure:
$ time git rev-list --objects --count HEAD >/dev/null
print_time_spent_resolving_hash_collisions 9:445591664
real 0m31.733s
user 0m31.220s
sys 0m0.463s
For fun I reverted 9a414486d9f0:
$ time git rev-list --objects --count HEAD >/dev/null
print_time_spent_resolving_hash_collisions 14:338605504
real 0m37.008s
user 0m36.524s
sys 0m0.423s
The 17% that Jeff measured in there, equals to 1-31.733/37.008 = 0.14
in these measurements.
The time spent in collision resolving went down by 1/3 on itself
(14.33s to 9.44s), so there is still room for improvement.
Signed-off-by: Stefan Beller <sbeller@google.com>
---
According to Jeff, sending patches that don't get accepted is the new hype!
builtin/rev-list.c | 2 ++
object.c | 36 ++++++++++++++++++++++++++++++++++++
object.h | 2 ++
3 files changed, 40 insertions(+)
diff --git a/builtin/rev-list.c b/builtin/rev-list.c
index 8479f6e..d0e5922 100644
--- a/builtin/rev-list.c
+++ b/builtin/rev-list.c
@@ -412,5 +412,7 @@ int cmd_rev_list(int argc, const char **argv, const char *prefix)
printf("%d\n", revs.count_left + revs.count_right);
}
+ print_time_spent_resolving_hash_collisions();
+
return 0;
}
diff --git a/object.c b/object.c
index 67d9a9e..e9e73e0 100644
--- a/object.c
+++ b/object.c
@@ -5,9 +5,40 @@
#include "commit.h"
#include "tag.h"
+#include <time.h>
+
static struct object **obj_hash;
static int nr_objs, obj_hash_size;
+struct timespec caching = {0, 0};
+
+void diff(struct timespec *start, struct timespec *end, struct timespec *dst)
+{
+ if ((end->tv_nsec-start->tv_nsec)<0) {
+ dst->tv_sec = end->tv_sec-start->tv_sec-1;
+ dst->tv_nsec = 1000000000+end->tv_nsec-start->tv_nsec;
+ } else {
+ dst->tv_sec = end->tv_sec-start->tv_sec;
+ dst->tv_nsec = end->tv_nsec-start->tv_nsec;
+ }
+}
+
+void add_time_to(struct timespec *dst, struct timespec *addend)
+{
+ dst->tv_sec += addend->tv_sec;
+ dst->tv_nsec += addend->tv_nsec;
+ while (dst->tv_nsec > 1000000000) {
+ dst->tv_nsec -= 1000000000;
+ dst->tv_sec ++;
+ }
+}
+
+void print_time_spent_resolving_hash_collisions(void)
+{
+ fprintf(stderr, "print_time_spent_resolving_hash_collisions %ld:%9ld\n",
+ (long)caching.tv_sec, (long)caching.tv_nsec);
+}
+
unsigned int get_max_object_index(void)
{
return obj_hash_size;
@@ -86,11 +117,13 @@ struct object *lookup_object(const unsigned char *sha1)
{
unsigned int i, first;
struct object *obj;
+ struct timespec time1, time2, t_diff;
if (!obj_hash)
return NULL;
first = i = hash_obj(sha1, obj_hash_size);
+ clock_gettime(CLOCK_MONOTONIC, &time1);
while ((obj = obj_hash[i]) != NULL) {
if (!hashcmp(sha1, obj->oid.hash))
break;
@@ -98,6 +131,9 @@ struct object *lookup_object(const unsigned char *sha1)
if (i == obj_hash_size)
i = 0;
}
+ clock_gettime(CLOCK_MONOTONIC, &time2);
+ diff(&time1, &time2, &t_diff);
+ add_time_to(&caching, &t_diff);
if (obj && i != first) {
/*
* Move object to where we started to look for it so
diff --git a/object.h b/object.h
index f8b6442..ee6ab3a 100644
--- a/object.h
+++ b/object.h
@@ -56,6 +56,8 @@ extern const char *typename(unsigned int type);
extern int type_from_string_gently(const char *str, ssize_t, int gentle);
#define type_from_string(str) type_from_string_gently(str, -1, 0)
+void print_time_spent_resolving_hash_collisions(void);
+
/*
* Return the current number of buckets in the object hashmap.
*/
--
2.10.0.130.gef2bcd7.dirty
^ permalink raw reply related
* Re: [PATCH 2/2] use zstd zlib wrapper
From: Stefan Beller @ 2016-09-15 1:22 UTC (permalink / raw)
To: Jeff King; +Cc: git@vger.kernel.org
In-Reply-To: <20160914235843.nacr54ekvl6rjipk@sigill.intra.peff.net>
On Wed, Sep 14, 2016 at 4:58 PM, Jeff King <peff@peff.net> wrote:
> There's a fancy new compression algorithm called "zstd". The
> idea is that it's supposed to get similar compression ratios
> to zlib, but with much faster compression and decompression
> times. And on top of that, a nice sliding scale to trade off
> size versus time on the compression side.
>
> The zstd site at https://facebook.github.io/zstd/ claims
> close to 3x speedup for both compression and decompression
> versus zlib, with similar compression ratios. There are
> other fast algorithms (like lz4), but they usually compress
> much worse (follow the link above for a nice table of
> results).
>
> Since any git operations that have to access objects need to
> do a zlib inflate, in theory we can speed up everything by
> using zstd. And then on the packing side, use higher
> compression levels when making on-disk packfiles (which will
> be accessed many times) and lower ones when making loose
> objects, or deflating packed objects on the fly when serving
> fetches.
>
> The catch, of course, is that it's a new incompatible
> format. This would be a pretty huge change and totally break
> backwards compatibility for git, not just on disk but
> on-the-wire as well. So my goal here was not a finished
> product but just a quick experiment to see if it did indeed
> bring the promise speedups.
>
> Disappointingly, the answer seems to be "no".
After having looked at the data, I disagree with the conclusion.
And for that I think we need to reason about the frequency
of the operations happening.
* As an enduser, happily hacking away at one repository,
I probably do not care about the pack size on disk as much
as I care about timing of the local operations. And I assume
that for each repack we have about 1000 reads (log/rev-list)
The 1000 is a wild speculation without any data to back it up.
So as an end user I'd be happy about [zstd, ~5]
For the end user LZ4 seems to be the best solution if it were available.
* As a service provider, I know we have a lot more reads than
writes, and repacking is annoying. Also at that scale the disk
isn't negligible cheap. So we need to weigh the numbers differently,
but how? I suspect depending on the weighting it could still be
considered beneficial to go with zstd5. (No hard numbers here)
^ permalink raw reply
* Re: [PATCH 1/2] obj_hash: convert to a critbit tree
From: Jeff King @ 2016-09-15 1:13 UTC (permalink / raw)
To: Stefan Beller; +Cc: git@vger.kernel.org
In-Reply-To: <CAGZ79kb4_k=+O6pzQkZBGHQty+kdYTnQX_3110RNt097AVn+Kg@mail.gmail.com>
On Wed, Sep 14, 2016 at 05:52:06PM -0700, Stefan Beller wrote:
> > So finding "1011" involves traversing the trie: down the "1"
> > side, then the "0" side, and then check that the rest
> > matches "11".
>
> So we stop building a tree as soon as we hit a unique data
> element (i.e. if we stick to the idea of encoding the hash along the way,
> we would have needed another node each for "11" as well as "00"
> that points to NULL and the ending data respectively.
>
> So we stop short as soon as we have a unique.
>
> That makes insertion very easy, because as soon as we hit
> a unique, we'd just introduce a node and add the two uniques
> left and right. (Well what happens if we were to insert
> 101010111 and 101010101 ? Both have a long prefix,
> I suppose we'd have 7 nodes and then the distiguishing
> node for those 2.)
I think it actually can represent the interior sequence as a single
node. That's what the "critical" part of critbit is for; the critical
bit is the one where we diverge. But I'm not 100% sure that it's
implemented that way.
In practice, though, sha1 would give us a pretty full tree near the
front, so I'd expect each bit to be "critical".
> > It's possible that (2) would be better if instead of a
> > critbit tree, we used a "critbyte" tree. That would involve
> > fewer node traversals, at the cost of making each node
> > larger (right now the internal nodes store 2 pointer slots;
> > they'd have to store 256 to handle a full byte). I didn't
> > try it, but I suspect it would still be slower for the same
> > reasons.
>
> I would expect to go for a crit-"variable-length" tree instead.
>
> The reason for this is that a higher fan out seems to be more
> beneficial in the earlier stages, e.g. we could use critbyte trees
> for the first 1-3 layers in the tree as that will have good memory
> efficiency (all 256 slots filled), but will be faster than the critbit trees
> (one lookup instead of 8 conditional jumps).
Yeah, I suspect a hybrid approach may be a better tradeoff. I encourage
you to try and measure. :)
> I guess when trying to improve the hashsets, someone tried trees
> as a collision resolver?
I don't think so. hashmap.c uses a linked list, but obj_hash just does
linear probing. Both are linear, but the latter is more memory efficient
and especially has a more compact cache footprint when resolving
collisions. The downside of open probing like that is that it's very
hard to delete an entry, but we don't care about that for obj_hash.
I don't think that improving collision resolution would help that much
for obj_hash, though. The fact that quadratic probing and cuckoo hashing
didn't yield big benefits implies that we don't spend most of our time
on collisions in the first place (OTOH, my 9a414486d9f0 that you found
does help _only_ when there are collisions, so maybe I'm wrong).
> So maybe we could have some software sided cache for hot entries?
> (I imagine a data structure consisting of 2 hash sets here, one
> hashset containing
> the complete data set, and the other hashset is a very small hashset with e.g.
> just 256(?) entries that are an LRU cache for the cache entries.
> Though this sounds like we'd be trying to outsmart the hardware... Not sure
> I'd expect gains from that)
Yeah. Basically what we want is the reverse of a bloom filter: it's OK
to be wrong, but it most be wrong to say "it's not here" when it is, not
the other way around. And so that's basically...a cache of a smaller
data-structure in front of the larger one.
But given that the hash table is already O(1)-ish, it's hard to beat
that (because remember when you have a cache miss, you then have to do
an extra lookup in the full table anyway).
I did play around with stuff like that when I was coming up with
9a414486d9f0, but was never able to make it faster. Patches welcome, of
course.
> I guess we rather want to split up the data sets on the application
> side: i.e. instead of
> having so many objects, have hash sets for e.g. blobs, trees, commits separately
> and then use slightly different strategies there (different load factors?)
My gut is that would not make a big difference (and sometimes it would
be worse, because we don't know what the object type is, or we want to
make _sure_ that we don't have the object known as a different type).
> Unrelated note about hashmaps:
> I wonder if we rather want to give good initial estimates of the size
> as one improvement
My recollection is that the growth isn't really relevant. At least for
obj_hash, we do _way_ more lookups than insertions, so the only thing
that really matters is lookup speed.
But as with everything, if you can come up with improved numbers, I'm
happy to look at the patches. :)
-Peff
^ permalink raw reply
* Re: Tracking down a segfault in delta_base_cache
From: Jeff King @ 2016-09-15 0:56 UTC (permalink / raw)
To: Jonathon Mah; +Cc: git
In-Reply-To: <3946EE74-219D-4E9C-9CED-69D53B940955@jonathonmah.com>
On Wed, Sep 14, 2016 at 05:42:29PM -0700, Jonathon Mah wrote:
> Hi git, I've been seeing git segfault over the past few days. I'm on Mac OS X 10.12, 64-bit, compiling with clang (Apple LLVM version 8.0.0 (clang-800.0.40)).
> [...]
> Bisect says:
>
> 8261e1f139db3f8aa6f9fd7d98c876cbeb0f927c is the first bad commit
> commit 8261e1f139db3f8aa6f9fd7d98c876cbeb0f927c
> Author: Jeff King <peff@peff.net>
> Date: Mon Aug 22 18:00:07 2016 -0400
>
> delta_base_cache: use hashmap.h
Have you tried with the patch in:
http://public-inbox.org/git/20160912164616.vg33kldazuthff3d@sigill.intra.peff.net/
?
> $ lldb /Users/jmah/Documents/Streams/git/git-log -- -u
> (lldb) target create "/Users/jmah/Documents/Streams/git/git-log"
> Current executable set to '/Users/jmah/Documents/Streams/git/git-log' (x86_64).
> (lldb) settings set -- target.run-args "-u"
> (lldb) process launch -o /dev/null
> Process 92815 launched: '/Users/jmah/Documents/Streams/git/git-log' (x86_64)
> Process 92815 stopped
> * thread #1: tid = 0x1c30677, 0x00000001001bba80 git-log`release_delta_base_cache(ent=0xffffffffffffffd0) + 16 at sha1_file.c:2171, queue = 'com.apple.main-thread', stop reason = EXC_BAD_ACCESS (code=1, address=0x10)
> frame #0: 0x00000001001bba80 git-log`release_delta_base_cache(ent=0xffffffffffffffd0) + 16 at sha1_file.c:2171
> 2168
> 2169 static inline void release_delta_base_cache(struct delta_base_cache_entry *ent)
> 2170 {
> -> 2171 free(ent->data);
> 2172 detach_delta_base_cache_entry(ent);
The problems I saw with valgrind weren't here, but would explain this.
We free() the previous node, then walk forward from its "next" pointer.
On my Linux box, that happens to work, but we could be feeding total
junk to the list pointer, which would meant ent->data is junk, and
free() notices.
-Peff
^ permalink raw reply
* Re: [PATCH 1/2] obj_hash: convert to a critbit tree
From: Stefan Beller @ 2016-09-15 0:52 UTC (permalink / raw)
To: Jeff King; +Cc: git@vger.kernel.org
In-Reply-To: <20160914235633.gofr534hvslkclzm@sigill.intra.peff.net>
On Wed, Sep 14, 2016 at 4:56 PM, Jeff King <peff@peff.net> wrote:
> For operations that traverse the whole reachability graph,
> like "rev-list --objects", the obj_hash in object.c shows up
> as a hotspot. We basically have to do "nr_commits *
> size_of_tree" hash lookups, because each tree we look at, we
> need to say "have we seen this sha1 yet?" (it's a little
> more complicated because we avoid digging into sub-trees we
> know we've already seen, but it's a reasonable
> approximation). So graph traversal operations like that are
> very sensitive to improvements in lookup time.
>
> For keys with length "m", a hash table is generally O(m) to
> compute the hash (and verify that you've found the correct
> key), and O(1) in finding the correct slot. The latter is
> subject to collisions and probing strategy, but we keep the
> load factor on the obj_hash table below 50%, which is quite
> low. And we have a good hash (we reuse the sha1s themselves,
> which are effectively random). So we'd expect relatively few
> collisions, and past experiments to tweak the probing
> strategy haven't yielded any benefit (we use linear probing;
> I tried quadratic probing at one point, and Junio tried
> cuckoo hashing).
>
> Another data structure with similar properties is sometimes
> known as a "critbit tree" (see http://cr.yp.to/critbit.html
> for discussion and history). The basic idea is a binary trie
> where each node is either an internal node, representing a
> single bit of a stored key, or a leaf node, representing one
> or more "remainder" bits. So if you were storing two bit
> sequences 1011 and "1100", you'd have three nodes (besides
> the root):
>
> (root)
> / \
> 0 1
> / \
> NULL (internal)
> / \
> 0 1
> / \
> "11" "00"
>
> So finding "1011" involves traversing the trie: down the "1"
> side, then the "0" side, and then check that the rest
> matches "11".
So we stop building a tree as soon as we hit a unique data
element (i.e. if we stick to the idea of encoding the hash along the way,
we would have needed another node each for "11" as well as "00"
that points to NULL and the ending data respectively.
So we stop short as soon as we have a unique.
That makes insertion very easy, because as soon as we hit
a unique, we'd just introduce a node and add the two uniques
left and right. (Well what happens if we were to insert
101010111 and 101010101 ? Both have a long prefix,
I suppose we'd have 7 nodes and then the distiguishing
node for those 2.)
>
> Looking up a key is O(m), and there's no issue with
> collisions or probing. It can use less memory than a hash
> table, because there's no load factor to care about.
>
> That's the good news. The bad news is that big-O analysis
> is not the whole story. You'll notice that we have to do a
> lot of conditional pointer-jumping to walk the trie. Whereas
> a hash table can jump right to a proposed key and do a
> memcmp() to see if we got it.
>
> So I wasn't overly optimistic that this would be any faster.
> And indeed it's not. It's about three times slower (about
> 4.5s versus 1.5s running "rev-list --objects --all" on
> git.git).
>
> The reason is, I think, a combination of:
>
> 0. We care much more about performance for this hash than
> memory efficiency. So we keep the load factor
> quite low, and thus reduce the number of collisions.
>
> 1. For many hash tables, computing the hash is expensive.
> Not so here, because we are storing sha1s. So it is
> literally just casting the first 4 bytes of the sha1 to
> an int; we don't even look at the other bytes until the
> collision check (and because of point 0, we don't
> generally have to do very many collision checks during
> our probe).
>
> 2. The collision check _does_ have to look at all 20 bytes
> of the sha1. And we may have to do it multiple times as
> we linearly probe the collisions. _But_ it can be done
> with memcmp(), which is optimized to compare 32 or 64
> bits at a time. So we our O(m) has a very nice constant
> factor.
>
> Whereas in the critbit tree, we pay an instruction for
> _each_ bit we look at.
>
> It's possible that (2) would be better if instead of a
> critbit tree, we used a "critbyte" tree. That would involve
> fewer node traversals, at the cost of making each node
> larger (right now the internal nodes store 2 pointer slots;
> they'd have to store 256 to handle a full byte). I didn't
> try it, but I suspect it would still be slower for the same
> reasons.
I would expect to go for a crit-"variable-length" tree instead.
The reason for this is that a higher fan out seems to be more
beneficial in the earlier stages, e.g. we could use critbyte trees
for the first 1-3 layers in the tree as that will have good memory
efficiency (all 256 slots filled), but will be faster than the critbit trees
(one lookup instead of 8 conditional jumps).
In a degenerated form a crit-"variable-length" tree could be
imagined as a hashmap with a critbit tree as the collision resolver,
as we flip from the one extreme of hashmaps (memory inefficient,
but O(1) lookup with low constant factors), to the other extreme
of critbits (memory efficient, but need O(log(size)) nodes which
each need time per bit)
So maybe we could start with a crit-word (16bit) node at the top,
then have critbyte tree nodes (8 bits), and then go for critbit nodes?
The crit-word would have 2**16 entries, i.e. 64k, which is more than
git.gits count of objects (~44k), so we'd get the memory inefficiency of
a hash map, combined with slightly higher costs for collision resolving.
I guess when trying to improve the hashsets, someone tried trees
as a collision resolver?
>
> The code is below for reference. I pulled the critbit
> implementation from:
>
> https://github.com/ennorehling/critbit
>
> but made a few tweaks:
>
> - the critbit does not store the keys themselves, as we
> already have them in the "struct object", and do not
> want to waste space. As a result, I baked in some
> assumptions about the 20-byte key-length (which is fine
> for our purposes here).
>
> - rather than malloc() each node, I used a pool allocator
> (to try to keep the nodes closer together in cache)
I think this is another interesting part, so let's talk about caching.
While we may not care about the memory inefficiency for the lost
memory, it is still bad for caches. And the cache inefficiency may
be worse than a few instructions of walking down a critbit for
example. I was instantly reminded of 9a414486d9f0
(lookup_object: prioritize recently found objects)
So maybe we could have some software sided cache for hot entries?
(I imagine a data structure consisting of 2 hash sets here, one
hashset containing
the complete data set, and the other hashset is a very small hashset with e.g.
just 256(?) entries that are an LRU cache for the cache entries.
Though this sounds like we'd be trying to outsmart the hardware... Not sure
I'd expect gains from that)
I guess we rather want to split up the data sets on the application
side: i.e. instead of
having so many objects, have hash sets for e.g. blobs, trees, commits separately
and then use slightly different strategies there (different load factors?)
Unrelated note about hashmaps:
I wonder if we rather want to give good initial estimates of the size
as one improvement
Thanks for posting these patches!
They are really fun to learn from!
Stefan
^ permalink raw reply
* Tracking down a segfault in delta_base_cache
From: Jonathon Mah @ 2016-09-15 0:42 UTC (permalink / raw)
To: git; +Cc: Jeff King
Hi git, I've been seeing git segfault over the past few days. I'm on Mac OS X 10.12, 64-bit, compiling with clang (Apple LLVM version 8.0.0 (clang-800.0.40)).
I first noticed it during a checkout, then also during `log -u`. I'm still debugging, but wanted to give a heads-up in case anyone else is seeing this.
~/D/S/A/HLT $ git-log -u -n1000 >/dev/null
fish: 'git-log' terminated by signal SIGSEGV (Address boundary error)
~/D/S/A/HLT $ git fsck
Checking object directories: 100% (256/256), done.
fish: 'git fsck' terminated by signal SIGSEGV (Address boundary error)
~/D/S/A/HLT $ git --version
git version 2.10.0.129.g35f6318
Running git-fsck from 2.9.2 validates the repository data.
Bisect says:
8261e1f139db3f8aa6f9fd7d98c876cbeb0f927c is the first bad commit
commit 8261e1f139db3f8aa6f9fd7d98c876cbeb0f927c
Author: Jeff King <peff@peff.net>
Date: Mon Aug 22 18:00:07 2016 -0400
delta_base_cache: use hashmap.h
Backtrace for the `log -u` case is below. I'll follow up with my progress.
-Jonathon
$ lldb /Users/jmah/Documents/Streams/git/git-log -- -u
(lldb) target create "/Users/jmah/Documents/Streams/git/git-log"
Current executable set to '/Users/jmah/Documents/Streams/git/git-log' (x86_64).
(lldb) settings set -- target.run-args "-u"
(lldb) process launch -o /dev/null
Process 92815 launched: '/Users/jmah/Documents/Streams/git/git-log' (x86_64)
Process 92815 stopped
* thread #1: tid = 0x1c30677, 0x00000001001bba80 git-log`release_delta_base_cache(ent=0xffffffffffffffd0) + 16 at sha1_file.c:2171, queue = 'com.apple.main-thread', stop reason = EXC_BAD_ACCESS (code=1, address=0x10)
frame #0: 0x00000001001bba80 git-log`release_delta_base_cache(ent=0xffffffffffffffd0) + 16 at sha1_file.c:2171
2168
2169 static inline void release_delta_base_cache(struct delta_base_cache_entry *ent)
2170 {
-> 2171 free(ent->data);
2172 detach_delta_base_cache_entry(ent);
2173 }
2174
(lldb) bt
warning: could not load any Objective-C class information. This will significantly reduce the quality of type information available.
* thread #1: tid = 0x1c30677, 0x00000001001bba80 git-log`release_delta_base_cache(ent=0xffffffffffffffd0) + 16 at sha1_file.c:2171, queue = 'com.apple.main-thread', stop reason = EXC_BAD_ACCESS (code=1, address=0x10)
* frame #0: 0x00000001001bba80 git-log`release_delta_base_cache(ent=0xffffffffffffffd0) + 16 at sha1_file.c:2171
frame #1: 0x00000001001bcadf git-log`add_delta_base_cache(p=0x00000001006062f0, base_offset=1792781, base=0x000000015749a000, base_size=1617761, type=OBJ_BLOB) + 143 at sha1_file.c:2199
frame #2: 0x00000001001bc0d6 git-log`unpack_entry(p=0x00000001006062f0, obj_offset=1792781, final_type=0x00007fff5fbfe7fc, final_size=0x000000010185a5a0) + 1590 at sha1_file.c:2345
frame #3: 0x00000001001c2209 git-log`cache_or_unpack_entry(p=0x00000001006062f0, base_offset=2692554, base_size=0x000000010185a5a0, type=0x00007fff5fbfe7fc) + 73 at sha1_file.c:2162
frame #4: 0x00000001001bed8d git-log`read_packed_sha1(sha1="?c?????}\x0e'\x81҄MH;yP?, type=0x00007fff5fbfe7fc, size=0x000000010185a5a0) + 93 at sha1_file.c:2765
frame #5: 0x00000001001bcc17 git-log`read_object(sha1="?c?????}\x0e'\x81҄MH;yP?, type=0x00007fff5fbfe7fc, size=0x000000010185a5a0) + 119 at sha1_file.c:2813
frame #6: 0x00000001001be013 git-log`read_sha1_file_extended(sha1="?c?????}\x0e'\x81҄MH;yP?, type=0x00007fff5fbfe7fc, size=0x000000010185a5a0, flag=1) + 67 at sha1_file.c:2841
frame #7: 0x00000001001073ba git-log`read_sha1_file(sha1="?c?????}\x0e'\x81҄MH;yP?, type=0x00007fff5fbfe7fc, size=0x000000010185a5a0) + 42 at cache.h:1056
frame #8: 0x0000000100106ce6 git-log`diff_populate_filespec(s=0x000000010185a570, flags=2) + 1334 at diff.c:2845
frame #9: 0x0000000100106670 git-log`diff_filespec_is_binary(one=0x000000010185a570) + 160 at diff.c:2248
frame #10: 0x00000001001124bc git-log`builtin_diff(name_a="Applications/IDE/PlugIns/IDEPlugIns/IDEPlugIns.xcodeproj/project.pbxproj", name_b="Applications/IDE/PlugIns/IDEPlugIns/IDEPlugIns.xcodeproj/project.pbxproj", one=0x000000010185a570, two=0x0000000101878310, xfrm_msg="index e063d6f..288f95f 100644\n", must_show_header=0, o=0x00007fff5fbff4b8, complete_rewrite=0) + 1852 at diff.c:2383
frame #11: 0x00000001001116ce git-log`run_diff_cmd(pgm=0x0000000000000000, name="Applications/IDE/PlugIns/IDEPlugIns/IDEPlugIns.xcodeproj/project.pbxproj", other=0x0000000000000000, attr_path="Applications/IDE/PlugIns/IDEPlugIns/IDEPlugIns.xcodeproj/project.pbxproj", one=0x000000010185a570, two=0x0000000101878310, msg=0x00007fff5fbfed18, o=0x00007fff5fbff4b8, p=0x000000010186c130) + 734 at diff.c:3134
frame #12: 0x0000000100111350 git-log`run_diff(p=0x000000010186c130, o=0x00007fff5fbff4b8) + 720 at diff.c:3222
frame #13: 0x000000010010d75d git-log`diff_flush_patch(p=0x000000010186c130, o=0x00007fff5fbff4b8) + 157 at diff.c:4202
frame #14: 0x000000010010b9bc git-log`diff_flush(options=0x00007fff5fbff4b8) + 1148 at diff.c:4722
frame #15: 0x000000010014418b git-log`log_tree_diff_flush(opt=0x00007fff5fbfefc0) + 507 at log-tree.c:781
frame #16: 0x00000001001445fe git-log`log_tree_diff(opt=0x00007fff5fbfefc0, commit=0x0000000153506540, log=0x00007fff5fbfeed8) + 606 at log-tree.c:848
frame #17: 0x000000010014428e git-log`log_tree_commit(opt=0x00007fff5fbfefc0, commit=0x0000000153506540) + 238 at log-tree.c:877
frame #18: 0x0000000100064b89 git-log`cmd_log_walk(rev=0x00007fff5fbfefc0) + 185 at log.c:360
frame #19: 0x0000000100066405 git-log`cmd_log(argc=2, argv=0x00007fff5fbff9d0, prefix=0x0000000000000000) + 309 at log.c:682
frame #20: 0x000000010000274f git-log`run_builtin(p=0x0000000100264970, argc=2, argv=0x00007fff5fbff9d0) + 431 at git.c:352
frame #21: 0x0000000100001a9a git-log`handle_builtin(argc=2, argv=0x00007fff5fbff9d0) + 138 at git.c:539
frame #22: 0x00000001000017e4 git-log`cmd_main(argc=2, argv=0x00007fff5fbff9d0) + 116 at git.c:635
frame #23: 0x00000001000c9eb4 git-log`main(argc=2, argv=0x00007fff5fbff9d0) + 68 at common-main.c:40
frame #24: 0x00007fffd87ff255 libdyld.dylib`start + 1
^ permalink raw reply
* [RFC] extending pathspec support to submodules
From: Brandon Williams @ 2016-09-14 23:57 UTC (permalink / raw)
To: git, pclouds; +Cc: Brandon Williams
---
I've been trying to think through how we could potentially add pathspec support
for --recurse-submodule options (for builtins like ls-files or grep down the
line). This is something that could be useful if the user supply's a pathspec
that could match to a file in a submodule. We could match the submodule to the
pathspec and then fork the process to recursively run the command on the
submodule which can be passed a modified pathspec.
For example with a pathspec 'sub/dir/a', where sub is a submodule in the root
directory of the supermodule's repo, we could match 'sub' to that spec and then
recursively call the git command with a pathspec of 'dir/a'. The child process
would then have the responsibility of matching 'dir/a' to files in its repo.
Does this seem like a reasonable feature to add? And if so are how is my
initial approach at solving the problem?
One idea I had was to add a submodule match flag in order to perform special
matching just in the --recurse-submodules cases since we'll want somethings to
match here that wouldn't normally match.
@@ -283,6 +284,29 @@ static int match_pathspec_item(const struct pathspec_item *item, int prefix,
item->nowildcard_len - prefix))
return MATCHED_FNMATCH;
+ /*
+ * Preform some checks to see if "name" is a super set of the pathspec
+ */
+ if (flags & DO_MATCH_SUBMODULE) {
+ struct strbuf buf = STRBUF_INIT;
+ strbuf_addstr(&buf, name);
+ strbuf_addch(&buf, '/');
+ /*
+ * Check if the name is a prefix of the pathspec
+ */
+ if ((item->match[namelen] == '/') &&
+ !ps_strncmp(item, match, name, namelen))
+ return MATCHED_RECURSIVELY;
+ /*
+ * Check if the name wildmatches to the pathspec
+ */
+ if (!wildmatch(item->match, buf.buf,
+ WM_PREFIX |
+ (item->magic & PATHSPEC_ICASE ? WM_CASEFOLD : 0),
+ NULL));
+ return MATCHED_FNMATCH;
+ }
+
return 0;
}
One of the main difficulties I was having is figuring out how wildmatching
should be applied in this case. What I believe we want is the ability for the
whole name of the submodule to match a prefix of the pathspec pattern. To do
this I was thinking of adding a flag to do prefix matching to the wildmatch
function like so:
diff --git a/wildmatch.c b/wildmatch.c
index 57c8765..f1e1725 100644
--- a/wildmatch.c
+++ b/wildmatch.c
@@ -60,8 +60,12 @@ static int dowild(const uchar *p, const uchar *text, unsigned int flags)
for ( ; (p_ch = *p) != '\0'; text++, p++) {
int matched, match_slash, negated;
uchar t_ch, prev_ch;
- if ((t_ch = *text) == '\0' && p_ch != '*')
- return WM_ABORT_ALL;
+ if ((t_ch = *text) == '\0' && p_ch != '*') {
+ if ((flags & WM_PREFIX) && (*(p-1) == '/'))
+ return WM_MATCH;
+ else
+ return WM_ABORT_ALL;
+ }
if ((flags & WM_CASEFOLD) && ISUPPER(t_ch))
t_ch = tolower(t_ch);
if ((flags & WM_CASEFOLD) && ISUPPER(p_ch))
diff --git a/wildmatch.h b/wildmatch.h
index 4090c8f..490db51 100644
--- a/wildmatch.h
+++ b/wildmatch.h
@@ -3,6 +3,7 @@
#define WM_CASEFOLD 1
#define WM_PATHNAME 2
+#define WM_PREFIX 4
#define WM_ABORT_MALFORMED 2
#define WM_NOMATCH 1
--
Any comments or thoughts on this would be appreciated.
Thanks,
Brandon
^ permalink raw reply related
* [PATCH 2/2] use zstd zlib wrapper
From: Jeff King @ 2016-09-14 23:58 UTC (permalink / raw)
To: git
In-Reply-To: <20160914235547.h3n2otje2hec6u7k@sigill.intra.peff.net>
There's a fancy new compression algorithm called "zstd". The
idea is that it's supposed to get similar compression ratios
to zlib, but with much faster compression and decompression
times. And on top of that, a nice sliding scale to trade off
size versus time on the compression side.
The zstd site at https://facebook.github.io/zstd/ claims
close to 3x speedup for both compression and decompression
versus zlib, with similar compression ratios. There are
other fast algorithms (like lz4), but they usually compress
much worse (follow the link above for a nice table of
results).
Since any git operations that have to access objects need to
do a zlib inflate, in theory we can speed up everything by
using zstd. And then on the packing side, use higher
compression levels when making on-disk packfiles (which will
be accessed many times) and lower ones when making loose
objects, or deflating packed objects on the fly when serving
fetches.
The catch, of course, is that it's a new incompatible
format. This would be a pretty huge change and totally break
backwards compatibility for git, not just on disk but
on-the-wire as well. So my goal here was not a finished
product but just a quick experiment to see if it did indeed
bring the promise speedups.
Disappointingly, the answer seems to be "no".
The patch is relatively straightforward. zstd ships with a
zlib-compatible wrapper that we can just plug in. For
compression, it writes with zlib by default, but you can
flip it to zstd mode with a run-time flag. For
decompression, it auto-detects and runs the correct choice
between zlib and zstd. So this patch _could_ serve as the
basis for a real solution. We'd bundle zstd and its wrapper
with the runtime switch off, and then after several years
people could enable zstd writing to cut off older clients.
In the patch below, that switch is just setting the
pack.compression level to "zstd1", "zstd2", etc.
I ran a series of tests where I would:
1. Repack with "-F" and a particular pack.compression
setting, measuring both the time to repack and the size
of the resulting pack.
2. Time "git rev-list --objects --all" to see the effect
on commit/tree traversal.
3. Run "log -Sfoo --raw" to see the additional effect on
accessing the blobs.
All the timings below are against linux.git. The rev-list
timings are best-of-five, but I didn't do multiple timings
for the repack or "log -S" tests, because they were so
painfully slow (and because the effect sizes seemed to be
larger than run-to-run noise).
I did these on top of my delta-cache and pack-mru patches,
since that would amplify any effect we see (i.e.,
those speed up other things so decompression time is a
relatively larger share of the pie).
Here's a baseline run using stock zlib, with the default
compression settings:
[zlib]
- pack
1122845190
- repack
real 4m43.817s
user 17m32.396s
sys 0m5.964s
- rev-list --objects --all
real 0m27.378s
user 0m27.132s
sys 0m0.240s
- log -Sfoo --raw
real 5m3.490s
user 5m0.040s
sys 0m3.424s
You can see that we're pretty much CPU bound. You can also
see that there's quite a bit of parallelism going on in the
repack (this is a quad-core with hyperthreading).
Here's the same thing built against the zstd wrapper, but
still using zlib:
[zstd, disabled]
- pack
1123089975
(+0.02%)
- repack
real 4m58.263s
user 17m50.596s
sys 0m7.200s
(+5%)
- rev-list --objects --all
real 0m28.143s
user 0m27.864s
sys 0m0.272s
(+3%)
- log -Sfoo --raw
real 5m12.742s
user 5m9.804s
sys 0m2.912s
(+3%)
The percentages show the difference against the baseline run
above (wall-clock times for the timing results). The pack
size is similar, which we'd expect (it's not identical
because the threaded delta search is slightly
non-deterministic).
Sadly we lose 3-5% just to the wrapper sitting in front of
zlib. So that's not a great start. But it also means we
might be able to recover that through code optimizations of
the wrapper, the way we call it, etc.
Now here it is with zstd turned onto its lowest setting:
[zstd, 1]
- pack
1199180502
(+7%)
- repack
real 4m11.740s
user 16m36.510s
sys 0m7.700s
(-11%)
- rev-list --objects --all
real 0m25.870s
user 0m25.632s
sys 0m0.232s
(-5.5%)
- log -Sfoo --raw
real 4m10.899s
user 4m7.788s
sys 0m3.056s
(-17%)
We've definitely traded off some space here. But the
compression is faster, _and_ so is the decompression.
So let's bump up the compression level a bit:
[zstd, 3]
- pack
1162416903
(+3.5%)
- repack
real 5m1.477s
user 19m34.476s
sys 0m17.720s
(+6.2%)
- rev-list --objects --all
real 0m25.716s
user 0m25.468s
sys 0m0.244s
(-6%)
- log -Sfoo --raw
real 4m10.547s
user 4m6.500s
sys 0m3.884s
(-17%)
OK, the size is improving. But now our compression is
noticeably slower. The decompression remains better, which
is nice. Let's go further:
[zstd, 5]
- pack
1144433165
(+2%)
- repack
real 5m46.050s
user 22m7.580s
sys 1m2.212s
(+22%)
- rev-list --objects --all
real 0m25.717s
user 0m25.404s
sys 0m0.304s
(-6%)
- log -Sfoo --raw
real 4m14.651s
user 4m11.176s
sys 0m3.448s
(-16%)
Getting close to parity on the size. But the compression
time keeps creeping up.
[zstd, 10]
- pack
1132922195
(+0.8%)
- repack
real 41m35.835s
user 166m36.956s
sys 50m11.436s
(+780%)
- rev-list --objects --all
real 0m25.946s
user 0m25.656s
sys 0m0.284s
(-5%)
- log -Sfoo --raw
real 4m8.490s
user 4m5.864s
sys 0m2.604s
(-18%)
Oof. We still haven't reached size parity, and now we're
using a _lot_ more CPU on the repack.
I wanted to take it all the way to "22", zstd's maximum,
just for fun. But after 2 hours (real time, so pegging all
my cores while doing so), we were only at 12% of the delta
phase. Yikes.
So...it doesn't seem like a great tradeoff (especially
considering the costs in migrating to a new algorithm). The
decompression _is_ delightfully faster, and it's true that
we decompress more than we compress. But the speed/size
tradeoff for compression doesn't seem great.
I'm not sure why we don't get numbers as nice as the ones on
the zstd website. Obviously possibility 0 is that I simply
screwed something up. But here are some other thoughts /
wild guesses:
1. There's clearly some overhead to using the zlib
wrapper. Even just calling into zlib with it to
compress seems to cost about 5%.
For zstd, it's less clear. The regular (non-wrapper)
zstd API seems to involve allocating a zstd compressor,
and then feeding multiple compressions to it. So that
would amortize the setup cost. I'd guess that the zlib
wrapper has to allocate a new zstd compressor for each
wrapped compression (obviously they could share one,
but then you run into threading issues).
The fact that the time increases as we bump the
compression level implies to me that the main cost is
in the actual compression, though, not in the setup.
It's possible that the setup cost is higher when the
compression level is higher, but I wouldn't really
expect it to scale as rapidly as my numbers show.
So this is probably making things worse than they
otherwise could be, but I doubt it's the real cause of
the problems.
2. Git tends to do a lot of relatively small compressions,
as we zlib-compress deltas. So I wonder (and this is a
wild guess) if zstd performs more favorably on "normal"
sized inputs.
That would be possible to test. Or could probably be
found out by digging more into the zstd benchmarks;
they used some standard corpuses, but the results are
summarized. More broken-down results may show patterns.
I didn't do those things, hence "wild guess".
And obviously even this patch series _does_ show an
improvement on the decompression side (which happens a lot
more often than compression!). So this isn't a total dead
end. But given the ratio/time tradeoff I saw on the
compression side, there are other options. E.g., lz4 is even
faster than zstd, but comes with a size penalty (though I
think it's more like 30%, not 7%).
I would have liked to test with lz4, too, but they don't
ship a totally trivial zlib wrapper. :)
Signed-off-by: Jeff King <peff@peff.net>
---
Makefile | 9 +++++++++
builtin/pack-objects.c | 19 +++++++++++++------
cache.h | 4 ++++
3 files changed, 26 insertions(+), 6 deletions(-)
diff --git a/Makefile b/Makefile
index 99e0eb2..e29edbe 100644
--- a/Makefile
+++ b/Makefile
@@ -1138,6 +1138,15 @@ else
endif
IMAP_SEND_LDFLAGS += $(OPENSSL_LINK) $(OPENSSL_LIBSSL) $(LIB_4_CRYPTO)
+# for linking straight out of zstd build directory
+ifdef ZSTD_PATH
+ LIB_OBJS += $(ZSTD_PATH)/zlibWrapper/zstd_zlibwrapper.o
+ BASIC_CFLAGS += -I$(ZSTD_PATH)/zlibWrapper
+ BASIC_CFLAGS += -I$(ZSTD_PATH)/lib -I$(ZSTD_PATH)/lib/common
+ BASIC_CFLAGS += -DUSE_ZSTD
+ EXTLIBS += $(ZSTD_PATH)/lib/libzstd.a
+endif
+
ifdef ZLIB_PATH
BASIC_CFLAGS += -I$(ZLIB_PATH)/include
EXTLIBS += -L$(ZLIB_PATH)/$(lib) $(CC_LD_DYNPATH)$(ZLIB_PATH)/$(lib)
diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
index 4a63398..b424465 100644
--- a/builtin/pack-objects.c
+++ b/builtin/pack-objects.c
@@ -62,6 +62,7 @@ static int num_preferred_base;
static struct progress *progress_state;
static int pack_compression_level = Z_DEFAULT_COMPRESSION;
static int pack_compression_seen;
+static int pack_compression_max = Z_BEST_COMPRESSION;
static struct packed_git *reuse_packfile;
static uint32_t reuse_packfile_objects;
@@ -2220,11 +2221,17 @@ static int git_pack_config(const char *k, const char *v, void *cb)
return 0;
}
if (!strcmp(k, "pack.compression")) {
- int level = git_config_int(k, v);
- if (level == -1)
- level = Z_DEFAULT_COMPRESSION;
- else if (level < 0 || level > Z_BEST_COMPRESSION)
- die("bad pack compression level %d", level);
+ int level;
+ if (!v)
+ return config_error_nonbool(k);
+#ifdef USE_ZSTD
+ if (skip_prefix(v, "zstd", &v)) {
+ useZSTD(1);
+ /* XXX doesn't seem to be a constant? */
+ pack_compression_max = 22;
+ }
+#endif
+ level = git_config_int(k, v);
pack_compression_level = level;
pack_compression_seen = 1;
return 0;
@@ -2765,7 +2772,7 @@ int cmd_pack_objects(int argc, const char **argv, const char *prefix)
reuse_delta = 0;
if (pack_compression_level == -1)
pack_compression_level = Z_DEFAULT_COMPRESSION;
- else if (pack_compression_level < 0 || pack_compression_level > Z_BEST_COMPRESSION)
+ else if (pack_compression_level < 0 || pack_compression_level > pack_compression_max)
die("bad pack compression level %d", pack_compression_level);
if (!delta_search_threads) /* --threads=0 means autodetect */
diff --git a/cache.h b/cache.h
index 3556326..9b2f8f5 100644
--- a/cache.h
+++ b/cache.h
@@ -37,7 +37,11 @@
#define git_SHA1_Update git_SHA1_Update_Chunked
#endif
+#ifdef USE_ZSTD
+#include "zstd_zlibwrapper.h"
+#else
#include <zlib.h>
+#endif
typedef struct git_zstream {
z_stream z;
unsigned long avail_in;
--
2.10.0.271.ge3ee153
^ permalink raw reply related
* [PATCH 1/2] obj_hash: convert to a critbit tree
From: Jeff King @ 2016-09-14 23:56 UTC (permalink / raw)
To: git
In-Reply-To: <20160914235547.h3n2otje2hec6u7k@sigill.intra.peff.net>
For operations that traverse the whole reachability graph,
like "rev-list --objects", the obj_hash in object.c shows up
as a hotspot. We basically have to do "nr_commits *
size_of_tree" hash lookups, because each tree we look at, we
need to say "have we seen this sha1 yet?" (it's a little
more complicated because we avoid digging into sub-trees we
know we've already seen, but it's a reasonable
approximation). So graph traversal operations like that are
very sensitive to improvements in lookup time.
For keys with length "m", a hash table is generally O(m) to
compute the hash (and verify that you've found the correct
key), and O(1) in finding the correct slot. The latter is
subject to collisions and probing strategy, but we keep the
load factor on the obj_hash table below 50%, which is quite
low. And we have a good hash (we reuse the sha1s themselves,
which are effectively random). So we'd expect relatively few
collisions, and past experiments to tweak the probing
strategy haven't yielded any benefit (we use linear probing;
I tried quadratic probing at one point, and Junio tried
cuckoo hashing).
Another data structure with similar properties is sometimes
known as a "critbit tree" (see http://cr.yp.to/critbit.html
for discussion and history). The basic idea is a binary trie
where each node is either an internal node, representing a
single bit of a stored key, or a leaf node, representing one
or more "remainder" bits. So if you were storing two bit
sequences 1011 and "1100", you'd have three nodes (besides
the root):
(root)
/ \
0 1
/ \
NULL (internal)
/ \
0 1
/ \
"11" "00"
So finding "1011" involves traversing the trie: down the "1"
side, then the "0" side, and then check that the rest
matches "11".
Looking up a key is O(m), and there's no issue with
collisions or probing. It can use less memory than a hash
table, because there's no load factor to care about.
That's the good news. The bad news is that big-O analysis
is not the whole story. You'll notice that we have to do a
lot of conditional pointer-jumping to walk the trie. Whereas
a hash table can jump right to a proposed key and do a
memcmp() to see if we got it.
So I wasn't overly optimistic that this would be any faster.
And indeed it's not. It's about three times slower (about
4.5s versus 1.5s running "rev-list --objects --all" on
git.git).
The reason is, I think, a combination of:
0. We care much more about performance for this hash than
memory efficiency. So we keep the load factor
quite low, and thus reduce the number of collisions.
1. For many hash tables, computing the hash is expensive.
Not so here, because we are storing sha1s. So it is
literally just casting the first 4 bytes of the sha1 to
an int; we don't even look at the other bytes until the
collision check (and because of point 0, we don't
generally have to do very many collision checks during
our probe).
2. The collision check _does_ have to look at all 20 bytes
of the sha1. And we may have to do it multiple times as
we linearly probe the collisions. _But_ it can be done
with memcmp(), which is optimized to compare 32 or 64
bits at a time. So we our O(m) has a very nice constant
factor.
Whereas in the critbit tree, we pay an instruction for
_each_ bit we look at.
It's possible that (2) would be better if instead of a
critbit tree, we used a "critbyte" tree. That would involve
fewer node traversals, at the cost of making each node
larger (right now the internal nodes store 2 pointer slots;
they'd have to store 256 to handle a full byte). I didn't
try it, but I suspect it would still be slower for the same
reasons.
The code is below for reference. I pulled the critbit
implementation from:
https://github.com/ennorehling/critbit
but made a few tweaks:
- the critbit does not store the keys themselves, as we
already have them in the "struct object", and do not
want to waste space. As a result, I baked in some
assumptions about the 20-byte key-length (which is fine
for our purposes here).
- rather than malloc() each node, I used a pool allocator
(to try to keep the nodes closer together in cache)
Signed-off-by: Jeff King <peff@peff.net>
---
Makefile | 1 +
critbit.c | 350 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
critbit.h | 60 +++++++++++
object.c | 85 ++-------------
4 files changed, 417 insertions(+), 79 deletions(-)
create mode 100644 critbit.c
create mode 100644 critbit.h
diff --git a/Makefile b/Makefile
index 7f18492..99e0eb2 100644
--- a/Makefile
+++ b/Makefile
@@ -720,6 +720,7 @@ LIB_OBJS += connected.o
LIB_OBJS += convert.o
LIB_OBJS += copy.o
LIB_OBJS += credential.o
+LIB_OBJS += critbit.o
LIB_OBJS += csum-file.o
LIB_OBJS += ctype.o
LIB_OBJS += date.o
diff --git a/critbit.c b/critbit.c
new file mode 100644
index 0000000..7354375
--- /dev/null
+++ b/critbit.c
@@ -0,0 +1,350 @@
+/*
+Copyright (c) 2012, Enno Rehling <enno@eressea.de>
+
+Permission to use, copy, modify, and/or distribute this software for any
+purpose with or without fee is hereby granted, provided that the above
+copyright notice and this permission notice appear in all copies.
+
+THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES
+WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF
+MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR
+ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
+WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN
+ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
+OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
+**/
+
+#include "cache.h"
+#include "critbit.h"
+
+/* see http://cr.yp.to/critbit.html */
+struct critbit_node {
+ void * child[2];
+ size_t byte;
+ unsigned int mask;
+};
+
+#define EXTERNAL_NODE 0
+#define INTERNAL_NODE 1
+
+static int decode_pointer(void ** ptr)
+{
+ ptrdiff_t numvalue = (char*)*ptr - (char*)0;
+ if (numvalue & 1) {
+ *ptr = (void*)(numvalue - 1);
+ return EXTERNAL_NODE;
+ }
+ return INTERNAL_NODE;
+}
+
+static void * make_external_node(const void * key, size_t keylen)
+{
+ return (unsigned char *)key + 1;
+}
+
+static void from_external_node(void * ptr, void **key, size_t *keylen)
+{
+ /* eek */
+ *keylen = 20;
+ *key = ptr;
+}
+
+static struct critbit_node *node_pool;
+static int node_pool_used;
+static int node_pool_alloc;
+
+static struct critbit_node * make_internal_node(void)
+{
+ if (node_pool_used == node_pool_alloc) {
+ node_pool_alloc = 1024;
+ node_pool_used = 0;
+ ALLOC_ARRAY(node_pool, node_pool_alloc);
+ }
+ return &node_pool[node_pool_used++];
+}
+
+static int cb_less(const struct critbit_node * a, const struct critbit_node * b)
+{
+ return a->byte < b->byte || (a->byte == b->byte && a->mask < b->mask);
+}
+
+int cb_insert(critbit_tree * cb, const void * key, size_t keylen)
+{
+ assert(cb);
+ assert(key);
+ if (!cb->root) {
+ cb->root = make_external_node(key, keylen);
+ return CB_SUCCESS;
+ }
+ else {
+ void ** iter = &cb->root;
+ struct critbit_node * prev = 0;
+ for (;;) {
+ void * ptr = *iter;
+ if (decode_pointer(&ptr) == INTERNAL_NODE) {
+ struct critbit_node * node = (struct critbit_node *)ptr;
+ unsigned char * bytes = (unsigned char *)key;
+ int branch = (keylen <= node->byte) ? 0 : ((1 + ((bytes[node->byte] | node->mask) & 0xFF)) >> 8);
+ iter = &node->child[branch];
+ prev = node;
+ }
+ else {
+ unsigned char *iptr, *bytes = (unsigned char *)key, *ikey = bytes;
+ void * vptr;
+ unsigned int mask, byte = 0;
+ int branch;
+ size_t len;
+ struct critbit_node * node;
+
+ from_external_node(ptr, &vptr, &len);
+
+ for (iptr = vptr; byte < keylen && byte < len && *ikey == *iptr;) {
+ ++ikey;
+ ++iptr;
+ ++byte;
+ }
+
+ if (byte == keylen && byte == len) {
+ return CB_EXISTS; /* duplicate entry */
+ }
+ node = make_internal_node();
+ node->byte = byte;
+ mask = *ikey ^ *iptr; /* these are all the bits that differ */
+ mask |= mask >> 1;
+ mask |= mask >> 2;
+ mask |= mask >> 4; /* now, every bit up to the MSB is set to 1 */
+ mask = (mask&~(mask >> 1)) ^ 0xFF;
+ node->mask = (unsigned char)mask;
+
+ /* find the right place to insert, iff prev's crit-bit is later in the string than new crit-bit */
+ if (prev && cb_less(node, prev)) {
+ for (iter = &cb->root;;) {
+ ptr = *iter;
+ if (decode_pointer(&ptr) == INTERNAL_NODE) {
+ struct critbit_node * next = (struct critbit_node *)ptr;
+ if (cb_less(next, node)) {
+ branch = ((1 + ((bytes[next->byte] | next->mask) & 0xFF)) >> 8);
+ iter = &next->child[branch];
+ }
+ else {
+ break;
+ }
+ }
+ else {
+ assert(!"should never get here");
+ }
+ }
+ }
+
+ branch = ((1 + ((*ikey | node->mask) & 0xFF)) >> 8);
+ node->child[branch] = make_external_node(key, keylen);
+ node->child[1 - branch] = *iter;
+ *iter = (void *)node;
+
+ return CB_SUCCESS;
+ }
+ }
+ }
+}
+
+static void * cb_find_top_i(const critbit_tree * cb, const void * key, size_t keylen)
+{
+ void *ptr, *top = 0;
+ assert(key);
+
+ if (!cb->root) {
+ return 0;
+ }
+ for (ptr = cb->root, top = cb->root;;) {
+ void * last = ptr;
+ if (decode_pointer(&ptr) == INTERNAL_NODE) {
+ struct critbit_node * node = (struct critbit_node *)ptr;
+ int branch;
+ if (keylen <= node->byte) {
+ break;
+ }
+ else {
+ unsigned char * bytes = (unsigned char *)key;
+ top = last;
+ branch = (1 + ((bytes[node->byte] | node->mask) & 0xFF)) >> 8;
+ ptr = node->child[branch];
+ }
+ }
+ else {
+ /* we reached an external node before exhausting the key length */
+ top = last;
+ break;
+ }
+ }
+ return top;
+}
+
+static int cb_find_prefix_i(void * ptr, const void * key, size_t keylen, void ** results, int numresults, int * offset, int next)
+{
+ assert(next <= numresults);
+ if (next == numresults) {
+ return next;
+ }
+ else if (decode_pointer(&ptr) == INTERNAL_NODE) {
+ struct critbit_node * node = (struct critbit_node *)ptr;
+ next = cb_find_prefix_i(node->child[0], key, keylen, results, numresults, offset, next);
+ if (next < numresults) {
+ next = cb_find_prefix_i(node->child[1], key, keylen, results, numresults, offset, next);
+ }
+ }
+ else {
+ /* reached an external node */
+ char * str;
+ void * vptr;
+ size_t len;
+
+ from_external_node(ptr, &vptr, &len);
+ str = vptr;
+ if (len >= keylen && memcmp(key, str, keylen) == 0) {
+ if (*offset>0) {
+ --*offset;
+ }
+ else {
+ results[next++] = str;
+ }
+ }
+ }
+ return next;
+}
+
+int cb_find_prefix(const critbit_tree * cb, const void * key, size_t keylen, void ** results, int numresults, int offset)
+{
+ if (numresults > 0) {
+ void *top = cb_find_top_i(cb, key, keylen);
+ if (top) {
+ /* recursively add all children except the ones from [0-offset) of top to the results */
+ return cb_find_prefix_i(top, key, keylen, results, numresults, &offset, 0);
+ }
+ }
+ return 0;
+}
+
+static int cb_foreach_i(void * ptr, const void * key, size_t keylen, int(*match_cb)(const void * match, const void * key, size_t keylen, void *), void *data)
+{
+ int result = 0;
+
+ if (decode_pointer(&ptr) == INTERNAL_NODE) {
+ struct critbit_node * node = (struct critbit_node *)ptr;
+ result = cb_foreach_i(node->child[0], key, keylen, match_cb, data);
+ if (!result) {
+ result = cb_foreach_i(node->child[1], key, keylen, match_cb, data);
+ }
+ }
+ else {
+ /* reached an external node */
+ void * match;
+ size_t len;
+
+ from_external_node(ptr, &match, &len);
+ if (len >= keylen && memcmp(key, match, keylen) == 0) {
+ return match_cb(match, key, keylen, data);
+ }
+ }
+ return result;
+}
+
+int cb_foreach(critbit_tree * cb, const void * key, size_t keylen, int(*match_cb)(const void * match, const void * key, size_t keylen, void *), void *data)
+{
+ void *top = cb_find_top_i(cb, key, keylen);
+ if (top) {
+ /* recursively add all children except the ones from [0-offset) of top to the results */
+ return cb_foreach_i(top, key, keylen, match_cb, data);
+ }
+ return 0;
+}
+
+const void * cb_find(critbit_tree * cb, const void * key, size_t keylen)
+{
+ void * str;
+ size_t len;
+ unsigned char * bytes = (unsigned char *)key;
+ void * ptr;
+
+ assert(cb);
+ assert(key);
+ if (!cb->root) return 0;
+ for (ptr = cb->root; decode_pointer(&ptr) == INTERNAL_NODE; ) {
+ struct critbit_node *node = (struct critbit_node *)ptr;
+ uint8_t c = 0;
+ int direction;
+ if (node->byte < keylen)
+ c = bytes[node->byte];
+ direction = (1+(c|node->mask))>>8;
+ ptr = node->child[direction];
+ }
+ from_external_node(ptr, &str, &len);
+ if (len >= keylen && memcmp(key, str, keylen) == 0) {
+ return str;
+ }
+ return 0;
+}
+
+int cb_erase(critbit_tree * cb, const void * key, size_t keylen)
+{
+ void **iter = NULL;
+ void *ptr = cb->root;
+ struct critbit_node *parent = 0;
+ unsigned char * bytes = (unsigned char *)key;
+ int branch;
+
+ if (!cb->root) return CB_NOTFOUND;
+
+ for (;;) {
+ int type;
+
+ type = decode_pointer(&ptr);
+ if (type == INTERNAL_NODE) {
+ iter = parent ? &parent->child[branch] : &cb->root;
+ parent = (struct critbit_node*)ptr;
+ branch = (keylen <= parent->byte) ? 0 : ((1 + ((bytes[parent->byte] | parent->mask) & 0xFF)) >> 8);
+ ptr = parent->child[branch];
+ }
+ else {
+ void * str;
+ size_t len;
+ from_external_node(ptr, &str, &len);
+ if (len == keylen && memcmp(key, str, len) == 0) {
+ free(ptr);
+ if (iter) {
+ *iter = parent->child[1 - branch];
+ free(parent);
+ }
+ else {
+ cb->root = 0;
+ }
+ return CB_SUCCESS;
+ }
+ return CB_NOTFOUND;
+ }
+ }
+}
+
+size_t cb_new_kv(const char *key, size_t keylen, const void * value, size_t len, void * out)
+{
+ char * dst = (char*)out;
+ if (dst != key) {
+ memmove(dst, key, keylen);
+ }
+ dst[keylen] = 0;
+ memmove(dst + keylen + 1, value, len);
+ return len + keylen + 1;
+}
+
+void cb_get_kv(const void *kv, void * value, size_t len)
+{
+ const char * key = (const char *)kv;
+ size_t keylen = strlen(key) + 1;
+ memmove(value, key + keylen, len);
+}
+
+void cb_get_kv_ex(void *kv, void ** value)
+{
+ char * key = (char *)kv;
+ size_t keylen = strlen(key) + 1;
+ *value = key + keylen;
+}
diff --git a/critbit.h b/critbit.h
new file mode 100644
index 0000000..ee774e8
--- /dev/null
+++ b/critbit.h
@@ -0,0 +1,60 @@
+/*
+Copyright (c) 2012, Enno Rehling <enno@eressea.de>
+
+Permission to use, copy, modify, and/or distribute this software for any
+purpose with or without fee is hereby granted, provided that the above
+copyright notice and this permission notice appear in all copies.
+
+THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES
+WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF
+MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR
+ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
+WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN
+ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
+OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
+**/
+
+#ifndef _CRITBITT_H
+#define _CRITBITT_H
+
+#ifdef __cplusplus
+extern "C" {
+#endif
+
+#include <stddef.h>
+
+typedef struct critbit_tree {
+ void * root;
+} critbit_tree;
+
+#define CB_SUCCESS 0
+#define CB_EXISTS 1
+#define CB_NOTFOUND 2
+
+#define CRITBIT_TREE() { 0 }
+
+int cb_insert(critbit_tree * cb, const void * key, size_t keylen);
+const void * cb_find(critbit_tree * cb, const void * key, size_t keylen);
+int cb_erase(critbit_tree * cb, const void * key, size_t keylen);
+int cb_find_prefix(const critbit_tree * cb, const void * key, size_t keylen, void ** results, int numresults, int offset);
+int cb_foreach(critbit_tree * cb, const void * key, size_t keylen, int (*match_cb)(const void * match, const void * key, size_t keylen, void *), void *data);
+void cb_clear(critbit_tree * cb);
+
+#define cb_insert_str(cb, key) \
+ cb_insert(cb, (void *)key, strlen(key)+1)
+#define cb_find_str(cb, key) \
+ (const char *)cb_find(cb, (void *)key, strlen(key)+1)
+#define cb_erase_str(cb, key) \
+ cb_erase(cb, (void *)key, strlen(key)+1)
+#define cb_find_prefix_str(cb, key, results, numresults, offset) \
+ cb_find_prefix(cb, (const void *)key, strlen(key), results, numresults, offset)
+
+#define CB_KV_SIZE(keylen, datalen) (keylen+datalen+1)
+size_t cb_new_kv(const char *key, size_t keylen, const void * value, size_t len, void * out);
+void cb_get_kv(const void *kv, void * value, size_t len);
+void cb_get_kv_ex(void *kv, void ** value);
+
+#ifdef __cplusplus
+}
+#endif
+#endif
diff --git a/object.c b/object.c
index 67d9a9e..24d26a8 100644
--- a/object.c
+++ b/object.c
@@ -4,6 +4,7 @@
#include "tree.h"
#include "commit.h"
#include "tag.h"
+#include "critbit.h"
static struct object **obj_hash;
static int nr_objs, obj_hash_size;
@@ -51,32 +52,8 @@ int type_from_string_gently(const char *str, ssize_t len, int gentle)
die("invalid object type \"%s\"", str);
}
-/*
- * Return a numerical hash value between 0 and n-1 for the object with
- * the specified sha1. n must be a power of 2. Please note that the
- * return value is *not* consistent across computer architectures.
- */
-static unsigned int hash_obj(const unsigned char *sha1, unsigned int n)
-{
- return sha1hash(sha1) & (n - 1);
-}
-
-/*
- * Insert obj into the hash table hash, which has length size (which
- * must be a power of 2). On collisions, simply overflow to the next
- * empty bucket.
- */
-static void insert_obj_hash(struct object *obj, struct object **hash, unsigned int size)
-{
- unsigned int j = hash_obj(obj->oid.hash, size);
- while (hash[j]) {
- j++;
- if (j >= size)
- j = 0;
- }
- hash[j] = obj;
-}
+struct critbit_tree objects;
/*
* Look up the record for the given sha1 in the hash map stored in
@@ -84,58 +61,10 @@ static void insert_obj_hash(struct object *obj, struct object **hash, unsigned i
*/
struct object *lookup_object(const unsigned char *sha1)
{
- unsigned int i, first;
- struct object *obj;
-
- if (!obj_hash)
+ const unsigned char *ret = cb_find(&objects, sha1, 20);
+ if (!ret)
return NULL;
-
- first = i = hash_obj(sha1, obj_hash_size);
- while ((obj = obj_hash[i]) != NULL) {
- if (!hashcmp(sha1, obj->oid.hash))
- break;
- i++;
- if (i == obj_hash_size)
- i = 0;
- }
- if (obj && i != first) {
- /*
- * Move object to where we started to look for it so
- * that we do not need to walk the hash table the next
- * time we look for it.
- */
- struct object *tmp = obj_hash[i];
- obj_hash[i] = obj_hash[first];
- obj_hash[first] = tmp;
- }
- return obj;
-}
-
-/*
- * Increase the size of the hash map stored in obj_hash to the next
- * power of 2 (but at least 32). Copy the existing values to the new
- * hash map.
- */
-static void grow_object_hash(void)
-{
- int i;
- /*
- * Note that this size must always be power-of-2 to match hash_obj
- * above.
- */
- int new_hash_size = obj_hash_size < 32 ? 32 : 2 * obj_hash_size;
- struct object **new_hash;
-
- new_hash = xcalloc(new_hash_size, sizeof(struct object *));
- for (i = 0; i < obj_hash_size; i++) {
- struct object *obj = obj_hash[i];
- if (!obj)
- continue;
- insert_obj_hash(obj, new_hash, new_hash_size);
- }
- free(obj_hash);
- obj_hash = new_hash;
- obj_hash_size = new_hash_size;
+ return (struct object *)(ret - offsetof(struct object, oid));
}
void *create_object(const unsigned char *sha1, void *o)
@@ -147,10 +76,8 @@ void *create_object(const unsigned char *sha1, void *o)
obj->flags = 0;
hashcpy(obj->oid.hash, sha1);
- if (obj_hash_size - 1 <= nr_objs * 2)
- grow_object_hash();
+ cb_insert(&objects, &obj->oid, 20);
- insert_obj_hash(obj, obj_hash, obj_hash_size);
nr_objs++;
return obj;
}
--
2.10.0.271.ge3ee153
^ permalink raw reply related
* Journal of Failed Git Experiments, Volume 1
From: Jeff King @ 2016-09-14 23:55 UTC (permalink / raw)
To: git
I try a lot of different experiments with git performance, some of them
more hare-brained than others. The ones that succeed end up as real
patches. But I hate for the ones that fail to die a quiet death. Then
nobody learns what _doesn't_ work, and nobody has the opportunity to
point out the spot where I made a stupid mistake that invalidates the
whole result.
Here are two performance experiments that did _not_ turn out. They
are in patch form, because I want to document exactly what I did
(especially because it helps in the "stupid mistake" case, or if
somebody wants to try building on my results).
So without further ado:
[1/2]: obj_hash: convert to a critbit tree
[2/2]: use zstd zlib wrapper
-Peff
^ permalink raw reply
* Re: [PATCH] Move format-patch base commit and prerequisites before email signature
From: Josh Triplett @ 2016-09-14 23:52 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Jeff King, git
In-Reply-To: <xmqqmvjaozrz.fsf@gitster.mtv.corp.google.com>
On Wed, Sep 14, 2016 at 03:57:36PM -0700, Junio C Hamano wrote:
> Junio C Hamano <gitster@pobox.com> writes:
>
> > I do not mind doing it myself, but I am already in today's
> > integration cycle (which will merge a handful of topics to
> > 'master'), so I won't get around to it for some time. If you are
> > inclined to, please be my guest ;-)
>
> I queued this on top for now; I think it can be just squashed into
> your patch. Please say "I agree" and I'll make it happen, or say
> "that's wrong" followed by a replacement patch ;-).
"I agree". :)
I'd suggest squashing in an *additional* patch to the testsuite to
ensure the presence of the blank line:
diff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh
index 535857e..8d90a6e 100755
--- a/t/t4014-format-patch.sh
+++ b/t/t4014-format-patch.sh
@@ -1515,8 +1515,9 @@ test_expect_success 'format-patch -o overrides format.outputDirectory' '
test_expect_success 'format-patch --base' '
git checkout side &&
- git format-patch --stdout --base=HEAD~3 -1 | tail -n 6 >actual &&
- echo "base-commit: $(git rev-parse HEAD~3)" >expected &&
+ git format-patch --stdout --base=HEAD~3 -1 | tail -n 7 >actual &&
+ echo >expected &&
+ echo "base-commit: $(git rev-parse HEAD~3)" >>expected &&
echo "prerequisite-patch-id: $(git show --patch HEAD~2 | git patch-id --stable | awk "{print \$1}")" >>expected &&
echo "prerequisite-patch-id: $(git show --patch HEAD~1 | git patch-id --stable | awk "{print \$1}")" >>expected &&
signature >> expected &&
^ permalink raw reply related
* What's cooking in git.git (Sep 2016, #04; Wed, 14)
From: Junio C Hamano @ 2016-09-14 23:01 UTC (permalink / raw)
To: git
Here are the topics that have been cooking. Commits prefixed with
'-' are only in 'pu' (proposed updates) while commits prefixed with
'+' are in 'next'. The ones marked with '.' do not appear in any of
the integration branches, but I am still holding onto them.
You can find the changes described here in the integration branches
of the repositories listed at
http://git-blame.blogspot.com/p/git-public-repositories.html
--------------------------------------------------
[Graduated to "master"]
* ep/use-git-trace-curl-in-tests (2016-09-07) 4 commits
(merged to 'next' on 2016-09-08 at 04372de)
+ t5551-http-fetch-smart.sh: use the GIT_TRACE_CURL environment var
+ t5550-http-fetch-dumb.sh: use the GIT_TRACE_CURL environment var
+ test-lib.sh: preserve GIT_TRACE_CURL from the environment
+ t5541-http-push-smart.sh: use the GIT_TRACE_CURL environment var
Update a few tests that used to use GIT_CURL_VERBOSE to use the
newer GIT_TRACE_CURL.
* jc/am-read-author-file (2016-08-30) 1 commit
(merged to 'next' on 2016-09-08 at d2db42f)
+ am: refactor read_author_script()
Extract a small helper out of the function that reads the authors
script file "git am" internally uses.
This by itself is not useful until a second caller appears in the
future for "rebase -i" helper.
* jc/forbid-symbolic-ref-d-HEAD (2016-09-02) 1 commit
(merged to 'next' on 2016-09-08 at cd8c1b3)
+ symbolic-ref -d: do not allow removal of HEAD
"git symbolic-ref -d HEAD" happily removes the symbolic ref, but
the resulting repository becomes an invalid one. Teach the command
to forbid removal of HEAD.
* jc/submodule-anchor-git-dir (2016-09-01) 1 commit
(merged to 'next' on 2016-09-08 at b6f20cf)
+ submodule: avoid auto-discovery in prepare_submodule_repo_env()
Having a submodule whose ".git" repository is somehow corrupt
caused a few commands that recurse into submodules loop forever.
* jk/diff-submodule-diff-inline (2016-08-31) 8 commits
(merged to 'next' on 2016-09-02 at 734e42c)
+ diff: teach diff to display submodule difference with an inline diff
+ submodule: refactor show_submodule_summary with helper function
+ submodule: convert show_submodule_summary to use struct object_id *
+ allow do_submodule_path to work even if submodule isn't checked out
+ diff: prepare for additional submodule formats
+ graph: add support for --line-prefix on all graph-aware output
+ diff.c: remove output_prefix_length field
+ cache: add empty_tree_oid object and helper function
The "git diff --submodule={short,log}" mechanism has been enhanced
to allow "--submodule=diff" to show the patch between the submodule
commits bound to the superproject.
* jk/squelch-false-warning-from-gcc-o3 (2016-08-31) 2 commits
(merged to 'next' on 2016-09-08 at c9a2af6)
+ color_parse_mem: initialize "struct color" temporary
+ error_errno: use constant return similar to error()
Compilation fix.
* jk/test-lib-drop-pid-from-results (2016-08-30) 1 commit
(merged to 'next' on 2016-09-08 at 0967b0b)
+ test-lib: drop PID from test-results/*.count
The test framework left the number of tests and success/failure
count in the t/test-results directory, keyed by the name of the
test script plus the process ID. The latter however turned out not
to serve any useful purpose. The process ID part of the filename
has been removed.
* js/t6026-clean-up (2016-09-07) 1 commit
(merged to 'next' on 2016-09-08 at 5ad2fc1)
+ t6026-merge-attr: clean up background process at end of test case
A test spawned a short-lived background process, which sometimes
prevented the test directory from getting removed at the end of the
script on some platforms.
* js/t9903-chaining (2016-09-07) 1 commit
(merged to 'next' on 2016-09-08 at 162a3c9)
+ t9903: fix broken && chain
Test fix.
* rs/compat-strdup (2016-09-07) 1 commit
(merged to 'next' on 2016-09-08 at 46acfdf)
+ compat: move strdup(3) replacement to its own file
Code cleanup.
* rs/hex2chr (2016-09-07) 1 commit
(merged to 'next' on 2016-09-08 at 7266d5b)
+ introduce hex2chr() for converting two hexadecimal digits to a character
Code cleanup.
* sb/transport-report-missing-submodule-on-stderr (2016-09-08) 1 commit
(merged to 'next' on 2016-09-08 at 3550831)
+ transport: report missing submodule pushes consistently on stderr
Message cleanup.
* sb/xdiff-remove-unused-static-decl (2016-09-07) 1 commit
(merged to 'next' on 2016-09-08 at 39e41dd)
+ xdiff: remove unneeded declarations
Code cleanup.
--------------------------------------------------
[New Topics]
* et/add-chmod-x (2016-09-12) 1 commit
- add: document the chmod option
(this branch is used by tg/add-chmod+x-fix.)
"git add --chmod=+x" added recently lacked documentation, which has
been corrected.
Will merge to 'next'.
* js/libify-require-clean-work-tree (2016-09-12) 5 commits
- wt-status: teach has_{unstaged,uncommitted}_changes() about submodules
- Export also the has_un{staged,committed}_changed() functions
- Make the require_clean_work_tree() function truly reusable
- pull: make code more similar to the shell script again
- pull: drop confusing prefix parameter of die_on_unclean_work_tree()
The require_clean_work_tree() helper was recreated in C when "git
pull" was rewritten from shell; the helper is now made available to
other callers in preparation for upcoming "rebase -i" work.
Waiting for comments.
Modulo a few minor nits, this looked almost ready.
cf. <xmqqtwdl2bhm.fsf@gitster.mtv.corp.google.com>
cf. <xmqqpoo92bdr.fsf@gitster.mtv.corp.google.com>
* ks/perf-build-with-autoconf (2016-09-13) 1 commit
- t/perf/run: Don't forget to copy config.mak.autogen & friends
Performance tests done via "t/perf" did not use the same set of
build configuration if the user relied on autoconf generated
configuration.
Will merge to 'next'.
* tg/add-chmod+x-fix (2016-09-14) 5 commits
- SQUASH???
- add: modify already added files when --chmod is given
- read-cache: introduce chmod_index_entry
- update-index: add test for chmod flags
- Merge branch 'ib/t3700-add-chmod-x-updates' into tg/add-chmod+x-fix
(this branch uses et/add-chmod-x.)
"git add --chmod=+x <pathspec>" added recently only toggled the
executable bit for paths that are either new or modified. This has
been corrected to flip the executable bit for all paths that match
the given pathspec.
Waiting for an ack for SQUASH???
* bw/ls-files-recurse-submodules (2016-09-13) 3 commits
- SQUASH??? Undecided
- SQUASH???
- ls-files: adding support for submodules
"git ls-files" learned "--recurse-submodules" option that can be
used to get a listing of tracked files across submodules (i.e. this
only works with "--cached" option, not for listing untracked or
ignored files). This would be a useful tool to sit on the upstream
side of a pipe that is read with xargs to work on all working tree
files from the top-level superproject.
Waiting for the discussion to conclude.
* bw/pathspec-remove-unused-extern-decl (2016-09-13) 1 commit
- pathspec: remove unnecessary function prototypes
Code cleanup.
Will merge to 'next'.
* ew/http-do-not-forget-to-call-curl-multi-remove-handle (2016-09-13) 3 commits
- http: always remove curl easy from curlm session on release
- http: consolidate #ifdefs for curl_multi_remove_handle
- http: warn on curl_multi_add_handle failures
The http transport (with curl-multi option, which is the default
these days) failed to remove curl-easy handle from a curlm session,
which led to unnecessary API failures.
Will merge to 'next'.
* jk/delta-base-cache (2016-09-12) 1 commit
- add_delta_base_cache: use list_for_each_safe
Recently we updated the code to manage the in-core cache that holds
objects that have recently been used to reconstitute other objects
that are stored as deltas against them, but the update used an
incorrect API function to manage the list of these objects. This
has been fixed.
Will merge to 'next'.
This is a last-minute fix to a topic that graduated to 'master'
post 2.10 release.
* jk/setup-sequence-update (2016-09-13) 16 commits
- t1007: factor out repeated setup
- init: reset cached config when entering new repo
- init: expand comments explaining config trickery
- config: only read .git/config from configured repos
- test-config: setup git directory
- t1302: use "git -C"
- pager: handle early config
- pager: use callbacks instead of configset
- pager: make pager_program a file-local static
- pager: stop loading git_default_config()
- pager: remove obsolete comment
- diff: always try to set up the repository
- diff: handle --no-index prefixes consistently
- diff: skip implicit no-index check when given --no-index
- patch-id: use RUN_SETUP_GENTLY
- hash-object: always try to set up the git repository
There were numerous corner cases in which the configuration files
are read and used or not read at all depending on the directory a
Git command was run, leading to inconsistent behaviour. The code
to set-up repository access at the beginning of a Git process has
been updated to fix them.
Will merge to 'next'.
* ls/filter-process (2016-09-13) 10 commits
- convert: add filter.<driver>.process option
- convert: make apply_filter() adhere to standard Git error handling
- convert: modernize tests
- convert: quote filter names in error messages
- pkt-line: add functions to read/write flush terminated packet streams
- pkt-line: add packet_write_gently()
- pkt-line: add packet_flush_gently()
- pkt-line: add packet_write_fmt_gently()
- pkt-line: extract set_packet_header()
- pkt-line: rename packet_write() to packet_write_fmt()
The smudge/clean filter API expect an external process is spawned
to filter the contents for each path that has a filter defined. A
new type of "process" filter API has been added to allow the first
request to run the filter for a path to spawn a single process, and
all filtering need is served by this single process for multiple
paths, reducing the process creation overhead.
Waiting for the discussion to conclude.
cf. <20160910062919.GB11001@tb-raspi> etc.
* rs/checkout-some-states-are-const (2016-09-13) 1 commit
- checkout: constify parameters of checkout_stage() and checkout_merged()
Code cleanup.
Will merge to 'next'.
* rs/pack-sort-with-llist-mergesort (2016-09-13) 1 commit
- sha1_file: use llist_mergesort() for sorting packs
Code cleanup.
Will merge to 'next'.
* rs/strbuf-remove-fix (2016-09-13) 1 commit
- strbuf: use valid pointer in strbuf_remove()
Code cleanup.
Will merge to 'next'.
* rs/unpack-trees-reduce-file-scope-global (2016-09-13) 1 commit
- unpack-trees: pass checkout state explicitly to check_updates()
Code cleanup.
Will merge to 'next'.
* mr/vcs-svn-printf-ulong (2016-09-14) 1 commit
- vcs-svn/fast_export: fix timestamp fmt specifiers
Code cleanup.
Will merge to 'next'.
* hv/submodule-not-yet-pushed-fix (2016-09-14) 2 commits
- serialize collection of refs that contain submodule changes
- serialize collection of changed submodules
--------------------------------------------------
[Stalled]
* jc/bundle (2016-03-03) 6 commits
- index-pack: --clone-bundle option
- Merge branch 'jc/index-pack' into jc/bundle
- bundle v3: the beginning
- bundle: keep a copy of bundle file name in the in-core bundle header
- bundle: plug resource leak
- bundle doc: 'verify' is not about verifying the bundle
The beginning of "split bundle", which could be one of the
ingredients to allow "git clone" traffic off of the core server
network to CDN.
While I think it would make it easier for people to experiment and
build on if the topic is merged to 'next', I am at the same time a
bit reluctant to merge an unproven new topic that introduces a new
file format, which we may end up having to support til the end of
time. It is likely that to support a "prime clone from CDN", it
would need a lot more than just "these are the heads and the pack
data is over there", so this may not be sufficient.
Will discard.
* jc/blame-reverse (2016-06-14) 2 commits
- blame: dwim "blame --reverse OLD" as "blame --reverse OLD.."
- blame: improve diagnosis for "--reverse NEW"
It is a common mistake to say "git blame --reverse OLD path",
expecting that the command line is dwimmed as if asking how lines
in path in an old revision OLD have survived up to the current
commit.
Has been waiting for positive responses without seeing any.
Will discard.
* jc/attr (2016-05-25) 18 commits
- attr: support quoting pathname patterns in C style
- attr: expose validity check for attribute names
- attr: add counted string version of git_attr()
- attr: add counted string version of git_check_attr()
- attr: retire git_check_attrs() API
- attr: convert git_check_attrs() callers to use the new API
- attr: convert git_all_attrs() to use "struct git_attr_check"
- attr: (re)introduce git_check_attr() and struct git_attr_check
- attr: rename function and struct related to checking attributes
- attr.c: plug small leak in parse_attr_line()
- attr.c: tighten constness around "git_attr" structure
- attr.c: simplify macroexpand_one()
- attr.c: mark where #if DEBUG ends more clearly
- attr.c: complete a sentence in a comment
- attr.c: explain the lack of attr-name syntax check in parse_attr()
- attr.c: update a stale comment on "struct match_attr"
- attr.c: use strchrnul() to scan for one line
- commit.c: use strchrnul() to scan for one line
(this branch is used by jc/attr-more, sb/pathspec-label and sb/submodule-default-paths.)
The attributes API has been updated so that it can later be
optimized using the knowledge of which attributes are queried.
I wanted to polish this topic further to make the attribute
subsystem thread-ready, but because other topics depend on this
topic and they do not (yet) need it to be thread-ready.
As the authors of topics that depend on this seem not in a hurry,
let's discard this and dependent topics and restart them some other
day.
Will discard.
* jc/attr-more (2016-06-09) 8 commits
- attr.c: outline the future plans by heavily commenting
- attr.c: always pass check[] to collect_some_attrs()
- attr.c: introduce empty_attr_check_elems()
- attr.c: correct ugly hack for git_all_attrs()
- attr.c: rename a local variable check
- fixup! d5ad6c13
- attr.c: pass struct git_attr_check down the callchain
- attr.c: add push_stack() helper
(this branch uses jc/attr; is tangled with sb/pathspec-label and sb/submodule-default-paths.)
The beginning of long and tortuous journey to clean-up attribute
subsystem implementation.
Needs to be redone.
Will discard.
* sb/submodule-default-paths (2016-06-20) 5 commits
- completion: clone can recurse into submodules
- clone: add --init-submodule=<pathspec> switch
- submodule update: add `--init-default-path` switch
- Merge branch 'sb/pathspec-label' into sb/submodule-default-paths
- Merge branch 'jc/attr' into sb/submodule-default-paths
(this branch uses jc/attr and sb/pathspec-label; is tangled with jc/attr-more.)
Allow specifying the set of submodules the user is interested in on
the command line of "git clone" that clones the superproject.
Will discard.
* sb/pathspec-label (2016-06-03) 6 commits
- pathspec: disable preload-index when attribute pathspec magic is in use
- pathspec: allow escaped query values
- pathspec: allow querying for attributes
- pathspec: move prefix check out of the inner loop
- pathspec: move long magic parsing out of prefix_pathspec
- Documentation: fix a typo
(this branch is used by sb/submodule-default-paths; uses jc/attr; is tangled with jc/attr-more.)
The pathspec mechanism learned ":(attr:X)$pattern" pathspec magic
to limit paths that match $pattern further by attribute settings.
The preload-index mechanism is disabled when the new pathspec magic
is in use (at least for now), because the attribute subsystem is
not thread-ready.
Will discard.
* mh/connect (2016-06-06) 10 commits
- connect: [host:port] is legacy for ssh
- connect: move ssh command line preparation to a separate function
- connect: actively reject git:// urls with a user part
- connect: change the --diag-url output to separate user and host
- connect: make parse_connect_url() return the user part of the url as a separate value
- connect: group CONNECT_DIAG_URL handling code
- connect: make parse_connect_url() return separated host and port
- connect: re-derive a host:port string from the separate host and port variables
- connect: call get_host_and_port() earlier
- connect: document why we sometimes call get_port after get_host_and_port
Rewrite Git-URL parsing routine (hopefully) without changing any
behaviour.
It has been two months without any support. We may want to discard
this.
* sb/bisect (2016-04-15) 22 commits
. SQUASH???
. bisect: get back halfway shortcut
. bisect: compute best bisection in compute_relevant_weights()
. bisect: use a bottom-up traversal to find relevant weights
. bisect: prepare for different algorithms based on find_all
. bisect: rename count_distance() to compute_weight()
. bisect: make total number of commits global
. bisect: introduce distance_direction()
. bisect: extract get_distance() function from code duplication
. bisect: use commit instead of commit list as arguments when appropriate
. bisect: replace clear_distance() by unique markers
. bisect: use struct node_data array instead of int array
. bisect: get rid of recursion in count_distance()
. bisect: make algorithm behavior independent of DEBUG_BISECT
. bisect: make bisect compile if DEBUG_BISECT is set
. bisect: plug the biggest memory leak
. bisect: add test for the bisect algorithm
. t6030: generalize test to not rely on current implementation
. t: use test_cmp_rev() where appropriate
. t/test-lib-functions.sh: generalize test_cmp_rev
. bisect: allow 'bisect run' if no good commit is known
. bisect: write about `bisect next` in documentation
The internal algorithm used in "git bisect" to find the next commit
to check has been optimized greatly.
Was expecting a reroll, but now pb/bisect topic starts removinging
more and more parts from git-bisect.sh, this needs to see a fresh
reroll.
Will discard.
cf. <1460294354-7031-1-git-send-email-s-beyer@gmx.net>
* sg/completion-updates (2016-02-28) 21 commits
. completion: cache the path to the repository
. completion: extract repository discovery from __gitdir()
. completion: don't guard git executions with __gitdir()
. completion: consolidate silencing errors from git commands
. completion: don't use __gitdir() for git commands
. completion: respect 'git -C <path>'
. completion: fix completion after 'git -C <path>'
. completion: don't offer commands when 'git --opt' needs an argument
. rev-parse: add '--absolute-git-dir' option
. completion: list short refs from a remote given as a URL
. completion: don't list 'HEAD' when trying refs completion outside of a repo
. completion: list refs from remote when remote's name matches a directory
. completion: respect 'git --git-dir=<path>' when listing remote refs
. completion: fix most spots not respecting 'git --git-dir=<path>'
. completion: ensure that the repository path given on the command line exists
. completion tests: add tests for the __git_refs() helper function
. completion tests: check __gitdir()'s output in the error cases
. completion tests: consolidate getting path of current working directory
. completion tests: make the $cur variable local to the test helper functions
. completion tests: don't add test cruft to the test repository
. completion: improve __git_refs()'s in-code documentation
Has been waiting for a reroll for too long.
cf. <1456754714-25237-1-git-send-email-szeder@ira.uka.de>
Will discard.
* ec/annotate-deleted (2015-11-20) 1 commit
- annotate: skip checking working tree if a revision is provided
Usability fix for annotate-specific "<file> <rev>" syntax with deleted
files.
Has been waiting for a review for too long without seeing anything.
Will discard.
* dk/gc-more-wo-pack (2016-01-13) 4 commits
- gc: clean garbage .bitmap files from pack dir
- t5304: ensure non-garbage files are not deleted
- t5304: test .bitmap garbage files
- prepare_packed_git(): find more garbage
Follow-on to dk/gc-idx-wo-pack topic, to clean up stale
.bitmap and .keep files.
Has been waiting for a reroll for too long.
cf. <xmqq60ypbeng.fsf@gitster.mtv.corp.google.com>
Will discard.
* jc/diff-b-m (2015-02-23) 5 commits
. WIPWIP
. WIP: diff-b-m
- diffcore-rename: allow easier debugging
- diffcore-rename.c: add locate_rename_src()
- diffcore-break: allow debugging
"git diff -B -M" produced incorrect patch when the postimage of a
completely rewritten file is similar to the preimage of a removed
file; such a resulting file must not be expressed as a rename from
other place.
The fix in this patch is broken, unfortunately.
Will discard.
--------------------------------------------------
[Cooking]
* jk/patch-ids-no-merges (2016-09-12) 2 commits
- patch-ids: refuse to compute patch-id for merge commit
- patch-ids: turn off rename detection
"git log --cherry-pick" used to include merge commits as candidates
to be matched up with other commits, resulting a lot of wasted time.
The patch-id generation logic has been updated to ignore merges to
avoid the wastage.
Will merge to 'next'.
* js/git-gui-commit-gpgsign (2016-09-11) 2 commits
(merged to 'next' on 2016-09-12 at 05350ab)
+ Merge branch 'js/commit-gpgsign' of ../git-gui into js/git-gui-commit-gpgsign
+ git-gui: respect commit.gpgsign again
"git commit-tree" stopped reading commit.gpgsign configuration
variable that was meant for Porcelain "git commit" in Git 2.9; we
forgot to update "git gui" to look at the configuration to match
this change.
Will merge to 'master'.
* bc/object-id (2016-09-07) 20 commits
- builtin/reset: convert to use struct object_id
- builtin/commit-tree: convert to struct object_id
- builtin/am: convert to struct object_id
- refs: add an update_ref_oid function.
- sha1_name: convert get_sha1_mb to struct object_id
- builtin/update-index: convert file to struct object_id
- notes: convert init_notes to use struct object_id
- builtin/rm: convert to use struct object_id
- builtin/blame: convert file to use struct object_id
- Convert read_mmblob to take struct object_id.
- notes-merge: convert struct notes_merge_pair to struct object_id
- builtin/checkout: convert some static functions to struct object_id
- streaming: make stream_blob_to_fd take struct object_id
- builtin: convert textconv_object to use struct object_id
- builtin/cat-file: convert some static functions to struct object_id
- builtin/cat-file: convert struct expand_data to use struct object_id
- builtin/log: convert some static functions to use struct object_id
- builtin/blame: convert struct origin to use struct object_id
- builtin/apply: convert static functions to struct object_id
- cache: convert struct cache_entry to use struct object_id
The "unsigned char sha1[20]" to "struct object_id" conversion
continues. Notable changes in this round includes that ce->sha1,
i.e. the object name recorded in the cache_entry, turns into an
object_id.
It had merge conflicts with a few topics in flight (Christian's
"apply.c split", Dscho's "cat-file --filters" and Jeff Hostetler's
"status --porcelain-v2"). Extra sets of eyes double-checking for
mismerges are highly appreciated.
Will merge to 'next'.
* jk/pack-tag-of-tag (2016-09-07) 5 commits
(merged to 'next' on 2016-09-12 at 62c62c0)
+ pack-objects: walk tag chains for --include-tag
+ t5305: simplify packname handling
+ t5305: use "git -C"
+ t5305: drop "dry-run" of unpack-objects
+ t5305: move cleanup into test block
"git pack-objects --include-tag" was taught that when we know that
we are sending an object C, we want a tag B that directly points at
C but also a tag A that points at the tag B. We used to miss the
intermediate tag B in some cases.
Will merge to 'master'.
* jt/accept-capability-advertisement-when-fetching-from-void (2016-09-09) 3 commits
- connect: advertized capability is not a ref
- connect: tighten check for unexpected early hang up
- tests: move test_lazy_prereq JGIT to test-lib.sh
JGit can show a fake ref "capabilities^{}" to "git fetch" when it
does not advertise any refs, but "git fetch" was not prepared to
see such an advertisement. When the other side disconnects without
giving any ref advertisement, we used to say "there may not be a
repository at that URL", but we may have seen other advertisement
like "shallow" and ".have" in which case we definitely know that a
repository is there. The code to detect this case has also been
updated.
Will merge to 'next'.
* rt/rebase-i-broken-insn-advise (2016-09-07) 1 commit
- rebase -i: improve advice on bad instruction lines
When "git rebase -i" is given a broken instruction, it told the
user to fix it with "--edit-todo", but didn't say what the step
after that was (i.e. "--continue").
Will hold.
Dscho's "rebase -i" hopefully will become available in 'pu', by
which time an equivalent of this fix would be ported to C. This is
queued merely as a reminder.
* sy/git-gui-i18n-ja (2016-09-07) 7 commits
(merged to 'next' on 2016-09-12 at 4a701c2)
+ Merge branch 'sy/i18n' of git-gui
+ git-gui: update Japanese information
+ git-gui: update Japanese translation
+ git-gui: add Japanese language code
+ git-gui: apply po template to Japanese translation
+ git-gui: consistently use the same word for "blame" in Japanese
+ git-gui: consistently use the same word for "remote" in Japanese
Update Japanese translation for "git-gui".
Will merge to 'master'.
* ah/misc-message-fixes (2016-09-08) 5 commits
(merged to 'next' on 2016-09-12 at a113aea)
+ unpack-trees: do not capitalize "working"
+ git-merge-octopus: do not capitalize "octopus"
+ git-rebase--interactive: fix English grammar
+ cat-file: put spaces around pipes in usage string
+ am: put spaces around pipe in usage string
Message cleanup.
Will merge to 'master'.
* jk/fix-remote-curl-url-wo-proto (2016-09-08) 1 commit
(merged to 'next' on 2016-09-12 at 7845867)
+ remote-curl: handle URLs without protocol
"git fetch http::/site/path" did not die correctly and segfaulted
instead.
Will merge to 'master'.
* jt/format-patch-base-info-above-sig (2016-09-14) 2 commits
- SQUASH???
- format-patch: show base info before email signature
"git format-patch --base=..." feature that was recently added
showed the base commit information after "-- " e-mail signature
line, which turned out to be inconvenient. The base information
has been moved above the signature line.
Waiting for an ack for SQUASH???
* nd/checkout-disambiguation (2016-09-09) 4 commits
- fixup! checkout.txt: document a common case that ignores ambiguation rules
- checkout: fix ambiguity check in subdir
- checkout.txt: document a common case that ignores ambiguation rules
- checkout: add some spaces between code and comment
"git checkout <word>" does not follow the usual disambiguation
rules when the <word> can be both a rev and a path, to allow
checking out a branch 'foo' in a project that happens to have a
file 'foo' in the working tree without having to disambiguate.
This was poorly documented and the check was incorrect when the
command was run from a subdirectory.
Waiting for an Ack for fixup!
* sb/diff-cleanup (2016-09-08) 3 commits
(merged to 'next' on 2016-09-12 at 5d16b28)
+ diff: remove dead code
+ diff: omit found pointer from emit_callback
+ diff.c: use diff_options directly
Code cleanup.
Will merge to 'master'.
* sg/fix-versioncmp-with-common-suffix (2016-09-08) 5 commits
- versioncmp: cope with common leading parts in versionsort.prereleaseSuffix
- versioncmp: pass full tagnames to swap_prereleases()
- t7004-tag: add version sort tests to show prerelease reordering issues
- t7004-tag: use test_config helper
- t7004-tag: delete unnecessary tags with test_when_finished
The prereleaseSuffix feature of version comparison that is used in
"git tag -l" did not correctly when two or more prereleases for the
same release were present (e.g. when 2.0, 2.0-beta1, and 2.0-beta2
are there and the code needs to compare 2.0-beta1 and 2.0-beta2).
Waiting for a reroll.
cf. <20160908223727.Horde.jVOOJ278ssZ3qkyjkmyqZD-@webmail.informatik.kit.edu>
* va/i18n (2016-09-11) 11 commits
- i18n: update-index: mark warning for translation
- i18n: show-branch: mark error messages for translation
- i18n: receive-pack: mark messages for translation
- notes: downcase the first word of error messages
- i18n: notes: mark error messages for translation
- i18n: merge-recursive: mark verbose message for translation
- i18n: merge-recursive: mark error messages for translation
- i18n: config: mark error message for translation
- i18n: branch: mark option description for translation
- SQUASH???
- i18n: blame: mark error messages for translation
More i18n.
Waiting for a reroll.
Some of them need to use Q_(); even when they always show numbers
that are greater than 1, some languages have different plural forms.
cf. <17140652.xHVhzLXte8@cayenne> etc.
* js/sequencer-wo-die (2016-09-09) 17 commits
(merged to 'next' on 2016-09-12 at d2154ea)
+ sequencer: ensure to release the lock when we could not read the index
+ sequencer: lib'ify checkout_fast_forward()
+ sequencer: lib'ify fast_forward_to()
+ sequencer: lib'ify save_opts()
+ sequencer: lib'ify save_todo()
+ sequencer: lib'ify save_head()
+ sequencer: lib'ify create_seq_dir()
+ sequencer: lib'ify read_populate_opts()
+ sequencer: lib'ify read_populate_todo()
+ sequencer: lib'ify read_and_refresh_cache()
+ sequencer: lib'ify prepare_revs()
+ sequencer: lib'ify walk_revs_populate_todo()
+ sequencer: lib'ify do_pick_commit()
+ sequencer: lib'ify do_recursive_merge()
+ sequencer: lib'ify write_message()
+ sequencer: do not die() in do_pick_commit()
+ sequencer: lib'ify sequencer_pick_revisions()
Lifts calls to exit(2) and die() higher in the callchain in
sequencer.c files so that more helper functions in it can be used
by callers that want to handle error conditions themselves.
Will merge to 'master'.
* cp/completion-negative-refs (2016-08-24) 1 commit
- completion: support excluding refs
The command-line completion script (in contrib/) learned to
complete "git cmd ^mas<HT>" to complete the negative end of
reference to "git cmd ^master".
Needs review.
* js/cat-file-filters (2016-09-11) 4 commits
- cat-file: support --textconv/--filters in batch mode
- cat-file --textconv/--filters: allow specifying the path separately
- cat-file: introduce the --filters option
- cat-file: fix a grammo in the man page
Even though "git hash-objects", which is a tool to take an
on-filesystem data stream and put it into the Git object store,
allowed to perform the "outside-world-to-Git" conversions (e.g.
end-of-line conversions and application of the clean-filter), and
it had the feature on by default from very early days, its reverse
operation "git cat-file", which takes an object from the Git object
store and externalize for the consumption by the outside world,
lacked an equivalent mechanism to run the "Git-to-outside-world"
conversion. The command learned the "--filters" option to do so.
Will merge to 'next'.
* sb/push-make-submodule-check-the-default (2016-08-24) 1 commit
- push: change submodule default to check
Turn the default of "push.recurseSubmodules" to "check".
Alas, this reveals that the "check" mode is too inefficient to use
in real projects, even in ones as small as git itself.
cf. <xmqqh9aaot49.fsf@gitster.mtv.corp.google.com>
* ak/curl-imap-send-explicit-scheme (2016-08-17) 1 commit
- imap-send: Tell cURL to use imap:// or imaps://
When we started cURL to talk to imap server when a new enough
version of cURL library is available, we forgot to explicitly add
imap(s):// before the destination. To some folks, that didn't work
and the library tried to make HTTP(s) requests instead.
Needs review and testing.
* jk/reduce-gc-aggressive-depth (2016-08-11) 1 commit
(merged to 'next' on 2016-08-11 at 6810c6f)
+ gc: default aggressive depth to 50
"git gc --aggressive" used to limit the delta-chain length to 250,
which is way too deep for gaining additional space savings and is
detrimental for runtime performance. The limit has been reduced to
50.
Will hold to see if people scream.
* ks/pack-objects-bitmap (2016-09-12) 2 commits
- pack-objects: use reachability bitmap index when generating non-stdout pack
- pack-objects: respect --local/--honor-pack-keep/--incremental when bitmap is in use
Some codepaths in "git pack-objects" were not ready to use an
existing pack bitmap; now they are and as the result they have
become faster.
Will merge to 'next'.
* mh/diff-indent-heuristic (2016-09-07) 9 commits
- SQAUSH???
- blame: honor the diff heuristic options and config
- parse-options: add parse_opt_unknown_cb()
- diff: improve positioning of add/delete blocks in diffs
- xdl_change_compact(): introduce the concept of a change group
- recs_match(): take two xrecord_t pointers as arguments
- is_blank_line(): take a single xrecord_t as argument
- xdl_change_compact(): only use heuristic if group can't be matched
- xdl_change_compact(): fix compaction heuristic to adjust ixo
Output from "git diff" can be made easier to read by selecting
which lines are common and which lines are added/deleted
intelligently when the lines before and after the changed section
are the same. A command line option is added to help with the
experiment to find a good heuristics.
Waiting for an ack to SQUASH??? Otherwise looked OK.
* cc/apply-am (2016-09-07) 41 commits
(merged to 'next' on 2016-09-12 at 854edde)
+ builtin/am: use apply API in run_apply()
+ apply: learn to use a different index file
+ apply: pass apply state to build_fake_ancestor()
+ apply: refactor `git apply` option parsing
+ apply: change error_routine when silent
+ usage: add get_error_routine() and get_warn_routine()
+ usage: add set_warn_routine()
+ apply: don't print on stdout in verbosity_silent mode
+ apply: make it possible to silently apply
+ apply: use error_errno() where possible
+ apply: make some parsing functions static again
+ apply: move libified code from builtin/apply.c to apply.{c,h}
+ apply: rename and move opt constants to apply.h
+ builtin/apply: rename option parsing functions
+ builtin/apply: make create_one_file() return -1 on error
+ builtin/apply: make try_create_file() return -1 on error
+ builtin/apply: make write_out_results() return -1 on error
+ builtin/apply: make write_out_one_result() return -1 on error
+ builtin/apply: make create_file() return -1 on error
+ builtin/apply: make add_index_file() return -1 on error
+ builtin/apply: make add_conflicted_stages_file() return -1 on error
+ builtin/apply: make remove_file() return -1 on error
+ builtin/apply: make build_fake_ancestor() return -1 on error
+ builtin/apply: change die_on_unsafe_path() to check_unsafe_path()
+ builtin/apply: make gitdiff_*() return -1 on error
+ builtin/apply: make gitdiff_*() return 1 at end of header
+ builtin/apply: make parse_traditional_patch() return -1 on error
+ builtin/apply: make apply_all_patches() return 128 or 1 on error
+ builtin/apply: move check_apply_state() to apply.c
+ builtin/apply: make check_apply_state() return -1 instead of die()ing
+ apply: make init_apply_state() return -1 instead of exit()ing
+ builtin/apply: move init_apply_state() to apply.c
+ builtin/apply: make parse_ignorewhitespace_option() return -1 instead of die()ing
+ builtin/apply: make parse_whitespace_option() return -1 instead of die()ing
+ builtin/apply: make parse_single_patch() return -1 on error
+ builtin/apply: make parse_chunk() return a negative integer on error
+ builtin/apply: make find_header() return -128 instead of die()ing
+ builtin/apply: read_patch_file() return -1 instead of die()ing
+ builtin/apply: make apply_patch() return -1 or -128 instead of die()ing
+ apply: move 'struct apply_state' to apply.h
+ apply: make some names more specific
"git am" has been taught to make an internal call to "git apply"'s
innards without spawning the latter as a separate process.
Will merge to 'master'.
* jk/pack-objects-optim-mru (2016-08-11) 4 commits
(merged to 'next' on 2016-08-11 at c0a7dae)
+ pack-objects: use mru list when iterating over packs
+ pack-objects: break delta cycles before delta-search phase
+ sha1_file: make packed_object_info public
+ provide an initializer for "struct object_info"
"git pack-objects" in a repository with many packfiles used to
spend a lot of time looking for/at objects in them; the accesses to
the packfiles are now optimized by checking the most-recently-used
packfile first.
Will hold to see if people scream.
* jk/rebase-i-drop-ident-check (2016-07-29) 1 commit
(merged to 'next' on 2016-08-14 at 6891bcd)
+ rebase-interactive: drop early check for valid ident
Even when "git pull --rebase=preserve" (and the underlying "git
rebase --preserve") can complete without creating any new commit
(i.e. fast-forwards), it still insisted on having a usable ident
information (read: user.email is set correctly), which was less
than nice. As the underlying commands used inside "git rebase"
would fail with a more meaningful error message and advice text
when the bogus ident matters, this extra check was removed.
Will hold to see if people scream.
cf. <20160729224944.GA23242@sigill.intra.peff.net>
* dp/autoconf-curl-ssl (2016-06-28) 1 commit
- ./configure.ac: detect SSL in libcurl using curl-config
The ./configure script generated from configure.ac was taught how
to detect support of SSL by libcurl better.
Needs review.
* jc/pull-rebase-ff (2016-07-28) 1 commit
- pull: fast-forward "pull --rebase=true"
"git pull --rebase", when there is no new commits on our side since
we forked from the upstream, should be able to fast-forward without
invoking "git rebase", but it didn't.
Needs a real log message and a few tests.
* ex/deprecate-empty-pathspec-as-match-all (2016-06-22) 1 commit
(merged to 'next' on 2016-07-13 at d9ca7fb)
+ pathspec: warn on empty strings as pathspec
An empty string used as a pathspec element has always meant
'everything matches', but it is too easy to write a script that
finds a path to remove in $path and run 'git rm "$paht"', which
ends up removing everything. Start warning about this use of an
empty string used for 'everything matches' and ask users to use a
more explicit '.' for that instead.
The hope is that existing users will not mind this change, and
eventually the warning can be turned into a hard error, upgrading
the deprecation into removal of this (mis)feature.
Will hold to see if people scream.
* mh/ref-store (2016-09-09) 38 commits
(merged to 'next' on 2016-09-12 at 1b0bd3c)
+ refs: implement iteration over only per-worktree refs
+ refs: make lock generic
+ refs: add method to rename refs
+ refs: add methods to init refs db
+ refs: make delete_refs() virtual
+ refs: add method for initial ref transaction commit
+ refs: add methods for reflog
+ refs: add method iterator_begin
+ files_ref_iterator_begin(): take a ref_store argument
+ split_symref_update(): add a files_ref_store argument
+ lock_ref_sha1_basic(): add a files_ref_store argument
+ lock_ref_for_update(): add a files_ref_store argument
+ commit_ref_update(): add a files_ref_store argument
+ lock_raw_ref(): add a files_ref_store argument
+ repack_without_refs(): add a files_ref_store argument
+ refs: make peel_ref() virtual
+ refs: make create_symref() virtual
+ refs: make pack_refs() virtual
+ refs: make verify_refname_available() virtual
+ refs: make read_raw_ref() virtual
+ resolve_gitlink_ref(): rename path parameter to submodule
+ resolve_gitlink_ref(): avoid memory allocation in many cases
+ resolve_gitlink_ref(): implement using resolve_ref_recursively()
+ resolve_ref_recursively(): new function
+ read_raw_ref(): take a (struct ref_store *) argument
+ resolve_gitlink_packed_ref(): remove function
+ resolve_packed_ref(): rename function from resolve_missing_loose_ref()
+ refs: reorder definitions
+ refs: add a transaction_commit() method
+ {lock,commit,rollback}_packed_refs(): add files_ref_store arguments
+ resolve_missing_loose_ref(): add a files_ref_store argument
+ get_packed_ref(): add a files_ref_store argument
+ add_packed_ref(): add a files_ref_store argument
+ refs: create a base class "ref_store" for files_ref_store
+ refs: add a backend method structure
+ refs: rename struct ref_cache to files_ref_store
+ rename_ref_available(): add docstring
+ resolve_gitlink_ref(): eliminate temporary variable
The ref-store abstraction was introduced to the refs API so that we
can plug in different backends to store references.
Will merge to 'master'.
* nd/shallow-deepen (2016-06-13) 27 commits
- fetch, upload-pack: --deepen=N extends shallow boundary by N commits
- upload-pack: add get_reachable_list()
- upload-pack: split check_unreachable() in two, prep for get_reachable_list()
- t5500, t5539: tests for shallow depth excluding a ref
- clone: define shallow clone boundary with --shallow-exclude
- fetch: define shallow boundary with --shallow-exclude
- upload-pack: support define shallow boundary by excluding revisions
- refs: add expand_ref()
- t5500, t5539: tests for shallow depth since a specific date
- clone: define shallow clone boundary based on time with --shallow-since
- fetch: define shallow boundary with --shallow-since
- upload-pack: add deepen-since to cut shallow repos based on time
- shallow.c: implement a generic shallow boundary finder based on rev-list
- fetch-pack: use a separate flag for fetch in deepening mode
- fetch-pack.c: mark strings for translating
- fetch-pack: use a common function for verbose printing
- fetch-pack: use skip_prefix() instead of starts_with()
- upload-pack: move rev-list code out of check_non_tip()
- upload-pack: make check_non_tip() clean things up on error
- upload-pack: tighten number parsing at "deepen" lines
- upload-pack: use skip_prefix() instead of starts_with()
- upload-pack: move "unshallow" sending code out of deepen()
- upload-pack: remove unused variable "backup"
- upload-pack: move "shallow" sending code out of deepen()
- upload-pack: move shallow deepen code out of receive_needs()
- transport-helper.c: refactor set_helper_option()
- remote-curl.c: convert fetch_git() to use argv_array
The existing "git fetch --depth=<n>" option was hard to use
correctly when making the history of an existing shallow clone
deeper. A new option, "--deepen=<n>", has been added to make this
easier to use. "git clone" also learned "--shallow-since=<date>"
and "--shallow-exclude=<tag>" options to make it easier to specify
"I am interested only in the recent N months worth of history" and
"Give me only the history since that version".
Needs review.
Rerolled. What this topic attempts to achieve is worthwhile, I
would think.
* pb/bisect (2016-08-23) 27 commits
. bisect--helper: remove the dequote in bisect_start()
. bisect--helper: retire `--bisect-auto-next` subcommand
. bisect--helper: retire `--bisect-autostart` subcommand
. bisect--helper: retire `--check-and-set-terms` subcommand
. bisect--helper: retire `--bisect-write` subcommand
. bisect--helper: `bisect_replay` shell function in C
. bisect--helper: `bisect_log` shell function in C
. bisect--helper: retire `--write-terms` subcommand
. bisect--helper: retire `--check-expected-revs` subcommand
. bisect--helper: `bisect_state` & `bisect_head` shell function in C
. bisect--helper: `bisect_autostart` shell function in C
. bisect--helper: retire `--next-all` subcommand
. bisect--helper: retire `--bisect-clean-state` subcommand
. bisect--helper: `bisect_next` and `bisect_auto_next` shell function in C
. bisect--helper: `bisect_start` shell function partially in C
. bisect--helper: `get_terms` & `bisect_terms` shell function in C
. bisect--helper: `bisect_next_check` & bisect_voc shell function in C
. bisect--helper: `check_and_set_terms` shell function in C
. bisect--helper: `bisect_write` shell function in C
. bisect--helper: `is_expected_rev` & `check_expected_revs` shell function in C
. bisect--helper: `bisect_reset` shell function in C
. wrapper: move is_empty_file() and rename it as is_empty_or_missing_file()
. t6030: explicitly test for bisection cleanup
. bisect--helper: `bisect_clean_state` shell function in C
. bisect--helper: `write_terms` shell function in C
. bisect: rewrite `check_term_format` shell function in C
. bisect--helper: use OPT_CMDMODE instead of OPT_BOOL
GSoC "bisect" topic.
I'd prefer to see early part solidified so that reviews can focus
on the later part that is still in flux. We are almost there but
not quite yet.
* kn/ref-filter-branch-list (2016-05-17) 17 commits
- branch: implement '--format' option
- branch: use ref-filter printing APIs
- branch, tag: use porcelain output
- ref-filter: allow porcelain to translate messages in the output
- ref-filter: add `:dir` and `:base` options for ref printing atoms
- ref-filter: make remote_ref_atom_parser() use refname_atom_parser_internal()
- ref-filter: introduce symref_atom_parser() and refname_atom_parser()
- ref-filter: introduce refname_atom_parser_internal()
- ref-filter: make "%(symref)" atom work with the ':short' modifier
- ref-filter: add support for %(upstream:track,nobracket)
- ref-filter: make %(upstream:track) prints "[gone]" for invalid upstreams
- ref-filter: introduce format_ref_array_item()
- ref-filter: move get_head_description() from branch.c
- ref-filter: modify "%(objectname:short)" to take length
- ref-filter: implement %(if:equals=<string>) and %(if:notequals=<string>)
- ref-filter: include reference to 'used_atom' within 'atom_value'
- ref-filter: implement %(if), %(then), and %(else) atoms
The code to list branches in "git branch" has been consolidated
with the more generic ref-filter API.
Rerolled.
Needs review.
* jc/merge-drop-old-syntax (2015-04-29) 1 commit
- merge: drop 'git merge <message> HEAD <commit>' syntax
Stop supporting "git merge <message> HEAD <commit>" syntax that has
been deprecated since October 2007, and issues a deprecation
warning message since v2.5.0.
It has been reported that git-gui still uses the deprecated syntax,
which needs to be fixed before this final step can proceed.
cf. <5671DB28.8020901@kdbg.org>
--------------------------------------------------
[Discarded]
* jn/fix-connect-unexpected-hangup-diag (2016-09-08) 1 commit
. connect: tighten check for unexpected early hang up
Now part of jt/accept-capability-advertisement-when-fetching-from-void
topic.
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox