Re: [PATCH v3 3/5] repack: add --sparse and pass to pack-objects

[Derrick Stolee <stolee@gmail.com>]

On 6/24/2019 8:07 AM, Nathaniel Filardo wrote:
> The sparse connectivity algorithm saves a whole lot of time when there
> are UNINTERESTING trees around.

Neat! Any chance for some example performance stats?

> ---

Looks like you forgot your Signed-off-by here.

> +# repack --sparse invokes pack-objects --sparse
> +test_expect_success 'repack --sparse and fsck' '
> +	git repack -a --sparse &&
> +	git fsck
> +'

This test may not be enough to properly test the sparse
algorithm, as it is only really enabled when the --revs
argument is given to the pack-objects process AND there
is a "NOT" ref (i.e. "!{oid}" over stdin). Using "-a" here
will not have any "NOT" references. OR maybe this already
is enough when you have the .keep packs. Is there a way
you could set up the packs in this test to explicitly be
in the situation you describe where the sparse option
speeds things up?

It's hard to check that the '--sparse' option is doing
anything in a test, but it is important that we run the
logic. One way to see if this test is doing anything is
to insert a die() somewhere. For example, this die()
statement will check if we ever needed to mark things
uninteresting in a workdir path with both UNINTERESTING
and INTERESTING trees:

diff --git a/revision.c b/revision.c
index eb8e51bc63..8835f8e7b1 100644
--- a/revision.c
+++ b/revision.c
@@ -227,6 +227,7 @@ void mark_trees_uninteresting_sparse(struct repository *r,
        if (!has_uninteresting || !has_interesting)
                return;

+       die("we exercised the logic!");
        paths_and_oids_init(&map);

        oidset_iter_init(trees, &iter);

The implementation of the argument looks straight-forward.
It appears we are passing the argument to the sub-process,
so the bitflag isn't used yet.

-Stolee