Skip to content

Commit a96e9a2

Browse files
pks-tgitster
authored andcommitted
reftable/merged: allocation-less dropping of shadowed records
The purpose of the merged reftable iterator is to iterate through all entries of a set of tables in the correct order. This is implemented by using a sub-iterator for each table, where the next entry of each of these iterators gets put into a priority queue. For each iteration, we do roughly the following steps: 1. Retrieve the top record of the priority queue. This is the entry we want to return to the caller. 2. Retrieve the next record of the sub-iterator that this record came from. If any, add it to the priority queue at the correct position. The position is determined by comparing the record keys, which e.g. corresponds to the refname for ref records. 3. Keep removing the top record of the priority queue until we hit the first entry whose key is larger than the returned record's key. This is required to drop "shadowed" records. The last step will lead to at least one comparison to the next entry, but may lead to many comparisons in case the reftable stack consists of many tables with shadowed records. It is thus part of the hot code path when iterating through records. The code to compare the entries with each other is quite inefficient though. Instead of comparing record keys with each other directly, we first format them into `struct strbuf`s and only then compare them with each other. While we already optimized this code path to reuse buffers in 829231d (reftable/merged: reuse buffer to compute record keys, 2023-12-11), the cost to format the keys into the buffers still adds up quite significantly. Refactor the code to use `reftable_record_cmp()` instead, which has been introduced in the preceding commit. This function compares records with each other directly without requiring any memory allocations or copying and is thus way more efficient. The following benchmark uses git-show-ref(1) to print a single ref matching a pattern out of 1 million refs. This is the most direct way to exercise ref iteration speed as we remove all overhead of having to show the refs, too. Benchmark 1: show-ref: single matching ref (revision = HEAD~) Time (mean ± σ): 180.7 ms ± 4.7 ms [User: 177.1 ms, System: 3.4 ms] Range (min … max): 174.9 ms … 211.7 ms 1000 runs Benchmark 2: show-ref: single matching ref (revision = HEAD) Time (mean ± σ): 162.1 ms ± 4.4 ms [User: 158.5 ms, System: 3.4 ms] Range (min … max): 155.4 ms … 189.3 ms 1000 runs Summary show-ref: single matching ref (revision = HEAD) ran 1.11 ± 0.04 times faster than show-ref: single matching ref (revision = HEAD~) Signed-off-by: Patrick Steinhardt <[email protected]> Signed-off-by: Junio C Hamano <[email protected]>
1 parent adb5d2c commit a96e9a2

File tree

2 files changed

+2
-11
lines changed

2 files changed

+2
-11
lines changed

reftable/merged.c

Lines changed: 2 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -51,8 +51,6 @@ static void merged_iter_close(void *p)
5151
reftable_iterator_destroy(&mi->stack[i]);
5252
}
5353
reftable_free(mi->stack);
54-
strbuf_release(&mi->key);
55-
strbuf_release(&mi->entry_key);
5654
}
5755

5856
static int merged_iter_advance_nonnull_subiter(struct merged_iter *mi,
@@ -105,14 +103,11 @@ static int merged_iter_next_entry(struct merged_iter *mi,
105103
such a deployment, the loop below must be changed to collect all
106104
entries for the same key, and return new the newest one.
107105
*/
108-
reftable_record_key(&entry.rec, &mi->entry_key);
109106
while (!merged_iter_pqueue_is_empty(mi->pq)) {
110107
struct pq_entry top = merged_iter_pqueue_top(mi->pq);
111-
int cmp = 0;
108+
int cmp;
112109

113-
reftable_record_key(&top.rec, &mi->key);
114-
115-
cmp = strbuf_cmp(&mi->key, &mi->entry_key);
110+
cmp = reftable_record_cmp(&top.rec, &entry.rec);
116111
if (cmp > 0)
117112
break;
118113

@@ -246,8 +241,6 @@ static int merged_table_seek_record(struct reftable_merged_table *mt,
246241
.typ = reftable_record_type(rec),
247242
.hash_id = mt->hash_id,
248243
.suppress_deletions = mt->suppress_deletions,
249-
.key = STRBUF_INIT,
250-
.entry_key = STRBUF_INIT,
251244
};
252245
int n = 0;
253246
int err = 0;

reftable/merged.h

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -31,8 +31,6 @@ struct merged_iter {
3131
uint8_t typ;
3232
int suppress_deletions;
3333
struct merged_iter_pqueue pq;
34-
struct strbuf key;
35-
struct strbuf entry_key;
3634
};
3735

3836
void merged_table_release(struct reftable_merged_table *mt);

0 commit comments

Comments
 (0)