Subject: Re: [PATCH 06/12] clone: disable save_commit_buffer

From: Junio C Hamano <[email-masked]>
Date: Wed, 25 Jan 2017 11:11:01 -0800

Jeff King <[email-masked]> writes:

> Normally git caches the raw commit object contents in
> "struct commit". This makes it fast to run parse_commit()
> followed by a pretty-print operation.
>
> For commands which don't actually pretty-print the commits,
> the caching is wasteful (and may use quite a lot of memory
> if git accesses a large number of commits).
>
> For fetching operations like clone, we already disable

s/clone/fetch/ you meant?

> In one real-world case with a large number of tags, this
> cut about 10MB off of clone's heap usage. Not spectacular,
> but there's really no downside.

"There is no downside" is especially true in the modern world post
v2.1, where get_commit_buffer() is what everybody has to go through
to access this information.  I would have been very hesitant to
accept a patch like this one if we didn't do that clean-up, as a
stray codepath could have just done "commit->buffer" and segfaulted
or said "ah, there is no message", neither of which is satisfactory.

Thanks.

> Signed-off-by: Jeff King <[email-masked]>
> ---
>  builtin/clone.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/builtin/clone.c b/builtin/clone.c
> index 5ef81927a..3fca45e7e 100644
> --- a/builtin/clone.c
> +++ b/builtin/clone.c
> @@ -858,6 +858,7 @@ int cmd_clone(int argc, const char **argv, const char *prefix)
>  	struct refspec *refspec;
>  	const char *fetch_pattern;
>  
> +	save_commit_buffer = 0;
>  	packet_trace_identity("clone");
>  	argc = parse_options(argc, argv, prefix, builtin_clone_options,
>  			     builtin_clone_usage, 0);