git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: What's the difference between `git show branch:file | diff -u - file` vs `git diff branch file`?

From
Junio C Hamano <gitster@pobox.com>
Date
Aug 29, 2011, 20:50 UTC
Message-ID
<7vliuc54ct.fsf@alter.siamese.dyndns.org>
In-Reply-To
<7vty9054qr.fsf@alter.siamese.dyndns.org>
Junio C Hamano <gitster@pobox.com> writes:
Show 10 quoted lines
> 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".

Show 171 quoted lines
>     (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];
>  	/*
Previous: Junio C HamanoNext: Junio C Hamano
Message 18 of 56 in “What's the difference between `git show branch:file | diff -u - file` vs `git diff branch file`?”
  1. Marat RadchenkoAug 23, 2011
  2. Michael J GruberAug 23, 2011
  3. Marat RadchenkoAug 23, 2011
  4. Michael WittenAug 23, 2011
  5. Michael J GruberAug 23, 2011
  6. Marat RadchenkoAug 23, 2011
  7. Junio C HamanoAug 23, 2011
  8. Marat RadchenkoAug 23, 2011
  9. Michael J GruberAug 23, 2011
  10. Marat RadchenkoAug 25, 2011
  11. Junio C HamanoAug 25, 2011
  12. Marat RadchenkoAug 26, 2011
  13. Nguyen Thai Ngoc DuyAug 29, 2011
  14. Marat RadchenkoAug 29, 2011
  15. Nguyen Thai Ngoc DuyAug 29, 2011
  16. Junio C HamanoAug 29, 2011
  17. Junio C HamanoAug 29, 2011
  18. Junio C HamanoAug 29, 2011
  19. Junio C HamanoAug 29, 2011
  20. 0/3 Un-pessimize "diff-index $commit -- $pathspec"Junio C Hamano, Aug 29, 2011
  21. 1/3 traverse_trees(): allow pruning with pathspecJunio C Hamano, Aug 29, 2011
  22. Nguyen Thai Ngoc DuyAug 30, 2011
  23. Junio C HamanoAug 30, 2011
  24. Nguyen Thai Ngoc DuyAug 31, 2011
  25. Michael HaggertyOct 9, 2011
  26. Nguyen Thai Ngoc DuyOct 9, 2011
  27. Michael HaggertyOct 10, 2011
  28. 2/3 unpack-trees: allow pruning with pathspecJunio C Hamano, Aug 29, 2011
  29. Nguyen Thai Ngoc DuyAug 30, 2011
  30. Junio C HamanoAug 30, 2011
  31. David Michael BarrAug 30, 2011
  32. 3/3 diff-index: pass pathspec down to unpack-trees machineryJunio C Hamano, Aug 29, 2011
  33. Jonathan NiederJan 11, 2012
  34. Junio C HamanoJan 11, 2012
  35. Nguyen Thai Ngoc DuyJan 11, 2012
  36. Nguyen Thai Ngoc DuyJan 11, 2012
  37. Junio C HamanoJan 11, 2012
  38. tree_entry_interesting: make recursive mode defaultNguyễn Thái Ngọc Duy, Jan 12, 2012
  39. Junio C HamanoJan 12, 2012
  40. Nguyen Thai Ngoc DuyJan 12, 2012
  41. 1/2 Document limited recursion pathspec matching with wildcardsNguyễn Thái Ngọc Duy, Jan 14, 2012
  42. 2/2 tree_entry_interesting: make recursive mode defaultNguyễn Thái Ngọc Duy, Jan 14, 2012
  43. Junio C HamanoJan 15, 2012
  44. Nguyen Thai Ngoc DuyJan 15, 2012
  45. Junio C HamanoJan 16, 2012
  46. Nguyen Thai Ngoc DuyJan 18, 2012
  47. Junio C HamanoJan 15, 2012
  48. Nguyen Thai Ngoc DuyJan 15, 2012
  49. Linus TorvaldsAug 29, 2011
  50. Junio C HamanoAug 29, 2011
  51. Linus TorvaldsAug 29, 2011
  52. Junio C HamanoAug 29, 2011
  53. Marat RadchenkoAug 30, 2011
  54. Junio C HamanoAug 31, 2011
  55. Michael J GruberAug 30, 2011
  56. Junio C HamanoAug 30, 2011

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.