packed_ref_cache: keep the `packed-refs` file mmapped if possible

Keep a copy of the `packed-refs` file contents in memory for as long as a `packed_ref_cache` object is in use: * If the system allows it, keep the `packed-refs` file mmapped. * If not (either because the system doesn't support `mmap()` at all, or because a file that is currently mmapped cannot be replaced via `rename()`), then make a copy of the file's contents in heap-allocated space, and keep that around instead. We base the choice of behavior on a new build-time switch, `MMAP_PREVENTS_DELETE`. By default, this switch is set for Windows variants. After this commit, `MMAP_NONE` and `MMAP_TEMPORARY` are still handled identically. But the next commit will introduce a difference. This whole change is still pointless, because we only read the `packed-refs` file contents immediately after instantiating the `packed_ref_cache`. But that will soon change. Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Michael Haggerty committed Sep 25, 2017 at 10:00 UTC 5b633610ec7a6c10f55e4758b047892652da8e05
3 files changed +152 -42
Makefile
+6
@@ -200,6 +200,9 @@ all::
200 #
201 # Define NO_MMAP if you want to avoid mmap.
202 #
203 +# Define MMAP_PREVENTS_DELETE if a file that is currently mmapped cannot be
204 +# deleted or cannot be replaced using rename().
205 +#
206 # Define NO_SYS_POLL_H if you don't have sys/poll.h.
207 #
208 # Define NO_POLL if you do not have or don't want to use poll().
@@ -1383,6 +1386,9 @@ else
1386 COMPAT_OBJS += compat/win32mmap.o
1387 endif
1388 endif
1389 +ifdef MMAP_PREVENTS_DELETE
1390 + BASIC_CFLAGS += -DMMAP_PREVENTS_DELETE
1391 +endif
1392 ifdef OBJECT_CREATION_USES_RENAMES
1393 COMPAT_CFLAGS += -DOBJECT_CREATION_MODE=1
1394 endif
config.mak.uname
+3
@@ -184,6 +184,7 @@ ifeq ($(uname_O),Cygwin)
184 UNRELIABLE_FSTAT = UnfortunatelyYes
185 SPARSE_FLAGS = -isystem /usr/include/w32api -Wno-one-bit-signed-bitfield
186 OBJECT_CREATION_USES_RENAMES = UnfortunatelyNeedsTo
187 + MMAP_PREVENTS_DELETE = UnfortunatelyYes
188 COMPAT_OBJS += compat/cygwin.o
189 FREAD_READS_DIRECTORIES = UnfortunatelyYes
190 endif
@@ -353,6 +354,7 @@ ifeq ($(uname_S),Windows)
354 NO_ST_BLOCKS_IN_STRUCT_STAT = YesPlease
355 NO_NSEC = YesPlease
356 USE_WIN32_MMAP = YesPlease
357 + MMAP_PREVENTS_DELETE = UnfortunatelyYes
358 # USE_NED_ALLOCATOR = YesPlease
359 UNRELIABLE_FSTAT = UnfortunatelyYes
360 OBJECT_CREATION_USES_RENAMES = UnfortunatelyNeedsTo
@@ -501,6 +503,7 @@ ifneq (,$(findstring MINGW,$(uname_S)))
503 NO_ST_BLOCKS_IN_STRUCT_STAT = YesPlease
504 NO_NSEC = YesPlease
505 USE_WIN32_MMAP = YesPlease
506 + MMAP_PREVENTS_DELETE = UnfortunatelyYes
507 USE_NED_ALLOCATOR = YesPlease
508 UNRELIABLE_FSTAT = UnfortunatelyYes
509 OBJECT_CREATION_USES_RENAMES = UnfortunatelyNeedsTo
refs/packed-backend.c
+143 -42
@@ -7,6 +7,35 @@
7 #include "../iterator.h"
8 #include "../lockfile.h"
9
10 +enum mmap_strategy {
11 + /*
12 + * Don't use mmap() at all for reading `packed-refs`.
13 + */
14 + MMAP_NONE,
15 +
16 + /*
17 + * Can use mmap() for reading `packed-refs`, but the file must
18 + * not remain mmapped. This is the usual option on Windows,
19 + * where you cannot rename a new version of a file onto a file
20 + * that is currently mmapped.
21 + */
22 + MMAP_TEMPORARY,
23 +
24 + /*
25 + * It is OK to leave the `packed-refs` file mmapped while
26 + * arbitrary other code is running.
27 + */
28 + MMAP_OK
29 +};
30 +
31 +#if defined(NO_MMAP)
32 +static enum mmap_strategy mmap_strategy = MMAP_NONE;
33 +#elif defined(MMAP_PREVENTS_DELETE)
34 +static enum mmap_strategy mmap_strategy = MMAP_TEMPORARY;
35 +#else
36 +static enum mmap_strategy mmap_strategy = MMAP_OK;
37 +#endif
38 +
39 struct packed_ref_store;
40
41 struct packed_ref_cache {
@@ -18,6 +47,21 @@ struct packed_ref_cache {
47
48 struct ref_cache *cache;
49
50 + /* Is the `packed-refs` file currently mmapped? */
51 + int mmapped;
52 +
53 + /*
54 + * The contents of the `packed-refs` file. If the file is
55 + * mmapped, this points at the mmapped contents of the file.
56 + * If not, this points at heap-allocated memory containing the
57 + * contents. If there were no contents (e.g., because the file
58 + * didn't exist), `buf` and `eof` are both NULL.
59 + */
60 + char *buf, *eof;
61 +
62 + /* The size of the header line, if any; otherwise, 0: */
63 + size_t header_len;
64 +
65 /*
66 * What is the peeled state of this cache? (This is usually
67 * determined from the header of the "packed-refs" file.)
@@ -76,6 +120,26 @@ static void acquire_packed_ref_cache(struct packed_ref_cache *packed_refs)
120 packed_refs->referrers++;
121 }
122
123 +/*
124 + * If the buffer in `packed_refs` is active, then either munmap the
125 + * memory and close the file, or free the memory. Then set the buffer
126 + * pointers to NULL.
127 + */
128 +static void release_packed_ref_buffer(struct packed_ref_cache *packed_refs)
129 +{
130 + if (packed_refs->mmapped) {
131 + if (munmap(packed_refs->buf,
132 + packed_refs->eof - packed_refs->buf))
133 + die_errno("error ummapping packed-refs file %s",
134 + packed_refs->refs->path);
135 + packed_refs->mmapped = 0;
136 + } else {
137 + free(packed_refs->buf);
138 + }
139 + packed_refs->buf = packed_refs->eof = NULL;
140 + packed_refs->header_len = 0;
141 +}
142 +
143 /*
144 * Decrease the reference count of *packed_refs. If it goes to zero,
145 * free *packed_refs and return true; otherwise return false.
@@ -85,6 +149,7 @@ static int release_packed_ref_cache(struct packed_ref_cache *packed_refs)
149 if (!--packed_refs->referrers) {
150 free_ref_cache(packed_refs->cache);
151 stat_validity_clear(&packed_refs->validity);
152 + release_packed_ref_buffer(packed_refs);
153 free(packed_refs);
154 return 1;
155 } else {
@@ -284,13 +349,15 @@ static struct ref_iterator_vtable mmapped_ref_iterator_vtable = {
349 };
350
351 struct ref_iterator *mmapped_ref_iterator_begin(
287 - const char *packed_refs_file,
352 struct packed_ref_cache *packed_refs,
353 const char *pos, const char *eof)
354 {
355 struct mmapped_ref_iterator *iter = xcalloc(1, sizeof(*iter));
356 struct ref_iterator *ref_iterator = &iter->base;
357
358 + if (!packed_refs->buf)
359 + return empty_ref_iterator_begin();
360 +
361 base_ref_iterator_init(ref_iterator, &mmapped_ref_iterator_vtable, 0);
362
363 iter->packed_refs = packed_refs;
@@ -304,6 +371,62 @@ struct ref_iterator *mmapped_ref_iterator_begin(
371 return ref_iterator;
372 }
373
374 +/*
375 + * Depending on `mmap_strategy`, either mmap or read the contents of
376 + * the `packed-refs` file into the `packed_refs` instance. Return 1 if
377 + * the file existed and was read, or 0 if the file was absent. Die on
378 + * errors.
379 + */
380 +static int load_contents(struct packed_ref_cache *packed_refs)
381 +{
382 + int fd;
383 + struct stat st;
384 + size_t size;
385 + ssize_t bytes_read;
386 +
387 + fd = open(packed_refs->refs->path, O_RDONLY);
388 + if (fd < 0) {
389 + if (errno == ENOENT) {
390 + /*
391 + * This is OK; it just means that no
392 + * "packed-refs" file has been written yet,
393 + * which is equivalent to it being empty,
394 + * which is its state when initialized with
395 + * zeros.
396 + */
397 + return 0;
398 + } else {
399 + die_errno("couldn't read %s", packed_refs->refs->path);
400 + }
401 + }
402 +
403 + stat_validity_update(&packed_refs->validity, fd);
404 +
405 + if (fstat(fd, &st) < 0)
406 + die_errno("couldn't stat %s", packed_refs->refs->path);
407 + size = xsize_t(st.st_size);
408 +
409 + switch (mmap_strategy) {
410 + case MMAP_NONE:
411 + case MMAP_TEMPORARY:
412 + packed_refs->buf = xmalloc(size);
413 + bytes_read = read_in_full(fd, packed_refs->buf, size);
414 + if (bytes_read < 0 || bytes_read != size)
415 + die_errno("couldn't read %s", packed_refs->refs->path);
416 + packed_refs->eof = packed_refs->buf + size;
417 + packed_refs->mmapped = 0;
418 + break;
419 + case MMAP_OK:
420 + packed_refs->buf = xmmap(NULL, size, PROT_READ, MAP_PRIVATE, fd, 0);
421 + packed_refs->eof = packed_refs->buf + size;
422 + packed_refs->mmapped = 1;
423 + break;
424 + }
425 + close(fd);
426 +
427 + return 1;
428 +}
429 +
430 /*
431 * Read from the `packed-refs` file into a newly-allocated
432 * `packed_ref_cache` and return it. The return value will already
@@ -336,11 +459,6 @@ struct ref_iterator *mmapped_ref_iterator_begin(
459 static struct packed_ref_cache *read_packed_refs(struct packed_ref_store *refs)
460 {
461 struct packed_ref_cache *packed_refs = xcalloc(1, sizeof(*packed_refs));
339 - int fd;
340 - struct stat st;
341 - size_t size;
342 - char *buf;
343 - const char *pos, *eof;
462 struct ref_dir *dir;
463 struct ref_iterator *iter;
464 int ok;
@@ -351,45 +469,29 @@ static struct packed_ref_cache *read_packed_refs(struct packed_ref_store *refs)
469 packed_refs->cache->root->flag &= ~REF_INCOMPLETE;
470 packed_refs->peeled = PEELED_NONE;
471
354 - fd = open(refs->path, O_RDONLY);
355 - if (fd < 0) {
356 - if (errno == ENOENT) {
357 - /*
358 - * This is OK; it just means that no
359 - * "packed-refs" file has been written yet,
360 - * which is equivalent to it being empty.
361 - */
362 - return packed_refs;
363 - } else {
364 - die_errno("couldn't read %s", refs->path);
365 - }
366 - }
367 -
368 - stat_validity_update(&packed_refs->validity, fd);
369 -
370 - if (fstat(fd, &st) < 0)
371 - die_errno("couldn't stat %s", refs->path);
372 -
373 - size = xsize_t(st.st_size);
374 - buf = xmmap(NULL, size, PROT_READ, MAP_PRIVATE, fd, 0);
375 - pos = buf;
376 - eof = buf + size;
472 + if (!load_contents(packed_refs))
473 + return packed_refs;
474
475 /* If the file has a header line, process it: */
379 - if (pos < eof && *pos == '#') {
476 + if (packed_refs->buf < packed_refs->eof && *packed_refs->buf == '#') {
477 struct strbuf tmp = STRBUF_INIT;
478 char *p;
479 const char *eol;
480 struct string_list traits = STRING_LIST_INIT_NODUP;
481
385 - eol = memchr(pos, '\n', eof - pos);
482 + eol = memchr(packed_refs->buf, '\n',
483 + packed_refs->eof - packed_refs->buf);
484 if (!eol)
387 - die_unterminated_line(refs->path, pos, eof - pos);
485 + die_unterminated_line(refs->path,
486 + packed_refs->buf,
487 + packed_refs->eof - packed_refs->buf);
488
389 - strbuf_add(&tmp, pos, eol - pos);
489 + strbuf_add(&tmp, packed_refs->buf, eol - packed_refs->buf);
490
491 if (!skip_prefix(tmp.buf, "# pack-refs with:", (const char **)&p))
392 - die_invalid_line(refs->path, pos, eof - pos);
492 + die_invalid_line(refs->path,
493 + packed_refs->buf,
494 + packed_refs->eof - packed_refs->buf);
495
496 string_list_split_in_place(&traits, p, ' ', -1);
497
@@ -400,14 +502,17 @@ static struct packed_ref_cache *read_packed_refs(struct packed_ref_store *refs)
502 /* perhaps other traits later as well */
503
504 /* The "+ 1" is for the LF character. */
403 - pos = eol + 1;
505 + packed_refs->header_len = eol + 1 - packed_refs->buf;
506
507 string_list_clear(&traits, 0);
508 strbuf_release(&tmp);
509 }
510
511 dir = get_ref_dir(packed_refs->cache->root);
410 - iter = mmapped_ref_iterator_begin(refs->path, packed_refs, pos, eof);
512 + iter = mmapped_ref_iterator_begin(
513 + packed_refs,
514 + packed_refs->buf + packed_refs->header_len,
515 + packed_refs->eof);
516 while ((ok = ref_iterator_advance(iter)) == ITER_OK) {
517 struct ref_entry *entry =
518 create_ref_entry(iter->refname, iter->oid, iter->flags);
@@ -420,11 +525,6 @@ static struct packed_ref_cache *read_packed_refs(struct packed_ref_store *refs)
525 if (ok != ITER_DONE)
526 die("error reading packed-refs file %s", refs->path);
527
423 - if (munmap(buf, size))
424 - die_errno("error ummapping packed-refs file %s", refs->path);
425 -
426 - close(fd);
427 -
528 return packed_refs;
529 }
530
@@ -1031,6 +1131,8 @@ static int packed_transaction_finish(struct ref_store *ref_store,
1131 int ret = TRANSACTION_GENERIC_ERROR;
1132 char *packed_refs_path;
1133
1134 + clear_packed_ref_cache(refs);
1135 +
1136 packed_refs_path = get_locked_file_path(&refs->lock);
1137 if (rename_tempfile(&refs->tempfile, packed_refs_path)) {
1138 strbuf_addf(err, "error replacing %s: %s",
@@ -1038,7 +1140,6 @@ static int packed_transaction_finish(struct ref_store *ref_store,
1140 goto cleanup;
1141 }
1142
1041 - clear_packed_ref_cache(refs);
1143 ret = 0;
1144
1145 cleanup: