@matheusbsbs / matheus-bagliano / commits / b00bcb7c49

reftable/reader: iterate to next block in place

The table iterator has to iterate towards the next block once it has yielded all records of the current block. This is done by creating a new table iterator, initializing it to the next block, releasing the old iterator and then copying over the data. Refactor the code to instead advance the table iterator in place. This is simpler and unlocks some optimizations in subsequent patches. Also, it allows us to avoid some allocations. The following measurements show a single matching ref out of 1 million refs. Before this change: HEAP SUMMARY: in use at exit: 13,603 bytes in 125 blocks total heap usage: 7,235 allocs, 7,110 frees, 301,481 bytes allocated After: HEAP SUMMARY: in use at exit: 13,603 bytes in 125 blocks total heap usage: 315 allocs, 190 frees, 107,027 bytes allocated Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Patrick Steinhardt committed Apr 8, 2024 at 14:16 UTC b00bcb7c49a4f96d39e4a448998b366bcd484de2
2 files changed +28 -21
reftable/block.c
+2
@@ -188,6 +188,8 @@ int block_reader_init(struct block_reader *br, struct reftable_block *block,
188 uint8_t *restart_bytes = NULL;
189 uint8_t *uncompressed = NULL;
190
191 + reftable_block_done(&br->block);
192 +
193 if (!reftable_is_block_type(typ)) {
194 err = REFTABLE_FORMAT_ERROR;
195 goto done;
reftable/reader.c
+26 -21
@@ -312,26 +312,20 @@ static void table_iter_close(struct table_iter *ti)
312 block_iter_close(&ti->bi);
313 }
314
315 -static int table_iter_next_block(struct table_iter *dest,
316 - struct table_iter *src)
315 +static int table_iter_next_block(struct table_iter *ti)
316 {
318 - uint64_t next_block_off = src->block_off + src->br.full_block_size;
317 + uint64_t next_block_off = ti->block_off + ti->br.full_block_size;
318 int err;
319
321 - dest->r = src->r;
322 - dest->typ = src->typ;
323 - dest->block_off = next_block_off;
324 -
325 - err = reader_init_block_reader(src->r, &dest->br, next_block_off, src->typ);
320 + err = reader_init_block_reader(ti->r, &ti->br, next_block_off, ti->typ);
321 if (err > 0)
327 - dest->is_finished = 1;
328 - if (err) {
329 - table_iter_block_done(dest);
322 + ti->is_finished = 1;
323 + if (err)
324 return err;
331 - }
325
333 - dest->is_finished = 0;
334 - block_iter_seek_start(&dest->bi, &dest->br);
326 + ti->block_off = next_block_off;
327 + ti->is_finished = 0;
328 + block_iter_seek_start(&ti->bi, &ti->br);
329
330 return 0;
331 }
@@ -342,7 +336,6 @@ static int table_iter_next(struct table_iter *ti, struct reftable_record *rec)
336 return REFTABLE_API_ERROR;
337
338 while (1) {
345 - struct table_iter next = TABLE_ITER_INIT;
339 int err;
340
341 if (ti->is_finished)
@@ -362,14 +355,11 @@ static int table_iter_next(struct table_iter *ti, struct reftable_record *rec)
355 * table and retry. If there are no more blocks then the
356 * iterator is drained.
357 */
365 - err = table_iter_next_block(&next, ti);
358 + err = table_iter_next_block(ti);
359 if (err) {
360 ti->is_finished = 1;
361 return err;
362 }
370 -
371 - table_iter_close(ti);
372 - *ti = next;
363 }
364 }
365
@@ -453,9 +443,24 @@ static int reader_seek_linear(struct table_iter *ti,
443 * have no other way to do this.
444 */
445 while (1) {
456 - struct table_iter next = TABLE_ITER_INIT;
446 + struct table_iter next = *ti;
447 +
448 + /*
449 + * We must be careful to not modify underlying data of `ti`
450 + * because we may find that `next` does not contain our desired
451 + * block, but that `ti` does. In that case, we would discard
452 + * `next` and continue with `ti`.
453 + *
454 + * This also means that we cannot reuse allocated memory for
455 + * `next` here. While it would be great if we could, it should
456 + * in practice not be too bad given that we should only ever
457 + * end up doing linear seeks with at most three blocks. As soon
458 + * as we have more than three blocks we would have an index, so
459 + * we would not do a linear search there anymore.
460 + */
461 + memset(&next.br.block, 0, sizeof(next.br.block));
462
458 - err = table_iter_next_block(&next, ti);
463 + err = table_iter_next_block(&next);
464 if (err < 0)
465 goto done;
466 if (err > 0)