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

From: Junio C Hamano <gitster@pobox.com>
Date: Jan 25, 2017, 19:11 UTC
Message-ID: <xmqqmvefaray.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <20170124004500.v7geae55w6zeax7m@sigill.intra.peff.net>

Jeff King <peff@peff.net> writes:

Show 9 quoted lines

> 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.

Show 17 quoted lines

> Signed-off-by: Jeff King <peff@peff.net>
> ---
>  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);