Skip to content

Commit

Permalink
Reduce Map::size_type to 32-bits. Protobuf containers can't have mo…
Browse files Browse the repository at this point in the history
…re than

2^32 elements anyway because they are not serializable.
This saves 16 bytes for each `map` field.

PiperOrigin-RevId: 571943973
  • Loading branch information
protobuf-github-bot authored and copybara-github committed Oct 9, 2023
1 parent d14a336 commit 95d2b2e
Show file tree
Hide file tree
Showing 2 changed files with 16 additions and 14 deletions.
4 changes: 2 additions & 2 deletions src/google/protobuf/map.cc
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ void UntypedMapBase::EraseFromTree(size_type b,
}
}

size_t UntypedMapBase::VariantBucketNumber(VariantKey key) const {
auto UntypedMapBase::VariantBucketNumber(VariantKey key) const -> size_type {
return BucketNumberFromHash(key.Hash());
}

Expand Down Expand Up @@ -206,7 +206,7 @@ size_t UntypedMapBase::SpaceUsedInTable(size_t sizeof_node) const {
size += sizeof_node * num_elements_;
// For each tree, count the overhead of those nodes.
// Two buckets at a time because we only care about trees.
for (size_t b = 0; b < num_buckets_; ++b) {
for (size_type b = 0; b < num_buckets_; ++b) {
if (TableEntryIsTree(b)) {
size += sizeof(Tree);
size += sizeof(Tree::value_type) * TableEntryToTree(table_[b])->size();
Expand Down
26 changes: 14 additions & 12 deletions src/google/protobuf/map.h
Original file line number Diff line number Diff line change
Expand Up @@ -467,6 +467,8 @@ class UntypedMapBase;

class UntypedMapIterator {
public:
using size_type = uint32_t;

// Invariants:
// node_ is always correct. This is handy because the most common
// operations are operator* and operator-> and they only use node_.
Expand All @@ -478,12 +480,12 @@ class UntypedMapIterator {

explicit UntypedMapIterator(const UntypedMapBase* m);

UntypedMapIterator(NodeBase* n, const UntypedMapBase* m, size_t index)
UntypedMapIterator(NodeBase* n, const UntypedMapBase* m, size_type index)
: node_(n), m_(m), bucket_index_(index) {}

// Advance through buckets, looking for the first that isn't empty.
// If nothing non-empty is found then leave node_ == nullptr.
void SearchFrom(size_t start_bucket);
void SearchFrom(size_type start_bucket);

// The definition of operator== is handled by the derived type. If we were
// to do it in this class it would allow comparing iterators of different
Expand All @@ -504,7 +506,7 @@ class UntypedMapIterator {

NodeBase* node_;
const UntypedMapBase* m_;
size_t bucket_index_;
size_type bucket_index_;
};

// Base class for all Map instantiations.
Expand All @@ -517,7 +519,7 @@ class PROTOBUF_EXPORT UntypedMapBase {
using Tree = internal::TreeForMap;

public:
using size_type = size_t;
using size_type = uint32_t;

explicit constexpr UntypedMapBase(Arena* arena)
: num_elements_(0),
Expand Down Expand Up @@ -545,9 +547,7 @@ class PROTOBUF_EXPORT UntypedMapBase {
std::swap(alloc_, other->alloc_);
}

static size_type max_size() {
return static_cast<size_type>(1) << (sizeof(void**) >= 8 ? 60 : 28);
}
static size_type max_size() { return std::numeric_limits<size_type>::max(); }
size_type size() const { return num_elements_; }
bool empty() const { return size() == 0; }

Expand Down Expand Up @@ -781,10 +781,10 @@ inline UntypedMapIterator::UntypedMapIterator(const UntypedMapBase* m) : m_(m) {
}
}

inline void UntypedMapIterator::SearchFrom(size_t start_bucket) {
inline void UntypedMapIterator::SearchFrom(size_type start_bucket) {
ABSL_DCHECK(m_->index_of_first_non_null_ == m_->num_buckets_ ||
!m_->TableEntryIsEmpty(m_->index_of_first_non_null_));
for (size_t i = start_bucket; i < m_->num_buckets_; ++i) {
for (size_type i = start_bucket; i < m_->num_buckets_; ++i) {
TableEntryPtr entry = m_->table_[i];
if (entry == TableEntryPtr{}) continue;
bucket_index_ = i;
Expand Down Expand Up @@ -1022,7 +1022,7 @@ class KeyMapBase : public UntypedMapBase {
}

// Resize to the given number of buckets.
void Resize(size_t new_num_buckets) {
void Resize(size_type new_num_buckets) {
if (num_buckets_ == kGlobalEmptyTableSize) {
// This is the global empty array.
// Just overwrite with a new one. No need to transfer or free anything.
Expand Down Expand Up @@ -1068,7 +1068,7 @@ class KeyMapBase : public UntypedMapBase {
// stale. Fix them as needed. Then return true iff node_ points to a
// Node in a list. If false is returned then *it is modified to be
// a valid iterator for node_.
bool revalidate_if_necessary(size_t& bucket_index, KeyNode* node,
bool revalidate_if_necessary(size_type& bucket_index, KeyNode* node,
TreeIterator* it) const {
// Force bucket_index to be in range.
bucket_index &= (num_buckets_ - 1);
Expand Down Expand Up @@ -1133,7 +1133,9 @@ class Map : private internal::KeyMapBase<internal::KeyForBase<Key>> {
using reference = value_type&;
using const_reference = const value_type&;

using size_type = size_t;
// The largest valid serialization for a message is INT_MAX, so we can't have
// more than 32-bits worth of elements.
using size_type = uint32_t;
using hasher = typename TS::hash;

constexpr Map() : Base(nullptr) { StaticValidityCheck(); }
Expand Down

0 comments on commit 95d2b2e

Please sign in to comment.