git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Jeff King <peff@peff.net>
To: git@vger.kernel.org
Cc: Michael Haggerty <mhagger@alum.mit.edu>,
	Kevin Willford <kewillf@microsoft.com>
Subject: [PATCH 2/2] patch-ids: skip merge commits
Date: Wed, 7 Sep 2016 03:54:25 -0400	[thread overview]
Message-ID: <20160907075425.6jgamuwm66xbklu3@sigill.intra.peff.net> (raw)
In-Reply-To: <20160907075346.z6wtmqnfc6bsunjb@sigill.intra.peff.net>

The patch-ids code which powers "log --cherry-pick" doesn't
look at whether each commit is a merge or not. It just feeds
the commit's first parent to the diff, and ignores any
additional parents.

In theory, this might be useful if you wanted to find
equivalence between, say, a merge commit and a squash-merge
that does the same thing.  But that may also be the wrong
thing; the diffs may be the same, but the meaning of the two
commits is definitely not identical. We should err on the
side of _not_ matching such commits.

Moreover, we may spend a lot of extra time computing these
merge diffs. In the case that inspired this patch, a "git
format-patch --cherry-pick" dropped from over 3 minutes to
less than 4 seconds. This seems pretty drastic, but is
easily explained. The command was invoked by a "git rebase"
of an older topic branch; there had been tens of thousands
of commits on the upstream branch in the meantime. In
addition, this project used a topic-branch workflow with
occasional "back-merges" from "master" to each topic (to
resolve conflicts on the topics rather than in the merge
commits). So there were not only extra merges, but the diffs
for these back-merges were generally quite large (because
they represented _everything_ that had been merged to master
since the topic branched).

This patch just ignores merge commits entirely when
generating patch-ids, meaning they will never be matched
(from either side of a symmetric-diff traversal).

Signed-off-by: Jeff King <peff@peff.net>
---
 patch-ids.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/patch-ids.c b/patch-ids.c
index 77e4663..b1f8514 100644
--- a/patch-ids.c
+++ b/patch-ids.c
@@ -7,10 +7,12 @@
 int commit_patch_id(struct commit *commit, struct diff_options *options,
 		    unsigned char *sha1, int diff_header_only)
 {
-	if (commit->parents)
+	if (commit->parents) {
+		if (commit->parents->next)
+			return 0;
 		diff_tree_sha1(commit->parents->item->object.oid.hash,
 			       commit->object.oid.hash, "", options);
-	else
+	} else
 		diff_root_tree_sha1(commit->object.oid.hash, "", options);
 	diffcore_std(options);
 	return diff_flush_patch_id(options, sha1, diff_header_only);
-- 
2.10.0.rc2.154.gb4a4b8b

  parent reply	other threads:[~2016-09-07  7:54 UTC|newest]

Thread overview: 35+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-09-07  7:53 [RFC/PATCH 0/2] more patch-id speedups Jeff King
2016-09-07  7:54 ` [PATCH 1/2] patch-ids: turn off rename detection Jeff King
2016-09-07 12:53   ` Johannes Schindelin
2016-09-07  7:54 ` Jeff King [this message]
2016-09-07 12:52   ` [PATCH 2/2] patch-ids: skip merge commits Johannes Schindelin
2016-09-07 18:46     ` Jeff King
2016-09-07 22:08       ` Jeff King
2016-09-07 13:06 ` [RFC/PATCH 0/2] more patch-id speedups Johannes Schindelin
2016-09-07 18:49   ` Jeff King
2016-09-07 22:01 ` [RFC/PATCH v2 0/3] patch-id for merges Jeff King
2016-09-07 22:02   ` [PATCH 1/3] patch-ids: turn off rename detection Jeff King
2016-09-07 22:12     ` Jacob Keller
2016-09-07 22:04   ` [PATCH 2/3] diff_flush_patch_id: stop returning error result Jeff King
2016-09-08  0:51     ` Ramsay Jones
2016-09-08  7:20       ` Jeff King
2016-09-09 10:28     ` Johannes Schindelin
2016-09-09 10:40       ` Jeff King
2016-09-09 12:58         ` Johannes Schindelin
2016-09-09 19:37           ` Jeff King
2016-09-07 22:04   ` [PATCH 3/3] patch-ids: use commit sha1 as patch-id for merge commits Jeff King
2016-09-07 22:28     ` Jacob Keller
2016-09-07 22:38       ` Jeff King
2016-09-07 22:51   ` [RFC/PATCH v2 0/3] patch-id for merges Josh Triplett
2016-09-08  7:30     ` Jeff King
2016-09-09 20:34   ` [PATCH v3 0/2] " Jeff King
2016-09-09 20:34     ` [PATCH v3 1/2] patch-ids: turn off rename detection Jeff King
2016-09-09 20:34     ` [PATCH v3 2/2] patch-ids: define patch-id of merge commits as "null" Jeff King
2016-09-09 20:37       ` Jeff King
2016-09-09 21:13       ` Junio C Hamano
2016-09-09 21:01     ` [PATCH v3 0/2] patch-id for merges Junio C Hamano
2016-09-12 15:59       ` Jeff King
2016-09-12 17:18         ` Junio C Hamano
2016-09-12 17:56           ` Jeff King
2016-09-12 20:44             ` Junio C Hamano
2016-09-25 18:25     ` Johannes Schindelin

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=20160907075425.6jgamuwm66xbklu3@sigill.intra.peff.net \
    --to=peff@peff.net \
    --cc=git@vger.kernel.org \
    --cc=kewillf@microsoft.com \
    --cc=mhagger@alum.mit.edu \
    /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).