reftable/block: make block iterators reseekable

Refactor the block iterators so that initialization and seeking are different from one another. This makes the iterator trivially reseekable by storing the pointer to the block at initialization time, which we can then reuse on every seek. This refactoring prepares the code for exposing a `reftable_iterator` interface for blocks in a subsequent commit. Callsites are adjusted accordingly. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Patrick Steinhardt committed Apr 7, 2025 at 15:16 UTC 6da48a5e00ae77c4092e78ac8ac8641a90660343
5 files changed +48 -35
reftable/block.c
+13 -10
@@ -381,11 +381,16 @@ static uint32_t block_restart_offset(const struct reftable_block *b, size_t idx)
381 return reftable_get_be24(b->block_data.data + b->restart_off + 3 * idx);
382 }
383
384 -void block_iter_seek_start(struct block_iter *it, const struct reftable_block *block)
384 +void block_iter_init(struct block_iter *it, const struct reftable_block *block)
385 {
386 it->block = block;
387 + block_iter_seek_start(it);
388 +}
389 +
390 +void block_iter_seek_start(struct block_iter *it)
391 +{
392 reftable_buf_reset(&it->last_key);
388 - it->next_off = block->header_off + 4;
393 + it->next_off = it->block->header_off + 4;
394 }
395
396 struct restart_needle_less_args {
@@ -473,12 +478,11 @@ void block_iter_close(struct block_iter *it)
478 reftable_buf_release(&it->scratch);
479 }
480
476 -int block_iter_seek_key(struct block_iter *it, const struct reftable_block *block,
477 - struct reftable_buf *want)
481 +int block_iter_seek_key(struct block_iter *it, struct reftable_buf *want)
482 {
483 struct restart_needle_less_args args = {
484 .needle = *want,
481 - .block = block,
485 + .block = it->block,
486 };
487 struct reftable_record rec;
488 int err = 0;
@@ -496,7 +500,7 @@ int block_iter_seek_key(struct block_iter *it, const struct reftable_block *bloc
500 * restart point. While that works alright, we would end up scanning
501 * too many record.
502 */
499 - i = binsearch(block->restart_count, &restart_needle_less, &args);
503 + i = binsearch(it->block->restart_count, &restart_needle_less, &args);
504 if (args.error) {
505 err = REFTABLE_FORMAT_ERROR;
506 goto done;
@@ -521,12 +525,11 @@ int block_iter_seek_key(struct block_iter *it, const struct reftable_block *bloc
525 * starting from the preceding restart point.
526 */
527 if (i > 0)
524 - it->next_off = block_restart_offset(block, i - 1);
528 + it->next_off = block_restart_offset(it->block, i - 1);
529 else
526 - it->next_off = block->header_off + 4;
527 - it->block = block;
530 + it->next_off = it->block->header_off + 4;
531
529 - err = reftable_record_init(&rec, reftable_block_type(block));
532 + err = reftable_record_init(&rec, reftable_block_type(it->block));
533 if (err < 0)
534 goto done;
535
reftable/block.h
+16 -5
@@ -79,12 +79,23 @@ struct block_iter {
79 .scratch = REFTABLE_BUF_INIT, \
80 }
81
82 -/* Position `it` at start of the block */
83 -void block_iter_seek_start(struct block_iter *it, const struct reftable_block *block);
82 +/*
83 + * Initialize the block iterator with the given block. The iterator will be
84 + * positioned at the first record contained in the block. The block must remain
85 + * valid until the end of the iterator's lifetime. It is valid to re-initialize
86 + * iterators multiple times.
87 + */
88 +void block_iter_init(struct block_iter *it, const struct reftable_block *block);
89 +
90 +/* Position the initialized iterator at the first record of its block. */
91 +void block_iter_seek_start(struct block_iter *it);
92
85 -/* Position `it` to the `want` key in the block */
86 -int block_iter_seek_key(struct block_iter *it, const struct reftable_block *block,
87 - struct reftable_buf *want);
93 +/*
94 + * Position the initialized iterator at the desired record key. It is not an
95 + * error in case the record cannot be found. If so, a subsequent call to
96 + * `block_iter_next()` will indicate that the iterator is exhausted.
97 + */
98 +int block_iter_seek_key(struct block_iter *it, struct reftable_buf *want);
99
100 /* return < 0 for error, 0 for OK, > 0 for EOF. */
101 int block_iter_next(struct block_iter *it, struct reftable_record *rec);
reftable/iter.c
+1 -1
@@ -139,7 +139,7 @@ static int indexed_table_ref_iter_next_block(struct indexed_table_ref_iter *it)
139 /* indexed block does not exist. */
140 return REFTABLE_FORMAT_ERROR;
141 }
142 - block_iter_seek_start(&it->cur, &it->block);
142 + block_iter_init(&it->cur, &it->block);
143 return 0;
144 }
145
reftable/table.c
+7 -4
@@ -208,7 +208,7 @@ static int table_iter_next_block(struct table_iter *ti)
208
209 ti->block_off = next_block_off;
210 ti->is_finished = 0;
211 - block_iter_seek_start(&ti->bi, &ti->block);
211 + block_iter_init(&ti->bi, &ti->block);
212
213 return 0;
214 }
@@ -256,7 +256,7 @@ static int table_iter_seek_to(struct table_iter *ti, uint64_t off, uint8_t typ)
256
257 ti->typ = reftable_block_type(&ti->block);
258 ti->block_off = off;
259 - block_iter_seek_start(&ti->bi, &ti->block);
259 + block_iter_init(&ti->bi, &ti->block);
260 ti->is_finished = 0;
261 return 0;
262 }
@@ -349,7 +349,8 @@ static int table_iter_seek_linear(struct table_iter *ti,
349 * the wanted key inside of it. If the block does not contain our key
350 * we know that the corresponding record does not exist.
351 */
352 - err = block_iter_seek_key(&ti->bi, &ti->block, &want_key);
352 + block_iter_init(&ti->bi, &ti->block);
353 + err = block_iter_seek_key(&ti->bi, &want_key);
354 if (err < 0)
355 goto done;
356 err = 0;
@@ -417,7 +418,9 @@ static int table_iter_seek_indexed(struct table_iter *ti,
418 if (err != 0)
419 goto done;
420
420 - err = block_iter_seek_key(&ti->bi, &ti->block, &want_index.u.idx.last_key);
421 + block_iter_init(&ti->bi, &ti->block);
422 +
423 + err = block_iter_seek_key(&ti->bi, &want_index.u.idx.last_key);
424 if (err < 0)
425 goto done;
426
t/unit-tests/t-reftable-block.c
+11 -15
@@ -66,7 +66,7 @@ static void t_ref_block_read_write(void)
66 block_source_from_buf(&source ,&block_data);
67 reftable_block_init(&block, &source, 0, header_off, block_size, REFTABLE_HASH_SIZE_SHA1);
68
69 - block_iter_seek_start(&it, &block);
69 + block_iter_init(&it, &block);
70
71 for (i = 0; ; i++) {
72 ret = block_iter_next(&it, &rec);
@@ -79,10 +79,9 @@ static void t_ref_block_read_write(void)
79 }
80
81 for (i = 0; i < N; i++) {
82 - block_iter_reset(&it);
82 reftable_record_key(&recs[i], &want);
83
85 - ret = block_iter_seek_key(&it, &block, &want);
84 + ret = block_iter_seek_key(&it, &want);
85 check_int(ret, ==, 0);
86
87 ret = block_iter_next(&it, &rec);
@@ -91,7 +90,7 @@ static void t_ref_block_read_write(void)
90 check(reftable_record_equal(&recs[i], &rec, REFTABLE_HASH_SIZE_SHA1));
91
92 want.len--;
94 - ret = block_iter_seek_key(&it, &block, &want);
93 + ret = block_iter_seek_key(&it, &want);
94 check_int(ret, ==, 0);
95
96 ret = block_iter_next(&it, &rec);
@@ -156,7 +155,7 @@ static void t_log_block_read_write(void)
155 block_source_from_buf(&source, &block_data);
156 reftable_block_init(&block, &source, 0, header_off, block_size, REFTABLE_HASH_SIZE_SHA1);
157
159 - block_iter_seek_start(&it, &block);
158 + block_iter_init(&it, &block);
159
160 for (i = 0; ; i++) {
161 ret = block_iter_next(&it, &rec);
@@ -169,11 +168,10 @@ static void t_log_block_read_write(void)
168 }
169
170 for (i = 0; i < N; i++) {
172 - block_iter_reset(&it);
171 reftable_buf_reset(&want);
172 check(!reftable_buf_addstr(&want, recs[i].u.log.refname));
173
176 - ret = block_iter_seek_key(&it, &block, &want);
174 + ret = block_iter_seek_key(&it, &want);
175 check_int(ret, ==, 0);
176
177 ret = block_iter_next(&it, &rec);
@@ -182,7 +180,7 @@ static void t_log_block_read_write(void)
180 check(reftable_record_equal(&recs[i], &rec, REFTABLE_HASH_SIZE_SHA1));
181
182 want.len--;
185 - ret = block_iter_seek_key(&it, &block, &want);
183 + ret = block_iter_seek_key(&it, &want);
184 check_int(ret, ==, 0);
185
186 ret = block_iter_next(&it, &rec);
@@ -249,7 +247,7 @@ static void t_obj_block_read_write(void)
247 block_source_from_buf(&source, &block_data);
248 reftable_block_init(&block, &source, 0, header_off, block_size, REFTABLE_HASH_SIZE_SHA1);
249
252 - block_iter_seek_start(&it, &block);
250 + block_iter_init(&it, &block);
251
252 for (i = 0; ; i++) {
253 ret = block_iter_next(&it, &rec);
@@ -262,10 +260,9 @@ static void t_obj_block_read_write(void)
260 }
261
262 for (i = 0; i < N; i++) {
265 - block_iter_reset(&it);
263 reftable_record_key(&recs[i], &want);
264
268 - ret = block_iter_seek_key(&it, &block, &want);
265 + ret = block_iter_seek_key(&it, &want);
266 check_int(ret, ==, 0);
267
268 ret = block_iter_next(&it, &rec);
@@ -334,7 +331,7 @@ static void t_index_block_read_write(void)
331 block_source_from_buf(&source, &block_data);
332 reftable_block_init(&block, &source, 0, header_off, block_size, REFTABLE_HASH_SIZE_SHA1);
333
337 - block_iter_seek_start(&it, &block);
334 + block_iter_init(&it, &block);
335
336 for (i = 0; ; i++) {
337 ret = block_iter_next(&it, &rec);
@@ -347,10 +344,9 @@ static void t_index_block_read_write(void)
344 }
345
346 for (i = 0; i < N; i++) {
350 - block_iter_reset(&it);
347 reftable_record_key(&recs[i], &want);
348
353 - ret = block_iter_seek_key(&it, &block, &want);
349 + ret = block_iter_seek_key(&it, &want);
350 check_int(ret, ==, 0);
351
352 ret = block_iter_next(&it, &rec);
@@ -359,7 +355,7 @@ static void t_index_block_read_write(void)
355 check(reftable_record_equal(&recs[i], &rec, REFTABLE_HASH_SIZE_SHA1));
356
357 want.len--;
362 - ret = block_iter_seek_key(&it, &block, &want);
358 + ret = block_iter_seek_key(&it, &want);
359 check_int(ret, ==, 0);
360
361 ret = block_iter_next(&it, &rec);