reftable/merged: reuse buffer to compute record keys
When iterating over entries in the merged iterator's queue, we compute the key of each of the entries and write it into a buffer. We do not reuse the buffer though and thus re-allocate it on every iteration, which is wasteful given that we never transfer ownership of the allocated bytes outside of the loop. Refactor the code to reuse the buffer. This also fixes a potential memory leak when `merged_iter_advance_subiter()` returns an error. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Patrick Steinhardt committed
Dec 11, 2023 at 10:08 UTC
829231dc203f777a32ebdbbaf66e7661a21ac74a
2 files changed
+18
-15
reftable/merged.c
+16
-15
@@ -52,6 +52,8 @@ static void merged_iter_close(void *p)
52
reftable_iterator_destroy(&mi->stack[i]);
53
}
54
reftable_free(mi->stack);
55
+ strbuf_release(&mi->key);
56
+ strbuf_release(&mi->entry_key);
57
}
58
59
static int merged_iter_advance_nonnull_subiter(struct merged_iter *mi,
@@ -85,7 +87,6 @@ static int merged_iter_advance_subiter(struct merged_iter *mi, size_t idx)
87
static int merged_iter_next_entry(struct merged_iter *mi,
88
struct reftable_record *rec)
89
{
88
- struct strbuf entry_key = STRBUF_INIT;
90
struct pq_entry entry = { 0 };
91
int err = 0;
92
@@ -105,33 +106,31 @@ static int merged_iter_next_entry(struct merged_iter *mi,
106
such a deployment, the loop below must be changed to collect all
107
entries for the same key, and return new the newest one.
108
*/
108
- reftable_record_key(&entry.rec, &entry_key);
109
+ reftable_record_key(&entry.rec, &mi->entry_key);
110
while (!merged_iter_pqueue_is_empty(mi->pq)) {
111
struct pq_entry top = merged_iter_pqueue_top(mi->pq);
111
- struct strbuf k = STRBUF_INIT;
112
- int err = 0, cmp = 0;
112
+ int cmp = 0;
113
114
- reftable_record_key(&top.rec, &k);
114
+ reftable_record_key(&top.rec, &mi->key);
115
116
- cmp = strbuf_cmp(&k, &entry_key);
117
- strbuf_release(&k);
118
-
119
- if (cmp > 0) {
116
+ cmp = strbuf_cmp(&mi->key, &mi->entry_key);
117
+ if (cmp > 0)
118
break;
121
- }
119
120
merged_iter_pqueue_remove(&mi->pq);
121
err = merged_iter_advance_subiter(mi, top.index);
125
- if (err < 0) {
126
- return err;
127
- }
122
+ if (err < 0)
123
+ goto done;
124
reftable_record_release(&top.rec);
125
}
126
127
reftable_record_copy_from(rec, &entry.rec, hash_size(mi->hash_id));
128
+
129
+done:
130
reftable_record_release(&entry.rec);
133
- strbuf_release(&entry_key);
134
- return 0;
131
+ strbuf_release(&mi->entry_key);
132
+ strbuf_release(&mi->key);
133
+ return err;
134
}
135
136
static int merged_iter_next(struct merged_iter *mi, struct reftable_record *rec)
@@ -248,6 +247,8 @@ static int merged_table_seek_record(struct reftable_merged_table *mt,
247
.typ = reftable_record_type(rec),
248
.hash_id = mt->hash_id,
249
.suppress_deletions = mt->suppress_deletions,
250
+ .key = STRBUF_INIT,
251
+ .entry_key = STRBUF_INIT,
252
};
253
int n = 0;
254
int err = 0;
reftable/merged.h
+2
@@ -31,6 +31,8 @@ struct merged_iter {
31
uint8_t typ;
32
int suppress_deletions;
33
struct merged_iter_pqueue pq;
34
+ struct strbuf key;
35
+ struct strbuf entry_key;
36
};
37
38
void merged_table_release(struct reftable_merged_table *mt);