ref-filter: use separate cache for contains_tag_algo
The algorithm which powers "tag --contains" uses the TMP_MARK and UNINTERESTING bits, but never cleans up after itself. As a result, stale UNINTERESTING bits may impact later traversals (like "--merged"). We could fix this by clearing the bits after we're done with the --contains traversal. That would be enough to fix the existing problem, but it leaves future developers in a bad spot: they cannot add other traversals that operate simultaneously with --contains (e.g., if you wanted to add "--no-contains" and use both filters at the same time). Instead, we can use a commit slab to store our cached results, which will store the bits outside of the commit structs entirely. This adds an extra level of indirection, but in my tests (running "git tag --contains HEAD" on linux.git), there was no measurable slowdown. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Jeff King committed
Mar 9, 2017 at 08:29 UTC
a91aca44bf4b3746c3ce03583a1b3418d1610ef7
1 file changed
+35
-20
ref-filter.c
+35
-20
@@ -15,6 +15,7 @@
15
#include "version.h"
16
#include "trailer.h"
17
#include "wt-status.h"
18
+#include "commit-slab.h"
19
20
static struct ref_msg {
21
const char *gone;
@@ -1470,15 +1471,22 @@ static void get_ref_atom_value(struct ref_array_item *ref, int atom, struct atom
1471
*v = &ref->value[atom];
1472
}
1473
1474
+/*
1475
+ * Unknown has to be "0" here, because that's the default value for
1476
+ * contains_cache slab entries that have not yet been assigned.
1477
+ */
1478
enum contains_result {
1474
- CONTAINS_UNKNOWN = -1,
1475
- CONTAINS_NO = 0,
1476
- CONTAINS_YES = 1
1479
+ CONTAINS_UNKNOWN = 0,
1480
+ CONTAINS_NO,
1481
+ CONTAINS_YES
1482
};
1483
1484
+define_commit_slab(contains_cache, enum contains_result);
1485
+
1486
struct ref_filter_cbdata {
1487
struct ref_array *array;
1488
struct ref_filter *filter;
1489
+ struct contains_cache contains_cache;
1490
};
1491
1492
/*
@@ -1509,20 +1517,22 @@ static int in_commit_list(const struct commit_list *want, struct commit *c)
1517
* Do not recurse to find out, though, but return -1 if inconclusive.
1518
*/
1519
static enum contains_result contains_test(struct commit *candidate,
1512
- const struct commit_list *want)
1520
+ const struct commit_list *want,
1521
+ struct contains_cache *cache)
1522
{
1514
- /* was it previously marked as containing a want commit? */
1515
- if (candidate->object.flags & TMP_MARK)
1516
- return CONTAINS_YES;
1517
- /* or marked as not possibly containing a want commit? */
1518
- if (candidate->object.flags & UNINTERESTING)
1519
- return CONTAINS_NO;
1523
+ enum contains_result *cached = contains_cache_at(cache, candidate);
1524
+
1525
+ /* If we already have the answer cached, return that. */
1526
+ if (*cached)
1527
+ return *cached;
1528
+
1529
/* or are we it? */
1530
if (in_commit_list(want, candidate)) {
1522
- candidate->object.flags |= TMP_MARK;
1531
+ *cached = CONTAINS_YES;
1532
return CONTAINS_YES;
1533
}
1534
1535
+ /* Otherwise, we don't know; prepare to recurse */
1536
parse_commit_or_die(candidate);
1537
return CONTAINS_UNKNOWN;
1538
}
@@ -1535,10 +1545,11 @@ static void push_to_contains_stack(struct commit *candidate, struct contains_sta
1545
}
1546
1547
static enum contains_result contains_tag_algo(struct commit *candidate,
1538
- const struct commit_list *want)
1548
+ const struct commit_list *want,
1549
+ struct contains_cache *cache)
1550
{
1551
struct contains_stack contains_stack = { 0, 0, NULL };
1541
- enum contains_result result = contains_test(candidate, want);
1552
+ enum contains_result result = contains_test(candidate, want, cache);
1553
1554
if (result != CONTAINS_UNKNOWN)
1555
return result;
@@ -1550,16 +1561,16 @@ static enum contains_result contains_tag_algo(struct commit *candidate,
1561
struct commit_list *parents = entry->parents;
1562
1563
if (!parents) {
1553
- commit->object.flags |= UNINTERESTING;
1564
+ *contains_cache_at(cache, commit) = CONTAINS_NO;
1565
contains_stack.nr--;
1566
}
1567
/*
1568
* If we just popped the stack, parents->item has been marked,
1569
* therefore contains_test will return a meaningful yes/no.
1570
*/
1560
- else switch (contains_test(parents->item, want)) {
1571
+ else switch (contains_test(parents->item, want, cache)) {
1572
case CONTAINS_YES:
1562
- commit->object.flags |= TMP_MARK;
1573
+ *contains_cache_at(cache, commit) = CONTAINS_YES;
1574
contains_stack.nr--;
1575
break;
1576
case CONTAINS_NO:
@@ -1571,13 +1582,14 @@ static enum contains_result contains_tag_algo(struct commit *candidate,
1582
}
1583
}
1584
free(contains_stack.contains_stack);
1574
- return contains_test(candidate, want);
1585
+ return contains_test(candidate, want, cache);
1586
}
1587
1577
-static int commit_contains(struct ref_filter *filter, struct commit *commit)
1588
+static int commit_contains(struct ref_filter *filter, struct commit *commit,
1589
+ struct contains_cache *cache)
1590
{
1591
if (filter->with_commit_tag_algo)
1580
- return contains_tag_algo(commit, filter->with_commit) == CONTAINS_YES;
1592
+ return contains_tag_algo(commit, filter->with_commit, cache) == CONTAINS_YES;
1593
return is_descendant_of(commit, filter->with_commit);
1594
}
1595
@@ -1774,7 +1786,7 @@ static int ref_filter_handler(const char *refname, const struct object_id *oid,
1786
return 0;
1787
/* We perform the filtering for the '--contains' option */
1788
if (filter->with_commit &&
1777
- !commit_contains(filter, commit))
1789
+ !commit_contains(filter, commit, &ref_cbdata->contains_cache))
1790
return 0;
1791
}
1792
@@ -1874,6 +1886,8 @@ int filter_refs(struct ref_array *array, struct ref_filter *filter, unsigned int
1886
broken = 1;
1887
filter->kind = type & FILTER_REFS_KIND_MASK;
1888
1889
+ init_contains_cache(&ref_cbdata.contains_cache);
1890
+
1891
/* Simple per-ref filtering */
1892
if (!filter->kind)
1893
die("filter_refs: invalid type");
@@ -1896,6 +1910,7 @@ int filter_refs(struct ref_array *array, struct ref_filter *filter, unsigned int
1910
head_ref(ref_filter_handler, &ref_cbdata);
1911
}
1912
1913
+ clear_contains_cache(&ref_cbdata.contains_cache);
1914
1915
/* Filters that need revision walking */
1916
if (filter->merge_commit)