[PATCH v3] packed_ref_cache: don't use mmap() for small files
- From
Kim Gybels <kgybels@infogroep.be>
- Date
- Jan 16, 2018, 19:38 UTC
- Message-ID
- <20180116193815.4568-1-kgybels@infogroep.be>
- In-Reply-To
- <20180115235251.GA21900@sigill.intra.peff.net>
Take a hint from commit ea68b0ce9f8 (hash-object: don't use mmap() for small files, 2010-02-21) and use read() instead of mmap() for small packed-refs files.
This also fixes the problem[1] where xmmap() returns NULL for zero length[2], for which munmap() later fails.
Alternatively, we could simply check for NULL before munmap(), or introduce xmunmap() that could be used together with xmmap(). However, always setting snapshot->buf to a valid pointer, by relying on xmalloc(0)'s fallback to 1-byte allocation, makes using snapshots easier.
[1] https://github.com/git-for-windows/git/issues/1410
[2] Logic introduced in commit 9130ac1e196 (Better error messages for
corrupt databases, 2007-01-11)Signed-off-by: Kim Gybels <kgybels@infogroep.be> ---
Change since v2: removed separate case for zero length as suggested by Peff, ensuring that snapshot->buf is always a valid pointer.
refs/packed-backend.c | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-)
diff --git a/refs/packed-backend.c b/refs/packed-backend.c index dab8a85d9a..b6e2bc3c1d 100644 --- a/refs/packed-backend.c +++ b/refs/packed-backend.c @@ -455,6 +455,8 @@ static void verify_buffer_safe(struct snapshot *snapshot) last_line, eof - last_line); } +#define SMALL_FILE_SIZE (32*1024) + /* * Depending on `mmap_strategy`, either mmap or read the contents of * the `packed-refs` file into the snapshot. Return 1 if the file @@ -489,21 +491,17 @@ static int load_contents(struct snapshot *snapshot) die_errno("couldn't stat %s", snapshot->refs->path); size = xsize_t(st.st_size); - switch (mmap_strategy) { - case MMAP_NONE: + if (size <= SMALL_FILE_SIZE || mmap_strategy == MMAP_NONE) { snapshot->buf = xmalloc(size); bytes_read = read_in_full(fd, snapshot->buf, size); if (bytes_read < 0 || bytes_read != size) die_errno("couldn't read %s", snapshot->refs->path); snapshot->eof = snapshot->buf + size; snapshot->mmapped = 0; - break; - case MMAP_TEMPORARY: - case MMAP_OK: + } else { snapshot->buf = xmmap(NULL, size, PROT_READ, MAP_PRIVATE, fd, 0); snapshot->eof = snapshot->buf + size; snapshot->mmapped = 1; - break; } close(fd);
-- 2.15.1.windows.2