revision.c: discount side branches when computing TREESAME
Use the BOTTOM flag to define relevance for pruning. Relevant commits are those that are !UNINTERESTING or BOTTOM, and this allows us to identify irrelevant side branches (UNINTERESTING && !BOTTOM). If a merge has relevant parents, and it is TREESAME to them, then do not let irrelevant parents cause the merge to be treated as !TREESAME. When considering simplification, don't always include all merges - merges with exactly one relevant parent can be simplified, if TREESAME according to the above rule. These two changes greatly increase simplification in limited, pruned revision lists. Signed-off-by: Kevin Bracey <kevin@bracey.fi> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Kevin Bracey committed
May 16, 2013 at 18:32 UTC
4d826608e9610851d29440ca290e54b701921608
3 files changed
+164
-27
revision.c
+150
-21
@@ -332,6 +332,80 @@ static int everybody_uninteresting(struct commit_list *orig)
332
return 1;
333
}
334
335
+/*
336
+ * A definition of "relevant" commit that we can use to simplify limited graphs
337
+ * by eliminating side branches.
338
+ *
339
+ * A "relevant" commit is one that is !UNINTERESTING (ie we are including it
340
+ * in our list), or that is a specified BOTTOM commit. Then after computing
341
+ * a limited list, during processing we can generally ignore boundary merges
342
+ * coming from outside the graph, (ie from irrelevant parents), and treat
343
+ * those merges as if they were single-parent. TREESAME is defined to consider
344
+ * only relevant parents, if any. If we are TREESAME to our on-graph parents,
345
+ * we don't care if we were !TREESAME to non-graph parents.
346
+ *
347
+ * Treating bottom commits as relevant ensures that a limited graph's
348
+ * connection to the actual bottom commit is not viewed as a side branch, but
349
+ * treated as part of the graph. For example:
350
+ *
351
+ * ....Z...A---X---o---o---B
352
+ * . /
353
+ * W---Y
354
+ *
355
+ * When computing "A..B", the A-X connection is at least as important as
356
+ * Y-X, despite A being flagged UNINTERESTING.
357
+ *
358
+ * And when computing --ancestry-path "A..B", the A-X connection is more
359
+ * important than Y-X, despite both A and Y being flagged UNINTERESTING.
360
+ */
361
+static inline int relevant_commit(struct commit *commit)
362
+{
363
+ return (commit->object.flags & (UNINTERESTING | BOTTOM)) != UNINTERESTING;
364
+}
365
+
366
+/*
367
+ * Return a single relevant commit from a parent list. If we are a TREESAME
368
+ * commit, and this selects one of our parents, then we can safely simplify to
369
+ * that parent.
370
+ */
371
+static struct commit *one_relevant_parent(const struct rev_info *revs,
372
+ struct commit_list *orig)
373
+{
374
+ struct commit_list *list = orig;
375
+ struct commit *relevant = NULL;
376
+
377
+ if (!orig)
378
+ return NULL;
379
+
380
+ /*
381
+ * For 1-parent commits, or if first-parent-only, then return that
382
+ * first parent (even if not "relevant" by the above definition).
383
+ * TREESAME will have been set purely on that parent.
384
+ */
385
+ if (revs->first_parent_only || !orig->next)
386
+ return orig->item;
387
+
388
+ /*
389
+ * For multi-parent commits, identify a sole relevant parent, if any.
390
+ * If we have only one relevant parent, then TREESAME will be set purely
391
+ * with regard to that parent, and we can simplify accordingly.
392
+ *
393
+ * If we have more than one relevant parent, or no relevant parents
394
+ * (and multiple irrelevant ones), then we can't select a parent here
395
+ * and return NULL.
396
+ */
397
+ while (list) {
398
+ struct commit *commit = list->item;
399
+ list = list->next;
400
+ if (relevant_commit(commit)) {
401
+ if (relevant)
402
+ return NULL;
403
+ relevant = commit;
404
+ }
405
+ }
406
+ return relevant;
407
+}
408
+
409
/*
410
* The goal is to get REV_TREE_NEW as the result only if the
411
* diff consists of all '+' (and no other changes), REV_TREE_OLD
@@ -502,27 +576,52 @@ static unsigned update_treesame(struct rev_info *revs, struct commit *commit)
576
if (commit->parents && commit->parents->next) {
577
unsigned n;
578
struct treesame_state *st;
579
+ struct commit_list *p;
580
+ unsigned relevant_parents;
581
+ unsigned relevant_change, irrelevant_change;
582
583
st = lookup_decoration(&revs->treesame, &commit->object);
584
if (!st)
585
die("update_treesame %s", sha1_to_hex(commit->object.sha1));
509
- commit->object.flags |= TREESAME;
510
- for (n = 0; n < st->nparents; n++) {
511
- if (!st->treesame[n]) {
512
- commit->object.flags &= ~TREESAME;
513
- break;
514
- }
586
+ relevant_parents = 0;
587
+ relevant_change = irrelevant_change = 0;
588
+ for (p = commit->parents, n = 0; p; n++, p = p->next) {
589
+ if (relevant_commit(p->item)) {
590
+ relevant_change |= !st->treesame[n];
591
+ relevant_parents++;
592
+ } else
593
+ irrelevant_change |= !st->treesame[n];
594
}
595
+ if (relevant_parents ? relevant_change : irrelevant_change)
596
+ commit->object.flags &= ~TREESAME;
597
+ else
598
+ commit->object.flags |= TREESAME;
599
}
600
601
return commit->object.flags & TREESAME;
602
}
603
604
+static inline int limiting_can_increase_treesame(const struct rev_info *revs)
605
+{
606
+ /*
607
+ * TREESAME is irrelevant unless prune && dense;
608
+ * if simplify_history is set, we can't have a mixture of TREESAME and
609
+ * !TREESAME INTERESTING parents (and we don't have treesame[]
610
+ * decoration anyway);
611
+ * if first_parent_only is set, then the TREESAME flag is locked
612
+ * against the first parent (and again we lack treesame[] decoration).
613
+ */
614
+ return revs->prune && revs->dense &&
615
+ !revs->simplify_history &&
616
+ !revs->first_parent_only;
617
+}
618
+
619
static void try_to_simplify_commit(struct rev_info *revs, struct commit *commit)
620
{
621
struct commit_list **pp, *parent;
622
struct treesame_state *ts = NULL;
525
- int tree_changed = 0, nth_parent;
623
+ int relevant_change = 0, irrelevant_change = 0;
624
+ int relevant_parents, nth_parent;
625
626
/*
627
* If we don't do pruning, everything is interesting
@@ -546,10 +645,12 @@ static void try_to_simplify_commit(struct rev_info *revs, struct commit *commit)
645
if (!revs->dense && !commit->parents->next)
646
return;
647
549
- for (pp = &commit->parents, nth_parent = 0;
648
+ for (pp = &commit->parents, nth_parent = 0, relevant_parents = 0;
649
(parent = *pp) != NULL;
650
pp = &parent->next, nth_parent++) {
651
struct commit *p = parent->item;
652
+ if (relevant_commit(p))
653
+ relevant_parents++;
654
655
if (nth_parent == 1) {
656
/*
@@ -573,7 +674,7 @@ static void try_to_simplify_commit(struct rev_info *revs, struct commit *commit)
674
!revs->simplify_history &&
675
!(commit->object.flags & UNINTERESTING)) {
676
ts = initialise_treesame(revs, commit);
576
- if (!tree_changed)
677
+ if (!(irrelevant_change || relevant_change))
678
ts->treesame[0] = 1;
679
}
680
}
@@ -619,14 +720,27 @@ static void try_to_simplify_commit(struct rev_info *revs, struct commit *commit)
720
/* fallthrough */
721
case REV_TREE_OLD:
722
case REV_TREE_DIFFERENT:
622
- tree_changed = 1;
723
+ if (relevant_commit(p))
724
+ relevant_change = 1;
725
+ else
726
+ irrelevant_change = 1;
727
continue;
728
}
729
die("bad tree compare for commit %s", sha1_to_hex(commit->object.sha1));
730
}
627
- if (tree_changed)
628
- return;
629
- commit->object.flags |= TREESAME;
731
+
732
+ /*
733
+ * TREESAME is straightforward for single-parent commits. For merge
734
+ * commits, it is most useful to define it so that "irrelevant"
735
+ * parents cannot make us !TREESAME - if we have any relevant
736
+ * parents, then we only consider TREESAMEness with respect to them,
737
+ * allowing irrelevant merges from uninteresting branches to be
738
+ * simplified away. Only if we have only irrelevant parents do we
739
+ * base TREESAME on them. Note that this logic is replicated in
740
+ * update_treesame, which should be kept in sync.
741
+ */
742
+ if (relevant_parents ? !relevant_change : !irrelevant_change)
743
+ commit->object.flags |= TREESAME;
744
}
745
746
static void commit_list_insert_by_date_cached(struct commit *p, struct commit_list **head,
@@ -998,6 +1112,18 @@ static int limit_list(struct rev_info *revs)
1112
free_commit_list(bottom);
1113
}
1114
1115
+ /*
1116
+ * Check if any commits have become TREESAME by some of their parents
1117
+ * becoming UNINTERESTING.
1118
+ */
1119
+ if (limiting_can_increase_treesame(revs))
1120
+ for (list = newlist; list; list = list->next) {
1121
+ struct commit *c = list->item;
1122
+ if (c->object.flags & (UNINTERESTING | TREESAME))
1123
+ continue;
1124
+ update_treesame(revs, c);
1125
+ }
1126
+
1127
revs->commits = newlist;
1128
return 0;
1129
}
@@ -2248,6 +2374,7 @@ static int remove_marked_parents(struct rev_info *revs, struct commit *commit)
2374
static struct commit_list **simplify_one(struct rev_info *revs, struct commit *commit, struct commit_list **tail)
2375
{
2376
struct commit_list *p;
2377
+ struct commit *parent;
2378
struct merge_simplify_state *st, *pst;
2379
int cnt;
2380
@@ -2336,19 +2463,20 @@ static struct commit_list **simplify_one(struct rev_info *revs, struct commit *c
2463
/*
2464
* A commit simplifies to itself if it is a root, if it is
2465
* UNINTERESTING, if it touches the given paths, or if it is a
2339
- * merge and its parents simplify to more than one commit
2466
+ * merge and its parents don't simplify to one relevant commit
2467
* (the first two cases are already handled at the beginning of
2468
* this function).
2469
*
2343
- * Otherwise, it simplifies to what its sole parent simplifies to.
2470
+ * Otherwise, it simplifies to what its sole relevant parent
2471
+ * simplifies to.
2472
*/
2473
if (!cnt ||
2474
(commit->object.flags & UNINTERESTING) ||
2475
!(commit->object.flags & TREESAME) ||
2348
- (1 < cnt))
2476
+ (parent = one_relevant_parent(revs, commit->parents)) == NULL)
2477
st->simplified = commit;
2478
else {
2351
- pst = locate_simplify_state(revs, commit->parents->item);
2479
+ pst = locate_simplify_state(revs, parent);
2480
st->simplified = pst->simplified;
2481
}
2482
return tail;
@@ -2445,7 +2573,8 @@ int prepare_revision_walk(struct rev_info *revs)
2573
free(list);
2574
2575
/* Signal whether we need per-parent treesame decoration */
2448
- if (revs->simplify_merges)
2576
+ if (revs->simplify_merges ||
2577
+ (revs->limited && limiting_can_increase_treesame(revs)))
2578
revs->treesame.name = "treesame";
2579
2580
if (revs->no_walk != REVISION_WALK_NO_WALK_UNSORTED)
@@ -2479,15 +2608,15 @@ static enum rewrite_result rewrite_one(struct rev_info *revs, struct commit **pp
2608
if (!revs->limited)
2609
if (add_parents_to_list(revs, p, &revs->commits, &cache) < 0)
2610
return rewrite_one_error;
2482
- if (p->parents && p->parents->next)
2483
- return rewrite_one_ok;
2611
if (p->object.flags & UNINTERESTING)
2612
return rewrite_one_ok;
2613
if (!(p->object.flags & TREESAME))
2614
return rewrite_one_ok;
2615
if (!p->parents)
2616
return rewrite_one_noparents;
2490
- *pp = p->parents->item;
2617
+ if ((p = one_relevant_parent(revs, p->parents)) == NULL)
2618
+ return rewrite_one_ok;
2619
+ *pp = p;
2620
}
2621
}
2622
t/t6019-rev-list-ancestry-path.sh
+10
-2
@@ -18,7 +18,8 @@ test_description='--ancestry-path'
18
# --ancestry-path F...I == F H I
19
#
20
# G..M -- G.t == [nothing - was dropped in "-s ours" merge L]
21
-# --ancestry-path G..M -- G.t == H J L
21
+# --ancestry-path G..M -- G.t == L
22
+# --ancestry-path --simplify-merges G^..M -- G.t == G L
23
24
. ./test-lib.sh
25
@@ -101,8 +102,15 @@ test_expect_success 'rev-list G..M -- G.t' '
102
'
103
104
test_expect_success 'rev-list --ancestry-path G..M -- G.t' '
104
- for c in H J L; do echo $c; done >expect &&
105
+ echo L >expect &&
106
git rev-list --ancestry-path --format=%s G..M -- G.t |
107
+ sed -e "/^commit /d" >actual &&
108
+ test_cmp expect actual
109
+'
110
+
111
+test_expect_success 'rev-list --ancestry-path --simplify-merges G^..M -- G.t' '
112
+ for c in G L; do echo $c; done >expect &&
113
+ git rev-list --ancestry-path --simplify-merges --format=%s G^..M -- G.t |
114
sed -e "/^commit /d" |
115
sort >actual &&
116
test_cmp expect actual
t/t6111-rev-list-treesame.sh
+4
-4
@@ -138,9 +138,9 @@ check_result 'M L G' F..M --first-parent -- file
138
# Note that G is pruned when E is the bottom, even if it's the same commit list
139
# If we want history since E, then we're quite happy to ignore G that took E.
140
check_result 'M L K J I H G' E..M --ancestry-path
141
-check_outcome failure 'M L J I H' E..M --ancestry-path -- file # includes G
141
+check_result 'M L J I H' E..M --ancestry-path -- file
142
check_outcome failure '(LH)M (K)L (EJ)K (I)J (E)I (E)H' E..M --ancestry-path --parents -- file # includes G
143
-check_outcome failure '(LH)M (E)H (J)L (I)J (E)I' E..M --ancestry-path --simplify-merges -- file # includes G
143
+check_result '(LH)M (E)H (J)L (I)J (E)I' E..M --ancestry-path --simplify-merges -- file
144
145
# Should still be able to ignore I-J branch in simple log, despite limiting
146
# to G.
@@ -167,9 +167,9 @@ check_result 'F D' B..F --full-history -- file
167
check_result '(D)F (BA)D' B..F --full-history --parents -- file
168
check_result '(B)F' B..F --simplify-merges -- file
169
check_result 'F D' B..F --ancestry-path
170
-check_outcome failure 'F' B..F --ancestry-path -- file # includes D
170
+check_result 'F' B..F --ancestry-path -- file
171
check_outcome failure 'F' B..F --ancestry-path --parents -- file # includes D
172
-check_outcome failure 'F' B..F --ancestry-path --simplify-merges -- file # includes D
172
+check_result 'F' B..F --ancestry-path --simplify-merges -- file
173
check_result 'F D' B..F --first-parent
174
check_result 'F' B..F --first-parent -- file
175