@matheusbsbs / matheus-bagliano / commits / aad8ad6fe1

reftable/merged: advance subiter on subsequent iteration

When advancing the merged iterator, we pop the topmost entry from its priority queue and then advance the sub-iterator that the entry belongs to, adding the result as a new entry. This is quite sensible in the case where the merged iterator is used to actually iterate through records. But the merged iterator is also used when we look up a single record, only, so advancing the sub-iterator is wasted effort because we would never even look at the result. Instead of immediately advancing the sub-iterator, we can also defer this to the next iteration of the merged iterator by storing the intent-to-advance. This results in a small speedup when reading many records. The following benchmark creates 10000 refs, which will also end up with many ref lookups: Benchmark 1: update-ref: create many refs (revision = HEAD~) Time (mean ± σ): 337.2 ms ± 7.3 ms [User: 200.1 ms, System: 136.9 ms] Range (min … max): 329.3 ms … 373.2 ms 100 runs Benchmark 2: update-ref: create many refs (revision = HEAD) Time (mean ± σ): 332.5 ms ± 5.9 ms [User: 197.2 ms, System: 135.1 ms] Range (min … max): 327.6 ms … 359.8 ms 100 runs Summary update-ref: create many refs (revision = HEAD) ran 1.01 ± 0.03 times faster than update-ref: create many refs (revision = HEAD~) While this speedup alone isn't really worth it, this refactoring will also allow two additional optimizations in subsequent patches. First, it will allow us to special-case when there is only a single sub-iter left to circumvent the priority queue altogether. And second, it makes it easier to avoid copying records to the caller. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Patrick Steinhardt committed Mar 4, 2024 at 11:48 UTC aad8ad6fe1fe15d4b024425cd64389928838efd3

1 file changed +12 -14

reftable/merged.c

+12 -14

@@ -19,11 +19,12 @@ https://developers.google.com/open-source/licenses/bsd

20 struct merged_iter {

21 struct reftable_iterator *stack;

22 + struct merged_iter_pqueue pq;

23 uint32_t hash_id;

24 size_t stack_len;

25 uint8_t typ;

26 int suppress_deletions;

26 - struct merged_iter_pqueue pq;

27 + ssize_t advance_index;

28 };

30 static int merged_iter_init(struct merged_iter *mi)

@@ -96,13 +97,17 @@ static int merged_iter_next_entry(struct merged_iter *mi,

97 struct pq_entry entry = { 0 };

98 int err = 0;

100 + if (mi->advance_index >= 0) {

101 + err = merged_iter_advance_subiter(mi, mi->advance_index);

102 + if (err < 0)

103 + return err;

104 + mi->advance_index = -1;

105 + }

106 +

107 if (merged_iter_pqueue_is_empty(mi->pq))

108 return 1;

109

110 entry = merged_iter_pqueue_remove(&mi->pq);

103 - err = merged_iter_advance_subiter(mi, entry.index);

104 - if (err < 0)

105 - return err;

111

112 /*

113 One can also use reftable as datacenter-local storage, where the ref

@@ -116,14 +121,6 @@ static int merged_iter_next_entry(struct merged_iter *mi,

121 struct pq_entry top = merged_iter_pqueue_top(mi->pq);

122 int cmp;

123

119 - /*

120 - * When the next entry comes from the same queue as the current

121 - * entry then it must by definition be larger. This avoids a

122 - * comparison in the most common case.

123 - */

124 - if (top.index == entry.index)

125 - break;

126 -

124 cmp = reftable_record_cmp(&top.rec, &entry.rec);

125 if (cmp > 0)

126 break;

@@ -137,6 +134,7 @@ static int merged_iter_next_entry(struct merged_iter *mi,

134

135 reftable_record_release(rec);

136 *rec = entry.rec;

137 + mi->advance_index = entry.index;

138

139 done:

140 if (err)

@@ -160,9 +158,8 @@ static int merged_iter_next(struct merged_iter *mi, struct reftable_record *rec)

158 static int merged_iter_next_void(void *p, struct reftable_record *rec)

159 {

160 struct merged_iter *mi = p;

163 - if (merged_iter_pqueue_is_empty(mi->pq))

161 + if (merged_iter_pqueue_is_empty(mi->pq) && mi->advance_index < 0)

162 return 1;

165 -

163 return merged_iter_next(mi, rec);

164 }

165

@@ -255,6 +252,7 @@ static int merged_table_seek_record(struct reftable_merged_table *mt,

252 .typ = reftable_record_type(rec),

253 .hash_id = mt->hash_id,

254 .suppress_deletions = mt->suppress_deletions,

255 + .advance_index = -1,

256 };

257 struct merged_iter *p;

258 int err;