Fix off-by-one bug in table shrinking after erasing many elements.

It was causing the table to have a larger than expected load factor for a
single insertion, and rehashing the table right away afterwards.

PiperOrigin-RevId: 707548927
pull/19700/head
Protobuf Team Bot 2 months ago committed by Copybara-Service
parent 5944662a50
commit d9c9420c28
  1. 4
      src/google/protobuf/map.h
  2. 21
      src/google/protobuf/map_test.cc

@ -822,7 +822,8 @@ class KeyMapBase : public UntypedMapBase {
// So, estimate how much to shrink by making sure we don't shrink so
// much that we would need to grow the table after a few inserts.
const size_type hypothetical_size = new_size * 5 / 4 + 1;
while ((hypothetical_size << lg2_of_size_reduction_factor) < hi_cutoff) {
while ((hypothetical_size << (1 + lg2_of_size_reduction_factor)) <
hi_cutoff) {
++lg2_of_size_reduction_factor;
}
size_type new_num_buckets = std::max<size_type>(
@ -862,6 +863,7 @@ class KeyMapBase : public UntypedMapBase {
}
}
DeleteTable(old_table, old_table_size);
AssertLoadFactor();
}
map_index_t BucketNumber(typename TS::ViewType k) const {

@ -274,6 +274,27 @@ TEST(MapTest, NaturalGrowthOnArenasReuseBlocks) {
EXPECT_THAT(arena.SpaceUsed(), AllOf(Ge(expected), Le(1.02 * expected)));
}
TEST(MapTest, ErasingEnoughCausesDownwardRehashOnNextInsert) {
for (size_t capacity = 1; capacity < 1000; capacity *= 2) {
const size_t max_size = MapTestPeer::CalculateHiCutoff(capacity);
for (size_t min_size = 1; min_size < max_size / 4; ++min_size) {
Map<int, int> m;
while (m.size() < max_size) m[m.size()];
const size_t num_buckets = MapTestPeer::NumBuckets(m);
while (m.size() > min_size) m.erase(m.size() - 1);
// Erasing doesn't shrink the table.
ASSERT_EQ(num_buckets, MapTestPeer::NumBuckets(m));
// This insertion causes a shrinking rehash because the load factor is too
// low.
m[99999];
size_t new_num_buckets = MapTestPeer::NumBuckets(m);
EXPECT_LT(new_num_buckets, num_buckets);
EXPECT_LE(m.size(),
MapTestPeer::CalculateHiCutoff(MapTestPeer::NumBuckets(m)));
}
}
}
// We changed the internal implementation to use a smaller size type, but the
// public API will still use size_t to avoid changing the API. Test that.
TEST(MapTest, SizeTypeIsSizeT) {

Loading…
Cancel
Save