Skip to content

Commit e3cc31a

Browse files
mhansencopybara-github
authored andcommitted
Reserve capacity in ProtobufArrayList when calling Builder.addAllRepeatedMessage(Collection)
This avoids some extra copies and garbage generation. There's still the extra default 10-sized backing array created by default in ProtobufArrayList which isn't fixed yet, which we see in allocation tests. That's from `ensureXIsMutable()` which allocates a new list without awareness of the list's size. Maybe we can fix that later: b/362848901. PiperOrigin-RevId: 670766317
1 parent bd1887e commit e3cc31a

File tree

2 files changed

+30
-6
lines changed

2 files changed

+30
-6
lines changed

java/core/src/main/java/com/google/protobuf/AbstractMessageLite.java

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -340,8 +340,14 @@ private String getReadingExceptionMessage(String target) {
340340

341341
// We check nulls as we iterate to avoid iterating over values twice.
342342
private static <T> void addAllCheckingNulls(Iterable<T> values, List<? super T> list) {
343-
if (list instanceof ArrayList && values instanceof Collection) {
344-
((ArrayList<T>) list).ensureCapacity(list.size() + ((Collection<T>) values).size());
343+
if (values instanceof Collection) {
344+
int growth = ((Collection<T>) values).size();
345+
if (list instanceof ArrayList) {
346+
((ArrayList<T>) list).ensureCapacity(list.size() + growth);
347+
}
348+
if (list instanceof ProtobufArrayList) {
349+
((ProtobufArrayList<T>) list).ensureCapacity(list.size() + growth);
350+
}
345351
}
346352
int begin = list.size();
347353
if (values instanceof List && values instanceof RandomAccess) {

java/core/src/main/java/com/google/protobuf/ProtobufArrayList.java

Lines changed: 22 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -52,8 +52,7 @@ public boolean add(E element) {
5252
ensureIsMutable();
5353

5454
if (size == array.length) {
55-
// Resize to 1.5x the size
56-
int length = ((size * 3) / 2) + 1;
55+
int length = growSize(size);
5756
E[] newArray = Arrays.copyOf(array, length);
5857

5958
array = newArray;
@@ -65,6 +64,11 @@ public boolean add(E element) {
6564
return true;
6665
}
6766

67+
private static int growSize(int previousSize) {
68+
// Resize to 1.5x the size
69+
return ((previousSize * 3) / 2) + 1;
70+
}
71+
6872
@Override
6973
public void add(int index, E element) {
7074
ensureIsMutable();
@@ -77,8 +81,7 @@ public void add(int index, E element) {
7781
// Shift everything over to make room
7882
System.arraycopy(array, index, array, index + 1, size - index);
7983
} else {
80-
// Resize to 1.5x the size
81-
int length = ((size * 3) / 2) + 1;
84+
int length = growSize(size);
8285
E[] newArray = createArray(length);
8386

8487
// Copy the first part directly
@@ -132,6 +135,21 @@ public int size() {
132135
return size;
133136
}
134137

138+
/** Ensures the backing array can fit at least minCapacity elements. */
139+
void ensureCapacity(int minCapacity) {
140+
if (minCapacity <= array.length) {
141+
return;
142+
}
143+
// To avoid quadratic copying when calling .addAllFoo(List) in a loop, we must not size to
144+
// exactly the requested capacity, but must exponentially grow instead. This is similar
145+
// behaviour to ArrayList.
146+
int n = size;
147+
while (n < minCapacity) {
148+
n = growSize(n);
149+
}
150+
array = Arrays.copyOf(array, n);
151+
}
152+
135153
@SuppressWarnings("unchecked")
136154
private static <E> E[] createArray(int capacity) {
137155
return (E[]) new Object[capacity];

0 commit comments

Comments
 (0)