@matheusbsbs / matheus-bagliano / commits / f6b58c1be4

reftable: introduce macros to grow arrays

Throughout the reftable library we have many cases where we need to grow arrays. In order to avoid too many reallocations, we roughly double the capacity of the array on each iteration. The resulting code pattern is duplicated across many sites. We have similar patterns in our main codebase, which is why we have eventually introduced an `ALLOC_GROW()` macro to abstract it away and avoid some code duplication. We cannot easily reuse this macro here though because `ALLOC_GROW()` uses `REALLOC_ARRAY()`, which in turn will call realloc(3P) to grow the array. The reftable code is structured as a library though (even if the boundaries are fuzzy), and one property this brings with it is that it is possible to plug in your own allocators. So instead of using realloc(3P), we need to use `reftable_realloc()` that knows to use the user-provided implementation. So let's introduce two new macros `REFTABLE_REALLOC_ARRAY()` and `REFTABLE_ALLOC_GROW()` that mirror what we do in our main codebase, with two modifications: - They use `reftable_realloc()`, as explained above. - They use a different growth factor of `2 * cap + 1` instead of `(cap + 16) * 3 / 2`. The second change is because we know a bit more about the allocation patterns in the reftable library. In most cases, we end up only having a handful of items in the array and don't end up growing them. The initial capacity that our normal growth factor uses (which is 24) would thus end up over-allocating in a lot of code paths. This effect is measurable: - Before change: HEAP SUMMARY: in use at exit: 671,983 bytes in 152 blocks total heap usage: 3,843,446 allocs, 3,843,294 frees, 223,761,402 bytes allocated - After change with a growth factor of `(2 * alloc + 1)`: HEAP SUMMARY: in use at exit: 671,983 bytes in 152 blocks total heap usage: 3,843,446 allocs, 3,843,294 frees, 223,761,410 bytes allocated - After change with a growth factor of `(alloc + 16)* 2 / 3`: HEAP SUMMARY: in use at exit: 671,983 bytes in 152 blocks total heap usage: 3,833,673 allocs, 3,833,521 frees, 4,728,251,742 bytes allocated While the total heap usage is roughly the same, we do end up allocating significantly more bytes with our usual growth factor (in fact, roughly 21 times as many). Convert the reftable library to use these new macros. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Patrick Steinhardt committed Feb 6, 2024 at 07:35 UTC f6b58c1be40ba4bd6e7f2364acfe5fa34ce04120

7 files changed +36 -61

reftable/basics.c

+2 -6

@@ -89,17 +89,13 @@ void parse_names(char *buf, int size, char ***namesp)

89 next = end;

90 }

91 if (p < next) {

92 - if (names_len == names_cap) {

93 - names_cap = 2 * names_cap + 1;

94 - names = reftable_realloc(

95 - names, names_cap * sizeof(*names));

96 - }

92 + REFTABLE_ALLOC_GROW(names, names_len + 1, names_cap);

93 names[names_len++] = xstrdup(p);

94 }

95 p = next + 1;

96 }

102 - names = reftable_realloc(names, (names_len + 1) * sizeof(*names));

98 + REFTABLE_REALLOC_ARRAY(names, names_len + 1);

99 names[names_len] = NULL;

100 *namesp = names;

101 }

reftable/basics.h

+11

@@ -53,6 +53,17 @@ void *reftable_realloc(void *p, size_t sz);

53 void reftable_free(void *p);

54 void *reftable_calloc(size_t sz);

56 +#define REFTABLE_REALLOC_ARRAY(x, alloc) (x) = reftable_realloc((x), st_mult(sizeof(*(x)), (alloc)))

57 +#define REFTABLE_ALLOC_GROW(x, nr, alloc) \

58 + do { \

59 + if ((nr) > alloc) { \

60 + alloc = 2 * (alloc) + 1; \

61 + if (alloc < (nr)) \

62 + alloc = (nr); \

63 + REFTABLE_REALLOC_ARRAY(x, alloc); \

64 + } \

65 + } while (0)

66 +

67 /* Find the longest shared prefix size of `a` and `b` */

68 struct strbuf;

69 int common_prefix_size(struct strbuf *a, struct strbuf *b);

reftable/block.c

+1 -6

@@ -51,12 +51,7 @@ static int block_writer_register_restart(struct block_writer *w, int n,

51 if (2 + 3 * rlen + n > w->block_size - w->next)

52 return -1;

53 if (is_restart) {

54 - if (w->restart_len == w->restart_cap) {

55 - w->restart_cap = w->restart_cap * 2 + 1;

56 - w->restarts = reftable_realloc(

57 - w->restarts, sizeof(uint32_t) * w->restart_cap);

58 - }

59 -

54 + REFTABLE_ALLOC_GROW(w->restarts, w->restart_len + 1, w->restart_cap);

55 w->restarts[w->restart_len++] = w->next;

56 }

reftable/merged_test.c

+6 -14

@@ -231,14 +231,10 @@ static void test_merged(void)

231 while (len < 100) { /* cap loops/recursion. */

232 struct reftable_ref_record ref = { NULL };

233 int err = reftable_iterator_next_ref(&it, &ref);

234 - if (err > 0) {

234 + if (err > 0)

235 break;

236 - }

237 - if (len == cap) {

238 - cap = 2 * cap + 1;

239 - out = reftable_realloc(

240 - out, sizeof(struct reftable_ref_record) * cap);

241 - }

236 +

237 + REFTABLE_ALLOC_GROW(out, len + 1, cap);

238 out[len++] = ref;

239 }

240 reftable_iterator_destroy(&it);

@@ -368,14 +364,10 @@ static void test_merged_logs(void)

364 while (len < 100) { /* cap loops/recursion. */

365 struct reftable_log_record log = { NULL };

366 int err = reftable_iterator_next_log(&it, &log);

371 - if (err > 0) {

367 + if (err > 0)

368 break;

373 - }

374 - if (len == cap) {

375 - cap = 2 * cap + 1;

376 - out = reftable_realloc(

377 - out, sizeof(struct reftable_log_record) * cap);

378 - }

369 +

370 + REFTABLE_ALLOC_GROW(out, len + 1, cap);

371 out[len++] = log;

372 }

373 reftable_iterator_destroy(&it);

reftable/pq.c

+2 -6

@@ -75,13 +75,9 @@ void merged_iter_pqueue_add(struct merged_iter_pqueue *pq, const struct pq_entry

75 {

76 int i = 0;

78 - if (pq->len == pq->cap) {

79 - pq->cap = 2 * pq->cap + 1;

80 - pq->heap = reftable_realloc(pq->heap,

81 - pq->cap * sizeof(struct pq_entry));

82 - }

83 -

78 + REFTABLE_ALLOC_GROW(pq->heap, pq->len + 1, pq->cap);

79 pq->heap[pq->len++] = *e;

80 +

81 i = pq->len - 1;

82 while (i > 0) {

83 int j = (i - 1) / 2;

reftable/stack.c

+12 -17

@@ -551,7 +551,7 @@ struct reftable_addition {

551 struct reftable_stack *stack;

552

553 char **new_tables;

554 - int new_tables_len;

554 + size_t new_tables_len, new_tables_cap;

555 uint64_t next_update_index;

556 };

557

@@ -602,8 +602,9 @@ done:

602

603 static void reftable_addition_close(struct reftable_addition *add)

604 {

605 - int i = 0;

605 struct strbuf nm = STRBUF_INIT;

606 + size_t i;

607 +

608 for (i = 0; i < add->new_tables_len; i++) {

609 stack_filename(&nm, add->stack, add->new_tables[i]);

610 unlink(nm.buf);

@@ -613,6 +614,7 @@ static void reftable_addition_close(struct reftable_addition *add)

614 reftable_free(add->new_tables);

615 add->new_tables = NULL;

616 add->new_tables_len = 0;

617 + add->new_tables_cap = 0;

618

619 delete_tempfile(&add->lock_file);

620 strbuf_release(&nm);

@@ -631,8 +633,8 @@ int reftable_addition_commit(struct reftable_addition *add)

633 {

634 struct strbuf table_list = STRBUF_INIT;

635 int lock_file_fd = get_tempfile_fd(add->lock_file);

634 - int i = 0;

636 int err = 0;

637 + size_t i;

638

639 if (add->new_tables_len == 0)

640 goto done;

@@ -660,12 +662,12 @@ int reftable_addition_commit(struct reftable_addition *add)

662 }

663

664 /* success, no more state to clean up. */

663 - for (i = 0; i < add->new_tables_len; i++) {

665 + for (i = 0; i < add->new_tables_len; i++)

666 reftable_free(add->new_tables[i]);

665 - }

667 reftable_free(add->new_tables);

668 add->new_tables = NULL;

669 add->new_tables_len = 0;

670 + add->new_tables_cap = 0;

671

672 err = reftable_stack_reload_maybe_reuse(add->stack, 1);

673 if (err)

@@ -792,11 +794,9 @@ int reftable_addition_add(struct reftable_addition *add,

794 goto done;

795 }

796

795 - add->new_tables = reftable_realloc(add->new_tables,

796 - sizeof(*add->new_tables) *

797 - (add->new_tables_len + 1));

798 - add->new_tables[add->new_tables_len] = strbuf_detach(&next_name, NULL);

799 - add->new_tables_len++;

797 + REFTABLE_ALLOC_GROW(add->new_tables, add->new_tables_len + 1,

798 + add->new_tables_cap);

799 + add->new_tables[add->new_tables_len++] = strbuf_detach(&next_name, NULL);

800 done:

801 if (tab_fd > 0) {

802 close(tab_fd);

@@ -1367,17 +1367,12 @@ static int stack_check_addition(struct reftable_stack *st,

1367 while (1) {

1368 struct reftable_ref_record ref = { NULL };

1369 err = reftable_iterator_next_ref(&it, &ref);

1370 - if (err > 0) {

1370 + if (err > 0)

1371 break;

1372 - }

1372 if (err < 0)

1373 goto done;

1374

1376 - if (len >= cap) {

1377 - cap = 2 * cap + 1;

1378 - refs = reftable_realloc(refs, cap * sizeof(refs[0]));

1379 - }

1380 -

1375 + REFTABLE_ALLOC_GROW(refs, len + 1, cap);

1376 refs[len++] = ref;

1377 }

1378

reftable/writer.c

+2 -12

@@ -200,12 +200,7 @@ static void writer_index_hash(struct reftable_writer *w, struct strbuf *hash)

200 return;

201 }

202

203 - if (key->offset_len == key->offset_cap) {

204 - key->offset_cap = 2 * key->offset_cap + 1;

205 - key->offsets = reftable_realloc(

206 - key->offsets, sizeof(uint64_t) * key->offset_cap);

207 - }

208 -

203 + REFTABLE_ALLOC_GROW(key->offsets, key->offset_len + 1, key->offset_cap);

204 key->offsets[key->offset_len++] = off;

205 }

206

@@ -674,12 +669,7 @@ static int writer_flush_nonempty_block(struct reftable_writer *w)

669 if (err < 0)

670 return err;

671

677 - if (w->index_cap == w->index_len) {

678 - w->index_cap = 2 * w->index_cap + 1;

679 - w->index = reftable_realloc(

680 - w->index,

681 - sizeof(struct reftable_index_record) * w->index_cap);

682 - }

672 + REFTABLE_ALLOC_GROW(w->index, w->index_len + 1, w->index_cap);

673

674 ir.offset = w->next;

675 strbuf_reset(&ir.last_key);