Avoid constructing the payload until we need it for reflection.

This saves memory and CPU when the map fields are only used as maps.

PiperOrigin-RevId: 695449397
pull/19217/head
Protobuf Team Bot 2 weeks ago committed by Copybara-Service
parent 82a7cf6205
commit eec53f9378
  1. 17
      csharp/src/Google.Protobuf/Reflection/FeatureSetDescriptor.g.cs
  2. 20
      src/google/protobuf/map_field.cc
  3. 10
      src/google/protobuf/map_field.h
  4. 15
      src/google/protobuf/map_field_inl.h
  5. 12
      src/google/protobuf/map_field_test.cc

@ -1,17 +0,0 @@
#region Copyright notice and license
// Protocol Buffers - Google's data interchange format
// Copyright 2008 Google Inc. All rights reserved.
//
// Use of this source code is governed by a BSD-style
// license that can be found in the LICENSE file or at
// https://developers.google.com/open-source/licenses/bsd
#endregion
namespace Google.Protobuf.Reflection;
internal sealed partial class FeatureSetDescriptor
{
// Canonical serialized form of the edition defaults, generated by embed_edition_defaults.
private const string DefaultsBase64 =
"ChMYhAciACoMCAEQAhgCIAMoATACChMY5wciACoMCAIQARgBIAIoATABChMY6AciDAgBEAEYASACKAEwASoAIOYHKOgH";
}

@ -158,10 +158,15 @@ size_t MapFieldBase::SpaceUsedExcludingSelfLong() const {
ConstAccess();
size_t size = 0;
if (auto* p = maybe_payload()) {
{
absl::MutexLock lock(&p->mutex);
size = SpaceUsedExcludingSelfNoLock();
}
absl::MutexLock lock(&p->mutex);
// Measure the map under the lock, because there could be some repeated
// field data that might be sync'd back into the map.
size = SpaceUsedExcludingSelfNoLock();
size += p->repeated_field.SpaceUsedExcludingSelfLong();
ConstAccess();
} else {
// Only measure the map without the repeated field, because it is not there.
size = SpaceUsedExcludingSelfNoLock();
ConstAccess();
}
return size;
@ -179,13 +184,6 @@ bool MapFieldBase::IsRepeatedFieldValid() const {
return state() != STATE_MODIFIED_MAP;
}
void MapFieldBase::SetMapDirty() {
MutableAccess();
// These are called by (non-const) mutator functions. So by our API it's the
// callers responsibility to have these calls properly ordered.
payload().state.store(STATE_MODIFIED_MAP, std::memory_order_relaxed);
}
void MapFieldBase::SetRepeatedDirty() {
MutableAccess();
// These are called by (non-const) mutator functions. So by our API it's the

@ -410,7 +410,15 @@ class PROTOBUF_EXPORT MapFieldBase : public MapFieldBaseForParse {
static void UnsafeShallowSwapImpl(MapFieldBase& lhs, MapFieldBase& rhs);
// Tells MapFieldBase that there is new change to Map.
void SetMapDirty();
void SetMapDirty() {
MutableAccess();
// These are called by (non-const) mutator functions. So by our API it's the
// callers responsibility to have these calls properly ordered.
if (auto* p = maybe_payload()) {
// If we don't have a payload, it is already assumed `STATE_MODIFIED_MAP`.
p->state.store(STATE_MODIFIED_MAP, std::memory_order_relaxed);
}
}
// Tells MapFieldBase that there is new change to RepeatedPtrField.
void SetRepeatedDirty();

@ -155,18 +155,13 @@ void TypeDefinedMapFieldBase<Key, T>::MergeFromImpl(MapFieldBase& base,
template <typename Key, typename T>
size_t TypeDefinedMapFieldBase<Key, T>::SpaceUsedExcludingSelfNoLockImpl(
const MapFieldBase& map) {
auto& self = static_cast<const TypeDefinedMapFieldBase&>(map);
size_t size = 0;
if (auto* p = self.maybe_payload()) {
size += p->repeated_field.SpaceUsedExcludingSelfLong();
}
// We can't compile this expression for DynamicMapField even though it is
// never used at runtime, so disable it at compile time.
std::get<std::is_same<Map<Key, T>, Map<MapKey, MapValueRef>>::value>(
std::make_tuple(
[&](const auto& map) { size += map.SpaceUsedExcludingSelfLong(); },
[](const auto&) {}))(self.map_);
return size;
if constexpr (!std::is_same<Map<Key, T>, Map<MapKey, MapValueRef>>::value) {
return static_cast<const TypeDefinedMapFieldBase&>(map)
.map_.SpaceUsedExcludingSelfLong();
}
return 0;
}
template <typename Key, typename T>

@ -452,6 +452,18 @@ TEST(MapFieldTest, ConstInit) {
EXPECT_EQ(field.size(), 0);
}
TEST(MapFieldTest, MutableMapDoesNotAllocatePayload) {
struct MaybePayload : MapFieldBase {
// Use a derived type to get access to the protected method.
// We steal the function pointer here to use below to inspect the instance.
static constexpr auto getter() { return &MaybePayload::maybe_payload; }
};
MyMapField field;
EXPECT_FALSE((field.*MaybePayload::getter())());
field.MutableMap();
EXPECT_FALSE((field.*MaybePayload::getter())());
}
} // namespace internal
} // namespace protobuf

Loading…
Cancel
Save