refs/packed-backend.c: add trace2 counters for jump list

The previous commit added low-level tests to ensure that the packed-refs
iterator did not enumerate excluded sections of the refspace.

However, there was no guarantee that these sections weren't being
visited, only that they were being suppressed from the output. To harden
these tests, add a trace2 counter which tracks the number of regions
skipped by the packed-refs iterator, and assert on its value.

Suggested-by: Derrick Stolee <derrickstolee@github.com>
Signed-off-by: Taylor Blau <me@ttaylorr.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/refs/packed-backend.c b/refs/packed-backend.c
index 092b50fa844f08e03ad14da8e16cf5a4c7a93cb2..97cc2090fdf11eb94259fc8c66d6db7fc7b07791 100644 (file)
--- a/refs/packed-backend.c
+++ b/refs/packed-backend.c
@@ -12,6 +12,7 @@
 #include "../chdir-notify.h"
 #include "../wrapper.h"
 #include "../write-or-die.h"
+#include "../trace2.h"
 
 enum mmap_strategy {
        /*
@@ -845,6 +846,7 @@ static int next_record(struct packed_ref_iterator *iter)
                iter->jump_cur++;
                if (iter->pos < curr->end) {
                        iter->pos = curr->end;
+                       trace2_counter_add(TRACE2_COUNTER_ID_PACKED_REFS_JUMPS, 1);
                        /* jumps are coalesced, so only one jump is necessary */
                        break;
                }

diff --git a/t/t1419-exclude-refs.sh b/t/t1419-exclude-refs.sh
index bc534c8ea18d8de23109180e5f5d1b35c723705c..5d8c86b657399e0d8852909a33491449f1eaefa9 100755 (executable)
--- a/t/t1419-exclude-refs.sh
+++ b/t/t1419-exclude-refs.sh
@@ -9,7 +9,8 @@ TEST_PASSES_SANITIZE_LEAK=true
 . ./test-lib.sh
 
 for_each_ref__exclude () {
-       test-tool ref-store main for-each-ref--exclude "$@" >actual.raw
+       GIT_TRACE2_PERF=1 test-tool ref-store main \
+               for-each-ref--exclude "$@" >actual.raw
        cut -d ' ' -f 2 actual.raw
 }
 
@@ -17,6 +18,17 @@ for_each_ref () {
        git for-each-ref --format='%(refname)' "$@"
 }
 
+assert_jumps () {
+       local nr="$1"
+       local trace="$2"
+
+       grep -q "name:jumps_made value:$nr$" $trace
+}
+
+assert_no_jumps () {
+       ! assert_jumps ".*" "$1"
+}
+
 test_expect_success 'setup' '
        test_commit --no-tag base &&
        base="$(git rev-parse HEAD)" &&
@@ -35,67 +47,76 @@ test_expect_success 'setup' '
 '
 
 test_expect_success 'excluded region in middle' '
-       for_each_ref__exclude refs/heads refs/heads/foo >actual &&
+       for_each_ref__exclude refs/heads refs/heads/foo >actual 2>perf &&
        for_each_ref refs/heads/bar refs/heads/baz refs/heads/quux >expect &&
 
-       test_cmp expect actual
+       test_cmp expect actual &&
+       assert_jumps 1 perf
 '
 
 test_expect_success 'excluded region at beginning' '
-       for_each_ref__exclude refs/heads refs/heads/bar >actual &&
+       for_each_ref__exclude refs/heads refs/heads/bar >actual 2>perf &&
        for_each_ref refs/heads/baz refs/heads/foo refs/heads/quux >expect &&
 
-       test_cmp expect actual
+       test_cmp expect actual &&
+       assert_jumps 1 perf
 '
 
 test_expect_success 'excluded region at end' '
-       for_each_ref__exclude refs/heads refs/heads/quux >actual &&
+       for_each_ref__exclude refs/heads refs/heads/quux >actual 2>perf &&
        for_each_ref refs/heads/foo refs/heads/bar refs/heads/baz >expect &&
 
-       test_cmp expect actual
+       test_cmp expect actual &&
+       assert_jumps 1 perf
 '
 
 test_expect_success 'disjoint excluded regions' '
-       for_each_ref__exclude refs/heads refs/heads/bar refs/heads/quux >actual &&
+       for_each_ref__exclude refs/heads refs/heads/bar refs/heads/quux >actual 2>perf &&
        for_each_ref refs/heads/baz refs/heads/foo >expect &&
 
-       test_cmp expect actual
+       test_cmp expect actual &&
+       assert_jumps 2 perf
 '
 
 test_expect_success 'adjacent, non-overlapping excluded regions' '
-       for_each_ref__exclude refs/heads refs/heads/bar refs/heads/baz >actual &&
+       for_each_ref__exclude refs/heads refs/heads/bar refs/heads/baz >actual 2>perf &&
        for_each_ref refs/heads/foo refs/heads/quux >expect &&
 
-       test_cmp expect actual
+       test_cmp expect actual &&
+       assert_jumps 1 perf
 '
 
 test_expect_success 'overlapping excluded regions' '
-       for_each_ref__exclude refs/heads refs/heads/ba refs/heads/baz >actual &&
+       for_each_ref__exclude refs/heads refs/heads/ba refs/heads/baz >actual 2>perf &&
        for_each_ref refs/heads/foo refs/heads/quux >expect &&
 
-       test_cmp expect actual
+       test_cmp expect actual &&
+       assert_jumps 1 perf
 '
 
 test_expect_success 'several overlapping excluded regions' '
        for_each_ref__exclude refs/heads \
-               refs/heads/bar refs/heads/baz refs/heads/foo >actual &&
+               refs/heads/bar refs/heads/baz refs/heads/foo >actual 2>perf &&
        for_each_ref refs/heads/quux >expect &&
 
-       test_cmp expect actual
+       test_cmp expect actual &&
+       assert_jumps 1 perf
 '
 
 test_expect_success 'non-matching excluded section' '
-       for_each_ref__exclude refs/heads refs/heads/does/not/exist >actual &&
+       for_each_ref__exclude refs/heads refs/heads/does/not/exist >actual 2>perf &&
        for_each_ref >expect &&
 
-       test_cmp expect actual
+       test_cmp expect actual &&
+       assert_no_jumps perf
 '
 
 test_expect_success 'meta-characters are discarded' '
-       for_each_ref__exclude refs/heads "refs/heads/ba*" >actual &&
+       for_each_ref__exclude refs/heads "refs/heads/ba*" >actual 2>perf &&
        for_each_ref >expect &&
 
-       test_cmp expect actual
+       test_cmp expect actual &&
+       assert_no_jumps perf
 '
 
 test_done

diff --git a/trace2.h b/trace2.h
index 4ced30c0db368b640fbaba439ad50049ba9041df..9452e291f5125ce70184b2c88d2297ba05ba1652 100644 (file)
--- a/trace2.h
+++ b/trace2.h
@@ -551,6 +551,8 @@ enum trace2_counter_id {
        TRACE2_COUNTER_ID_TEST1 = 0, /* emits summary event only */
        TRACE2_COUNTER_ID_TEST2,     /* emits summary and thread events */
 
+       TRACE2_COUNTER_ID_PACKED_REFS_JUMPS, /* counts number of jumps */
+
        /* Add additional counter definitions before here. */
        TRACE2_NUMBER_OF_COUNTERS
 };

diff --git a/trace2/tr2_ctr.c b/trace2/tr2_ctr.c
index b342d3b1a3c033bf539037bb5035ae57af119d6d..50570d0165324f6eaaaff581fdea664a1261068f 100644 (file)
--- a/trace2/tr2_ctr.c
+++ b/trace2/tr2_ctr.c
@@ -27,6 +27,11 @@ static struct tr2_counter_metadata tr2_counter_metadata[TRACE2_NUMBER_OF_COUNTER
                .name = "test2",
                .want_per_thread_events = 1,
        },
+       [TRACE2_COUNTER_ID_PACKED_REFS_JUMPS] = {
+               .category = "packed-refs",
+               .name = "jumps_made",
+               .want_per_thread_events = 0,
+       },
 
        /* Add additional metadata before here. */
 };