@matheusbsbs / matheus-bagliano / commits / 1970333644

reftable: fix perf regression when reading blocks of unwanted type

In fd888311fbc (reftable/table: move reading block into block reader, 2025-04-07), we have refactored how reftable blocks are read so that most of the logic is contained in the "block.c" subsystem itself. Most importantly, the whole logic to read the data itself is now contained in that subsystem. This change caused a significant performance regression though when reading blocks that aren't of the specific type one is searching for: Benchmark 1: update-ref: create 100k refs (revision = fd888311fbc~) Time (mean ± σ): 2.171 s ± 0.028 s [User: 1.189 s, System: 0.977 s] Range (min … max): 2.117 s … 2.206 s 10 runs Benchmark 2: update-ref: create 100k refs (revision = fd888311fbc) Time (mean ± σ): 3.418 s ± 0.030 s [User: 2.371 s, System: 1.037 s] Range (min … max): 3.377 s … 3.473 s 10 runs Summary update-ref: create 100k refs (revision = fd888311fbc~) ran 1.57 ± 0.02 times faster than update-ref: create 100k refs (revision = fd888311fbc) The root caute of the performance regression is that we changed when exactly blocks of an uninteresting type are being discarded. Previous to the refactoring in the mentioned commit we'd load the block data, read its type, notice that it's not the wanted type and discard the block. After the commit though we don't discard the block immediately, but we fully decode it only to realize that it's not the desired type. We then discard the block again, but have already performed a bunch of pointless work. Fix the regression by making `reftable_block_init()` return early in case the block is not of the desired type. This fixes the performance hit: Benchmark 1: update-ref: create 100k refs (revision = HEAD~) Time (mean ± σ): 2.712 s ± 0.018 s [User: 1.990 s, System: 0.716 s] Range (min … max): 2.682 s … 2.741 s 10 runs Benchmark 2: update-ref: create 100k refs (revision = HEAD) Time (mean ± σ): 1.670 s ± 0.012 s [User: 0.991 s, System: 0.676 s] Range (min … max): 1.652 s … 1.693 s 10 runs Summary update-ref: create 100k refs (revision = HEAD) ran 1.62 ± 0.02 times faster than update-ref: create 100k refs (revision = HEAD~) Note that the baseline performance is lower than in the original due to a couple of unrelated performance improvements that have landed since the original commit. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Patrick Steinhardt committed May 12, 2025 at 17:15 UTC 1970333644fad127c68046697b9b86fd8d7f28c2

4 files changed +19 -17

reftable/block.c

+6 -1

@@ -226,7 +226,8 @@ static int read_block(struct reftable_block_source *source,

226 int reftable_block_init(struct reftable_block *block,

227 struct reftable_block_source *source,

228 uint32_t offset, uint32_t header_size,

229 - uint32_t table_block_size, uint32_t hash_size)

229 + uint32_t table_block_size, uint32_t hash_size,

230 + uint8_t want_type)

231 {

232 uint32_t guess_block_size = table_block_size ?

233 table_block_size : DEFAULT_BLOCK_SIZE;

@@ -246,6 +247,10 @@ int reftable_block_init(struct reftable_block *block,

247 err = REFTABLE_FORMAT_ERROR;

248 goto done;

249 }

250 + if (want_type != REFTABLE_BLOCK_TYPE_ANY && block_type != want_type) {

251 + err = 1;

252 + goto done;

253 + }

254

255 block_size = reftable_get_be24(block->block_data.data + header_size + 1);

256 if (block_size > guess_block_size) {

reftable/reftable-block.h

+2 -1

@@ -56,7 +56,8 @@ struct reftable_block {

56 int reftable_block_init(struct reftable_block *b,

57 struct reftable_block_source *source,

58 uint32_t offset, uint32_t header_size,

59 - uint32_t table_block_size, uint32_t hash_size);

59 + uint32_t table_block_size, uint32_t hash_size,

60 + uint8_t want_type);

62 /* Release resources allocated by the block. */

63 void reftable_block_release(struct reftable_block *b);

reftable/table.c

+1 -10

@@ -173,16 +173,7 @@ int table_init_block(struct reftable_table *t, struct reftable_block *block,

173 return 1;

174

175 err = reftable_block_init(block, &t->source, next_off, header_off,

176 - t->block_size, hash_size(t->hash_id));

177 - if (err < 0)

178 - goto done;

179 -

180 - if (want_typ != REFTABLE_BLOCK_TYPE_ANY && block->block_type != want_typ) {

181 - err = 1;

182 - goto done;

183 - }

184 -

185 -done:

176 + t->block_size, hash_size(t->hash_id), want_typ);

177 if (err)

178 reftable_block_release(block);

179 return err;

t/unit-tests/t-reftable-block.c

+10 -5

@@ -64,7 +64,8 @@ static void t_ref_block_read_write(void)

64 block_writer_release(&bw);

66 block_source_from_buf(&source ,&block_data);

67 - reftable_block_init(&block, &source, 0, header_off, block_size, REFTABLE_HASH_SIZE_SHA1);

67 + reftable_block_init(&block, &source, 0, header_off, block_size,

68 + REFTABLE_HASH_SIZE_SHA1, REFTABLE_BLOCK_TYPE_REF);

70 block_iter_init(&it, &block);

@@ -153,7 +154,8 @@ static void t_log_block_read_write(void)

154 block_writer_release(&bw);

155

156 block_source_from_buf(&source, &block_data);

156 - reftable_block_init(&block, &source, 0, header_off, block_size, REFTABLE_HASH_SIZE_SHA1);

157 + reftable_block_init(&block, &source, 0, header_off, block_size,

158 + REFTABLE_HASH_SIZE_SHA1, REFTABLE_BLOCK_TYPE_LOG);

159

160 block_iter_init(&it, &block);

161

@@ -245,7 +247,8 @@ static void t_obj_block_read_write(void)

247 block_writer_release(&bw);

248

249 block_source_from_buf(&source, &block_data);

248 - reftable_block_init(&block, &source, 0, header_off, block_size, REFTABLE_HASH_SIZE_SHA1);

250 + reftable_block_init(&block, &source, 0, header_off, block_size,

251 + REFTABLE_HASH_SIZE_SHA1, REFTABLE_BLOCK_TYPE_OBJ);

252

253 block_iter_init(&it, &block);

254

@@ -329,7 +332,8 @@ static void t_index_block_read_write(void)

332 block_writer_release(&bw);

333

334 block_source_from_buf(&source, &block_data);

332 - reftable_block_init(&block, &source, 0, header_off, block_size, REFTABLE_HASH_SIZE_SHA1);

335 + reftable_block_init(&block, &source, 0, header_off, block_size,

336 + REFTABLE_HASH_SIZE_SHA1, REFTABLE_BLOCK_TYPE_INDEX);

337

338 block_iter_init(&it, &block);

339

@@ -411,7 +415,8 @@ static void t_block_iterator(void)

415 check_int(err, >, 0);

416

417 block_source_from_buf(&source, &data);

414 - reftable_block_init(&block, &source, 0, 0, data.len, REFTABLE_HASH_SIZE_SHA1);

418 + reftable_block_init(&block, &source, 0, 0, data.len,

419 + REFTABLE_HASH_SIZE_SHA1, REFTABLE_BLOCK_TYPE_REF);

420

421 err = reftable_block_init_iterator(&block, &it);

422 check_int(err, ==, 0);