git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Thomas Gummerer <t.gummerer@gmail.com>
To: git@vger.kernel.org
Cc: "Jonathan Nieder" <jrnieder@gmail.com>,
	"Thomas Gummerer" <t.gummerer@gmail.com>,
	"Jens Lehmann" <Jens.Lehmann@web.de>,
	"René Scharfe" <l.s.r@web.de>,
	"Tim Henigan" <tim.henigan@gmail.com>,
	"Junio C Hamano" <gitster@pobox.com>,
	"Alexey Borzenkov" <snaury@gmail.com>,
	"Bobby Powers" <bobbypowers@gmail.com>,
	"Michael Haggerty" <mhagger@alum.mit.edu>,
	"Jeff King" <peff@peff.net>
Subject: [PATCH v2] diff: don't read index when --no-index is given
Date: Mon,  9 Dec 2013 21:40:34 +0100	[thread overview]
Message-ID: <1386621634-25444-1-git-send-email-t.gummerer@gmail.com> (raw)
In-Reply-To: <20131209192000.GS29959@google.com>

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>
---

Thanks to Jonathan and Jens for comments on the previous round.
Changes:
 - Don't all setup_git_directory_gently when --no-index is given
 - Add performance test
 - Commit message improvements

 builtin/diff.c                | 16 +++++++++++++---
 t/perf/p4001-diff-no-index.sh | 17 +++++++++++++++++
 t/t4053-diff-no-index.sh      |  6 ++++++
 3 files changed, 36 insertions(+), 3 deletions(-)
 create mode 100755 t/perf/p4001-diff-no-index.sh

diff --git a/builtin/diff.c b/builtin/diff.c
index adb93a9..5f09a0b 100644
--- a/builtin/diff.c
+++ b/builtin/diff.c
@@ -257,7 +257,7 @@ int cmd_diff(int argc, const char **argv, const char *prefix)
 	int blobs = 0, paths = 0;
 	const char *path = NULL;
 	struct blobinfo blob[2];
-	int nongit;
+	int nongit, no_index = 0;
 	int result = 0;
 
 	/*
@@ -282,9 +282,19 @@ int cmd_diff(int argc, const char **argv, const char *prefix)
 	 *
 	 * Other cases are errors.
 	 */
+	for (i = 1; i < argc; i++) {
+		if (!strcmp(argv[i], "--"))
+			break;
+		if (!strcmp(argv[i], "--no-index")) {
+			no_index = 1;
+			break;
+		}
+	}
 
-	prefix = setup_git_directory_gently(&nongit);
-	gitmodules_config();
+	if (!no_index) {
+		prefix = setup_git_directory_gently(&nongit);
+		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..81c7aa0
--- /dev/null
+++ b/t/perf/p4001-diff-no-index.sh
@@ -0,0 +1,17 @@
+#!/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_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..d3dbf6b 100755
--- a/t/t4053-diff-no-index.sh
+++ b/t/t4053-diff-no-index.sh
@@ -29,4 +29,10 @@ 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_done
-- 
1.8.5.4.g8639e57

  reply	other threads:[~2013-12-09 20:40 UTC|newest]

Thread overview: 30+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-12-09 12:05 [PATCH] diff: don't read index when --no-index is given Thomas Gummerer
2013-12-09 15:16 ` Jonathan Nieder
2013-12-09 17:27   ` Jens Lehmann
2013-12-09 19:06     ` Jonathan Nieder
2013-12-09 19:14   ` Thomas Gummerer
2013-12-09 19:20     ` Jonathan Nieder
2013-12-09 20:40       ` Thomas Gummerer [this message]
2013-12-09 20:55         ` [PATCH v2] " Torsten Bögershausen
2013-12-09 20:57         ` Eric Sunshine
2013-12-09 21:17           ` Thomas Gummerer
2013-12-09 20:30 ` [PATCH] " Junio C Hamano
2013-12-09 21:13   ` Thomas Gummerer
2013-12-10 17:52   ` [PATCH v3 1/2] diff: move no-index detection to builtin/diff.c Thomas Gummerer
2013-12-10 17:52     ` [PATCH v3 2/2] diff: don't read index when --no-index is given Thomas Gummerer
2013-12-10 18:18       ` Jonathan Nieder
2013-12-10 18:55         ` Thomas Gummerer
2013-12-10 18:16     ` [PATCH v3 1/2] diff: move no-index detection to builtin/diff.c Jonathan Nieder
2013-12-10 18:46       ` Thomas Gummerer
2013-12-11  9:58       ` [PATCH v4 " Thomas Gummerer
2013-12-11  9:58         ` [PATCH v4 2/2] diff: don't read index when --no-index is given Thomas Gummerer
2013-12-12 20:25           ` Junio C Hamano
2013-12-14  0:44           ` Jonathan Nieder
2013-12-14  0:43         ` [PATCH v4 1/2] diff: move no-index detection to builtin/diff.c Jonathan Nieder
2013-12-14 10:42           ` Thomas Gummerer
2013-12-16 17:48             ` Junio C Hamano
2013-12-16 19:23               ` [PATCH 1/2] diff: add test for --no-index executed outside repo Thomas Gummerer
2013-12-16 19:23                 ` [PATCH 2/2] diff: avoid some nesting Thomas Gummerer
2013-12-16 19:42                 ` [PATCH 1/2] diff: add test for --no-index executed outside repo Junio C Hamano
2013-12-14 13:07           ` [PATCH v5 1/2] diff: move no-index detection to builtin/diff.c Thomas Gummerer
2013-12-14 13:07             ` [PATCH v5 2/2] diff: don't read index when --no-index is given Thomas Gummerer

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

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1386621634-25444-1-git-send-email-t.gummerer@gmail.com \
    --to=t.gummerer@gmail.com \
    --cc=Jens.Lehmann@web.de \
    --cc=bobbypowers@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=jrnieder@gmail.com \
    --cc=l.s.r@web.de \
    --cc=mhagger@alum.mit.edu \
    --cc=peff@peff.net \
    --cc=snaury@gmail.com \
    --cc=tim.henigan@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.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

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 read-only IMAP folder(s) and NNTP newsgroup(s).