git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "René Scharfe" <l.s.r@web.de>
To: ZheNing Hu via GitGitGadget <gitgitgadget@gmail.com>,
	git@vger.kernel.org
Cc: Junio C Hamano <gitster@pobox.com>,
	"brian m. carlson" <sandals@crustytoothpaste.net>,
	Christian Couder <christian.couder@gmail.com>,
	ZheNing Hu <adlternative@gmail.com>
Subject: Re: [PATCH] read-cache.c: reduce unnecessary cache entry name copying
Date: Mon, 6 Jun 2022 11:48:13 +0200	[thread overview]
Message-ID: <9195fe51-a696-1fb7-3617-a5b65358a806@web.de> (raw)
In-Reply-To: <pull.1249.git.1654436248249.gitgitgadget@gmail.com>

Am 05.06.22 um 15:37 schrieb ZheNing Hu via GitGitGadget:
> 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

  reply	other threads:[~2022-06-06  9:48 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-06-05 13:37 [PATCH] read-cache.c: reduce unnecessary cache entry name copying ZheNing Hu via GitGitGadget
2022-06-06  9:48 ` René Scharfe [this message]
2022-06-06 17:23   ` Junio C Hamano

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=9195fe51-a696-1fb7-3617-a5b65358a806@web.de \
    --to=l.s.r@web.de \
    --cc=adlternative@gmail.com \
    --cc=christian.couder@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitgitgadget@gmail.com \
    --cc=gitster@pobox.com \
    --cc=sandals@crustytoothpaste.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).