Re: [PATCH v3 3/3] packed-backend: mmap large "packed-refs" file during fsck

From: Jeff King <peff@peff.net>
Date: May 12, 2025, 13:08 UTC
Message-ID: <20250512130815.GC1191360@coredump.intra.peff.net>
In-Reply-To: <aCCt1zJ2yviOz--l@ArchLinux>

On Sun, May 11, 2025 at 10:01:59PM +0800, shejialuo wrote:

Show 22 quoted lines

> @@ -2113,24 +2113,19 @@ static int packed_fsck(struct ref_store *ref_store,
>  		goto cleanup;
>  	}
>  
> -	if (!st.st_size)
> +	if (!allocate_snapshot_buffer(&snapshot, fd, &st))
>  		goto cleanup;
>  
> -	if (strbuf_read(&packed_ref_content, fd, 0) < 0) {
> -		ret = error_errno(_("unable to read '%s'"), refs->path);
> -		goto cleanup;
> -	}
> -
> -	ret = packed_fsck_ref_content(o, ref_store, &sorted, packed_ref_content.buf,
> -				      packed_ref_content.buf + packed_ref_content.len);
> +	ret = packed_fsck_ref_content(o, ref_store, &sorted, snapshot.start,
> +				      snapshot.eof);
>  	if (!ret && sorted)
> -		ret = packed_fsck_ref_sorted(o, ref_store, packed_ref_content.buf,
> -					     packed_ref_content.buf + packed_ref_content.len);
> +		ret = packed_fsck_ref_sorted(o, ref_store, snapshot.start,
> +					     snapshot.eof);

OK, so we still use allocate_snapshot_buffer(), but then we hold on to the snapshot. Good.

I do think just using xmmap() would have been sufficient (and not needed patch 2 then), but I'm OK with this direction under the logic that we may end up sharing more code between the normal and fsck code paths eventually.

-Peff