@matheusbsbs / matheus-bagliano / commits / 9a59b65dba

reftable/reader: avoid copying index iterator

When doing an indexed seek we need to walk down the multi-level index until we finally hit a record of the desired indexed type. This loop performs a copy of the index iterator on every iteration, which is both hard to understand and completely unnecessary. Refactor the code so that we use a single iterator to walk down the indices, only. Note that while this should improve performance, the improvement is negligible in all but the most unreasonable repositories. This is because the effect is only really noticeable when we have to walk down many levels of indices, which is not something that a repository would typically have. So the motivation for this change is really only about readability. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Patrick Steinhardt committed May 13, 2024 at 10:47 UTC 9a59b65dba0820671753f636e9417bfd63ea20c1
1 file changed +14 -24
reftable/reader.c
+14 -24
@@ -510,13 +510,11 @@ static int reader_seek_indexed(struct reftable_reader *r,
510 .type = BLOCK_TYPE_INDEX,
511 .u.idx = { .last_key = STRBUF_INIT },
512 };
513 - struct table_iter index_iter = TABLE_ITER_INIT;
514 - struct table_iter empty = TABLE_ITER_INIT;
515 - struct table_iter next = TABLE_ITER_INIT;
513 + struct table_iter ti = TABLE_ITER_INIT, *malloced;
514 int err = 0;
515
516 reftable_record_key(rec, &want_index.u.idx.last_key);
519 - err = reader_start(r, &index_iter, reftable_record_type(rec), 1);
517 + err = reader_start(r, &ti, reftable_record_type(rec), 1);
518 if (err < 0)
519 goto done;
520
@@ -526,7 +524,7 @@ static int reader_seek_indexed(struct reftable_reader *r,
524 * highest layer that identifies the relevant index block as well as
525 * the record inside that block that corresponds to our wanted key.
526 */
529 - err = reader_seek_linear(&index_iter, &want_index);
527 + err = reader_seek_linear(&ti, &want_index);
528 if (err < 0)
529 goto done;
530
@@ -552,44 +550,36 @@ static int reader_seek_indexed(struct reftable_reader *r,
550 * all levels of the index only to find out that the key does
551 * not exist.
552 */
555 - err = table_iter_next(&index_iter, &index_result);
553 + err = table_iter_next(&ti, &index_result);
554 if (err != 0)
555 goto done;
556
559 - err = reader_table_iter_at(r, &next, index_result.u.idx.offset,
560 - 0);
557 + err = reader_table_iter_at(r, &ti, index_result.u.idx.offset, 0);
558 if (err != 0)
559 goto done;
560
564 - err = block_iter_seek_key(&next.bi, &next.br, &want_index.u.idx.last_key);
561 + err = block_iter_seek_key(&ti.bi, &ti.br, &want_index.u.idx.last_key);
562 if (err < 0)
563 goto done;
564
568 - if (next.typ == reftable_record_type(rec)) {
565 + if (ti.typ == reftable_record_type(rec)) {
566 err = 0;
567 break;
568 }
569
573 - if (next.typ != BLOCK_TYPE_INDEX) {
570 + if (ti.typ != BLOCK_TYPE_INDEX) {
571 err = REFTABLE_FORMAT_ERROR;
575 - break;
572 + goto done;
573 }
577 -
578 - table_iter_close(&index_iter);
579 - index_iter = next;
580 - next = empty;
574 }
575
583 - if (err == 0) {
584 - struct table_iter *malloced = reftable_calloc(1, sizeof(*malloced));
585 - *malloced = next;
586 - next = empty;
587 - iterator_from_table_iter(it, malloced);
588 - }
576 + REFTABLE_ALLOC_ARRAY(malloced, 1);
577 + *malloced = ti;
578 + iterator_from_table_iter(it, malloced);
579
580 done:
591 - table_iter_close(&next);
592 - table_iter_close(&index_iter);
581 + if (err)
582 + table_iter_close(&ti);
583 reftable_record_release(&want_index);
584 reftable_record_release(&index_result);
585 return err;