Re: [PATCH] read-cache.c: reduce unnecessary cache entry name copying

From: René Scharfe <l.s.r@web.de>
Date: Jun 6, 2022, 09:48 UTC
Message-ID: <9195fe51-a696-1fb7-3617-a5b65358a806@web.de>
In-Reply-To: <pull.1249.git.1654436248249.gitgitgadget@gmail.com>

Am 05.06.22 um 15:37 schrieb ZheNing Hu via GitGitGadget:

Show 30 quoted lines

> From: ZheNing Hu <adlternative@gmail.com>
>
> In function create_from_disk, we have already copy the cache
> entries name from disk or previous cache entry, we can reduce
> unnecessary copy before that.
>
> Signed-off-by: ZheNing Hu <adlternative@gmail.com>
> ---
>     read-cache.c: reduce unnecessary cache entry name copying
>
>     Index cache entries name are copied twice wrongly, so reduce the first
>     one to fix it.
>
> Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1249%2Fadlternative%2Fzh%2Frm-ce-copy-v1
> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1249/adlternative/zh/rm-ce-copy-v1
> Pull-Request: https://github.com/gitgitgadget/git/pull/1249
>
>  read-cache.c | 2 --
>  1 file changed, 2 deletions(-)
>
> diff --git a/read-cache.c b/read-cache.c
> index 96ce489c7c5..e61af3a3d4d 100644
> --- a/read-cache.c
> +++ b/read-cache.c
> @@ -1944,8 +1944,6 @@ static struct cache_entry *create_from_disk(struct mem_pool *ce_mem_pool,
>  	ce->ce_namelen = len;
>  	ce->index = 0;
>  	oidread(&ce->oid, ondisk->data);
> -	memcpy(ce->name, name, len);
> -	ce->name[len] = '\0';

This removal looks correct to me. The extra copying was added by 575fa8a3ed (read-cache: read data in a hash-independent way, 2019-02-19) but its commit message doesn't mention it.

>
>  	if (expand_name_field) {
>  		if (copy_len)

Some more context lines would help to see the redundancy; here they are:

	if (expand_name_field) {
		if (copy_len)
			memcpy(ce->name, previous_ce->name, copy_len);
		memcpy(ce->name + copy_len, name, len + 1 - copy_len);
		*ent_size = (name - ((char *)ondisk)) + len + 1 - copy_len;
	} else {
		memcpy(ce->name, name, len + 1);
		*ent_size = ondisk_ce_size(ce);
	}

>
> base-commit: ab336e8f1c8009c8b1aab8deb592148e69217085