Re: [PATCH 1/3] is_promisor_object(): free tree buffer after parsing

[Junio C Hamano <gitster@pobox.com>]

Jeff King <peff@peff.net> writes:

> The added perf test shows only a tiny improvement on my machine for
> git.git, since 1.7GB isn't enough to cause any real memory pressure:
>
>   Test                                 HEAD^               HEAD
>   --------------------------------------------------------------------------------
>   5600.4: fsck                         21.26(20.90+0.35)   20.84(20.79+0.04) -2.0%
>
> With linux.git the absolute change is a bit bigger, though still a small
> percentage:
>
>   Test                          HEAD^                 HEAD
>   -----------------------------------------------------------------------------
>   5600.4: fsck                  262.26(259.13+3.12)   254.92(254.62+0.29) -2.8%
>
> I didn't have the patience to run it under massif with linux.git, but
> it's probably on the order of about 14GB improvement, since that's the
> sum of the sizes of all of the uncompressed trees (but still isn't
> enough to create memory pressure on this particular machine, which has
> 64GB of RAM). Smaller machines would probably see a bigger effect on
> runtime (and sadly our perf suite does not measure peak heap).
>
> Signed-off-by: Jeff King <peff@peff.net>
> ---
>  packfile.c                    | 1 +
>  t/perf/p5600-partial-clone.sh | 4 ++++
>  2 files changed, 5 insertions(+)
>
> diff --git a/packfile.c b/packfile.c
> index 8668345d93..b79cbc8cd4 100644
> --- a/packfile.c
> +++ b/packfile.c
> @@ -2247,6 +2247,7 @@ static int add_promisor_object(const struct object_id *oid,
>  			return 0;
>  		while (tree_entry_gently(&desc, &entry))
>  			oidset_insert(set, &entry.oid);
> +		free_tree_buffer(tree);
>  	} else if (obj->type == OBJ_COMMIT) {
>  		struct commit *commit = (struct commit *) obj;
>  		struct commit_list *parents = commit->parents;

Hmph, does an added free() without removing one later mean we've
been leaking?

Nicely done.  Thanks.


> diff --git a/t/perf/p5600-partial-clone.sh b/t/perf/p5600-partial-clone.sh
> index 3e04bd2ae1..754aaec3dc 100755
> --- a/t/perf/p5600-partial-clone.sh
> +++ b/t/perf/p5600-partial-clone.sh
> @@ -23,4 +23,8 @@ test_perf 'checkout of result' '
>  	git -C worktree checkout -f
>  '
>  
> +test_perf 'fsck' '
> +	git -C bare.git fsck
> +'
> +
>  test_done