From bd1887e436d2c6cc35db1eede8ebbe1bee1fb78f Mon Sep 17 00:00:00 2001 From: Mark Hansen Date: Tue, 3 Sep 2024 17:22:04 -0700 Subject: [PATCH] Avoid allocating iterators when calling Message.Builder.addAllFoo(RandomAccess List) I've tried to keep the hot part of the loop (not null) in the loop without requiring an extra function call, and only extracted the cold part (null handling) to avoid repetition. PiperOrigin-RevId: 670760103 --- .../google/protobuf/AbstractMessageLite.java | 34 ++++++++++++++----- 1 file changed, 26 insertions(+), 8 deletions(-) diff --git a/java/core/src/main/java/com/google/protobuf/AbstractMessageLite.java b/java/core/src/main/java/com/google/protobuf/AbstractMessageLite.java index dde30c1b7e85..15c783195d9b 100644 --- a/java/core/src/main/java/com/google/protobuf/AbstractMessageLite.java +++ b/java/core/src/main/java/com/google/protobuf/AbstractMessageLite.java @@ -16,6 +16,7 @@ import java.util.ArrayList; import java.util.Collection; import java.util.List; +import java.util.RandomAccess; /** * A partial implementation of the {@link MessageLite} interface which implements as many methods of @@ -343,19 +344,36 @@ private static void addAllCheckingNulls(Iterable values, List ((ArrayList) list).ensureCapacity(list.size() + ((Collection) values).size()); } int begin = list.size(); - for (T value : values) { - if (value == null) { - // encountered a null value so we must undo our modifications prior to throwing - String message = "Element at index " + (list.size() - begin) + " is null."; - for (int i = list.size() - 1; i >= begin; i--) { - list.remove(i); + if (values instanceof List && values instanceof RandomAccess) { + List valuesList = (List) values; + int n = valuesList.size(); + // Optimisation: avoid allocating Iterator for RandomAccess lists. + for (int i = 0; i < n; i++) { + T value = valuesList.get(i); + if (value == null) { + resetListAndThrow(list, begin); + } + list.add(value); + } + } else { + for (T value : values) { + if (value == null) { + resetListAndThrow(list, begin); } - throw new NullPointerException(message); + list.add(value); } - list.add(value); } } + /** Remove elements after index begin from the List and throw NullPointerException. */ + private static void resetListAndThrow(List list, int begin) { + String message = "Element at index " + (list.size() - begin) + " is null."; + for (int i = list.size() - 1; i >= begin; i--) { + list.remove(i); + } + throw new NullPointerException(message); + } + /** Construct an UninitializedMessageException reporting missing fields in the given message. */ protected static UninitializedMessageException newUninitializedMessageException( MessageLite message) {