reftable/generic: move seeking of records into the iterator
Reftable iterators are created by seeking on the parent structure of a corresponding record. For example, to create an iterator for the merged table you would call `reftable_merged_table_seek_ref()`. Most notably, it is not posible to create an iterator and then seek it afterwards. While this may be a bit easier to reason about, it comes with two significant downsides. The first downside is that the logic to find records is split up between the parent data structure and the iterator itself. Conceptually, it is more straight forward if all that logic was contained in a single place, which should be the iterator. The second and more significant downside is that it is impossible to reuse iterators for multiple seeks. Whenever you want to look up a record, you need to re-create the whole infrastructure again, which is quite a waste of time. Furthermore, it is impossible to optimize seeks, such as when seeking the same record multiple times. To address this, we essentially split up the concerns properly such that the parent data structure is responsible for setting up the iterator via a new `init_iter()` callback, whereas the iterator handles seeks via a new `seek()` callback. This will eventually allow us to call `seek()` on the iterator multiple times, where every iterator can potentially optimize for certain cases. Note that at this point in time we are not yet ready to reuse the iterators. This will be left for a future patch series. 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
5bf96e0c393827481afab55d3b73ae09ec1ea0de
5 files changed
+177
-117
reftable/generic.c
+36
-11
@@ -12,25 +12,39 @@ https://developers.google.com/open-source/licenses/bsd
12
#include "reftable-iterator.h"
13
#include "reftable-generic.h"
14
15
+void table_init_iter(struct reftable_table *tab,
16
+ struct reftable_iterator *it,
17
+ uint8_t typ)
18
+{
19
+
20
+ tab->ops->init_iter(tab->table_arg, it, typ);
21
+}
22
+
23
int reftable_table_seek_ref(struct reftable_table *tab,
24
struct reftable_iterator *it, const char *name)
25
{
18
- struct reftable_record rec = { .type = BLOCK_TYPE_REF,
19
- .u.ref = {
20
- .refname = (char *)name,
21
- } };
22
- return tab->ops->seek_record(tab->table_arg, it, &rec);
26
+ struct reftable_record want = {
27
+ .type = BLOCK_TYPE_REF,
28
+ .u.ref = {
29
+ .refname = (char *)name,
30
+ },
31
+ };
32
+ table_init_iter(tab, it, BLOCK_TYPE_REF);
33
+ return it->ops->seek(it->iter_arg, &want);
34
}
35
36
int reftable_table_seek_log(struct reftable_table *tab,
37
struct reftable_iterator *it, const char *name)
38
{
28
- struct reftable_record rec = { .type = BLOCK_TYPE_LOG,
29
- .u.log = {
30
- .refname = (char *)name,
31
- .update_index = ~((uint64_t)0),
32
- } };
33
- return tab->ops->seek_record(tab->table_arg, it, &rec);
39
+ struct reftable_record want = {
40
+ .type = BLOCK_TYPE_LOG,
41
+ .u.log = {
42
+ .refname = (char *)name,
43
+ .update_index = ~((uint64_t)0),
44
+ },
45
+ };
46
+ table_init_iter(tab, it, BLOCK_TYPE_LOG);
47
+ return it->ops->seek(it->iter_arg, &want);
48
}
49
50
int reftable_table_read_ref(struct reftable_table *tab, const char *name,
@@ -152,11 +166,21 @@ int reftable_iterator_next_log(struct reftable_iterator *it,
166
return err;
167
}
168
169
+int iterator_seek(struct reftable_iterator *it, struct reftable_record *want)
170
+{
171
+ return it->ops->seek(it->iter_arg, want);
172
+}
173
+
174
int iterator_next(struct reftable_iterator *it, struct reftable_record *rec)
175
{
176
return it->ops->next(it->iter_arg, rec);
177
}
178
179
+static int empty_iterator_seek(void *arg, struct reftable_record *want)
180
+{
181
+ return 0;
182
+}
183
+
184
static int empty_iterator_next(void *arg, struct reftable_record *rec)
185
{
186
return 1;
@@ -167,6 +191,7 @@ static void empty_iterator_close(void *arg)
191
}
192
193
static struct reftable_iterator_vtable empty_vtable = {
194
+ .seek = &empty_iterator_seek,
195
.next = &empty_iterator_next,
196
.close = &empty_iterator_close,
197
};
reftable/generic.h
+7
-2
@@ -14,19 +14,24 @@ https://developers.google.com/open-source/licenses/bsd
14
15
/* generic interface to reftables */
16
struct reftable_table_vtable {
17
- int (*seek_record)(void *tab, struct reftable_iterator *it,
18
- struct reftable_record *);
17
+ void (*init_iter)(void *tab, struct reftable_iterator *it, uint8_t typ);
18
uint32_t (*hash_id)(void *tab);
19
uint64_t (*min_update_index)(void *tab);
20
uint64_t (*max_update_index)(void *tab);
21
};
22
23
+void table_init_iter(struct reftable_table *tab,
24
+ struct reftable_iterator *it,
25
+ uint8_t typ);
26
+
27
struct reftable_iterator_vtable {
28
+ int (*seek)(void *iter_arg, struct reftable_record *want);
29
int (*next)(void *iter_arg, struct reftable_record *rec);
30
void (*close)(void *iter_arg);
31
};
32
33
void iterator_set_empty(struct reftable_iterator *it);
34
+int iterator_seek(struct reftable_iterator *it, struct reftable_record *want);
35
int iterator_next(struct reftable_iterator *it, struct reftable_record *rec);
36
37
#endif
reftable/iter.c
+15
@@ -23,6 +23,13 @@ static void filtering_ref_iterator_close(void *iter_arg)
23
reftable_iterator_destroy(&fri->it);
24
}
25
26
+static int filtering_ref_iterator_seek(void *iter_arg,
27
+ struct reftable_record *want)
28
+{
29
+ struct filtering_ref_iterator *fri = iter_arg;
30
+ return iterator_seek(&fri->it, want);
31
+}
32
+
33
static int filtering_ref_iterator_next(void *iter_arg,
34
struct reftable_record *rec)
35
{
@@ -73,6 +80,7 @@ static int filtering_ref_iterator_next(void *iter_arg,
80
}
81
82
static struct reftable_iterator_vtable filtering_ref_iterator_vtable = {
83
+ .seek = &filtering_ref_iterator_seek,
84
.next = &filtering_ref_iterator_next,
85
.close = &filtering_ref_iterator_close,
86
};
@@ -119,6 +127,12 @@ static int indexed_table_ref_iter_next_block(struct indexed_table_ref_iter *it)
127
return 0;
128
}
129
130
+static int indexed_table_ref_iter_seek(void *p, struct reftable_record *want)
131
+{
132
+ BUG("seeking indexed table is not supported");
133
+ return -1;
134
+}
135
+
136
static int indexed_table_ref_iter_next(void *p, struct reftable_record *rec)
137
{
138
struct indexed_table_ref_iter *it = p;
@@ -175,6 +189,7 @@ int new_indexed_table_ref_iter(struct indexed_table_ref_iter **dest,
189
}
190
191
static struct reftable_iterator_vtable indexed_table_ref_iter_vtable = {
192
+ .seek = &indexed_table_ref_iter_seek,
193
.next = &indexed_table_ref_iter_next,
194
.close = &indexed_table_ref_iter_close,
195
};
reftable/merged.c
+49
-48
@@ -31,12 +31,18 @@ struct merged_iter {
31
};
32
33
static void merged_iter_init(struct merged_iter *mi,
34
- struct reftable_merged_table *mt)
34
+ struct reftable_merged_table *mt,
35
+ uint8_t typ)
36
{
37
memset(mi, 0, sizeof(*mi));
38
mi->advance_index = -1;
39
mi->suppress_deletions = mt->suppress_deletions;
40
+
41
REFTABLE_CALLOC_ARRAY(mi->subiters, mt->stack_len);
42
+ for (size_t i = 0; i < mt->stack_len; i++) {
43
+ reftable_record_init(&mi->subiters[i].rec, typ);
44
+ table_init_iter(&mt->stack[i], &mi->subiters[i].iter, typ);
45
+ }
46
mi->stack_len = mt->stack_len;
47
}
48
@@ -68,6 +74,27 @@ static int merged_iter_advance_subiter(struct merged_iter *mi, size_t idx)
74
return 0;
75
}
76
77
+static int merged_iter_seek(struct merged_iter *mi, struct reftable_record *want)
78
+{
79
+ int err;
80
+
81
+ mi->advance_index = -1;
82
+
83
+ for (size_t i = 0; i < mi->stack_len; i++) {
84
+ err = iterator_seek(&mi->subiters[i].iter, want);
85
+ if (err < 0)
86
+ return err;
87
+ if (err > 0)
88
+ continue;
89
+
90
+ err = merged_iter_advance_subiter(mi, i);
91
+ if (err < 0)
92
+ return err;
93
+ }
94
+
95
+ return 0;
96
+}
97
+
98
static int merged_iter_next_entry(struct merged_iter *mi,
99
struct reftable_record *rec)
100
{
@@ -133,6 +160,11 @@ static int merged_iter_next_entry(struct merged_iter *mi,
160
return 0;
161
}
162
163
+static int merged_iter_seek_void(void *it, struct reftable_record *want)
164
+{
165
+ return merged_iter_seek(it, want);
166
+}
167
+
168
static int merged_iter_next_void(void *p, struct reftable_record *rec)
169
{
170
struct merged_iter *mi = p;
@@ -147,6 +179,7 @@ static int merged_iter_next_void(void *p, struct reftable_record *rec)
179
}
180
181
static struct reftable_iterator_vtable merged_iter_vtable = {
182
+ .seek = merged_iter_seek_void,
183
.next = &merged_iter_next_void,
184
.close = &merged_iter_close,
185
};
@@ -220,47 +253,13 @@ reftable_merged_table_min_update_index(struct reftable_merged_table *mt)
253
return mt->min;
254
}
255
223
-static int reftable_table_seek_record(struct reftable_table *tab,
224
- struct reftable_iterator *it,
225
- struct reftable_record *rec)
226
-{
227
- return tab->ops->seek_record(tab->table_arg, it, rec);
228
-}
229
-
230
-static int merged_table_seek_record(struct reftable_merged_table *mt,
231
- struct reftable_iterator *it,
232
- struct reftable_record *rec)
256
+static void merged_table_init_iter(struct reftable_merged_table *mt,
257
+ struct reftable_iterator *it,
258
+ uint8_t typ)
259
{
234
- struct merged_iter merged, *p;
235
- int err;
236
-
237
- merged_iter_init(&merged, mt);
238
-
239
- for (size_t i = 0; i < mt->stack_len; i++) {
240
- reftable_record_init(&merged.subiters[i].rec,
241
- reftable_record_type(rec));
242
-
243
- err = reftable_table_seek_record(&mt->stack[i],
244
- &merged.subiters[i].iter, rec);
245
- if (err < 0)
246
- goto out;
247
- if (err > 0)
248
- continue;
249
-
250
- err = merged_iter_advance_subiter(&merged, i);
251
- if (err < 0)
252
- goto out;
253
- }
254
-
255
- p = reftable_malloc(sizeof(*p));
256
- *p = merged;
257
- iterator_from_merged_iter(it, p);
258
- err = 0;
259
-
260
-out:
261
- if (err < 0)
262
- merged_iter_close(&merged);
263
- return err;
260
+ struct merged_iter *mi = reftable_malloc(sizeof(*mi));
261
+ merged_iter_init(mi, mt, typ);
262
+ iterator_from_merged_iter(it, mi);
263
}
264
265
int reftable_merged_table_seek_ref(struct reftable_merged_table *mt,
@@ -273,7 +272,8 @@ int reftable_merged_table_seek_ref(struct reftable_merged_table *mt,
272
.refname = (char *)name,
273
},
274
};
276
- return merged_table_seek_record(mt, it, &rec);
275
+ merged_table_init_iter(mt, it, BLOCK_TYPE_REF);
276
+ return iterator_seek(it, &rec);
277
}
278
279
int reftable_merged_table_seek_log_at(struct reftable_merged_table *mt,
@@ -285,7 +285,8 @@ int reftable_merged_table_seek_log_at(struct reftable_merged_table *mt,
285
.refname = (char *)name,
286
.update_index = update_index,
287
} };
288
- return merged_table_seek_record(mt, it, &rec);
288
+ merged_table_init_iter(mt, it, BLOCK_TYPE_LOG);
289
+ return iterator_seek(it, &rec);
290
}
291
292
int reftable_merged_table_seek_log(struct reftable_merged_table *mt,
@@ -301,11 +302,11 @@ uint32_t reftable_merged_table_hash_id(struct reftable_merged_table *mt)
302
return mt->hash_id;
303
}
304
304
-static int reftable_merged_table_seek_void(void *tab,
305
- struct reftable_iterator *it,
306
- struct reftable_record *rec)
305
+static void reftable_merged_table_init_iter_void(void *tab,
306
+ struct reftable_iterator *it,
307
+ uint8_t typ)
308
{
308
- return merged_table_seek_record(tab, it, rec);
309
+ merged_table_init_iter(tab, it, typ);
310
}
311
312
static uint32_t reftable_merged_table_hash_id_void(void *tab)
@@ -324,7 +325,7 @@ static uint64_t reftable_merged_table_max_update_index_void(void *tab)
325
}
326
327
static struct reftable_table_vtable merged_table_vtable = {
327
- .seek_record = reftable_merged_table_seek_void,
328
+ .init_iter = reftable_merged_table_init_iter_void,
329
.hash_id = reftable_merged_table_hash_id_void,
330
.min_update_index = reftable_merged_table_min_update_index_void,
331
.max_update_index = reftable_merged_table_max_update_index_void,
reftable/reader.c
+70
-56
@@ -369,29 +369,6 @@ static int table_iter_next(struct table_iter *ti, struct reftable_record *rec)
369
}
370
}
371
372
-static int table_iter_next_void(void *ti, struct reftable_record *rec)
373
-{
374
- return table_iter_next(ti, rec);
375
-}
376
-
377
-static void table_iter_close_void(void *ti)
378
-{
379
- table_iter_close(ti);
380
-}
381
-
382
-static struct reftable_iterator_vtable table_iter_vtable = {
383
- .next = &table_iter_next_void,
384
- .close = &table_iter_close_void,
385
-};
386
-
387
-static void iterator_from_table_iter(struct reftable_iterator *it,
388
- struct table_iter *ti)
389
-{
390
- assert(!it->ops);
391
- it->iter_arg = ti;
392
- it->ops = &table_iter_vtable;
393
-}
394
-
372
static int table_iter_seek_to(struct table_iter *ti, uint64_t off, uint8_t typ)
373
{
374
int err;
@@ -576,43 +553,74 @@ done:
553
return err;
554
}
555
579
-static int reader_seek(struct reftable_reader *r, struct reftable_iterator *it,
580
- struct reftable_record *rec)
556
+static int table_iter_seek(struct table_iter *ti,
557
+ struct reftable_record *want)
558
{
582
- uint8_t typ = reftable_record_type(rec);
583
- struct reftable_reader_offsets *offs = reader_offsets_for(r, typ);
584
- struct table_iter ti, *p;
559
+ uint8_t typ = reftable_record_type(want);
560
+ struct reftable_reader_offsets *offs = reader_offsets_for(ti->r, typ);
561
int err;
562
587
- if (!offs->is_present) {
588
- iterator_set_empty(it);
589
- return 0;
590
- }
591
-
592
- table_iter_init(&ti, r);
593
-
594
- err = table_iter_seek_start(&ti, reftable_record_type(rec),
563
+ err = table_iter_seek_start(ti, reftable_record_type(want),
564
!!offs->index_offset);
565
if (err < 0)
566
goto out;
567
568
if (offs->index_offset)
600
- err = table_iter_seek_indexed(&ti, rec);
569
+ err = table_iter_seek_indexed(ti, want);
570
else
602
- err = table_iter_seek_linear(&ti, rec);
571
+ err = table_iter_seek_linear(ti, want);
572
if (err)
573
goto out;
574
606
- REFTABLE_ALLOC_ARRAY(p, 1);
607
- *p = ti;
608
- iterator_from_table_iter(it, p);
609
-
575
out:
611
- if (err)
612
- table_iter_close(&ti);
576
return err;
577
}
578
579
+static int table_iter_seek_void(void *ti, struct reftable_record *want)
580
+{
581
+ return table_iter_seek(ti, want);
582
+}
583
+
584
+static int table_iter_next_void(void *ti, struct reftable_record *rec)
585
+{
586
+ return table_iter_next(ti, rec);
587
+}
588
+
589
+static void table_iter_close_void(void *ti)
590
+{
591
+ table_iter_close(ti);
592
+}
593
+
594
+static struct reftable_iterator_vtable table_iter_vtable = {
595
+ .seek = &table_iter_seek_void,
596
+ .next = &table_iter_next_void,
597
+ .close = &table_iter_close_void,
598
+};
599
+
600
+static void iterator_from_table_iter(struct reftable_iterator *it,
601
+ struct table_iter *ti)
602
+{
603
+ assert(!it->ops);
604
+ it->iter_arg = ti;
605
+ it->ops = &table_iter_vtable;
606
+}
607
+
608
+static void reader_init_iter(struct reftable_reader *r,
609
+ struct reftable_iterator *it,
610
+ uint8_t typ)
611
+{
612
+ struct reftable_reader_offsets *offs = reader_offsets_for(r, typ);
613
+
614
+ if (offs->is_present) {
615
+ struct table_iter *ti;
616
+ REFTABLE_ALLOC_ARRAY(ti, 1);
617
+ table_iter_init(ti, r);
618
+ iterator_from_table_iter(it, ti);
619
+ } else {
620
+ iterator_set_empty(it);
621
+ }
622
+}
623
+
624
int reftable_reader_seek_ref(struct reftable_reader *r,
625
struct reftable_iterator *it, const char *name)
626
{
@@ -622,19 +630,23 @@ int reftable_reader_seek_ref(struct reftable_reader *r,
630
.refname = (char *)name,
631
},
632
};
625
- return reader_seek(r, it, &rec);
633
+ reader_init_iter(r, it, BLOCK_TYPE_REF);
634
+ return iterator_seek(it, &rec);
635
}
636
637
int reftable_reader_seek_log_at(struct reftable_reader *r,
638
struct reftable_iterator *it, const char *name,
639
uint64_t update_index)
640
{
632
- struct reftable_record rec = { .type = BLOCK_TYPE_LOG,
633
- .u.log = {
634
- .refname = (char *)name,
635
- .update_index = update_index,
636
- } };
637
- return reader_seek(r, it, &rec);
641
+ struct reftable_record rec = {
642
+ .type = BLOCK_TYPE_LOG,
643
+ .u.log = {
644
+ .refname = (char *)name,
645
+ .update_index = update_index,
646
+ },
647
+ };
648
+ reader_init_iter(r, it, BLOCK_TYPE_LOG);
649
+ return iterator_seek(it, &rec);
650
}
651
652
int reftable_reader_seek_log(struct reftable_reader *r,
@@ -692,7 +704,8 @@ static int reftable_reader_refs_for_indexed(struct reftable_reader *r,
704
struct indexed_table_ref_iter *itr = NULL;
705
706
/* Look through the reverse index. */
695
- err = reader_seek(r, &oit, &want);
707
+ reader_init_iter(r, &oit, BLOCK_TYPE_OBJ);
708
+ err = iterator_seek(&oit, &want);
709
if (err != 0)
710
goto done;
711
@@ -773,10 +786,11 @@ uint64_t reftable_reader_min_update_index(struct reftable_reader *r)
786
787
/* generic table interface. */
788
776
-static int reftable_reader_seek_void(void *tab, struct reftable_iterator *it,
777
- struct reftable_record *rec)
789
+static void reftable_reader_init_iter_void(void *tab,
790
+ struct reftable_iterator *it,
791
+ uint8_t typ)
792
{
779
- return reader_seek(tab, it, rec);
793
+ reader_init_iter(tab, it, typ);
794
}
795
796
static uint32_t reftable_reader_hash_id_void(void *tab)
@@ -795,7 +809,7 @@ static uint64_t reftable_reader_max_update_index_void(void *tab)
809
}
810
811
static struct reftable_table_vtable reader_vtable = {
798
- .seek_record = reftable_reader_seek_void,
812
+ .init_iter = reftable_reader_init_iter_void,
813
.hash_id = reftable_reader_hash_id_void,
814
.min_update_index = reftable_reader_min_update_index_void,
815
.max_update_index = reftable_reader_max_update_index_void,