Re: [PATCH 4/4] packed-backend: use mmap when opening large "packed-refs" file

[shejialuo <shejialuo@gmail.com>]

On Tue, May 06, 2025 at 12:00:39PM -0700, Junio C Hamano wrote:
> shejialuo <shejialuo@gmail.com> writes:
> 
> > We use "strbuf_read" to read the content of "packed-refs". However, this
> > is a bad practice which would consume a lot of memory usage if there are
> > multiple processes reading large "packed-refs".
> 
> Neither this nor the commit title says that the issue is limited to
> the code path that runs fsck on packed-refs file, but I thought the
> code paths to use packed-refs to resolve refs correctly uses mmap()
> and does not share this issue?  If it is limited to one single code
> path, please mention it explicitly.
> 

Nice advice, I will improve this in the next version.

> Also, I think it was already pointed out that "multiple processes"
> is not all that interesting issue.  Even if there is a single
> process using a single large packed-refs file, alloc+read gives the
> system more memory pressure than the read-only mmap like we do.
> 

That's right. Even there is only one process, we could avoid copying the
file content from kernel space to the user space. I will improve the
commit message.

> As to the title
> 
> 	packed-backend: use mmap when opening large "packed-refs" file
> 	packed-backend: mmap large "packed-refs" file during fsck
> 
> would be shorter and clearer.
> 

Yes, the second one is better. Will improve this.