@matheusbsbs / matheus-bagliano / commits / 9da5c992dd

reftable/block: avoid copying block iterators on seek

When seeking a reftable record in a block we need to position the iterator _before_ the sought-after record so that the next call to `block_iter_next()` would yield that record. To achieve this, the loop that performs the linear seeks to restore the previous position once it has found the record. This is done by advancing two `block_iter`s: one to check whether the next record is our sought-after record, and one that we update after every iteration. This of course involves quite a lot of copying and also leads to needless memory allocations. Refactor the code to get rid of the `next` iterator and the copying this involves. Instead, we can restore the previous offset such that the call to `next` will return the correct record. Next to being simpler conceptually this also leads to a nice speedup. The following benchmark parser 10k refs out of 100k existing refs via `git-rev-list --no-walk`: Benchmark 1: rev-list: print many refs (HEAD~) Time (mean ± σ): 170.2 ms ± 1.7 ms [User: 86.1 ms, System: 83.6 ms] Range (min … max): 166.4 ms … 180.3 ms 500 runs Benchmark 2: rev-list: print many refs (HEAD~) Time (mean ± σ): 161.6 ms ± 1.6 ms [User: 78.1 ms, System: 83.0 ms] Range (min … max): 158.4 ms … 172.3 ms 500 runs Summary rev-list: print many refs (HEAD) ran 1.05 ± 0.01 times faster than rev-list: print many refs (HEAD~) 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:17 UTC 9da5c992dd8856035003cb06ff9c996a23956951

2 files changed +14 -20

reftable/block.c

+14 -18

@@ -365,16 +365,6 @@ static int restart_needle_less(size_t idx, void *_args)

365 return args->needle.len < suffix_len;

366 }

367

368 -void block_iter_copy_from(struct block_iter *dest, const struct block_iter *src)

369 -{

370 - dest->block = src->block;

371 - dest->block_len = src->block_len;

372 - dest->hash_size = src->hash_size;

373 - dest->next_off = src->next_off;

374 - strbuf_reset(&dest->last_key);

375 - strbuf_addbuf(&dest->last_key, &src->last_key);

376 -}

377 -

368 int block_iter_next(struct block_iter *it, struct reftable_record *rec)

369 {

370 struct string_view in = {

@@ -427,7 +417,6 @@ int block_iter_seek_key(struct block_iter *it, const struct block_reader *br,

417 .needle = *want,

418 .reader = br,

419 };

430 - struct block_iter next = BLOCK_ITER_INIT;

420 struct reftable_record rec;

421 int err = 0;

422 size_t i;

@@ -486,11 +475,13 @@ int block_iter_seek_key(struct block_iter *it, const struct block_reader *br,

475 * far and then back up.

476 */

477 while (1) {

489 - block_iter_copy_from(&next, it);

490 - err = block_iter_next(&next, &rec);

478 + size_t prev_off = it->next_off;

479 +

480 + err = block_iter_next(it, &rec);

481 if (err < 0)

482 goto done;

483 if (err > 0) {

484 + it->next_off = prev_off;

485 err = 0;

486 goto done;

487 }

@@ -501,18 +492,23 @@ int block_iter_seek_key(struct block_iter *it, const struct block_reader *br,

492 * record does not exist in the block and can thus abort early.

493 * In case it is equal to the sought-after key we have found

494 * the desired record.

495 + *

496 + * Note that we store the next record's key record directly in

497 + * `last_key` without restoring the key of the preceding record

498 + * in case we need to go one record back. This is safe to do as

499 + * `block_iter_next()` would return the ref whose key is equal

500 + * to `last_key` now, and naturally all keys share a prefix

501 + * with themselves.

502 */

503 reftable_record_key(&rec, &it->last_key);

506 - if (strbuf_cmp(&it->last_key, want) >= 0)

504 + if (strbuf_cmp(&it->last_key, want) >= 0) {

505 + it->next_off = prev_off;

506 goto done;

508 -

509 - block_iter_copy_from(it, &next);

507 + }

508 }

509

510 done:

513 - block_iter_close(&next);

511 reftable_record_release(&rec);

515 -

512 return err;

513 }

514

reftable/block.h

-2

@@ -121,8 +121,6 @@ void block_iter_seek_start(struct block_iter *it, const struct block_reader *br)

121 int block_iter_seek_key(struct block_iter *it, const struct block_reader *br,

122 struct strbuf *want);

123

124 -void block_iter_copy_from(struct block_iter *dest, const struct block_iter *src);

125 -

124 /* return < 0 for error, 0 for OK, > 0 for EOF. */

125 int block_iter_next(struct block_iter *it, struct reftable_record *rec);

126