Merge "protozero: avoid trivial heap expansions"

include/perfetto/protozero/proto_decoder.h

diff --git a/include/perfetto/protozero/proto_decoder.h b/include/perfetto/protozero/proto_decoder.h
index ce5bb56..c8e46dd 100644
--- a/include/perfetto/protozero/proto_decoder.h
+++ b/include/perfetto/protozero/proto_decoder.h
@@ -357,7 +357,10 @@
       : ProtoDecoder(buffer, length),
         fields_(storage),
         num_fields_(num_fields),
-        size_(std::min(num_fields, capacity)),
+        // The reason for "capacity -1" is to avoid hitting the expansion path
+        // in TypedProtoDecoderBase::ParseAllFields() when we are just setting
+        // fields < INITIAL_STACK_CAPACITY (which is the most common case).
+        size_(std::min(num_fields, capacity - 1)),
         capacity_(capacity) {
     // The reason why Field needs to be trivially de/constructible is to avoid
     // implicit initializers on all the ~1000 entries. We need it to initialize
@@ -368,6 +371,7 @@
                       std::is_trivial<Field>::value,
                   "Field must be a trivial aggregate type");
     memset(fields_, 0, sizeof(Field) * capacity_);
+    PERFETTO_DCHECK(capacity > 0);
   }
 
   void ParseAllFields();
@@ -398,7 +402,7 @@
   uint32_t num_fields_;
 
   // Number of active |fields_| entries. This is initially equal to
-  // min(num_fields_, INITIAL_STACK_CAPACITY) and after ExpandHeapStorage() it
+  // min(num_fields_, INITIAL_STACK_CAPACITY - 1) and after ExpandHeapStorage()
   // becomes == |num_fields_|. If the message has non-packed repeated fields, it
   // can grow further, up to |capacity_|.
   // |size_| is always <= |capacity_|. But |num_fields_| can be > |size_|.

src/protozero/proto_decoder.cc

diff --git a/src/protozero/proto_decoder.cc b/src/protozero/proto_decoder.cc
index c13be9a..a766a28 100644
--- a/src/protozero/proto_decoder.cc
+++ b/src/protozero/proto_decoder.cc
@@ -233,14 +233,17 @@
 
 void TypedProtoDecoderBase::ExpandHeapStorage() {
   // When we expand the heap we must ensure that we have at very last capacity
-  // to deal with all known fields plus at least one repeated field. We go +32
-  // to avoid trivial re-allocations when dealing with repeated fields of a
-  // message that has > INITIAL_STACK_CAPACITY fields.
-  const uint32_t min_capacity = num_fields_ + 32;  // Any number >= +1 will do.
+  // to deal with all known fields plus at least one repeated field. We go +2048
+  // here based on observations on a large 4GB android trace. This is to avoid
+  // trivial re-allocations when dealing with repeated fields of a message that
+  // has > INITIAL_STACK_CAPACITY fields.
+  const uint32_t min_capacity = num_fields_ + 2048;  // Any num >= +1 will do.
   const uint32_t new_capacity = std::max(capacity_ * 2, min_capacity);
   PERFETTO_CHECK(new_capacity > size_ && new_capacity > num_fields_);
   std::unique_ptr<Field[]> new_storage(new Field[new_capacity]);
 
+  static_assert(std::is_trivially_constructible<Field>::value,
+                "Field must be trivially constructible");
   static_assert(std::is_trivially_copyable<Field>::value,
                 "Field must be trivially copyable");