Skip to content

Commit

Permalink
Breaking change: Remove MutableRepeatedFieldRef::Reserve() (reflect…
Browse files Browse the repository at this point in the history
…ion)

An upcoming performance improvement in RepeatedPtrField is incompatible with this API. The improvement is projected to accelerate repeated access to the elements of `RepeatedPtrField`, in particular and especially sequential access.

PA: https://protobuf.dev/news/2024-12-13/
PiperOrigin-RevId: 707309375
  • Loading branch information
CEL Dev Team authored and copybara-github committed Dec 18, 2024
1 parent 8a5e074 commit 22b8dd4
Show file tree
Hide file tree
Showing 5 changed files with 55 additions and 0 deletions.
1 change: 1 addition & 0 deletions common/values/parsed_map_field_value.cc
Original file line number Diff line number Diff line change
Expand Up @@ -165,6 +165,7 @@ ParsedMapFieldValue ParsedMapFieldValue::Clone(Allocator<> allocator) const {
auto cloned_field =
cloned->GetReflection()->GetMutableRepeatedFieldRef<google::protobuf::Message>(
cel::to_address(cloned), field_);
cloned_field.Reserve(field.size());
cloned_field.CopyFrom(field);
return ParsedMapFieldValue(std::move(cloned), field_);
}
Expand Down
1 change: 1 addition & 0 deletions common/values/parsed_repeated_field_value.cc
Original file line number Diff line number Diff line change
Expand Up @@ -160,6 +160,7 @@ ParsedRepeatedFieldValue ParsedRepeatedFieldValue::Clone(
auto cloned_field =
cloned->GetReflection()->GetMutableRepeatedFieldRef<google::protobuf::Message>(
cel::to_address(cloned), field_);
cloned_field.Reserve(field.size());
cloned_field.CopyFrom(field);
return ParsedRepeatedFieldValue(std::move(cloned), field_);
}
Expand Down
34 changes: 34 additions & 0 deletions internal/json.cc
Original file line number Diff line number Diff line change
Expand Up @@ -806,6 +806,7 @@ class MessageToJsonState {
if (size == 0) {
return absl::OkStatus();
}
ReserveValues(result, size);
CEL_ASSIGN_OR_RETURN(const auto to_value, GetRepeatedFieldToValue(field));
for (int index = 0; index < size; ++index) {
CEL_RETURN_IF_ERROR((this->*to_value)(reflection, message, field, index,
Expand Down Expand Up @@ -994,6 +995,9 @@ class MessageToJsonState {
virtual absl::Nonnull<google::protobuf::MessageLite*> MutableStructValue(
absl::Nonnull<google::protobuf::MessageLite*> message) const = 0;

virtual void ReserveValues(absl::Nonnull<google::protobuf::MessageLite*> message,
int capacity) const = 0;

virtual absl::Nonnull<google::protobuf::MessageLite*> AddValues(
absl::Nonnull<google::protobuf::MessageLite*> message) const = 0;

Expand Down Expand Up @@ -1072,6 +1076,13 @@ class GeneratedMessageToJsonState final : public MessageToJsonState {
google::protobuf::DownCastMessage<google::protobuf::Value>(message));
}

void ReserveValues(absl::Nonnull<google::protobuf::MessageLite*> message,
int capacity) const override {
ListValueReflection::ReserveValues(
google::protobuf::DownCastMessage<google::protobuf::ListValue>(message),
capacity);
}

absl::Nonnull<google::protobuf::MessageLite*> AddValues(
absl::Nonnull<google::protobuf::MessageLite*> message) const override {
return ListValueReflection::AddValues(
Expand Down Expand Up @@ -1152,6 +1163,12 @@ class DynamicMessageToJsonState final : public MessageToJsonState {
google::protobuf::DownCastMessage<google::protobuf::Message>(message));
}

void ReserveValues(absl::Nonnull<google::protobuf::MessageLite*> message,
int capacity) const override {
reflection_.ListValue().ReserveValues(
google::protobuf::DownCastMessage<google::protobuf::Message>(message), capacity);
}

absl::Nonnull<google::protobuf::MessageLite*> AddValues(
absl::Nonnull<google::protobuf::MessageLite*> message) const override {
return reflection_.ListValue().AddValues(
Expand Down Expand Up @@ -2155,6 +2172,9 @@ class JsonMutator {
virtual absl::Nonnull<google::protobuf::MessageLite*> MutableListValue(
absl::Nonnull<google::protobuf::MessageLite*> message) const = 0;

virtual void ReserveValues(absl::Nonnull<google::protobuf::MessageLite*> message,
int capacity) const = 0;

virtual absl::Nonnull<google::protobuf::MessageLite*> AddValues(
absl::Nonnull<google::protobuf::MessageLite*> message) const = 0;

Expand Down Expand Up @@ -2203,6 +2223,13 @@ class GeneratedJsonMutator final : public JsonMutator {
google::protobuf::DownCastMessage<google::protobuf::Value>(message));
}

void ReserveValues(absl::Nonnull<google::protobuf::MessageLite*> message,
int capacity) const override {
ListValueReflection::ReserveValues(
google::protobuf::DownCastMessage<google::protobuf::ListValue>(message),
capacity);
}

absl::Nonnull<google::protobuf::MessageLite*> AddValues(
absl::Nonnull<google::protobuf::MessageLite*> message) const override {
return ListValueReflection::AddValues(
Expand Down Expand Up @@ -2285,6 +2312,12 @@ class DynamicJsonMutator final : public JsonMutator {
google::protobuf::DownCastMessage<google::protobuf::Message>(message));
}

void ReserveValues(absl::Nonnull<google::protobuf::MessageLite*> message,
int capacity) const override {
list_value_reflection_.ReserveValues(
google::protobuf::DownCastMessage<google::protobuf::Message>(message), capacity);
}

absl::Nonnull<google::protobuf::MessageLite*> AddValues(
absl::Nonnull<google::protobuf::MessageLite*> message) const override {
return list_value_reflection_.AddValues(
Expand Down Expand Up @@ -2346,6 +2379,7 @@ class NativeJsonToProtoJsonState {

absl::Status ToProtoJsonList(const JsonArray& json,
absl::Nonnull<google::protobuf::MessageLite*> proto) {
mutator_->ReserveValues(proto, static_cast<int>(json.size()));
for (const auto& element : json) {
CEL_RETURN_IF_ERROR(ToProtoJson(element, mutator_->AddValues(proto)));
}
Expand Down
9 changes: 9 additions & 0 deletions internal/well_known_types.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1534,6 +1534,15 @@ absl::Nonnull<google::protobuf::Message*> ListValueReflection::AddValues(
return message->GetReflection()->AddMessage(message, values_field_);
}

void ListValueReflection::ReserveValues(absl::Nonnull<google::protobuf::Message*> message,
int capacity) const {
ABSL_DCHECK(IsInitialized());
ABSL_DCHECK_EQ(message->GetDescriptor(), descriptor_);
if (capacity > 0) {
MutableValues(message).Reserve(capacity);
}
}

absl::StatusOr<ListValueReflection> GetListValueReflection(
absl::Nonnull<const Descriptor*> descriptor) {
ListValueReflection reflection;
Expand Down
10 changes: 10 additions & 0 deletions internal/well_known_types.h
Original file line number Diff line number Diff line change
Expand Up @@ -1066,6 +1066,13 @@ class ListValueReflection final {
return message->add_values();
}

static void ReserveValues(absl::Nonnull<GeneratedMessageType*> message,
int capacity) {
if (capacity > 0) {
message->mutable_values()->Reserve(capacity);
}
}

absl::Status Initialize(absl::Nonnull<const google::protobuf::DescriptorPool*> pool);

absl::Status Initialize(absl::Nonnull<const google::protobuf::Descriptor*> descriptor);
Expand Down Expand Up @@ -1103,6 +1110,9 @@ class ListValueReflection final {
absl::Nonnull<google::protobuf::Message*> AddValues(
absl::Nonnull<google::protobuf::Message*> message) const;

void ReserveValues(absl::Nonnull<google::protobuf::Message*> message,
int capacity) const;

private:
absl::Nullable<const google::protobuf::Descriptor*> descriptor_ = nullptr;
absl::Nullable<const google::protobuf::FieldDescriptor*> values_field_ = nullptr;
Expand Down

0 comments on commit 22b8dd4

Please sign in to comment.