@matheusbsbs / matheus-bagliano / commits / 87ff723018

reftable/record: convert old and new object IDs to arrays

In 7af607c58d (reftable/record: store "val1" hashes as static arrays, 2024-01-03) and b31e3cc620 (reftable/record: store "val2" hashes as static arrays, 2024-01-03) we have converted ref records to store their object IDs in a static array. Convert log records to do the same so that their old and new object IDs are arrays, too. This change results in two allocations less per log record that we're iterating over. Before: HEAP SUMMARY: in use at exit: 13,473 bytes in 122 blocks total heap usage: 8,068,495 allocs, 8,068,373 frees, 401,011,862 bytes allocated After: HEAP SUMMARY: in use at exit: 13,473 bytes in 122 blocks total heap usage: 6,068,489 allocs, 6,068,367 frees, 361,011,822 bytes allocated Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Patrick Steinhardt committed Mar 5, 2024 at 13:10 UTC 87ff723018bfca588b5d68e110ab04494c451ebd

7 files changed +61 -151

refs/reftable-backend.c

+9 -30

@@ -171,23 +171,6 @@ static int should_write_log(struct ref_store *refs, const char *refname)

171 }

172 }

173

174 -static void clear_reftable_log_record(struct reftable_log_record *log)

175 -{

176 - switch (log->value_type) {

177 - case REFTABLE_LOG_UPDATE:

178 - /*

179 - * When we write log records, the hashes are owned by the

180 - * caller and thus shouldn't be free'd.

181 - */

182 - log->value.update.old_hash = NULL;

183 - log->value.update.new_hash = NULL;

184 - break;

185 - case REFTABLE_LOG_DELETION:

186 - break;

187 - }

188 - reftable_log_record_release(log);

189 -}

190 -

174 static void fill_reftable_log_record(struct reftable_log_record *log)

175 {

176 const char *info = git_committer_info(0);

@@ -1102,8 +1085,8 @@ static int write_transaction_table(struct reftable_writer *writer, void *cb_data

1085 fill_reftable_log_record(log);

1086 log->update_index = ts;

1087 log->refname = xstrdup(u->refname);

1105 - log->value.update.new_hash = u->new_oid.hash;

1106 - log->value.update.old_hash = tx_update->current_oid.hash;

1088 + memcpy(log->value.update.new_hash, u->new_oid.hash, GIT_MAX_RAWSZ);

1089 + memcpy(log->value.update.old_hash, tx_update->current_oid.hash, GIT_MAX_RAWSZ);

1090 log->value.update.message =

1091 xstrndup(u->msg, arg->refs->write_options.block_size / 2);

1092 }

@@ -1158,7 +1141,7 @@ static int write_transaction_table(struct reftable_writer *writer, void *cb_data

1141 done:

1142 assert(ret != REFTABLE_API_ERROR);

1143 for (i = 0; i < logs_nr; i++)

1161 - clear_reftable_log_record(&logs[i]);

1144 + reftable_log_record_release(&logs[i]);

1145 free(logs);

1146 return ret;

1147 }

@@ -1275,13 +1258,13 @@ static int write_create_symref_table(struct reftable_writer *writer, void *cb_da

1258 log.update_index = ts;

1259 log.value.update.message = xstrndup(create->logmsg,

1260 create->refs->write_options.block_size / 2);

1278 - log.value.update.new_hash = new_oid.hash;

1261 + memcpy(log.value.update.new_hash, new_oid.hash, GIT_MAX_RAWSZ);

1262 if (refs_resolve_ref_unsafe(&create->refs->base, create->refname,

1263 RESOLVE_REF_READING, &old_oid, NULL))

1281 - log.value.update.old_hash = old_oid.hash;

1264 + memcpy(log.value.update.old_hash, old_oid.hash, GIT_MAX_RAWSZ);

1265

1266 ret = reftable_writer_add_log(writer, &log);

1284 - clear_reftable_log_record(&log);

1267 + reftable_log_record_release(&log);

1268 return ret;

1269 }

1270

@@ -1420,7 +1403,7 @@ static int write_copy_table(struct reftable_writer *writer, void *cb_data)

1403 logs[logs_nr].update_index = deletion_ts;

1404 logs[logs_nr].value.update.message =

1405 xstrndup(arg->logmsg, arg->refs->write_options.block_size / 2);

1423 - logs[logs_nr].value.update.old_hash = old_ref.value.val1;

1406 + memcpy(logs[logs_nr].value.update.old_hash, old_ref.value.val1, GIT_MAX_RAWSZ);

1407 logs_nr++;

1408

1409 ret = read_ref_without_reload(arg->stack, "HEAD", &head_oid, &head_referent, &head_type);

@@ -1452,7 +1435,7 @@ static int write_copy_table(struct reftable_writer *writer, void *cb_data)

1435 logs[logs_nr].update_index = creation_ts;

1436 logs[logs_nr].value.update.message =

1437 xstrndup(arg->logmsg, arg->refs->write_options.block_size / 2);

1455 - logs[logs_nr].value.update.new_hash = old_ref.value.val1;

1438 + memcpy(logs[logs_nr].value.update.new_hash, old_ref.value.val1, GIT_MAX_RAWSZ);

1439 logs_nr++;

1440

1441 /*

@@ -1515,10 +1498,6 @@ done:

1498 for (i = 0; i < logs_nr; i++) {

1499 if (!strcmp(logs[i].refname, "HEAD"))

1500 continue;

1518 - if (logs[i].value.update.old_hash == old_ref.value.val1)

1519 - logs[i].value.update.old_hash = NULL;

1520 - if (logs[i].value.update.new_hash == old_ref.value.val1)

1521 - logs[i].value.update.new_hash = NULL;

1501 logs[i].refname = NULL;

1502 reftable_log_record_release(&logs[i]);

1503 }

@@ -2180,7 +2159,7 @@ static int reftable_be_reflog_expire(struct ref_store *ref_store,

2159 dest->value_type = REFTABLE_LOG_DELETION;

2160 } else {

2161 if ((flags & EXPIRE_REFLOGS_REWRITE) && last_hash)

2183 - dest->value.update.old_hash = last_hash;

2162 + memcpy(dest->value.update.old_hash, last_hash, GIT_MAX_RAWSZ);

2163 last_hash = logs[i].value.update.new_hash;

2164 }

2165 }

reftable/merged_test.c

+4 -7

@@ -289,16 +289,13 @@ merged_table_from_log_records(struct reftable_log_record **logs,

289

290 static void test_merged_logs(void)

291 {

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

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

294 - uint8_t hash3[GIT_SHA1_RAWSZ] = { 3 };

292 struct reftable_log_record r1[] = {

293 {

294 .refname = "a",

295 .update_index = 2,

296 .value_type = REFTABLE_LOG_UPDATE,

297 .value.update = {

301 - .old_hash = hash2,

298 + .old_hash = { 2 },

299 /* deletion */

300 .name = "jane doe",

301 .email = "jane@invalid",

@@ -310,8 +307,8 @@ static void test_merged_logs(void)

307 .update_index = 1,

308 .value_type = REFTABLE_LOG_UPDATE,

309 .value.update = {

313 - .old_hash = hash1,

314 - .new_hash = hash2,

310 + .old_hash = { 1 },

311 + .new_hash = { 2 },

312 .name = "jane doe",

313 .email = "jane@invalid",

314 .message = "message1",

@@ -324,7 +321,7 @@ static void test_merged_logs(void)

321 .update_index = 3,

322 .value_type = REFTABLE_LOG_UPDATE,

323 .value.update = {

327 - .new_hash = hash3,

324 + .new_hash = { 3 },

325 .name = "jane doe",

326 .email = "jane@invalid",

327 .message = "message3",

reftable/readwrite_test.c

+25 -37

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

77 }

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

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

80 char name[100];

81 int n;

84 - set_test_hash(hash, i);

85 -

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

85 log.refname = name;

86 log.update_index = update_index;

87 log.value_type = REFTABLE_LOG_UPDATE;

91 - log.value.update.new_hash = hash;

88 + set_test_hash(log.value.update.new_hash, i);

89 log.value.update.message = "message";

91 n = reftable_writer_add_log(w, &log);

@@ -137,13 +134,10 @@ static void test_log_buffer_size(void)

134 /* This tests buffer extension for log compression. Must use a random

135 hash, to ensure that the compressed part is larger than the original.

136 */

140 - uint8_t hash1[GIT_SHA1_RAWSZ], hash2[GIT_SHA1_RAWSZ];

137 for (i = 0; i < GIT_SHA1_RAWSZ; i++) {

142 - hash1[i] = (uint8_t)(git_rand() % 256);

143 - hash2[i] = (uint8_t)(git_rand() % 256);

138 + log.value.update.old_hash[i] = (uint8_t)(git_rand() % 256);

139 + log.value.update.new_hash[i] = (uint8_t)(git_rand() % 256);

140 }

145 - log.value.update.old_hash = hash1;

146 - log.value.update.new_hash = hash2;

141 reftable_writer_set_limits(w, update_index, update_index);

142 err = reftable_writer_add_log(w, &log);

143 EXPECT_ERR(err);

@@ -161,25 +155,26 @@ static void test_log_overflow(void)

155 .block_size = ARRAY_SIZE(msg),

156 };

157 int err;

164 - struct reftable_log_record

165 - log = { .refname = "refs/heads/master",

166 - .update_index = 0xa,

167 - .value_type = REFTABLE_LOG_UPDATE,

168 - .value = { .update = {

169 - .name = "Han-Wen Nienhuys",

170 - .email = "hanwen@google.com",

171 - .tz_offset = 100,

172 - .time = 0x5e430672,

173 - .message = msg,

174 - } } };

158 + struct reftable_log_record log = {

159 + .refname = "refs/heads/master",

160 + .update_index = 0xa,

161 + .value_type = REFTABLE_LOG_UPDATE,

162 + .value = {

163 + .update = {

164 + .old_hash = { 1 },

165 + .new_hash = { 2 },

166 + .name = "Han-Wen Nienhuys",

167 + .email = "hanwen@google.com",

168 + .tz_offset = 100,

169 + .time = 0x5e430672,

170 + .message = msg,

171 + },

172 + },

173 + };

174 struct reftable_writer *w =

175 reftable_new_writer(&strbuf_add_void, &noop_flush, &buf, &opts);

176

178 - uint8_t hash1[GIT_SHA1_RAWSZ] = {1}, hash2[GIT_SHA1_RAWSZ] = { 2 };

179 -

177 memset(msg, 'x', sizeof(msg) - 1);

181 - log.value.update.old_hash = hash1;

182 - log.value.update.new_hash = hash2;

178 reftable_writer_set_limits(w, update_index, update_index);

179 err = reftable_writer_add_log(w, &log);

180 EXPECT(err == REFTABLE_ENTRY_TOO_BIG_ERROR);

@@ -219,16 +214,13 @@ static void test_log_write_read(void)

214 EXPECT_ERR(err);

215 }

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

222 - uint8_t hash1[GIT_SHA1_RAWSZ], hash2[GIT_SHA1_RAWSZ];

217 struct reftable_log_record log = { NULL };

224 - set_test_hash(hash1, i);

225 - set_test_hash(hash2, i + 1);

218

219 log.refname = names[i];

220 log.update_index = i;

221 log.value_type = REFTABLE_LOG_UPDATE;

230 - log.value.update.old_hash = hash1;

231 - log.value.update.new_hash = hash2;

222 + set_test_hash(log.value.update.old_hash, i);

223 + set_test_hash(log.value.update.new_hash, i + 1);

224

225 err = reftable_writer_add_log(w, &log);

226 EXPECT_ERR(err);

@@ -298,18 +290,15 @@ static void test_log_zlib_corruption(void)

290 struct reftable_writer *w =

291 reftable_new_writer(&strbuf_add_void, &noop_flush, &buf, &opts);

292 const struct reftable_stats *stats = NULL;

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

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

293 char message[100] = { 0 };

294 int err, i, n;

305 -

295 struct reftable_log_record log = {

296 .refname = "refname",

297 .value_type = REFTABLE_LOG_UPDATE,

298 .value = {

299 .update = {

311 - .new_hash = hash1,

312 - .old_hash = hash2,

300 + .new_hash = { 1 },

301 + .old_hash = { 2 },

302 .name = "My Name",

303 .email = "myname@invalid",

304 .message = message,

@@ -821,13 +810,12 @@ static void test_write_multiple_indices(void)

810 }

811

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

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

813 struct reftable_log_record log = {

814 .update_index = 1,

815 .value_type = REFTABLE_LOG_UPDATE,

816 .value.update = {

829 - .old_hash = hash,

830 - .new_hash = hash,

817 + .old_hash = { i },

818 + .new_hash = { i },

819 },

820 };

821

reftable/record.c

+10 -49

@@ -763,16 +763,10 @@ static void reftable_log_record_copy_from(void *rec, const void *src_rec,

763 xstrdup(dst->value.update.message);

764 }

765

766 - if (dst->value.update.new_hash) {

767 - REFTABLE_ALLOC_ARRAY(dst->value.update.new_hash, hash_size);

768 - memcpy(dst->value.update.new_hash,

769 - src->value.update.new_hash, hash_size);

770 - }

771 - if (dst->value.update.old_hash) {

772 - REFTABLE_ALLOC_ARRAY(dst->value.update.old_hash, hash_size);

773 - memcpy(dst->value.update.old_hash,

774 - src->value.update.old_hash, hash_size);

775 - }

766 + memcpy(dst->value.update.new_hash,

767 + src->value.update.new_hash, hash_size);

768 + memcpy(dst->value.update.old_hash,

769 + src->value.update.old_hash, hash_size);

770 break;

771 }

772 }

@@ -790,8 +784,6 @@ void reftable_log_record_release(struct reftable_log_record *r)

784 case REFTABLE_LOG_DELETION:

785 break;

786 case REFTABLE_LOG_UPDATE:

793 - reftable_free(r->value.update.new_hash);

794 - reftable_free(r->value.update.old_hash);

787 reftable_free(r->value.update.name);

788 reftable_free(r->value.update.email);

789 reftable_free(r->value.update.message);

@@ -808,33 +800,20 @@ static uint8_t reftable_log_record_val_type(const void *rec)

800 return reftable_log_record_is_deletion(log) ? 0 : 1;

801 }

802

811 -static uint8_t zero[GIT_SHA256_RAWSZ] = { 0 };

812 -

803 static int reftable_log_record_encode(const void *rec, struct string_view s,

804 int hash_size)

805 {

806 const struct reftable_log_record *r = rec;

807 struct string_view start = s;

808 int n = 0;

819 - uint8_t *oldh = NULL;

820 - uint8_t *newh = NULL;

809 if (reftable_log_record_is_deletion(r))

810 return 0;

811

824 - oldh = r->value.update.old_hash;

825 - newh = r->value.update.new_hash;

826 - if (!oldh) {

827 - oldh = zero;

828 - }

829 - if (!newh) {

830 - newh = zero;

831 - }

832 -

812 if (s.len < 2 * hash_size)

813 return -1;

814

836 - memcpy(s.buf, oldh, hash_size);

837 - memcpy(s.buf + hash_size, newh, hash_size);

815 + memcpy(s.buf, r->value.update.old_hash, hash_size);

816 + memcpy(s.buf + hash_size, r->value.update.new_hash, hash_size);

817 string_view_consume(&s, 2 * hash_size);

818

819 n = encode_string(r->value.update.name ? r->value.update.name : "", s);

@@ -891,8 +870,6 @@ static int reftable_log_record_decode(void *rec, struct strbuf key,

870 if (val_type != r->value_type) {

871 switch (r->value_type) {

872 case REFTABLE_LOG_UPDATE:

894 - FREE_AND_NULL(r->value.update.old_hash);

895 - FREE_AND_NULL(r->value.update.new_hash);

873 FREE_AND_NULL(r->value.update.message);

874 FREE_AND_NULL(r->value.update.email);

875 FREE_AND_NULL(r->value.update.name);

@@ -909,11 +886,6 @@ static int reftable_log_record_decode(void *rec, struct strbuf key,

886 if (in.len < 2 * hash_size)

887 return REFTABLE_FORMAT_ERROR;

888

912 - r->value.update.old_hash =

913 - reftable_realloc(r->value.update.old_hash, hash_size);

914 - r->value.update.new_hash =

915 - reftable_realloc(r->value.update.new_hash, hash_size);

916 -

889 memcpy(r->value.update.old_hash, in.buf, hash_size);

890 memcpy(r->value.update.new_hash, in.buf + hash_size, hash_size);

891

@@ -983,17 +955,6 @@ static int null_streq(char *a, char *b)

955 return 0 == strcmp(a, b);

956 }

957

986 -static int zero_hash_eq(uint8_t *a, uint8_t *b, int sz)

987 -{

988 - if (!a)

989 - a = zero;

990 -

991 - if (!b)

992 - b = zero;

993 -

994 - return !memcmp(a, b, sz);

995 -}

996 -

958 static int reftable_log_record_equal_void(const void *a,

959 const void *b, int hash_size)

960 {

@@ -1037,10 +998,10 @@ int reftable_log_record_equal(const struct reftable_log_record *a,

998 b->value.update.email) &&

999 null_streq(a->value.update.message,

1000 b->value.update.message) &&

1040 - zero_hash_eq(a->value.update.old_hash,

1041 - b->value.update.old_hash, hash_size) &&

1042 - zero_hash_eq(a->value.update.new_hash,

1043 - b->value.update.new_hash, hash_size);

1001 + !memcmp(a->value.update.old_hash,

1002 + b->value.update.old_hash, hash_size) &&

1003 + !memcmp(a->value.update.new_hash,

1004 + b->value.update.new_hash, hash_size);

1005 }

1006

1007 abort();

reftable/record_test.c

-11

@@ -183,8 +183,6 @@ static void test_reftable_log_record_roundtrip(void)

183 .value_type = REFTABLE_LOG_UPDATE,

184 .value = {

185 .update = {

186 - .old_hash = reftable_malloc(GIT_SHA1_RAWSZ),

187 - .new_hash = reftable_malloc(GIT_SHA1_RAWSZ),

186 .name = xstrdup("han-wen"),

187 .email = xstrdup("hanwen@google.com"),

188 .message = xstrdup("test"),

@@ -202,13 +200,6 @@ static void test_reftable_log_record_roundtrip(void)

200 .refname = xstrdup("branch"),

201 .update_index = 33,

202 .value_type = REFTABLE_LOG_UPDATE,

205 - .value = {

206 - .update = {

207 - .old_hash = reftable_malloc(GIT_SHA1_RAWSZ),

208 - .new_hash = reftable_malloc(GIT_SHA1_RAWSZ),

209 - /* rest of fields left empty. */

210 - },

211 - },

203 }

204 };

205 set_test_hash(in[0].value.update.new_hash, 1);

@@ -231,8 +222,6 @@ static void test_reftable_log_record_roundtrip(void)

222 .value_type = REFTABLE_LOG_UPDATE,

223 .value = {

224 .update = {

234 - .new_hash = reftable_calloc(GIT_SHA1_RAWSZ, 1),

235 - .old_hash = reftable_calloc(GIT_SHA1_RAWSZ, 1),

225 .name = xstrdup("old name"),

226 .email = xstrdup("old@email"),

227 .message = xstrdup("old message"),

reftable/reftable-record.h

+2 -2

@@ -88,8 +88,8 @@ struct reftable_log_record {

89 union {

90 struct {

91 - uint8_t *new_hash;

92 - uint8_t *old_hash;

91 + unsigned char new_hash[GIT_MAX_RAWSZ];

92 + unsigned char old_hash[GIT_MAX_RAWSZ];

93 char *name;

94 char *email;

95 uint64_t time;

reftable/stack_test.c

+11 -15

@@ -468,8 +468,6 @@ static void test_reftable_stack_add(void)

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

469 logs[i].update_index = N + i + 1;

470 logs[i].value_type = REFTABLE_LOG_UPDATE;

471 -

472 - logs[i].value.update.new_hash = reftable_malloc(GIT_SHA1_RAWSZ);

471 logs[i].value.update.email = xstrdup("identity@invalid");

472 set_test_hash(logs[i].value.update.new_hash, i);

473 }

@@ -547,16 +545,17 @@ static void test_reftable_stack_log_normalize(void)

545 };

546 struct reftable_stack *st = NULL;

547 char *dir = get_tmp_dir(__LINE__);

550 -

551 - uint8_t h1[GIT_SHA1_RAWSZ] = { 0x01 }, h2[GIT_SHA1_RAWSZ] = { 0x02 };

552 -

553 - struct reftable_log_record input = { .refname = "branch",

554 - .update_index = 1,

555 - .value_type = REFTABLE_LOG_UPDATE,

556 - .value = { .update = {

557 - .new_hash = h1,

558 - .old_hash = h2,

559 - } } };

548 + struct reftable_log_record input = {

549 + .refname = "branch",

550 + .update_index = 1,

551 + .value_type = REFTABLE_LOG_UPDATE,

552 + .value = {

553 + .update = {

554 + .new_hash = { 1 },

555 + .old_hash = { 2 },

556 + },

557 + },

558 + };

559 struct reftable_log_record dest = {

560 .update_index = 0,

561 };

@@ -627,8 +626,6 @@ static void test_reftable_stack_tombstone(void)

626 logs[i].update_index = 42;

627 if (i % 2 == 0) {

628 logs[i].value_type = REFTABLE_LOG_UPDATE;

630 - logs[i].value.update.new_hash =

631 - reftable_malloc(GIT_SHA1_RAWSZ);

629 set_test_hash(logs[i].value.update.new_hash, i);

630 logs[i].value.update.email =

631 xstrdup("identity@invalid");

@@ -810,7 +807,6 @@ static void test_reflog_expire(void)

807 logs[i].update_index = i;

808 logs[i].value_type = REFTABLE_LOG_UPDATE;

809 logs[i].value.update.time = i;

813 - logs[i].value.update.new_hash = reftable_malloc(GIT_SHA1_RAWSZ);

810 logs[i].value.update.email = xstrdup("identity@invalid");

811 set_test_hash(logs[i].value.update.new_hash, i);

812 }