Skip to content

Commit 66c0dab

Browse files
hanwengitster
hanwen
authored and
gitster
committed
reftable: make reftable_record a tagged union
This reduces the amount of glue code, because we don't need a void pointer or vtable within the structure. The only snag is that reftable_index_record contain a strbuf, so it cannot be zero-initialized. To address this, use reftable_new_record() to return fresh instance, given a record type. Since reftable_new_record() doesn't cause heap allocation anymore, it should be balanced with reftable_record_release() rather than reftable_record_destroy(). Thanks to Peff for the suggestion. Helped-by: Jeff King <peff@peff.net> Signed-off-by: Han-Wen Nienhuys <hanwen@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
1 parent 9391b88 commit 66c0dab

File tree

12 files changed

+334
-337
lines changed

12 files changed

+334
-337
lines changed

‎reftable/block.c‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -421,7 +421,7 @@ int block_reader_seek(struct block_reader *br, struct block_iter *it,
421421
done:
422422
strbuf_release(&key);
423423
strbuf_release(&next.last_key);
424-
reftable_record_destroy(&rec);
424+
reftable_record_release(&rec);
425425

426426
return err;
427427
}

‎reftable/block_test.c‎

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -26,8 +26,9 @@ static void test_block_read_write(void)
2626
struct block_writer bw = {
2727
.last_key = STRBUF_INIT,
2828
};
29-
struct reftable_ref_record ref = { NULL };
30-
struct reftable_record rec = { NULL };
29+
struct reftable_record rec = {
30+
.type = BLOCK_TYPE_REF,
31+
};
3132
int i = 0;
3233
int n;
3334
struct block_reader br = { 0 };
@@ -40,22 +41,21 @@ static void test_block_read_write(void)
4041
block.source = malloc_block_source();
4142
block_writer_init(&bw, BLOCK_TYPE_REF, block.data, block_size,
4243
header_off, hash_size(GIT_SHA1_FORMAT_ID));
43-
reftable_record_from_ref(&rec, &ref);
4444

4545
for (i = 0; i < N; i++) {
4646
char name[100];
4747
uint8_t hash[GIT_SHA1_RAWSZ];
4848
snprintf(name, sizeof(name), "branch%02d", i);
4949
memset(hash, i, sizeof(hash));
5050

51-
ref.refname = name;
52-
ref.value_type = REFTABLE_REF_VAL1;
53-
ref.value.val1 = hash;
51+
rec.u.ref.refname = name;
52+
rec.u.ref.value_type = REFTABLE_REF_VAL1;
53+
rec.u.ref.value.val1 = hash;
5454

5555
names[i] = xstrdup(name);
5656
n = block_writer_add(&bw, &rec);
57-
ref.refname = NULL;
58-
ref.value_type = REFTABLE_REF_DELETION;
57+
rec.u.ref.refname = NULL;
58+
rec.u.ref.value_type = REFTABLE_REF_DELETION;
5959
EXPECT(n == 0);
6060
}
6161

@@ -74,7 +74,7 @@ static void test_block_read_write(void)
7474
if (r > 0) {
7575
break;
7676
}
77-
EXPECT_STREQ(names[j], ref.refname);
77+
EXPECT_STREQ(names[j], rec.u.ref.refname);
7878
j++;
7979
}
8080

@@ -92,15 +92,15 @@ static void test_block_read_write(void)
9292
n = block_iter_next(&it, &rec);
9393
EXPECT(n == 0);
9494

95-
EXPECT_STREQ(names[i], ref.refname);
95+
EXPECT_STREQ(names[i], rec.u.ref.refname);
9696

9797
want.len--;
9898
n = block_reader_seek(&br, &it, &want);
9999
EXPECT(n == 0);
100100

101101
n = block_iter_next(&it, &rec);
102102
EXPECT(n == 0);
103-
EXPECT_STREQ(names[10 * (i / 10)], ref.refname);
103+
EXPECT_STREQ(names[10 * (i / 10)], rec.u.ref.refname);
104104

105105
block_iter_close(&it);
106106
}

‎reftable/generic.c‎

Lines changed: 24 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ license that can be found in the LICENSE file or at
77
*/
88

99
#include "basics.h"
10+
#include "constants.h"
1011
#include "record.h"
1112
#include "generic.h"
1213
#include "reftable-iterator.h"
@@ -15,23 +16,21 @@ license that can be found in the LICENSE file or at
1516
int reftable_table_seek_ref(struct reftable_table *tab,
1617
struct reftable_iterator *it, const char *name)
1718
{
18-
struct reftable_ref_record ref = {
19-
.refname = (char *)name,
20-
};
21-
struct reftable_record rec = { NULL };
22-
reftable_record_from_ref(&rec, &ref);
19+
struct reftable_record rec = { .type = BLOCK_TYPE_REF,
20+
.u.ref = {
21+
.refname = (char *)name,
22+
} };
2323
return tab->ops->seek_record(tab->table_arg, it, &rec);
2424
}
2525

2626
int reftable_table_seek_log(struct reftable_table *tab,
2727
struct reftable_iterator *it, const char *name)
2828
{
29-
struct reftable_log_record log = {
30-
.refname = (char *)name,
31-
.update_index = ~((uint64_t)0),
32-
};
33-
struct reftable_record rec = { NULL };
34-
reftable_record_from_log(&rec, &log);
29+
struct reftable_record rec = { .type = BLOCK_TYPE_LOG,
30+
.u.log = {
31+
.refname = (char *)name,
32+
.update_index = ~((uint64_t)0),
33+
} };
3534
return tab->ops->seek_record(tab->table_arg, it, &rec);
3635
}
3736

@@ -129,17 +128,25 @@ void reftable_iterator_destroy(struct reftable_iterator *it)
129128
int reftable_iterator_next_ref(struct reftable_iterator *it,
130129
struct reftable_ref_record *ref)
131130
{
132-
struct reftable_record rec = { NULL };
133-
reftable_record_from_ref(&rec, ref);
134-
return iterator_next(it, &rec);
131+
struct reftable_record rec = {
132+
.type = BLOCK_TYPE_REF,
133+
.u.ref = *ref,
134+
};
135+
int err = iterator_next(it, &rec);
136+
*ref = rec.u.ref;
137+
return err;
135138
}
136139

137140
int reftable_iterator_next_log(struct reftable_iterator *it,
138141
struct reftable_log_record *log)
139142
{
140-
struct reftable_record rec = { NULL };
141-
reftable_record_from_log(&rec, log);
142-
return iterator_next(it, &rec);
143+
struct reftable_record rec = {
144+
.type = BLOCK_TYPE_LOG,
145+
.u.log = *log,
146+
};
147+
int err = iterator_next(it, &rec);
148+
*log = rec.u.log;
149+
return err;
143150
}
144151

145152
int iterator_next(struct reftable_iterator *it, struct reftable_record *rec)

‎reftable/iter.c‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ static int filtering_ref_iterator_next(void *iter_arg,
3232
struct reftable_record *rec)
3333
{
3434
struct filtering_ref_iterator *fri = iter_arg;
35-
struct reftable_ref_record *ref = rec->data;
35+
struct reftable_ref_record *ref = &rec->u.ref;
3636
int err = 0;
3737
while (1) {
3838
err = reftable_iterator_next_ref(&fri->it, ref);
@@ -127,7 +127,7 @@ static int indexed_table_ref_iter_next_block(struct indexed_table_ref_iter *it)
127127
static int indexed_table_ref_iter_next(void *p, struct reftable_record *rec)
128128
{
129129
struct indexed_table_ref_iter *it = p;
130-
struct reftable_ref_record *ref = rec->data;
130+
struct reftable_ref_record *ref = &rec->u.ref;
131131

132132
while (1) {
133133
int err = block_iter_next(&it->cur, rec);

‎reftable/merged.c‎

Lines changed: 16 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ static int merged_iter_init(struct merged_iter *mi)
3030

3131
if (err > 0) {
3232
reftable_iterator_destroy(&mi->stack[i]);
33-
reftable_record_destroy(&rec);
33+
reftable_record_release(&rec);
3434
} else {
3535
struct pq_entry e = {
3636
.rec = rec,
@@ -57,18 +57,17 @@ static void merged_iter_close(void *p)
5757
static int merged_iter_advance_nonnull_subiter(struct merged_iter *mi,
5858
size_t idx)
5959
{
60-
struct reftable_record rec = reftable_new_record(mi->typ);
6160
struct pq_entry e = {
62-
.rec = rec,
61+
.rec = reftable_new_record(mi->typ),
6362
.index = idx,
6463
};
65-
int err = iterator_next(&mi->stack[idx], &rec);
64+
int err = iterator_next(&mi->stack[idx], &e.rec);
6665
if (err < 0)
6766
return err;
6867

6968
if (err > 0) {
7069
reftable_iterator_destroy(&mi->stack[idx]);
71-
reftable_record_destroy(&rec);
70+
reftable_record_release(&e.rec);
7271
return 0;
7372
}
7473

@@ -126,11 +125,11 @@ static int merged_iter_next_entry(struct merged_iter *mi,
126125
if (err < 0) {
127126
return err;
128127
}
129-
reftable_record_destroy(&top.rec);
128+
reftable_record_release(&top.rec);
130129
}
131130

132131
reftable_record_copy_from(rec, &entry.rec, hash_size(mi->hash_id));
133-
reftable_record_destroy(&entry.rec);
132+
reftable_record_release(&entry.rec);
134133
strbuf_release(&entry_key);
135134
return 0;
136135
}
@@ -290,24 +289,24 @@ int reftable_merged_table_seek_ref(struct reftable_merged_table *mt,
290289
struct reftable_iterator *it,
291290
const char *name)
292291
{
293-
struct reftable_ref_record ref = {
294-
.refname = (char *)name,
292+
struct reftable_record rec = {
293+
.type = BLOCK_TYPE_REF,
294+
.u.ref = {
295+
.refname = (char *)name,
296+
},
295297
};
296-
struct reftable_record rec = { NULL };
297-
reftable_record_from_ref(&rec, &ref);
298298
return merged_table_seek_record(mt, it, &rec);
299299
}
300300

301301
int reftable_merged_table_seek_log_at(struct reftable_merged_table *mt,
302302
struct reftable_iterator *it,
303303
const char *name, uint64_t update_index)
304304
{
305-
struct reftable_log_record log = {
306-
.refname = (char *)name,
307-
.update_index = update_index,
308-
};
309-
struct reftable_record rec = { NULL };
310-
reftable_record_from_log(&rec, &log);
305+
struct reftable_record rec = { .type = BLOCK_TYPE_LOG,
306+
.u.log = {
307+
.refname = (char *)name,
308+
.update_index = update_index,
309+
} };
311310
return merged_table_seek_record(mt, it, &rec);
312311
}
313312

‎reftable/pq.c‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -74,6 +74,7 @@ struct pq_entry merged_iter_pqueue_remove(struct merged_iter_pqueue *pq)
7474
void merged_iter_pqueue_add(struct merged_iter_pqueue *pq, struct pq_entry e)
7575
{
7676
int i = 0;
77+
7778
if (pq->len == pq->cap) {
7879
pq->cap = 2 * pq->cap + 1;
7980
pq->heap = reftable_realloc(pq->heap,
@@ -98,7 +99,7 @@ void merged_iter_pqueue_release(struct merged_iter_pqueue *pq)
9899
{
99100
int i = 0;
100101
for (i = 0; i < pq->len; i++) {
101-
reftable_record_destroy(&pq->heap[i].rec);
102+
reftable_record_release(&pq->heap[i].rec);
102103
}
103104
FREE_AND_NULL(pq->heap);
104105
pq->len = pq->cap = 0;

‎reftable/pq_test.c‎

Lines changed: 12 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ static void test_pq(void)
3131
int N = ARRAY_SIZE(names) - 1;
3232

3333
struct merged_iter_pqueue pq = { NULL };
34-
const char *last = NULL;
34+
char *last = NULL;
3535

3636
int i = 0;
3737
for (i = 0; i < N; i++) {
@@ -42,32 +42,29 @@ static void test_pq(void)
4242

4343
i = 1;
4444
do {
45-
struct reftable_record rec =
46-
reftable_new_record(BLOCK_TYPE_REF);
47-
struct pq_entry e = { 0 };
48-
49-
reftable_record_as_ref(&rec)->refname = names[i];
50-
e.rec = rec;
45+
struct pq_entry e = { .rec = { .type = BLOCK_TYPE_REF,
46+
.u.ref = {
47+
.refname = names[i],
48+
} } };
5149
merged_iter_pqueue_add(&pq, e);
5250
merged_iter_pqueue_check(pq);
5351
i = (i * 7) % N;
5452
} while (i != 1);
5553

5654
while (!merged_iter_pqueue_is_empty(pq)) {
5755
struct pq_entry e = merged_iter_pqueue_remove(&pq);
58-
struct reftable_ref_record *ref =
59-
reftable_record_as_ref(&e.rec);
60-
56+
struct reftable_record *rec = &e.rec;
6157
merged_iter_pqueue_check(pq);
6258

59+
EXPECT(reftable_record_type(rec) == BLOCK_TYPE_REF);
6360
if (last) {
64-
EXPECT(strcmp(last, ref->refname) < 0);
61+
EXPECT(strcmp(last, rec->u.ref.refname) < 0);
6562
}
66-
last = ref->refname;
67-
ref->refname = NULL;
68-
reftable_free(ref);
63+
// this is names[i], so don't dealloc.
64+
last = rec->u.ref.refname;
65+
rec->u.ref.refname = NULL;
66+
reftable_record_release(rec);
6967
}
70-
7168
for (i = 0; i < N; i++) {
7269
reftable_free(names[i]);
7370
}

0 commit comments

Comments
 (0)