Skip to content

Commit

Permalink
Change & enforce element factory signature in RepeatedPtrField
Browse files Browse the repository at this point in the history
This is a pre-work for the contiguous, cache-prefetchable repeated T's.

This also removes templatization from `RepeatedPtrField::AddInternal()`.

PiperOrigin-RevId: 669459607
  • Loading branch information
protobuf-github-bot authored and copybara-github committed Sep 4, 2024
1 parent b8764f0 commit 84fb0cd
Show file tree
Hide file tree
Showing 10 changed files with 156 additions and 199 deletions.
4 changes: 1 addition & 3 deletions src/google/protobuf/extension_set.cc
Original file line number Diff line number Diff line change
Expand Up @@ -811,9 +811,7 @@ MessageLite* ExtensionSet::AddMessage(int number, FieldType type,
ABSL_DCHECK_TYPE(*extension, REPEATED_FIELD, MESSAGE);
}

return reinterpret_cast<internal::RepeatedPtrFieldBase*>(
extension->ptr.repeated_message_value)
->AddMessage(&prototype);
return extension->ptr.repeated_message_value->AddFromPrototype(&prototype);
}

// Defined in extension_set_heavy.cc.
Expand Down
3 changes: 2 additions & 1 deletion src/google/protobuf/generated_message_tctable_lite.cc
Original file line number Diff line number Diff line change
Expand Up @@ -360,10 +360,11 @@ inline PROTOBUF_ALWAYS_INLINE MessageLite* TcParser::NewMessage(
return table->class_data->New(arena);
}

// DO NOT SUBMIT: Redo via `field.AddMessage()`, and remove `NewMessage()`.
MessageLite* TcParser::AddMessage(const TcParseTableBase* table,
RepeatedPtrFieldBase& field) {
return static_cast<MessageLite*>(field.AddInternal(
[table](Arena* arena) { return NewMessage(table, arena); }));
[table](Arena* arena, void*& ptr) { ptr = NewMessage(table, arena); }));
}

template <typename TagType, bool group_coding, bool aux_is_table>
Expand Down
3 changes: 2 additions & 1 deletion src/google/protobuf/lazy_repeated_field.h
Original file line number Diff line number Diff line change
Expand Up @@ -328,7 +328,8 @@ class LazyRepeatedPtrField {
const char* ptr2 = ptr;
TagType next_tag;
do {
MessageLite* submsg = value->AddMessage(prototype);
MessageLite* submsg =
value->AddFromPrototype<GenericTypeHandler<MessageLite>>(prototype);
// ptr2 points to the start of the element's encoded length.
ptr = ctx->ParseMessage(submsg, ptr2);
if (PROTOBUF_PREDICT_FALSE(ptr == nullptr)) return nullptr;
Expand Down
22 changes: 0 additions & 22 deletions src/google/protobuf/message.cc
Original file line number Diff line number Diff line change
Expand Up @@ -495,28 +495,6 @@ const internal::RepeatedFieldAccessor* Reflection::RepeatedFieldAccessor(
}

namespace internal {
template <>
#if defined(_MSC_VER) && (_MSC_VER >= 1800)
// Note: force noinline to workaround MSVC compiler bug with /Zc:inline, issue
// #240
PROTOBUF_NOINLINE
#endif
Message*
GenericTypeHandler<Message>::NewFromPrototype(const Message* prototype,
Arena* arena) {
return prototype->New(arena);
}
template <>
#if defined(_MSC_VER) && (_MSC_VER >= 1800)
// Note: force noinline to workaround MSVC compiler bug with /Zc:inline, issue
// #240
PROTOBUF_NOINLINE
#endif
Arena*
GenericTypeHandler<Message>::GetArena(Message* value) {
return value->GetArena();
}

template void InternalMetadata::DoClear<UnknownFieldSet>();
template void InternalMetadata::DoMergeFrom<UnknownFieldSet>(
const UnknownFieldSet& other);
Expand Down
10 changes: 0 additions & 10 deletions src/google/protobuf/message_lite.cc
Original file line number Diff line number Diff line change
Expand Up @@ -703,16 +703,6 @@ absl::Cord MessageLite::SerializePartialAsCord() const {

namespace internal {

MessageLite* NewFromPrototypeHelper(const MessageLite* prototype,
Arena* arena) {
return prototype->New(arena);
}
template <>
void GenericTypeHandler<MessageLite>::Merge(const MessageLite& from,
MessageLite* to) {
to->CheckTypeAndMergeFrom(from);
}

// Non-inline variants of std::string specializations for
// various InternalMetadata routines.
template <>
Expand Down
5 changes: 0 additions & 5 deletions src/google/protobuf/message_lite.h
Original file line number Diff line number Diff line change
Expand Up @@ -225,9 +225,6 @@ class WireFormatLite;
class WeakFieldMap;
class RustMapHelper;

template <typename Type>
class GenericTypeHandler; // defined in repeated_field.h

// We compute sizes as size_t but cache them as int. This function converts a
// computed size to a cached size. Since we don't proceed with serialization
// if the total size was > INT_MAX, it is not important what this function
Expand Down Expand Up @@ -963,8 +960,6 @@ class PROTOBUF_EXPORT MessageLite {

template <typename Type>
friend class Arena::InternalHelper;
template <typename Type>
friend class internal::GenericTypeHandler;

friend auto internal::GetClassData(const MessageLite& msg);

Expand Down
7 changes: 4 additions & 3 deletions src/google/protobuf/repeated_field.h
Original file line number Diff line number Diff line change
Expand Up @@ -28,15 +28,13 @@
#include <iterator>
#include <limits>
#include <memory>
#include <string>
#include <type_traits>
#include <utility>

#include "absl/base/attributes.h"
#include "absl/base/dynamic_annotations.h"
#include "absl/base/optimization.h"
#include "absl/log/absl_check.h"
#include "absl/log/absl_log.h"
#include "absl/meta/type_traits.h"
#include "absl/strings/cord.h"
#include "google/protobuf/arena.h"
Expand All @@ -46,7 +44,6 @@
#include "google/protobuf/port.h"
#include "google/protobuf/repeated_ptr_field.h"


// Must be included last.
#include "google/protobuf/port_def.inc"

Expand Down Expand Up @@ -79,7 +76,11 @@ constexpr int RepeatedFieldLowerClampLimit() {
// overflows when multiplied by 2 (which is undefined behavior). Sizes above
// this will clamp to the maximum int value instead of following exponential
// growth when growing a repeated field.
#if defined(__cpp_inline_variables)
inline constexpr int kRepeatedFieldUpperClampLimit =
#else
constexpr int kRepeatedFieldUpperClampLimit =
#endif
(std::numeric_limits<int>::max() / 2) + 1;

template <typename Element>
Expand Down
2 changes: 0 additions & 2 deletions src/google/protobuf/repeated_field_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@
#include <cstdint>
#include <cstdlib>
#include <cstring>
#include <functional>
#include <iterator>
#include <limits>
#include <list>
Expand All @@ -34,7 +33,6 @@
#include <gtest/gtest.h>
#include "absl/log/absl_check.h"
#include "absl/numeric/bits.h"
#include "absl/random/random.h"
#include "absl/strings/cord.h"
#include "absl/strings/str_cat.h"
#include "absl/types/span.h"
Expand Down
65 changes: 25 additions & 40 deletions src/google/protobuf/repeated_ptr_field.cc
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,9 @@
#include <cstdint>
#include <cstring>
#include <limits>
#include <new>
#include <string>

#include "absl/base/prefetch.h"
#include "absl/log/absl_check.h"
#include "google/protobuf/arena.h"
#include "google/protobuf/message_lite.h"
Expand All @@ -35,25 +35,28 @@ namespace internal {

void** RepeatedPtrFieldBase::InternalExtend(int extend_amount) {
ABSL_DCHECK(extend_amount > 0);
constexpr size_t ptr_size = sizeof(rep()->elements[0]);
int capacity = Capacity();
int new_capacity = capacity + extend_amount;
constexpr size_t kPtrSize = sizeof(rep()->elements[0]);
const int old_capacity = Capacity();
Arena* arena = GetArena();
new_capacity = internal::CalculateReserveSize<void*, kRepHeaderSize>(
capacity, new_capacity);
ABSL_CHECK_LE(
static_cast<int64_t>(new_capacity),
static_cast<int64_t>(
(std::numeric_limits<size_t>::max() - kRepHeaderSize) / ptr_size))
<< "Requested size is too large to fit into size_t.";
size_t bytes = kRepHeaderSize + ptr_size * new_capacity;
Rep* new_rep;
if (arena == nullptr) {
internal::SizedPtr res = internal::AllocateAtLeast(bytes);
new_capacity = static_cast<int>((res.n - kRepHeaderSize) / ptr_size);
new_rep = reinterpret_cast<Rep*>(res.p);
} else {
new_rep = reinterpret_cast<Rep*>(Arena::CreateArray<char>(arena, bytes));
Rep* new_rep = nullptr;
{
int new_capacity = internal::CalculateReserveSize<void*, kRepHeaderSize>(
old_capacity, old_capacity + extend_amount);
const size_t new_size = kRepHeaderSize + kPtrSize * new_capacity;
if (arena == nullptr) {
const internal::SizedPtr alloc = internal::AllocateAtLeast(new_size);
new_capacity = static_cast<int>((alloc.n - kRepHeaderSize) / kPtrSize);
new_rep = reinterpret_cast<Rep*>(alloc.p);
} else {
auto* alloc = Arena::CreateArray<char>(arena, new_size);
new_rep = reinterpret_cast<Rep*>(alloc);
}
ABSL_CHECK_LE(
static_cast<int64_t>(new_capacity),
static_cast<int64_t>(
(std::numeric_limits<size_t>::max() - kRepHeaderSize) / kPtrSize))
<< "Requested size is too large to fit into size_t: " << new_capacity;
capacity_proxy_ = new_capacity - kSSOCapacity;
}

if (using_sso()) {
Expand All @@ -62,9 +65,8 @@ void** RepeatedPtrFieldBase::InternalExtend(int extend_amount) {
} else {
Rep* old_rep = rep();
memcpy(new_rep, old_rep,
old_rep->allocated_size * ptr_size + kRepHeaderSize);

size_t old_size = capacity * ptr_size + kRepHeaderSize;
old_rep->allocated_size * kPtrSize + kRepHeaderSize);
size_t old_size = old_capacity * kPtrSize + kRepHeaderSize;
if (arena == nullptr) {
internal::SizedDelete(old_rep, old_size);
} else {
Expand All @@ -74,7 +76,7 @@ void** RepeatedPtrFieldBase::InternalExtend(int extend_amount) {

tagged_rep_or_elem_ =
reinterpret_cast<void*>(reinterpret_cast<uintptr_t>(new_rep) + 1);
capacity_proxy_ = new_capacity - kSSOCapacity;

return &new_rep->elements[current_size_];
}

Expand All @@ -93,14 +95,6 @@ void RepeatedPtrFieldBase::DestroyProtos() {
tagged_rep_or_elem_ = nullptr;
}

void* RepeatedPtrFieldBase::AddMessageLite(ElementFactory factory) {
return AddInternal(factory);
}

void* RepeatedPtrFieldBase::AddString() {
return AddInternal([](Arena* arena) { return NewStringElement(arena); });
}

void RepeatedPtrFieldBase::CloseGap(int start, int num) {
if (using_sso()) {
if (start == 0 && num == 1) {
Expand All @@ -116,11 +110,6 @@ void RepeatedPtrFieldBase::CloseGap(int start, int num) {
ExchangeCurrentSize(current_size_ - num);
}

MessageLite* RepeatedPtrFieldBase::AddMessage(const MessageLite* prototype) {
return static_cast<MessageLite*>(
AddInternal([prototype](Arena* a) { return prototype->New(a); }));
}

void InternalOutOfLineDeleteMessageLite(MessageLite* message) {
delete message;
}
Expand Down Expand Up @@ -223,10 +212,6 @@ void RepeatedPtrFieldBase::MergeFrom<MessageLite>(
}
}

void* NewStringElement(Arena* arena) {
return Arena::Create<std::string>(arena);
}

} // namespace internal
} // namespace protobuf
} // namespace google
Expand Down
Loading

0 comments on commit 84fb0cd

Please sign in to comment.