Optimize ImmutableCollection.toArray(T[]) as per https://shipilev.net…

…/blog/2016/arrays-wisdom-ancients/, the key goal being to avoid the necessity of zeroing a newly created array where possible. Addresses #3209. RELNOTES=n/a ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=207635563
google · Aug 9, 2018 · f29f5d3 · f29f5d3
1 parent bd3a471
commit f29f5d3
Show file tree

Hide file tree

Showing 15 changed files with 231 additions and 14 deletions.
diff --git a/android/guava/src/com/google/common/collect/ImmutableCollection.java b/android/guava/src/com/google/common/collect/ImmutableCollection.java
@@ -171,21 +171,20 @@ public abstract class ImmutableCollection<E> extends AbstractCollection<E> imple
 
   @Override
   public final Object[] toArray() {
-    int size = size();
-    if (size == 0) {
-      return EMPTY_ARRAY;
-    }
-    Object[] result = new Object[size];
-    copyIntoArray(result, 0);
-    return result;
+    return toArray(EMPTY_ARRAY);
   }
 
   @CanIgnoreReturnValue
   @Override
   public final <T> T[] toArray(T[] other) {
     checkNotNull(other);
     int size = size();
+
     if (other.length < size) {
+      Object[] internal = internalArray();
+      if (internal != null) {
+        return Platform.copy(internal, internalArrayStart(), internalArrayEnd(), other);
+      }
       other = ObjectArrays.newArray(other, size);
     } else if (other.length > size) {
       other[size] = null;
@@ -194,6 +193,27 @@ public final <T> T[] toArray(T[] other) {
     return other;
   }
 
+  /** If this collection is backed by an array of its elements in insertion order, returns it. */
+  Object[] internalArray() {
+    return null;
+  }
+
+  /**
+   * If this collection is backed by an array of its elements in insertion order, returns the offset
+   * where this collection's elements start.
+   */
+  int internalArrayStart() {
+    throw new UnsupportedOperationException();
+  }
+
+  /**
+   * If this collection is backed by an array of its elements in insertion order, returns the offset
+   * where this collection's elements end.
+   */
+  int internalArrayEnd() {
+    throw new UnsupportedOperationException();
+  }
+
   @Override
   public abstract boolean contains(@NullableDecl Object object);
 

diff --git a/android/guava/src/com/google/common/collect/ImmutableList.java b/android/guava/src/com/google/common/collect/ImmutableList.java
@@ -444,6 +444,21 @@ public int size() {
       return length;
     }
 
+    @Override
+    Object[] internalArray() {
+      return ImmutableList.this.internalArray();
+    }
+
+    @Override
+    int internalArrayStart() {
+      return ImmutableList.this.internalArrayStart() + offset;
+    }
+
+    @Override
+    int internalArrayEnd() {
+      return ImmutableList.this.internalArrayStart() + offset + length;
+    }
+
     @Override
     public E get(int index) {
       checkElementIndex(index, length);

diff --git a/android/guava/src/com/google/common/collect/Platform.java b/android/guava/src/com/google/common/collect/Platform.java
@@ -18,6 +18,7 @@
 
 import com.google.common.annotations.GwtCompatible;
 import java.lang.reflect.Array;
+import java.util.Arrays;
 import java.util.Map;
 import java.util.Set;
 
@@ -86,6 +87,11 @@ static <T> T[] newArray(T[] reference, int length) {
     return result;
   }
 
+  /** Equivalent to Arrays.copyOfRange(source, from, to, arrayOfType.getClass()). */
+  static <T> T[] copy(Object[] source, int from, int to, T[] arrayOfType) {
+    return Arrays.copyOfRange(source, from, to, (Class<? extends T[]>) arrayOfType.getClass());
+  }
+
   /**
    * Configures the given map maker to use weak keys, if possible; does nothing otherwise (i.e., in
    * GWT). This is sometimes acceptable, when only server-side code could generate enough volume

diff --git a/android/guava/src/com/google/common/collect/RegularImmutableAsList.java b/android/guava/src/com/google/common/collect/RegularImmutableAsList.java
@@ -65,6 +65,21 @@ int copyIntoArray(Object[] dst, int offset) {
     return delegateList.copyIntoArray(dst, offset);
   }
 
+  @Override
+  Object[] internalArray() {
+    return delegateList.internalArray();
+  }
+
+  @Override
+  int internalArrayStart() {
+    return delegateList.internalArrayStart();
+  }
+
+  @Override
+  int internalArrayEnd() {
+    return delegateList.internalArrayEnd();
+  }
+
   @Override
   public E get(int index) {
     return delegateList.get(index);

diff --git a/android/guava/src/com/google/common/collect/RegularImmutableList.java b/android/guava/src/com/google/common/collect/RegularImmutableList.java
@@ -49,6 +49,21 @@ boolean isPartialView() {
     return false;
   }
 
+  @Override
+  Object[] internalArray() {
+    return array;
+  }
+
+  @Override
+  int internalArrayStart() {
+    return 0;
+  }
+
+  @Override
+  int internalArrayEnd() {
+    return size;
+  }
+
   @Override
   int copyIntoArray(Object[] dst, int dstOff) {
     System.arraycopy(array, 0, dst, dstOff, size);

diff --git a/android/guava/src/com/google/common/collect/RegularImmutableSet.java b/android/guava/src/com/google/common/collect/RegularImmutableSet.java
@@ -74,6 +74,21 @@ public UnmodifiableIterator<E> iterator() {
     return asList().iterator();
   }
 
+  @Override
+  Object[] internalArray() {
+    return elements;
+  }
+
+  @Override
+  int internalArrayStart() {
+    return 0;
+  }
+
+  @Override
+  int internalArrayEnd() {
+    return size;
+  }
+
   @Override
   int copyIntoArray(Object[] dst, int offset) {
     System.arraycopy(elements, 0, dst, offset, size);

diff --git a/android/guava/src/com/google/common/collect/RegularImmutableSortedSet.java b/android/guava/src/com/google/common/collect/RegularImmutableSortedSet.java
@@ -49,6 +49,21 @@ final class RegularImmutableSortedSet<E> extends ImmutableSortedSet<E> {
     this.elements = elements;
   }
 
+  @Override
+  Object[] internalArray() {
+    return elements.internalArray();
+  }
+
+  @Override
+  int internalArrayStart() {
+    return elements.internalArrayStart();
+  }
+
+  @Override
+  int internalArrayEnd() {
+    return elements.internalArrayEnd();
+  }
+
   @Override
   public UnmodifiableIterator<E> iterator() {
     return elements.iterator();

diff --git a/...-super/com/google/common/collect/super/com/google/common/collect/ImmutableCollection.java b/...-super/com/google/common/collect/super/com/google/common/collect/ImmutableCollection.java
@@ -89,6 +89,28 @@ ImmutableList<E> createAsList() {
     }
   }
 
+  /** If this collection is backed by an array of its elements in insertion order, returns it. */
+  @Nullable
+  Object[] internalArray() {
+    return null;
+  }
+
+  /**
+   * If this collection is backed by an array of its elements in insertion order, returns the offset
+   * where this collection's elements start.
+   */
+  int internalArrayStart() {
+    throw new UnsupportedOperationException();
+  }
+
+  /**
+   * If this collection is backed by an array of its elements in insertion order, returns the offset
+   * where this collection's elements end.
+   */
+  int internalArrayEnd() {
+    throw new UnsupportedOperationException();
+  }
+
   static <E> ImmutableCollection<E> unsafeDelegate(Collection<E> delegate) {
     return new ForwardingImmutableCollection<E>(delegate);
   }

diff --git a/guava-gwt/src-super/com/google/common/collect/super/com/google/common/collect/Platform.java b/guava-gwt/src-super/com/google/common/collect/super/com/google/common/collect/Platform.java
@@ -73,6 +73,13 @@ private static void resizeArray(Object array, int newSize) {
     ((NativeArray) array).setLength(newSize);
   }
 
+  /** Equivalent to Arrays.copyOfRange(source, from, to, arrayOfType.getClass()). */
+  static <T> T[] copy(Object[] source, int from, int to, T[] arrayOfType) {
+    T[] result = newArray(arrayOfType, to - from);
+    System.arraycopy(source, from, result, 0, to - from);
+    return result;
+  }
+
   // TODO(user): Move this logic to a utility class.
   @JsType(isNative = true, name = "Array", namespace = JsPackage.GLOBAL)
   private interface NativeArray {

diff --git a/guava/src/com/google/common/collect/ImmutableCollection.java b/guava/src/com/google/common/collect/ImmutableCollection.java
@@ -183,21 +183,20 @@ public Spliterator<E> spliterator() {
 
   @Override
   public final Object[] toArray() {
-    int size = size();
-    if (size == 0) {
-      return EMPTY_ARRAY;
-    }
-    Object[] result = new Object[size];
-    copyIntoArray(result, 0);
-    return result;
+    return toArray(EMPTY_ARRAY);
   }
 
   @CanIgnoreReturnValue
   @Override
   public final <T> T[] toArray(T[] other) {
     checkNotNull(other);
     int size = size();
+
     if (other.length < size) {
+      Object[] internal = internalArray();
+      if (internal != null) {
+        return Platform.copy(internal, internalArrayStart(), internalArrayEnd(), other);
+      }
       other = ObjectArrays.newArray(other, size);
     } else if (other.length > size) {
       other[size] = null;
@@ -206,6 +205,28 @@ public final <T> T[] toArray(T[] other) {
     return other;
   }
 
+  /** If this collection is backed by an array of its elements in insertion order, returns it. */
+  @Nullable
+  Object[] internalArray() {
+    return null;
+  }
+
+  /**
+   * If this collection is backed by an array of its elements in insertion order, returns the offset
+   * where this collection's elements start.
+   */
+  int internalArrayStart() {
+    throw new UnsupportedOperationException();
+  }
+
+  /**
+   * If this collection is backed by an array of its elements in insertion order, returns the offset
+   * where this collection's elements end.
+   */
+  int internalArrayEnd() {
+    throw new UnsupportedOperationException();
+  }
+
   @Override
   public abstract boolean contains(@Nullable Object object);
 

diff --git a/guava/src/com/google/common/collect/Platform.java b/guava/src/com/google/common/collect/Platform.java
@@ -18,6 +18,7 @@
 
 import com.google.common.annotations.GwtCompatible;
 import java.lang.reflect.Array;
+import java.util.Arrays;
 import java.util.Map;
 import java.util.Set;
 
@@ -86,6 +87,11 @@ static <T> T[] newArray(T[] reference, int length) {
     return result;
   }
 
+  /** Equivalent to Arrays.copyOfRange(source, from, to, arrayOfType.getClass()). */
+  static <T> T[] copy(Object[] source, int from, int to, T[] arrayOfType) {
+    return Arrays.copyOfRange(source, from, to, (Class<? extends T[]>) arrayOfType.getClass());
+  }
+
   /**
    * Configures the given map maker to use weak keys, if possible; does nothing otherwise (i.e., in
    * GWT). This is sometimes acceptable, when only server-side code could generate enough volume

diff --git a/guava/src/com/google/common/collect/RegularImmutableAsList.java b/guava/src/com/google/common/collect/RegularImmutableAsList.java
@@ -68,6 +68,21 @@ int copyIntoArray(Object[] dst, int offset) {
     return delegateList.copyIntoArray(dst, offset);
   }
 
+  @Override
+  Object[] internalArray() {
+    return delegateList.internalArray();
+  }
+
+  @Override
+  int internalArrayStart() {
+    return delegateList.internalArrayStart();
+  }
+
+  @Override
+  int internalArrayEnd() {
+    return delegateList.internalArrayEnd();
+  }
+
   @Override
   public E get(int index) {
     return delegateList.get(index);

diff --git a/guava/src/com/google/common/collect/RegularImmutableList.java b/guava/src/com/google/common/collect/RegularImmutableList.java
@@ -47,6 +47,21 @@ boolean isPartialView() {
     return false;
   }
 
+  @Override
+  Object[] internalArray() {
+    return array;
+  }
+
+  @Override
+  int internalArrayStart() {
+    return 0;
+  }
+
+  @Override
+  int internalArrayEnd() {
+    return array.length;
+  }
+
   @Override
   int copyIntoArray(Object[] dst, int dstOff) {
     System.arraycopy(array, 0, dst, dstOff, array.length);

diff --git a/guava/src/com/google/common/collect/RegularImmutableSet.java b/guava/src/com/google/common/collect/RegularImmutableSet.java
@@ -79,6 +79,21 @@ public Spliterator<E> spliterator() {
     return Spliterators.spliterator(elements, SPLITERATOR_CHARACTERISTICS);
   }
 
+  @Override
+  Object[] internalArray() {
+    return elements;
+  }
+
+  @Override
+  int internalArrayStart() {
+    return 0;
+  }
+
+  @Override
+  int internalArrayEnd() {
+    return elements.length;
+  }
+
   @Override
   int copyIntoArray(Object[] dst, int offset) {
     System.arraycopy(elements, 0, dst, offset, elements.length);

diff --git a/guava/src/com/google/common/collect/RegularImmutableSortedSet.java b/guava/src/com/google/common/collect/RegularImmutableSortedSet.java
@@ -50,6 +50,21 @@ final class RegularImmutableSortedSet<E> extends ImmutableSortedSet<E> {
     this.elements = elements;
   }
 
+  @Override
+  Object[] internalArray() {
+    return elements.internalArray();
+  }
+
+  @Override
+  int internalArrayStart() {
+    return elements.internalArrayStart();
+  }
+
+  @Override
+  int internalArrayEnd() {
+    return elements.internalArrayEnd();
+  }
+
   @Override
   public UnmodifiableIterator<E> iterator() {
     return elements.iterator();