On Tue, May 06, 2025 at 12:00:39PM -0700, Junio C Hamano wrote: > shejialuo <shejialuo@xxxxxxxxx> 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.
