From 05a8a4012351b07010afbc76892fe7919d0cb964 Mon Sep 17 00:00:00 2001 From: Mark Hansen Date: Wed, 11 Sep 2024 18:11:19 -0700 Subject: [PATCH] Protobuf Lite ArrayLists: Defer allocating backing array until we have some idea how much to allocate. This avoids allocating a backing array of size 10 when we are adding >10 elements. - If we are adding objects one at a time, inflate a Object[10] and continue. - If we are adding objects from a Collection, assume this is the only collection we are adding, and inflate a `Object[collection.size()]` - If the existing array is non-empty, resize the array by 1.5x exponential growth as usual. There's another small change where if we're addAll(<10 elements), we allocate an exact-sized backing array (e.g. size=3), rather than rounding up to size 10. See android/LiteAllocationTest. I think this is good: this will save memory in the common case of just calling .addAll() once, and if we call addAll twice, we grow still exponentially. But we could decide to avoid this or split it out into its own change. This change involves moving some logic out of GeneratedMessageLite. I think the default size of the backing array is better handled inside ProtobufArrayList than inside GeneratedMessageLite's wrapper function. PiperOrigin-RevId: 673612155 --- .../com/google/protobuf/BooleanArrayList.java | 18 ++++++++++---- .../com/google/protobuf/DoubleArrayList.java | 18 ++++++++++---- .../com/google/protobuf/FloatArrayList.java | 18 ++++++++++---- .../google/protobuf/GeneratedMessageLite.java | 18 +++++--------- .../com/google/protobuf/IntArrayList.java | 18 ++++++++++---- .../com/google/protobuf/LongArrayList.java | 18 ++++++++++---- .../com/google/protobuf/MessageSchema.java | 4 +--- .../google/protobuf/ProtobufArrayList.java | 24 +++++++++++++------ 8 files changed, 89 insertions(+), 47 deletions(-) diff --git a/java/core/src/main/java/com/google/protobuf/BooleanArrayList.java b/java/core/src/main/java/com/google/protobuf/BooleanArrayList.java index 353486db8311e..3b8c72704b165 100644 --- a/java/core/src/main/java/com/google/protobuf/BooleanArrayList.java +++ b/java/core/src/main/java/com/google/protobuf/BooleanArrayList.java @@ -8,6 +8,7 @@ package com.google.protobuf; import static com.google.protobuf.Internal.checkNotNull; +import static java.lang.Math.max; import com.google.protobuf.Internal.BooleanList; import java.util.Arrays; @@ -22,7 +23,9 @@ final class BooleanArrayList extends AbstractProtobufList implements BooleanList, RandomAccess, PrimitiveNonBoxingCollection { - private static final BooleanArrayList EMPTY_LIST = new BooleanArrayList(new boolean[0], 0, false); + private static final boolean[] EMPTY_ARRAY = new boolean[0]; + + private static final BooleanArrayList EMPTY_LIST = new BooleanArrayList(EMPTY_ARRAY, 0, false); public static BooleanArrayList emptyList() { return EMPTY_LIST; @@ -39,7 +42,7 @@ public static BooleanArrayList emptyList() { /** Constructs a new mutable {@code BooleanArrayList} with default capacity. */ BooleanArrayList() { - this(new boolean[DEFAULT_CAPACITY], 0, true); + this(EMPTY_ARRAY, 0, true); } /** @@ -101,7 +104,8 @@ public BooleanList mutableCopyWithCapacity(int capacity) { if (capacity < size) { throw new IllegalArgumentException(); } - return new BooleanArrayList(Arrays.copyOf(array, capacity), size, true); + boolean[] newArray = capacity == 0 ? EMPTY_ARRAY : Arrays.copyOf(array, capacity); + return new BooleanArrayList(newArray, size, true); } @Override @@ -258,6 +262,10 @@ void ensureCapacity(int minCapacity) { if (minCapacity <= array.length) { return; } + if (array.length == 0) { + array = new boolean[max(minCapacity, DEFAULT_CAPACITY)]; + return; + } // To avoid quadratic copying when calling .addAllFoo(List) in a loop, we must not size to // exactly the requested capacity, but must exponentially grow instead. This is similar // behaviour to ArrayList. @@ -269,8 +277,8 @@ void ensureCapacity(int minCapacity) { } private static int growSize(int previousSize) { - // Resize to 1.5x the size - return ((previousSize * 3) / 2) + 1; + // Resize to 1.5x the size, rounding up to DEFAULT_CAPACITY. + return max(((previousSize * 3) / 2) + 1, DEFAULT_CAPACITY); } /** diff --git a/java/core/src/main/java/com/google/protobuf/DoubleArrayList.java b/java/core/src/main/java/com/google/protobuf/DoubleArrayList.java index 1c7b009dc7d28..57d30d8737641 100644 --- a/java/core/src/main/java/com/google/protobuf/DoubleArrayList.java +++ b/java/core/src/main/java/com/google/protobuf/DoubleArrayList.java @@ -8,6 +8,7 @@ package com.google.protobuf; import static com.google.protobuf.Internal.checkNotNull; +import static java.lang.Math.max; import com.google.protobuf.Internal.DoubleList; import java.util.Arrays; @@ -22,7 +23,9 @@ final class DoubleArrayList extends AbstractProtobufList implements DoubleList, RandomAccess, PrimitiveNonBoxingCollection { - private static final DoubleArrayList EMPTY_LIST = new DoubleArrayList(new double[0], 0, false); + private static final double[] EMPTY_ARRAY = new double[0]; + + private static final DoubleArrayList EMPTY_LIST = new DoubleArrayList(EMPTY_ARRAY, 0, false); public static DoubleArrayList emptyList() { return EMPTY_LIST; @@ -39,7 +42,7 @@ public static DoubleArrayList emptyList() { /** Constructs a new mutable {@code DoubleArrayList} with default capacity. */ DoubleArrayList() { - this(new double[DEFAULT_CAPACITY], 0, true); + this(EMPTY_ARRAY, 0, true); } /** @@ -101,7 +104,8 @@ public DoubleList mutableCopyWithCapacity(int capacity) { if (capacity < size) { throw new IllegalArgumentException(); } - return new DoubleArrayList(Arrays.copyOf(array, capacity), size, true); + double[] newArray = capacity == 0 ? EMPTY_ARRAY : Arrays.copyOf(array, capacity); + return new DoubleArrayList(newArray, size, true); } @Override @@ -258,6 +262,10 @@ void ensureCapacity(int minCapacity) { if (minCapacity <= array.length) { return; } + if (array.length == 0) { + array = new double[max(minCapacity, DEFAULT_CAPACITY)]; + return; + } // To avoid quadratic copying when calling .addAllFoo(List) in a loop, we must not size to // exactly the requested capacity, but must exponentially grow instead. This is similar // behaviour to ArrayList. @@ -269,8 +277,8 @@ void ensureCapacity(int minCapacity) { } private static int growSize(int previousSize) { - // Resize to 1.5x the size - return ((previousSize * 3) / 2) + 1; + // Resize to 1.5x the size, rounding up to DEFAULT_CAPACITY. + return max(((previousSize * 3) / 2) + 1, DEFAULT_CAPACITY); } /** diff --git a/java/core/src/main/java/com/google/protobuf/FloatArrayList.java b/java/core/src/main/java/com/google/protobuf/FloatArrayList.java index a9479ad657c5c..7cb35db1eeb5e 100644 --- a/java/core/src/main/java/com/google/protobuf/FloatArrayList.java +++ b/java/core/src/main/java/com/google/protobuf/FloatArrayList.java @@ -8,6 +8,7 @@ package com.google.protobuf; import static com.google.protobuf.Internal.checkNotNull; +import static java.lang.Math.max; import com.google.protobuf.Internal.FloatList; import java.util.Arrays; @@ -22,7 +23,9 @@ final class FloatArrayList extends AbstractProtobufList implements FloatList, RandomAccess, PrimitiveNonBoxingCollection { - private static final FloatArrayList EMPTY_LIST = new FloatArrayList(new float[0], 0, false); + private static final float[] EMPTY_ARRAY = new float[0]; + + private static final FloatArrayList EMPTY_LIST = new FloatArrayList(EMPTY_ARRAY, 0, false); public static FloatArrayList emptyList() { return EMPTY_LIST; @@ -39,7 +42,7 @@ public static FloatArrayList emptyList() { /** Constructs a new mutable {@code FloatArrayList} with default capacity. */ FloatArrayList() { - this(new float[DEFAULT_CAPACITY], 0, true); + this(EMPTY_ARRAY, 0, true); } /** @@ -100,7 +103,8 @@ public FloatList mutableCopyWithCapacity(int capacity) { if (capacity < size) { throw new IllegalArgumentException(); } - return new FloatArrayList(Arrays.copyOf(array, capacity), size, true); + float[] newArray = capacity == 0 ? EMPTY_ARRAY : Arrays.copyOf(array, capacity); + return new FloatArrayList(newArray, size, true); } @Override @@ -257,6 +261,10 @@ void ensureCapacity(int minCapacity) { if (minCapacity <= array.length) { return; } + if (array.length == 0) { + array = new float[max(minCapacity, DEFAULT_CAPACITY)]; + return; + } // To avoid quadratic copying when calling .addAllFoo(List) in a loop, we must not size to // exactly the requested capacity, but must exponentially grow instead. This is similar // behaviour to ArrayList. @@ -268,8 +276,8 @@ void ensureCapacity(int minCapacity) { } private static int growSize(int previousSize) { - // Resize to 1.5x the size - return ((previousSize * 3) / 2) + 1; + // Resize to 1.5x the size, rounding up to DEFAULT_CAPACITY. + return max(((previousSize * 3) / 2) + 1, DEFAULT_CAPACITY); } /** diff --git a/java/core/src/main/java/com/google/protobuf/GeneratedMessageLite.java b/java/core/src/main/java/com/google/protobuf/GeneratedMessageLite.java index d7d7a8c3ffeee..5dbac067dbebb 100644 --- a/java/core/src/main/java/com/google/protobuf/GeneratedMessageLite.java +++ b/java/core/src/main/java/com/google/protobuf/GeneratedMessageLite.java @@ -1496,8 +1496,7 @@ protected static IntList emptyIntList() { protected static IntList mutableCopy(IntList list) { int size = list.size(); - return list.mutableCopyWithCapacity( - size == 0 ? AbstractProtobufList.DEFAULT_CAPACITY : size * 2); + return list.mutableCopyWithCapacity(size * 2); } protected static LongList emptyLongList() { @@ -1506,8 +1505,7 @@ protected static LongList emptyLongList() { protected static LongList mutableCopy(LongList list) { int size = list.size(); - return list.mutableCopyWithCapacity( - size == 0 ? AbstractProtobufList.DEFAULT_CAPACITY : size * 2); + return list.mutableCopyWithCapacity(size * 2); } protected static FloatList emptyFloatList() { @@ -1516,8 +1514,7 @@ protected static FloatList emptyFloatList() { protected static FloatList mutableCopy(FloatList list) { int size = list.size(); - return list.mutableCopyWithCapacity( - size == 0 ? AbstractProtobufList.DEFAULT_CAPACITY : size * 2); + return list.mutableCopyWithCapacity(size * 2); } protected static DoubleList emptyDoubleList() { @@ -1526,8 +1523,7 @@ protected static DoubleList emptyDoubleList() { protected static DoubleList mutableCopy(DoubleList list) { int size = list.size(); - return list.mutableCopyWithCapacity( - size == 0 ? AbstractProtobufList.DEFAULT_CAPACITY : size * 2); + return list.mutableCopyWithCapacity(size * 2); } protected static BooleanList emptyBooleanList() { @@ -1536,8 +1532,7 @@ protected static BooleanList emptyBooleanList() { protected static BooleanList mutableCopy(BooleanList list) { int size = list.size(); - return list.mutableCopyWithCapacity( - size == 0 ? AbstractProtobufList.DEFAULT_CAPACITY : size * 2); + return list.mutableCopyWithCapacity(size * 2); } protected static ProtobufList emptyProtobufList() { @@ -1546,8 +1541,7 @@ protected static ProtobufList emptyProtobufList() { protected static ProtobufList mutableCopy(ProtobufList list) { int size = list.size(); - return list.mutableCopyWithCapacity( - size == 0 ? AbstractProtobufList.DEFAULT_CAPACITY : size * 2); + return list.mutableCopyWithCapacity(size * 2); } /** diff --git a/java/core/src/main/java/com/google/protobuf/IntArrayList.java b/java/core/src/main/java/com/google/protobuf/IntArrayList.java index f821669489938..fe654e0ba14cb 100644 --- a/java/core/src/main/java/com/google/protobuf/IntArrayList.java +++ b/java/core/src/main/java/com/google/protobuf/IntArrayList.java @@ -8,6 +8,7 @@ package com.google.protobuf; import static com.google.protobuf.Internal.checkNotNull; +import static java.lang.Math.max; import com.google.protobuf.Internal.IntList; import java.util.Arrays; @@ -22,7 +23,9 @@ final class IntArrayList extends AbstractProtobufList implements IntList, RandomAccess, PrimitiveNonBoxingCollection { - private static final IntArrayList EMPTY_LIST = new IntArrayList(new int[0], 0, false); + private static final int[] EMPTY_ARRAY = new int[0]; + + private static final IntArrayList EMPTY_LIST = new IntArrayList(EMPTY_ARRAY, 0, false); public static IntArrayList emptyList() { return EMPTY_LIST; @@ -39,7 +42,7 @@ public static IntArrayList emptyList() { /** Constructs a new mutable {@code IntArrayList} with default capacity. */ IntArrayList() { - this(new int[DEFAULT_CAPACITY], 0, true); + this(EMPTY_ARRAY, 0, true); } /** @@ -100,7 +103,8 @@ public IntList mutableCopyWithCapacity(int capacity) { if (capacity < size) { throw new IllegalArgumentException(); } - return new IntArrayList(Arrays.copyOf(array, capacity), size, true); + int[] newArray = capacity == 0 ? EMPTY_ARRAY : Arrays.copyOf(array, capacity); + return new IntArrayList(newArray, size, true); } @Override @@ -257,6 +261,10 @@ void ensureCapacity(int minCapacity) { if (minCapacity <= array.length) { return; } + if (array.length == 0) { + array = new int[max(minCapacity, DEFAULT_CAPACITY)]; + return; + } // To avoid quadratic copying when calling .addAllFoo(List) in a loop, we must not size to // exactly the requested capacity, but must exponentially grow instead. This is similar // behaviour to ArrayList. @@ -268,8 +276,8 @@ void ensureCapacity(int minCapacity) { } private static int growSize(int previousSize) { - // Resize to 1.5x the size - return ((previousSize * 3) / 2) + 1; + // Resize to 1.5x the size, rounding up to DEFAULT_CAPACITY. + return max(((previousSize * 3) / 2) + 1, DEFAULT_CAPACITY); } /** diff --git a/java/core/src/main/java/com/google/protobuf/LongArrayList.java b/java/core/src/main/java/com/google/protobuf/LongArrayList.java index 417ee1f7add5d..f3c14a5a5482b 100644 --- a/java/core/src/main/java/com/google/protobuf/LongArrayList.java +++ b/java/core/src/main/java/com/google/protobuf/LongArrayList.java @@ -8,6 +8,7 @@ package com.google.protobuf; import static com.google.protobuf.Internal.checkNotNull; +import static java.lang.Math.max; import com.google.protobuf.Internal.LongList; import java.util.Arrays; @@ -22,7 +23,9 @@ final class LongArrayList extends AbstractProtobufList implements LongList, RandomAccess, PrimitiveNonBoxingCollection { - private static final LongArrayList EMPTY_LIST = new LongArrayList(new long[0], 0, false); + private static final long[] EMPTY_ARRAY = new long[0]; + + private static final LongArrayList EMPTY_LIST = new LongArrayList(EMPTY_ARRAY, 0, false); public static LongArrayList emptyList() { return EMPTY_LIST; @@ -39,7 +42,7 @@ public static LongArrayList emptyList() { /** Constructs a new mutable {@code LongArrayList} with default capacity. */ LongArrayList() { - this(new long[DEFAULT_CAPACITY], 0, true); + this(EMPTY_ARRAY, 0, true); } /** @@ -100,7 +103,8 @@ public LongList mutableCopyWithCapacity(int capacity) { if (capacity < size) { throw new IllegalArgumentException(); } - return new LongArrayList(Arrays.copyOf(array, capacity), size, true); + long[] newArray = capacity == 0 ? EMPTY_ARRAY : Arrays.copyOf(array, capacity); + return new LongArrayList(newArray, size, true); } @Override @@ -257,6 +261,10 @@ void ensureCapacity(int minCapacity) { if (minCapacity <= array.length) { return; } + if (array.length == 0) { + array = new long[max(minCapacity, DEFAULT_CAPACITY)]; + return; + } // To avoid quadratic copying when calling .addAllFoo(List) in a loop, we must not size to // exactly the requested capacity, but must exponentially grow instead. This is similar // behaviour to ArrayList. @@ -268,8 +276,8 @@ void ensureCapacity(int minCapacity) { } private static int growSize(int previousSize) { - // Resize to 1.5x the size - return ((previousSize * 3) / 2) + 1; + // Resize to 1.5x the size, rounding up to DEFAULT_CAPACITY. + return max(((previousSize * 3) / 2) + 1, DEFAULT_CAPACITY); } /** diff --git a/java/core/src/main/java/com/google/protobuf/MessageSchema.java b/java/core/src/main/java/com/google/protobuf/MessageSchema.java index 3649639d310f6..d8c63b9b955f0 100644 --- a/java/core/src/main/java/com/google/protobuf/MessageSchema.java +++ b/java/core/src/main/java/com/google/protobuf/MessageSchema.java @@ -3577,9 +3577,7 @@ private int parseRepeatedField( ProtobufList list = (ProtobufList) UNSAFE.getObject(message, fieldOffset); if (!list.isModifiable()) { final int size = list.size(); - list = - list.mutableCopyWithCapacity( - size == 0 ? AbstractProtobufList.DEFAULT_CAPACITY : size * 2); + list = list.mutableCopyWithCapacity(size * 2); UNSAFE.putObject(message, fieldOffset, list); } switch (fieldType) { diff --git a/java/core/src/main/java/com/google/protobuf/ProtobufArrayList.java b/java/core/src/main/java/com/google/protobuf/ProtobufArrayList.java index 193c42200b54c..9407760f4f9e2 100644 --- a/java/core/src/main/java/com/google/protobuf/ProtobufArrayList.java +++ b/java/core/src/main/java/com/google/protobuf/ProtobufArrayList.java @@ -7,6 +7,8 @@ package com.google.protobuf; +import static java.lang.Math.max; + import com.google.protobuf.Internal.ProtobufList; import java.util.Arrays; import java.util.RandomAccess; @@ -14,8 +16,10 @@ /** Implements {@link ProtobufList} for non-primitive and {@link String} types. */ final class ProtobufArrayList extends AbstractProtobufList implements RandomAccess { + private static final Object[] EMPTY_ARRAY = new Object[0]; + private static final ProtobufArrayList EMPTY_LIST = - new ProtobufArrayList(new Object[0], 0, false); + new ProtobufArrayList<>(EMPTY_ARRAY, 0, false); @SuppressWarnings("unchecked") // Guaranteed safe by runtime. public static ProtobufArrayList emptyList() { @@ -27,7 +31,7 @@ public static ProtobufArrayList emptyList() { @SuppressWarnings("unchecked") ProtobufArrayList() { - this((E[]) new Object[DEFAULT_CAPACITY], 0, true); + this((E[]) EMPTY_ARRAY, 0, true); } private ProtobufArrayList(E[] array, int size, boolean isMutable) { @@ -36,13 +40,14 @@ private ProtobufArrayList(E[] array, int size, boolean isMutable) { this.size = size; } + @SuppressWarnings("unchecked") // Casting an empty Object[] to a generic array is safe. @Override public ProtobufArrayList mutableCopyWithCapacity(int capacity) { if (capacity < size) { throw new IllegalArgumentException(); } - E[] newArray = Arrays.copyOf(array, capacity); + E[] newArray = capacity == 0 ? (E[]) EMPTY_ARRAY : Arrays.copyOf(array, capacity); return new ProtobufArrayList(newArray, size, true); } @@ -52,7 +57,7 @@ public boolean add(E element) { ensureIsMutable(); if (size == array.length) { - int length = growSize(size); + int length = growSize(array.length); E[] newArray = Arrays.copyOf(array, length); array = newArray; @@ -65,8 +70,8 @@ public boolean add(E element) { } private static int growSize(int previousSize) { - // Resize to 1.5x the size - return ((previousSize * 3) / 2) + 1; + // Resize to 1.5x the size, rounding up to DEFAULT_CAPACITY. + return max(((previousSize * 3) / 2) + 1, DEFAULT_CAPACITY); } @Override @@ -81,7 +86,7 @@ public void add(int index, E element) { // Shift everything over to make room System.arraycopy(array, index, array, index + 1, size - index); } else { - int length = growSize(size); + int length = growSize(array.length); E[] newArray = createArray(length); // Copy the first part directly @@ -136,10 +141,15 @@ public int size() { } /** Ensures the backing array can fit at least minCapacity elements. */ + @SuppressWarnings("unchecked") // Casting an Object[] with no values inside to E[] is safe. void ensureCapacity(int minCapacity) { if (minCapacity <= array.length) { return; } + if (array.length == 0) { + array = (E[]) new Object[max(minCapacity, DEFAULT_CAPACITY)]; + return; + } // To avoid quadratic copying when calling .addAllFoo(List) in a loop, we must not size to // exactly the requested capacity, but must exponentially grow instead. This is similar // behaviour to ArrayList.