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

Re: [PATCH v4 2/2] diff: don't read index when --no-index is given

From
Junio C Hamano <gitster@pobox.com>
Date
Dec 12, 2013, 20:25 UTC
Message-ID
<xmqqwqj9stos.fsf@gitster.dls.corp.google.com>
In-Reply-To
<1386755923-22132-2-git-send-email-t.gummerer@gmail.com>
Thomas Gummerer <t.gummerer@gmail.com> writes:
Show 24 quoted lines
> git diff --no-index ... currently reads the index, during setup, when
> calling gitmodules_config().  This results in worse performance when the
> index is not actually needed.  This patch avoids calling
> gitmodules_config() when the --no-index option is given.  The times for
> executing "git diff --no-index" in the WebKit repository are improved as
> follows:
>
> Test                      HEAD~3            HEAD
> ------------------------------------------------------------------
> 4001.1: diff --no-index   0.24(0.15+0.09)   0.01(0.00+0.00) -95.8%
>
> An additional improvement of this patch is that "git diff --no-index" no
> longer breaks when the index file is corrupt, which makes it possible to
> use it for investigating the broken repository.
>
> To improve the possible usage as investigation tool for broken
> repositories, setup_git_directory_gently() is also not called when the
> --no-index option is given.
>
> Also add a test to guard against future breakages, and a performance
> test to show the improvements.
>
> Signed-off-by: Thomas Gummerer <t.gummerer@gmail.com>
> ---
Looks reasonable.
Thanks.  Will queue.
Show 83 quoted lines
>  builtin/diff.c                |  7 +++++--
>  t/perf/p4001-diff-no-index.sh | 22 ++++++++++++++++++++++
>  t/t4053-diff-no-index.sh      | 15 +++++++++++++++
>  3 files changed, 42 insertions(+), 2 deletions(-)
>  create mode 100755 t/perf/p4001-diff-no-index.sh
>
> diff --git a/builtin/diff.c b/builtin/diff.c
> index da69e4a..ea1dd65 100644
> --- a/builtin/diff.c
> +++ b/builtin/diff.c
> @@ -298,7 +298,9 @@ int cmd_diff(int argc, const char **argv, const char *prefix)
>  			break;
>  	}
>  
> -	prefix = setup_git_directory_gently(&nongit);
> +	if (!no_index)
> +		prefix = setup_git_directory_gently(&nongit);
> +
>  	/*
>  	 * Treat git diff with at least one path outside of the
>  	 * repo the same as if the command would have been executed
> @@ -311,7 +313,8 @@ int cmd_diff(int argc, const char **argv, const char *prefix)
>  			!path_inside_repo(prefix, argv[i + 1]))))
>  		no_index = DIFF_NO_INDEX_IMPLICIT;
>  
> -	gitmodules_config();
> +	if (!no_index)
> +		gitmodules_config();
>  	git_config(git_diff_ui_config, NULL);
>  
>  	init_revisions(&rev, prefix);
> diff --git a/t/perf/p4001-diff-no-index.sh b/t/perf/p4001-diff-no-index.sh
> new file mode 100755
> index 0000000..683be69
> --- /dev/null
> +++ b/t/perf/p4001-diff-no-index.sh
> @@ -0,0 +1,22 @@
> +#!/bin/sh
> +
> +test_description="Test diff --no-index performance"
> +
> +. ./perf-lib.sh
> +
> +test_perf_large_repo
> +test_checkout_worktree
> +
> +file1=$(git ls-files | tail -n 2 | head -1)
> +file2=$(git ls-files | tail -n 1 | head -1)
> +
> +test_expect_success "empty files, so they take no time to diff" "
> +	echo >$file1 &&
> +	echo >$file2
> +"
> +
> +test_perf "diff --no-index" "
> +	git diff --no-index $file1 $file2 >/dev/null
> +"
> +
> +test_done
> diff --git a/t/t4053-diff-no-index.sh b/t/t4053-diff-no-index.sh
> index 979e983..077c775 100755
> --- a/t/t4053-diff-no-index.sh
> +++ b/t/t4053-diff-no-index.sh
> @@ -29,4 +29,19 @@ test_expect_success 'git diff --no-index relative path outside repo' '
>  	)
>  '
>  
> +test_expect_success 'git diff --no-index with broken index' '
> +	(
> +		cd repo &&
> +		echo broken >.git/index &&
> +		git diff --no-index a ../non/git/a
> +	)
> +'
> +
> +test_expect_success 'git diff outside repo with broken index' '
> +	(
> +		cd repo &&
> +		git diff ../non/git/a ../non/git/b
> +	)
> +'
> +
>  test_done
Previous: Thomas GummererNext: Jonathan Nieder
Message 21 of 30 in “diff: don't read index when --no-index is given”
  1. diff: don't read index when --no-index is givenThomas Gummerer, Dec 9, 2013
  2. Jonathan NiederDec 9, 2013
  3. Jens LehmannDec 9, 2013
  4. Jonathan NiederDec 9, 2013
  5. Thomas GummererDec 9, 2013
  6. Jonathan NiederDec 9, 2013
  7. diff: don't read index when --no-index is givenThomas Gummerer, Dec 9, 2013
  8. Torsten BögershausenDec 9, 2013
  9. Eric SunshineDec 9, 2013
  10. Thomas GummererDec 9, 2013
  11. Junio C HamanoDec 9, 2013
  12. Thomas GummererDec 9, 2013
  13. 1/2 diff: move no-index detection to builtin/diff.cThomas Gummerer, Dec 10, 2013
  14. 2/2 diff: don't read index when --no-index is givenThomas Gummerer, Dec 10, 2013
  15. Jonathan NiederDec 10, 2013
  16. Thomas GummererDec 10, 2013
  17. Jonathan NiederDec 10, 2013
  18. Thomas GummererDec 10, 2013
  19. 1/2 diff: move no-index detection to builtin/diff.cThomas Gummerer, Dec 11, 2013
  20. 2/2 diff: don't read index when --no-index is givenThomas Gummerer, Dec 11, 2013
  21. Junio C HamanoDec 12, 2013
  22. Jonathan NiederDec 14, 2013
  23. Jonathan NiederDec 14, 2013
  24. Thomas GummererDec 14, 2013
  25. Junio C HamanoDec 16, 2013
  26. 1/2 diff: add test for --no-index executed outside repoThomas Gummerer, Dec 16, 2013
  27. 2/2 diff: avoid some nestingThomas Gummerer, Dec 16, 2013
  28. Junio C HamanoDec 16, 2013
  29. 1/2 diff: move no-index detection to builtin/diff.cThomas Gummerer, Dec 14, 2013
  30. 2/2 diff: don't read index when --no-index is givenThomas Gummerer, Dec 14, 2013

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.