Skip to content

Commit f3f3d15

Browse files
ezbrcopybara-github
authored andcommitted
Optimize btree_iterator increment/decrement to avoid aliasing issues by using local variables instead of repeatedly writing to this.
Also update the benchmark to use DoNotOptimize to make sure the iterators are actually materialized. PiperOrigin-RevId: 736229349 Change-Id: I7422544398210a48a7a40d81d341fd4beaf71861
1 parent 3bddebd commit f3f3d15

File tree

2 files changed

+35
-35
lines changed

2 files changed

+35
-35
lines changed

absl/container/btree_benchmark.cc

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -406,22 +406,18 @@ void BM_Fifo(benchmark::State& state) {
406406
template <typename T>
407407
void BM_FwdIter(benchmark::State& state) {
408408
using V = typename remove_pair_const<typename T::value_type>::type;
409-
using R = typename T::value_type const*;
410409

411410
std::vector<V> values = GenerateValues<V>(kBenchmarkValues);
412411
T container(values.begin(), values.end());
413412

414413
auto iter = container.end();
415414

416-
R r = nullptr;
417-
418415
while (state.KeepRunning()) {
419416
if (iter == container.end()) iter = container.begin();
420-
r = &(*iter);
421-
++iter;
417+
auto *v = &(*iter);
418+
benchmark::DoNotOptimize(v);
419+
benchmark::DoNotOptimize(++iter);
422420
}
423-
424-
benchmark::DoNotOptimize(r);
425421
}
426422

427423
// Benchmark random range-construction of a container.

absl/container/internal/btree.h

Lines changed: 32 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -2151,50 +2151,54 @@ auto btree_iterator<N, R, P>::distance_slow(const_iterator other) const
21512151

21522152
template <typename N, typename R, typename P>
21532153
void btree_iterator<N, R, P>::increment_slow() {
2154-
if (node_->is_leaf()) {
2155-
assert(position_ >= node_->finish());
2156-
btree_iterator save(*this);
2157-
while (position_ == node_->finish() && !node_->is_root()) {
2158-
assert(node_->parent()->child(node_->position()) == node_);
2159-
position_ = node_->position();
2160-
node_ = node_->parent();
2154+
N* node = node_;
2155+
int position = position_;
2156+
if (node->is_leaf()) {
2157+
assert(position >= node->finish());
2158+
while (position == node->finish() && !node->is_root()) {
2159+
assert(node->parent()->child(node->position()) == node);
2160+
position = node->position();
2161+
node = node->parent();
21612162
}
21622163
// TODO(ezb): assert we aren't incrementing end() instead of handling.
2163-
if (position_ == node_->finish()) {
2164-
*this = save;
2164+
if (position == node->finish()) {
2165+
return;
21652166
}
21662167
} else {
2167-
assert(position_ < node_->finish());
2168-
node_ = node_->child(static_cast<field_type>(position_ + 1));
2169-
while (node_->is_internal()) {
2170-
node_ = node_->start_child();
2168+
assert(position < node->finish());
2169+
node = node->child(static_cast<field_type>(position + 1));
2170+
while (node->is_internal()) {
2171+
node = node->start_child();
21712172
}
2172-
position_ = node_->start();
2173+
position = node->start();
21732174
}
2175+
*this = {node, position};
21742176
}
21752177

21762178
template <typename N, typename R, typename P>
21772179
void btree_iterator<N, R, P>::decrement_slow() {
2178-
if (node_->is_leaf()) {
2179-
assert(position_ <= -1);
2180-
btree_iterator save(*this);
2181-
while (position_ < node_->start() && !node_->is_root()) {
2182-
assert(node_->parent()->child(node_->position()) == node_);
2183-
position_ = node_->position() - 1;
2184-
node_ = node_->parent();
2180+
N* node = node_;
2181+
int position = position_;
2182+
if (node->is_leaf()) {
2183+
assert(position <= -1);
2184+
while (position < node->start() && !node->is_root()) {
2185+
assert(node->parent()->child(node->position()) == node);
2186+
position = node->position() - 1;
2187+
node = node->parent();
21852188
}
21862189
// TODO(ezb): assert we aren't decrementing begin() instead of handling.
2187-
if (position_ < node_->start()) {
2188-
*this = save;
2190+
if (position < node->start()) {
2191+
return;
21892192
}
21902193
} else {
2191-
assert(position_ >= node_->start());
2192-
node_ = node_->child(static_cast<field_type>(position_));
2193-
while (node_->is_internal()) {
2194-
node_ = node_->child(node_->finish());
2194+
assert(position >= node->start());
2195+
node = node->child(static_cast<field_type>(position));
2196+
while (node->is_internal()) {
2197+
node = node->child(node->finish());
21952198
}
2196-
position_ = node_->finish() - 1;
2199+
position = node->finish() - 1;
21972200
}
2201+
*this = {node, position};
21982202
}
21992203

22002204
template <typename N, typename R, typename P>

0 commit comments

Comments
 (0)