@matheusbsbs / matheus-bagliano / commits / 7af607c58d

reftable/record: store "val1" hashes as static arrays

When reading ref records of type "val1", we store its object ID in an allocated array. This results in an additional allocation for every single ref record we read, which is rather inefficient especially when iterating over refs. Refactor the code to instead use an embedded array of `GIT_MAX_RAWSZ` bytes. While this means that `struct ref_record` is bigger now, we typically do not store all refs in an array anyway and instead only handle a limited number of records at the same point in time. Using `git show-ref --quiet` in a repository with ~350k refs this leads to a significant drop in allocations. Before: HEAP SUMMARY: in use at exit: 21,098 bytes in 192 blocks total heap usage: 2,116,683 allocs, 2,116,491 frees, 76,098,060 bytes allocated After: HEAP SUMMARY: in use at exit: 21,098 bytes in 192 blocks total heap usage: 1,419,031 allocs, 1,418,839 frees, 62,145,036 bytes allocated Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Patrick Steinhardt committed Jan 3, 2024 at 07:22 UTC 7af607c58d7985a0eb70fc3bca6eef8eb2381f14

7 files changed +13 -30

reftable/block_test.c

+1 -3

@@ -49,13 +49,11 @@ static void test_block_read_write(void)

50 for (i = 0; i < N; i++) {

51 char name[100];

52 - uint8_t hash[GIT_SHA1_RAWSZ];

52 snprintf(name, sizeof(name), "branch%02d", i);

54 - memset(hash, i, sizeof(hash));

54 rec.u.ref.refname = name;

55 rec.u.ref.value_type = REFTABLE_REF_VAL1;

58 - rec.u.ref.value.val1 = hash;

56 + memset(rec.u.ref.value.val1, i, GIT_SHA1_RAWSZ);

58 names[i] = xstrdup(name);

59 n = block_writer_add(&bw, &rec);

reftable/merged_test.c

+6 -10

@@ -123,13 +123,11 @@ static void readers_destroy(struct reftable_reader **readers, size_t n)

123

124 static void test_merged_between(void)

125 {

126 - uint8_t hash1[GIT_SHA1_RAWSZ] = { 1, 2, 3, 0 };

127 -

126 struct reftable_ref_record r1[] = { {

127 .refname = "b",

128 .update_index = 1,

129 .value_type = REFTABLE_REF_VAL1,

132 - .value.val1 = hash1,

130 + .value.val1 = { 1, 2, 3, 0 },

131 } };

132 struct reftable_ref_record r2[] = { {

133 .refname = "a",

@@ -165,26 +163,24 @@ static void test_merged_between(void)

163

164 static void test_merged(void)

165 {

168 - uint8_t hash1[GIT_SHA1_RAWSZ] = { 1 };

169 - uint8_t hash2[GIT_SHA1_RAWSZ] = { 2 };

166 struct reftable_ref_record r1[] = {

167 {

168 .refname = "a",

169 .update_index = 1,

170 .value_type = REFTABLE_REF_VAL1,

175 - .value.val1 = hash1,

171 + .value.val1 = { 1 },

172 },

173 {

174 .refname = "b",

175 .update_index = 1,

176 .value_type = REFTABLE_REF_VAL1,

181 - .value.val1 = hash1,

177 + .value.val1 = { 1 },

178 },

179 {

180 .refname = "c",

181 .update_index = 1,

182 .value_type = REFTABLE_REF_VAL1,

187 - .value.val1 = hash1,

183 + .value.val1 = { 1 },

184 }

185 };

186 struct reftable_ref_record r2[] = { {

@@ -197,13 +193,13 @@ static void test_merged(void)

193 .refname = "c",

194 .update_index = 3,

195 .value_type = REFTABLE_REF_VAL1,

200 - .value.val1 = hash2,

196 + .value.val1 = { 2 },

197 },

198 {

199 .refname = "d",

200 .update_index = 3,

201 .value_type = REFTABLE_REF_VAL1,

206 - .value.val1 = hash1,

202 + .value.val1 = { 1 },

203 },

204 };

205

reftable/readwrite_test.c

+4 -10

@@ -60,18 +60,15 @@ static void write_table(char ***names, struct strbuf *buf, int N,

60 *names = reftable_calloc(sizeof(char *) * (N + 1));

61 reftable_writer_set_limits(w, update_index, update_index);

62 for (i = 0; i < N; i++) {

63 - uint8_t hash[GIT_SHA256_RAWSZ] = { 0 };

63 char name[100];

64 int n;

67 - set_test_hash(hash, i);

68 -

66 snprintf(name, sizeof(name), "refs/heads/branch%02d", i);

68 ref.refname = name;

69 ref.update_index = update_index;

70 ref.value_type = REFTABLE_REF_VAL1;

74 - ref.value.val1 = hash;

71 + set_test_hash(ref.value.val1, i);

72 (*names)[i] = xstrdup(name);

74 n = reftable_writer_add_ref(w, &ref);

@@ -675,11 +672,10 @@ static void test_write_object_id_min_length(void)

672 struct strbuf buf = STRBUF_INIT;

673 struct reftable_writer *w =

674 reftable_new_writer(&strbuf_add_void, &buf, &opts);

678 - uint8_t hash[GIT_SHA1_RAWSZ] = {42};

675 struct reftable_ref_record ref = {

676 .update_index = 1,

677 .value_type = REFTABLE_REF_VAL1,

682 - .value.val1 = hash,

678 + .value.val1 = {42},

679 };

680 int err;

681 int i;

@@ -711,11 +707,10 @@ static void test_write_object_id_length(void)

707 struct strbuf buf = STRBUF_INIT;

708 struct reftable_writer *w =

709 reftable_new_writer(&strbuf_add_void, &buf, &opts);

714 - uint8_t hash[GIT_SHA1_RAWSZ] = {42};

710 struct reftable_ref_record ref = {

711 .update_index = 1,

712 .value_type = REFTABLE_REF_VAL1,

718 - .value.val1 = hash,

713 + .value.val1 = {42},

714 };

715 int err;

716 int i;

@@ -814,11 +809,10 @@ static void test_write_multiple_indices(void)

809 writer = reftable_new_writer(&strbuf_add_void, &writer_buf, &opts);

810 reftable_writer_set_limits(writer, 1, 1);

811 for (i = 0; i < 100; i++) {

817 - unsigned char hash[GIT_SHA1_RAWSZ] = {i};

812 struct reftable_ref_record ref = {

813 .update_index = 1,

814 .value_type = REFTABLE_REF_VAL1,

821 - .value.val1 = hash,

815 + .value.val1 = {i},

816 };

817

818 strbuf_reset(&buf);

reftable/record.c

-3

@@ -219,7 +219,6 @@ static void reftable_ref_record_copy_from(void *rec, const void *src_rec,

219 case REFTABLE_REF_DELETION:

220 break;

221 case REFTABLE_REF_VAL1:

222 - ref->value.val1 = reftable_malloc(hash_size);

222 memcpy(ref->value.val1, src->value.val1, hash_size);

223 break;

224 case REFTABLE_REF_VAL2:

@@ -303,7 +302,6 @@ void reftable_ref_record_release(struct reftable_ref_record *ref)

302 reftable_free(ref->value.val2.value);

303 break;

304 case REFTABLE_REF_VAL1:

306 - reftable_free(ref->value.val1);

305 break;

306 case REFTABLE_REF_DELETION:

307 break;

@@ -394,7 +392,6 @@ static int reftable_ref_record_decode(void *rec, struct strbuf key,

392 return -1;

393 }

394

397 - r->value.val1 = reftable_malloc(hash_size);

395 memcpy(r->value.val1, in.buf, hash_size);

396 string_view_consume(&in, hash_size);

397 break;

reftable/record_test.c

-1

@@ -119,7 +119,6 @@ static void test_reftable_ref_record_roundtrip(void)

119 case REFTABLE_REF_DELETION:

120 break;

121 case REFTABLE_REF_VAL1:

122 - in.u.ref.value.val1 = reftable_malloc(GIT_SHA1_RAWSZ);

122 set_hash(in.u.ref.value.val1, 1);

123 break;

124 case REFTABLE_REF_VAL2:

reftable/reftable-record.h

+2 -1

@@ -9,6 +9,7 @@ https://developers.google.com/open-source/licenses/bsd

9 #ifndef REFTABLE_RECORD_H

10 #define REFTABLE_RECORD_H

12 +#include "hash-ll.h"

13 #include <stdint.h>

15 /*

@@ -38,7 +39,7 @@ struct reftable_ref_record {

39 #define REFTABLE_NR_REF_VALUETYPES 4

40 } value_type;

41 union {

41 - uint8_t *val1; /* malloced hash. */

42 + unsigned char val1[GIT_MAX_RAWSZ];

43 struct {

44 uint8_t *value; /* first value, malloced hash */

45 uint8_t *target_value; /* second value, malloced hash */

reftable/stack_test.c

-2

@@ -463,7 +463,6 @@ static void test_reftable_stack_add(void)

463 refs[i].refname = xstrdup(buf);

464 refs[i].update_index = i + 1;

465 refs[i].value_type = REFTABLE_REF_VAL1;

466 - refs[i].value.val1 = reftable_malloc(GIT_SHA1_RAWSZ);

466 set_test_hash(refs[i].value.val1, i);

467

468 logs[i].refname = xstrdup(buf);

@@ -600,7 +599,6 @@ static void test_reftable_stack_tombstone(void)

599 refs[i].update_index = i + 1;

600 if (i % 2 == 0) {

601 refs[i].value_type = REFTABLE_REF_VAL1;

603 - refs[i].value.val1 = reftable_malloc(GIT_SHA1_RAWSZ);

602 set_test_hash(refs[i].value.val1, i);

603 }

604