From: Junio C Hamano <gitster@pobox.com>
To: Nguyen Thai Ngoc Duy <pclouds@gmail.com>
Cc: Marat Radchenko <marat@slonopotamus.org>, git@vger.kernel.org
Subject: Re: What's the difference between `git show branch:file | diff -u - file` vs `git diff branch file`?
Date: Mon, 29 Aug 2011 13:50:58 -0700 [thread overview]
Message-ID: <7vliuc54ct.fsf@alter.siamese.dyndns.org> (raw)
In-Reply-To: <7vty9054qr.fsf@alter.siamese.dyndns.org> (Junio C. Hamano's message of "Mon, 29 Aug 2011 13:42:36 -0700")
Junio C Hamano <gitster@pobox.com> writes:
> Junio C Hamano <gitster@pobox.com> writes:
>
>> Of course the merge machinery does not know anything about pruning with
>> pathspec, so it is understandable (not justifiable) it would walk the full
>> tree.
>>
>> Will try to find time this week to cook up something.
>
> This is still rough, but seems to pass the test suite, and gives me some
> performance boost when applied to the kernel tree:
Ehh, no, the patch does not apply to the kernel tree, but I meant "when
used on the kernel tree".
> (without patch)
> $ time git diff --raw --cached v2.6.30 -- virt/kvm
> real 0m0.114s
> user 0m0.088s
> sys 0m0.028s
>
> (with patch)
> $ time ./git diff --raw --cached v2.6.30 -- virt/kvm
> real 0m0.075s
> user 0m0.068s
> sys 0m0.008s
>
> What I do not like about it most is that we have an infrastructure that
> links traverse_info across stackframes to store the paths unexpanded and
> without extra linear allocation, but tree_entry_interesting() wants the
> path as a single string. Hence unpack_trees() carries an extra baggage
> "base" string, even though the general callback in tree-walk machinery
> does not need it.
>
> I think this can be trivially optimized to keep a pointer to a single
> strbuf in the traverse_info (initialize it at the same points as this
> patch sets info.pathspec), extending it as it digs deeper (copy the same
> pointer to the strbuf to a child traverse_info and tuck the name of the
> directory it descends into it, at the same points as this patch copies
> info->pathspec from the parent), and resetting the length back when the
> traversal into a subdirectory comes back.
>
>
>
> diff-lib.c | 1 +
> tree-walk.c | 39 +++++++++++++++++++++++++++++++++------
> tree-walk.h | 1 +
> unpack-trees.c | 2 ++
> unpack-trees.h | 1 +
> 5 files changed, 38 insertions(+), 6 deletions(-)
>
> diff --git a/diff-lib.c b/diff-lib.c
> index f8454dd..ebe751e 100644
> --- a/diff-lib.c
> +++ b/diff-lib.c
> @@ -468,6 +468,7 @@ static int diff_cache(struct rev_info *revs,
> opts.unpack_data = revs;
> opts.src_index = &the_index;
> opts.dst_index = NULL;
> + opts.pathspec = &revs->diffopt.pathspec;
>
> init_tree_desc(&t, tree->buffer, tree->size);
> return unpack_trees(1, &t, &opts);
> diff --git a/tree-walk.c b/tree-walk.c
> index 33f749e..808bb55 100644
> --- a/tree-walk.c
> +++ b/tree-walk.c
> @@ -309,6 +309,18 @@ static void free_extended_entry(struct tree_desc_x *t)
> }
> }
>
> +static inline int prune_traversal(struct name_entry *e,
> + struct traverse_info *info,
> + struct strbuf *base,
> + int still_interesting)
> +{
> + if (!info->pathspec || still_interesting == 2)
> + return 2;
> + if (still_interesting < 0)
> + return still_interesting;
> + return tree_entry_interesting(e, base, 0, info->pathspec);
> +}
> +
> int traverse_trees(int n, struct tree_desc *t, struct traverse_info *info)
> {
> int ret = 0;
> @@ -316,10 +328,18 @@ int traverse_trees(int n, struct tree_desc *t, struct traverse_info *info)
> struct name_entry *entry = xmalloc(n*sizeof(*entry));
> int i;
> struct tree_desc_x *tx = xcalloc(n, sizeof(*tx));
> + struct strbuf base = STRBUF_INIT;
> + int interesting = 1;
>
> for (i = 0; i < n; i++)
> tx[i].d = t[i];
>
> + if (info->prev) {
> + strbuf_grow(&base, info->pathlen);
> + make_traverse_path(base.buf, info->prev, &info->name);
> + base.buf[info->pathlen-1] = '/';
> + strbuf_setlen(&base, info->pathlen);
> + }
> for (;;) {
> unsigned long mask, dirmask;
> const char *first = NULL;
> @@ -376,16 +396,22 @@ int traverse_trees(int n, struct tree_desc *t, struct traverse_info *info)
> mask |= 1ul << i;
> if (S_ISDIR(entry[i].mode))
> dirmask |= 1ul << i;
> + e = &entry[i];
> }
> if (!mask)
> break;
> - ret = info->fn(n, mask, dirmask, entry, info);
> - if (ret < 0) {
> - error = ret;
> - if (!info->show_all_errors)
> - break;
> + interesting = prune_traversal(e, info, &base, interesting);
> + if (interesting < 0)
> + break;
> + if (interesting) {
> + ret = info->fn(n, mask, dirmask, entry, info);
> + if (ret < 0) {
> + error = ret;
> + if (!info->show_all_errors)
> + break;
> + }
> + mask &= ret;
> }
> - mask &= ret;
> ret = 0;
> for (i = 0; i < n; i++)
> if (mask & (1ul << i))
> @@ -395,6 +421,7 @@ int traverse_trees(int n, struct tree_desc *t, struct traverse_info *info)
> for (i = 0; i < n; i++)
> free_extended_entry(tx + i);
> free(tx);
> + strbuf_release(&base);
> return error;
> }
>
> diff --git a/tree-walk.h b/tree-walk.h
> index 39524b7..0089581 100644
> --- a/tree-walk.h
> +++ b/tree-walk.h
> @@ -44,6 +44,7 @@ struct traverse_info {
> struct traverse_info *prev;
> struct name_entry name;
> int pathlen;
> + struct pathspec *pathspec;
>
> unsigned long conflicts;
> traverse_callback_t fn;
> diff --git a/unpack-trees.c b/unpack-trees.c
> index cc616c3..670b464 100644
> --- a/unpack-trees.c
> +++ b/unpack-trees.c
> @@ -444,6 +444,7 @@ static int traverse_trees_recursive(int n, unsigned long dirmask,
>
> newinfo = *info;
> newinfo.prev = info;
> + newinfo.pathspec = info->pathspec;
> newinfo.name = *p;
> newinfo.pathlen += tree_entry_len(p->path, p->sha1) + 1;
> newinfo.conflicts |= df_conflicts;
> @@ -1040,6 +1041,7 @@ int unpack_trees(unsigned len, struct tree_desc *t, struct unpack_trees_options
> info.fn = unpack_callback;
> info.data = o;
> info.show_all_errors = o->show_all_errors;
> + info.pathspec = o->pathspec;
>
> if (o->prefix) {
> /*
> diff --git a/unpack-trees.h b/unpack-trees.h
> index 7998948..5e432f5 100644
> --- a/unpack-trees.h
> +++ b/unpack-trees.h
> @@ -52,6 +52,7 @@ struct unpack_trees_options {
> const char *prefix;
> int cache_bottom;
> struct dir_struct *dir;
> + struct pathspec *pathspec;
> merge_fn_t fn;
> const char *msgs[NB_UNPACK_TREES_ERROR_TYPES];
> /*
next prev parent reply other threads:[~2011-08-29 20:51 UTC|newest]
Thread overview: 56+ messages / expand[flat|nested] mbox.gz Atom feed top
2011-08-23 7:25 What's the difference between `git show branch:file | diff -u - file` vs `git diff branch file`? Marat Radchenko
2011-08-23 10:03 ` Michael J Gruber
2011-08-23 10:52 ` Marat Radchenko
2011-08-23 15:20 ` Michael Witten
2011-08-23 15:34 ` Michael J Gruber
2011-08-23 16:45 ` Marat Radchenko
2011-08-23 17:15 ` Junio C Hamano
2011-08-23 18:21 ` Marat Radchenko
2011-08-23 20:07 ` Michael J Gruber
2011-08-25 16:09 ` Marat Radchenko
2011-08-25 21:10 ` Junio C Hamano
2011-08-26 9:43 ` Marat Radchenko
2011-08-29 7:41 ` Nguyen Thai Ngoc Duy
2011-08-29 14:48 ` Marat Radchenko
2011-08-29 16:09 ` Nguyen Thai Ngoc Duy
2011-08-29 17:18 ` Junio C Hamano
2011-08-29 20:42 ` Junio C Hamano
2011-08-29 20:50 ` Junio C Hamano [this message]
2011-08-29 21:09 ` Junio C Hamano
2011-08-29 21:33 ` [PATCH 0/3] Un-pessimize "diff-index $commit -- $pathspec" Junio C Hamano
2011-08-29 21:33 ` [PATCH 1/3] traverse_trees(): allow pruning with pathspec Junio C Hamano
2011-08-30 12:53 ` Nguyen Thai Ngoc Duy
2011-08-30 17:44 ` Junio C Hamano
2011-08-31 1:35 ` Nguyen Thai Ngoc Duy
2011-10-09 15:39 ` Michael Haggerty
2011-10-09 21:35 ` Nguyen Thai Ngoc Duy
2011-10-10 4:42 ` Michael Haggerty
2011-08-29 21:33 ` [PATCH 2/3] unpack-trees: " Junio C Hamano
2011-08-30 13:03 ` Nguyen Thai Ngoc Duy
2011-08-30 17:32 ` Junio C Hamano
2011-08-30 15:24 ` David Michael Barr
2011-08-29 21:33 ` [PATCH 3/3] diff-index: pass pathspec down to unpack-trees machinery Junio C Hamano
2012-01-11 6:31 ` Jonathan Nieder
2012-01-11 8:05 ` Junio C Hamano
2012-01-11 12:33 ` Nguyen Thai Ngoc Duy
2012-01-11 12:47 ` Nguyen Thai Ngoc Duy
2012-01-11 20:40 ` Junio C Hamano
2012-01-12 4:09 ` [PATCH] tree_entry_interesting: make recursive mode default Nguyễn Thái Ngọc Duy
2012-01-12 5:04 ` Junio C Hamano
2012-01-12 5:44 ` Nguyen Thai Ngoc Duy
2012-01-14 9:23 ` [PATCH v2 1/2] Document limited recursion pathspec matching with wildcards Nguyễn Thái Ngọc Duy
2012-01-14 9:23 ` [PATCH v2 2/2] tree_entry_interesting: make recursive mode default Nguyễn Thái Ngọc Duy
2012-01-15 3:12 ` Junio C Hamano
2012-01-15 10:03 ` Nguyen Thai Ngoc Duy
2012-01-16 22:15 ` Junio C Hamano
2012-01-18 8:59 ` Nguyen Thai Ngoc Duy
2012-01-15 2:38 ` [PATCH v2 1/2] Document limited recursion pathspec matching with wildcards Junio C Hamano
2012-01-15 9:48 ` Nguyen Thai Ngoc Duy
2011-08-29 21:56 ` [PATCH 0/3] Un-pessimize "diff-index $commit -- $pathspec" Linus Torvalds
2011-08-29 22:05 ` Junio C Hamano
2011-08-29 22:11 ` Linus Torvalds
2011-08-29 23:42 ` Junio C Hamano
2011-08-30 6:16 ` Marat Radchenko
2011-08-31 0:18 ` Junio C Hamano
2011-08-30 10:04 ` Michael J Gruber
2011-08-30 17:03 ` Junio C Hamano
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=7vliuc54ct.fsf@alter.siamese.dyndns.org \
--to=gitster@pobox.com \
--cc=git@vger.kernel.org \
--cc=marat@slonopotamus.org \
--cc=pclouds@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).