[RFC/PATCH] bisect: clean flags after checking merge bases

git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed

* [RFC/PATCH] bisect: clean flags after checking merge bases
@ 2014-10-30 19:01 Junio C Hamano
  2014-10-30 19:19 ` [PATCH] get_merge_bases(): always clean-up object flags Junio C Hamano
  0 siblings, 1 reply; 2+ messages in thread
From: Junio C Hamano @ 2014-10-30 19:01 UTC (permalink / raw)
  To: git; +Cc: Christian Couder

Unless there is a good reason to belieave that a particular
invocation of a get_merge_bases*() is the last one that cares about
the object flags the computation of merge bases leaves on the
objects, the "cleanup" parameter should always be true, and I do not
think there is one in this codepath.

Found by code inspection.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 * I am planning to eventually change the function signature for
   this function to drop "cleanup" and change rare selected
   codepaths that know what they are doing to call a separate
   function that allows the flags not to be cleaned up.  Two calls
   in builtin/merge-base.c are both the last ones that can leave the
   flags on the objects, but I do not think this one, which happens
   before the main traversal even starts, should pass 0 here.

 bisect.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/bisect.c b/bisect.c
index df09cbc..ccca3b5 100644
--- a/bisect.c
+++ b/bisect.c
@@ -777,7 +777,7 @@ static void check_merge_bases(int no_checkout)
 	int rev_nr;
 	struct commit **rev = get_bad_and_good_commits(&rev_nr);
 
-	result = get_merge_bases_many(rev[0], rev_nr - 1, rev + 1, 0);
+	result = get_merge_bases_many(rev[0], rev_nr - 1, rev + 1, 1);
 
 	for (; result; result = result->next) {
 		const unsigned char *mb = result->item->object.sha1;

^ permalink raw reply related	[flat|nested] 2+ messages in thread

* [PATCH] get_merge_bases(): always clean-up object flags
  2014-10-30 19:01 [RFC/PATCH] bisect: clean flags after checking merge bases Junio C Hamano
@ 2014-10-30 19:19 ` Junio C Hamano
  0 siblings, 0 replies; 2+ messages in thread
From: Junio C Hamano @ 2014-10-30 19:19 UTC (permalink / raw)
  To: git

The callers of get_merge_bases() can choose to leave object flags
used during the merge-base traversal by passing cleanup=0 as a
parameter, but in practice a very few callers can afford to do so
(namely, "git merge-base"), as they need to compute merge base in
preparation for other processing of their own and they need to see
the object without contaminate flags.

Change the function signature of get_merge_bases_many() and
get_merge_bases() to drop the cleanup parameter, so that the
majority of the callers do not have to say ", 1" at the end.

Give a new get_merge_bases_many_dirty() API to support only a few
callers that know they do not need to spend cycles cleaning up the
object flags.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 * This comes on top of the previous "is it kosher for bisect to use
   the 'cleanup=0' variant in the merge-base computation upfront? I
   do not think so" patch.

 bisect.c             |  2 +-
 builtin/merge-base.c |  4 ++--
 builtin/merge.c      |  4 ++--
 builtin/rev-parse.c  |  2 +-
 commit.c             | 29 +++++++++++++++++++++--------
 commit.h             |  7 +++++--
 merge-recursive.c    |  2 +-
 notes-merge.c        |  2 +-
 revision.c           |  4 ++--
 sha1_name.c          |  2 +-
 submodule.c          |  2 +-
 11 files changed, 38 insertions(+), 22 deletions(-)

diff --git a/bisect.c b/bisect.c
index ccca3b5..8c6d843 100644
--- a/bisect.c
+++ b/bisect.c
@@ -777,7 +777,7 @@ static void check_merge_bases(int no_checkout)
 	int rev_nr;
 	struct commit **rev = get_bad_and_good_commits(&rev_nr);
 
-	result = get_merge_bases_many(rev[0], rev_nr - 1, rev + 1, 1);
+	result = get_merge_bases_many(rev[0], rev_nr - 1, rev + 1);
 
 	for (; result; result = result->next) {
 		const unsigned char *mb = result->item->object.sha1;
diff --git a/builtin/merge-base.c b/builtin/merge-base.c
index 0ecde8d..fdebef6 100644
--- a/builtin/merge-base.c
+++ b/builtin/merge-base.c
@@ -10,7 +10,7 @@ static int show_merge_base(struct commit **rev, int rev_nr, int show_all)
 {
 	struct commit_list *result;
 
-	result = get_merge_bases_many(rev[0], rev_nr - 1, rev + 1, 0);
+	result = get_merge_bases_many_dirty(rev[0], rev_nr - 1, rev + 1);
 
 	if (!result)
 		return 1;
@@ -176,7 +176,7 @@ static int handle_fork_point(int argc, const char **argv)
 	for (i = 0; i < revs.nr; i++)
 		revs.commit[i]->object.flags &= ~TMP_MARK;
 
-	bases = get_merge_bases_many(derived, revs.nr, revs.commit, 0);
+	bases = get_merge_bases_many_dirty(derived, revs.nr, revs.commit);
 
 	/*
 	 * There should be one and only one merge base, when we found
diff --git a/builtin/merge.c b/builtin/merge.c
index bebbe5b..5249f4e 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -1320,7 +1320,7 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
 	if (!remoteheads)
 		; /* already up-to-date */
 	else if (!remoteheads->next)
-		common = get_merge_bases(head_commit, remoteheads->item, 1);
+		common = get_merge_bases(head_commit, remoteheads->item);
 	else {
 		struct commit_list *list = remoteheads;
 		commit_list_insert(head_commit, &list);
@@ -1417,7 +1417,7 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
 			 * merge_bases again, otherwise "git merge HEAD^
 			 * HEAD^^" would be missed.
 			 */
-			common_one = get_merge_bases(head_commit, j->item, 1);
+			common_one = get_merge_bases(head_commit, j->item);
 			if (hashcmp(common_one->item->object.sha1,
 				j->item->object.sha1)) {
 				up_to_date = 0;
diff --git a/builtin/rev-parse.c b/builtin/rev-parse.c
index 35d3c43..95328b8 100644
--- a/builtin/rev-parse.c
+++ b/builtin/rev-parse.c
@@ -279,7 +279,7 @@ static int try_difference(const char *arg)
 			struct commit *a, *b;
 			a = lookup_commit_reference(sha1);
 			b = lookup_commit_reference(end);
-			exclude = get_merge_bases(a, b, 1);
+			exclude = get_merge_bases(a, b);
 			while (exclude) {
 				struct commit_list *n = exclude->next;
 				show_rev(REVERSED,
diff --git a/commit.c b/commit.c
index 19cf8f9..d13250e 100644
--- a/commit.c
+++ b/commit.c
@@ -867,7 +867,7 @@ struct commit_list *get_octopus_merge_bases(struct commit_list *in)
 
 		for (j = ret; j; j = j->next) {
 			struct commit_list *bases;
-			bases = get_merge_bases(i->item, j->item, 1);
+			bases = get_merge_bases(i->item, j->item);
 			if (!new)
 				new = bases;
 			else
@@ -936,10 +936,10 @@ static int remove_redundant(struct commit **array, int cnt)
 	return filled;
 }
 
-struct commit_list *get_merge_bases_many(struct commit *one,
-					 int n,
-					 struct commit **twos,
-					 int cleanup)
+static struct commit_list *get_merge_bases_many_0(struct commit *one,
+						  int n,
+						  struct commit **twos,
+						  int cleanup)
 {
 	struct commit_list *list;
 	struct commit **rslt;
@@ -977,10 +977,23 @@ struct commit_list *get_merge_bases_many(struct commit *one,
 	return result;
 }
 
-struct commit_list *get_merge_bases(struct commit *one, struct commit *two,
-				    int cleanup)
+struct commit_list *get_merge_bases_many(struct commit *one,
+					 int n,
+					 struct commit **twos)
+{
+	return get_merge_bases_many_0(one, n, twos, 1);
+}
+
+struct commit_list *get_merge_bases_many_dirty(struct commit *one,
+					       int n,
+					       struct commit **twos)
+{
+	return get_merge_bases_many_0(one, n, twos, 0);
+}
+
+struct commit_list *get_merge_bases(struct commit *one, struct commit *two)
 {
-	return get_merge_bases_many(one, 1, &two, cleanup);
+	return get_merge_bases_many_0(one, 1, &two, 1);
 }
 
 /*
diff --git a/commit.h b/commit.h
index bc68ccb..28bc553 100644
--- a/commit.h
+++ b/commit.h
@@ -236,10 +236,13 @@ struct commit_graft *read_graft_line(char *buf, int len);
 int register_commit_graft(struct commit_graft *, int);
 struct commit_graft *lookup_commit_graft(const unsigned char *sha1);
 
-extern struct commit_list *get_merge_bases(struct commit *rev1, struct commit *rev2, int cleanup);
-extern struct commit_list *get_merge_bases_many(struct commit *one, int n, struct commit **twos, int cleanup);
+extern struct commit_list *get_merge_bases(struct commit *rev1, struct commit *rev2);
+extern struct commit_list *get_merge_bases_many(struct commit *one, int n, struct commit **twos);
 extern struct commit_list *get_octopus_merge_bases(struct commit_list *in);
 
+/* To be used only when object flags after this call no longer matter */
+extern struct commit_list *get_merge_bases_many_dirty(struct commit *one, int n, struct commit **twos);
+
 /* largest positive number a signed 32-bit integer can contain */
 #define INFINITE_DEPTH 0x7fffffff
 
diff --git a/merge-recursive.c b/merge-recursive.c
index fdb7d0f..f758f7e 100644
--- a/merge-recursive.c
+++ b/merge-recursive.c
@@ -1904,7 +1904,7 @@ int merge_recursive(struct merge_options *o,
 	}
 
 	if (!ca) {
-		ca = get_merge_bases(h1, h2, 1);
+		ca = get_merge_bases(h1, h2);
 		ca = reverse_commit_list(ca);
 	}
 
diff --git a/notes-merge.c b/notes-merge.c
index 7eb9d7a..109ff4e 100644
--- a/notes-merge.c
+++ b/notes-merge.c
@@ -594,7 +594,7 @@ int notes_merge(struct notes_merge_options *o,
 	assert(local && remote);
 
 	/* Find merge bases */
-	bases = get_merge_bases(local, remote, 1);
+	bases = get_merge_bases(local, remote);
 	if (!bases) {
 		base_sha1 = null_sha1;
 		base_tree_sha1 = EMPTY_TREE_SHA1_BIN;
diff --git a/revision.c b/revision.c
index 75dda92..14e0e03 100644
--- a/revision.c
+++ b/revision.c
@@ -1441,7 +1441,7 @@ static void prepare_show_merge(struct rev_info *revs)
 	other = lookup_commit_or_die(sha1, "MERGE_HEAD");
 	add_pending_object(revs, &head->object, "HEAD");
 	add_pending_object(revs, &other->object, "MERGE_HEAD");
-	bases = get_merge_bases(head, other, 1);
+	bases = get_merge_bases(head, other);
 	add_rev_cmdline_list(revs, bases, REV_CMD_MERGE_BASE, UNINTERESTING | BOTTOM);
 	add_pending_commit_list(revs, bases, UNINTERESTING | BOTTOM);
 	free_commit_list(bases);
@@ -1546,7 +1546,7 @@ int handle_revision_arg(const char *arg_, struct rev_info *revs, int flags, unsi
 				     : lookup_commit_reference(b_obj->sha1));
 				if (!a || !b)
 					goto missing;
-				exclude = get_merge_bases(a, b, 1);
+				exclude = get_merge_bases(a, b);
 				add_rev_cmdline_list(revs, exclude,
 						     REV_CMD_MERGE_BASE,
 						     flags_exclude);
diff --git a/sha1_name.c b/sha1_name.c
index 5b004f5..9737557 100644
--- a/sha1_name.c
+++ b/sha1_name.c
@@ -993,7 +993,7 @@ int get_sha1_mb(const char *name, unsigned char *sha1)
 	two = lookup_commit_reference_gently(sha1_tmp, 0);
 	if (!two)
 		return -1;
-	mbs = get_merge_bases(one, two, 1);
+	mbs = get_merge_bases(one, two);
 	if (!mbs || mbs->next)
 		st = -1;
 	else {
diff --git a/submodule.c b/submodule.c
index 0690dc5..d37d400 100644
--- a/submodule.c
+++ b/submodule.c
@@ -301,7 +301,7 @@ static int prepare_submodule_summary(struct rev_info *rev, const char *path,
 	left->object.flags |= SYMMETRIC_LEFT;
 	add_pending_object(rev, &left->object, path);
 	add_pending_object(rev, &right->object, path);
-	merge_bases = get_merge_bases(left, right, 1);
+	merge_bases = get_merge_bases(left, right);
 	if (merge_bases) {
 		if (merge_bases->item == left)
 			*fast_forward = 1;

^ permalink raw reply related	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2014-10-30 19:19 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-10-30 19:01 [RFC/PATCH] bisect: clean flags after checking merge bases Junio C Hamano
2014-10-30 19:19 ` [PATCH] get_merge_bases(): always clean-up object flags Junio C Hamano

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