commit 70876bd87ea9050b46c8efdf191c246cf452349e
author Protobuf Team Bot <protobuf-github-bot@google.com> Wed Nov 15 07:25:54 2023 -0800
committer Copybara-Service <copybara-worker@google.com> Wed Nov 15 07:27:32 2023 -0800
tree 9680a0a55e04d51ba89dea189953b0b4ec0ff54c
parent 531596c5f95c494c2436f0a959a2427510fd4137

Short-circuit destruction of an empty field using SSO.

```
name                                   old cpu/op   new cpu/op   delta
BM_RepeatedPtrField_Ctor                133ns ± 2%     78ns ± 1%  -41.64%   (p=0.000 n=80+87)

```

PiperOrigin-RevId: 582667999

src/google/protobuf/implicit_weak_message.h

diff --git a/src/google/protobuf/implicit_weak_message.h b/src/google/protobuf/implicit_weak_message.h
index 7c94d39..ebd78f1 100644
--- a/src/google/protobuf/implicit_weak_message.h
+++ b/src/google/protobuf/implicit_weak_message.h
@@ -167,7 +167,11 @@
   // TODO: make this constructor private
   explicit WeakRepeatedPtrField(Arena* arena) : weak(arena) {}
 
-  ~WeakRepeatedPtrField() { weak.template Destroy<TypeHandler>(); }
+  ~WeakRepeatedPtrField() {
+    if (weak.NeedsDestroy()) {
+      weak.DestroyProtos();
+    }
+  }
 
   typedef internal::RepeatedPtrIterator<MessageLite> iterator;
   typedef internal::RepeatedPtrIterator<const MessageLite> const_iterator;

src/google/protobuf/repeated_ptr_field.cc

diff --git a/src/google/protobuf/repeated_ptr_field.cc b/src/google/protobuf/repeated_ptr_field.cc
index 2a98da2..2afe3ad 100644
--- a/src/google/protobuf/repeated_ptr_field.cc
+++ b/src/google/protobuf/repeated_ptr_field.cc
@@ -88,20 +88,7 @@
 }
 
 void RepeatedPtrFieldBase::DestroyProtos() {
-  ABSL_DCHECK(tagged_rep_or_elem_);
-  ABSL_DCHECK(arena_ == nullptr);
-  if (using_sso()) {
-    delete static_cast<MessageLite*>(tagged_rep_or_elem_);
-  } else {
-    Rep* r = rep();
-    int n = r->allocated_size;
-    void* const* elements = r->elements;
-    for (int i = 0; i < n; i++) {
-      delete static_cast<MessageLite*>(elements[i]);
-    }
-    const size_t size = Capacity() * sizeof(elements[0]) + kRepHeaderSize;
-    internal::SizedDelete(r, size);
-  }
+  Destroy<GenericTypeHandler<MessageLite>>();
 
   // TODO:  Eliminate this store when invoked from the destructor,
   // since it is dead.

src/google/protobuf/repeated_ptr_field.h

diff --git a/src/google/protobuf/repeated_ptr_field.h b/src/google/protobuf/repeated_ptr_field.h
index 11ddd15..1666c9c 100644
--- a/src/google/protobuf/repeated_ptr_field.h
+++ b/src/google/protobuf/repeated_ptr_field.h
@@ -251,10 +251,12 @@
   }
 
   // Must be called from destructor.
+  //
+  // Pre-condition: NeedsDestroy() returns true.
   template <typename TypeHandler>
   void Destroy() {
+    ABSL_DCHECK(NeedsDestroy());
     using H = CommonHandler<TypeHandler>;
-    if (arena_ != nullptr) return;
     int n = allocated_size();
     void** elems = elements();
     for (int i = 0; i < n; i++) {
@@ -267,7 +269,10 @@
   }
 
   bool NeedsDestroy() const {
-    return tagged_rep_or_elem_ != nullptr && arena_ == nullptr;
+    // TODO: arena check is redundant once all `RepeatedPtrField`s
+    // with non-null arena are owned by the arena.
+    return tagged_rep_or_elem_ != nullptr &&
+           PROTOBUF_PREDICT_FALSE(arena_ == nullptr);
   }
   void DestroyProtos();  // implemented in the cc file
 
@@ -630,7 +635,9 @@
     }
     this->CopyFrom<TypeHandler>(*other);
     other->InternalSwap(&temp);
-    temp.Destroy<TypeHandler>();  // Frees rep_ if `other` had no arena.
+    if (temp.NeedsDestroy()) {
+      temp.Destroy<TypeHandler>();
+    }
   }
 
   // Gets the Arena on which this RepeatedPtrField stores its elements.
@@ -1374,12 +1381,13 @@
 template <typename Element>
 RepeatedPtrField<Element>::~RepeatedPtrField() {
   StaticValidityCheck();
+  if (!NeedsDestroy()) return;
 #ifdef __cpp_if_constexpr
   if constexpr (std::is_base_of<MessageLite, Element>::value) {
 #else
   if (std::is_base_of<MessageLite, Element>::value) {
 #endif
-    if (NeedsDestroy()) DestroyProtos();
+    DestroyProtos();
   } else {
     Destroy<TypeHandler>();
   }