Backoff autovalue optimization if any of the properties is an array.

This is in preparation for reusing the ValueType abstraction to optimize Java record types.

PiperOrigin-RevId: 863273459

Author: rluble

jre/java/javaemul/internal/ValueType.java

@@ -15,7 +15,7 @@
  */
 package javaemul.internal;
 
-import java.util.Arrays;
+import java.util.Objects;
 import java.util.StringJoiner;
 import javaemul.internal.annotations.DoNotAutobox;
 import jsinterop.annotations.JsMethod;
@@ -69,33 +69,14 @@ static boolean equals(ValueType thisObject, Object o) {
     for (String p : thisKeys) {
       Object p1 = JsUtils.getProperty(thisObject, p);
       Object p2 = JsUtils.getProperty(thatObject, p);
-      if (!isAutoValueEqual(p1, p2)) {
+      if (!Objects.equals(p1, p2)) {
         return false;
       }
     }
 
     return true;
   }
 
-  private static boolean isAutoValueEqual(Object a, Object b) {
-    if (a == b) {
-      return true;
-    }
-    if (a == null || b == null) {
-      return false;
-    }
-
-    if (ArrayStamper.isStamped(a) && ArrayStamper.isStamped(b)) {
-      // AutoValue only supports primitive arrays so if the array is stamped we can safely assume
-      // this is a primitive array. So here we just check that they have same types and compare the
-      // contents.
-      return a.getClass() == b.getClass()
-          && Arrays.equals(JsUtils.<Object[]>uncheckedCast(a), JsUtils.<Object[]>uncheckedCast(b));
-    }
-
-    return a.equals(b);
-  }
-
   // Package private to ensure clinit is run when called from types that use this class as a mixin.
   static int hashCode(ValueType thisObject) {
     int hashCode = 1;
@@ -104,12 +85,7 @@ static int hashCode(ValueType thisObject) {
       if (value == null) {
         continue;
       }
-      // AutoValue supports primitives arrays, that needs special handling for hashCode.
-      int h =
-          ArrayStamper.isStamped(value)
-              ? Arrays.hashCode(JsUtils.<Object[]>uncheckedCast(value))
-              : value.hashCode();
-      hashCode = (1000003 * hashCode) ^ h;
+      hashCode = (1000003 * hashCode) ^ value.hashCode();
     }
     return hashCode;
   }

transpiler/java/com/google/j2cl/transpiler/passes/OptimizeAutoValue.java

@@ -189,6 +189,10 @@ public <T extends TypeDescriptor> T apply(T t) {
 
   private static boolean canBeInlinedTo(Type type) {
     if (AstUtils.isAnnotatedWithAutoValueBuilder(type.getDeclaration())) {
+      if (!isOptimizableAutoValue(type.getDeclaration().getEnclosingTypeDeclaration())) {
+        // If the enclosing @AutoValue is not optimizable, the builder is also not optimizable.
+        return false;
+      }
       // Note that AutoValue.Builder will generate default ctor so would be only safe to inline
       // the implementation if user didn't declare non-empty one.
       // Most complete logic for safety here would be cross-checking all generated ctors against
@@ -197,7 +201,17 @@ private static boolean canBeInlinedTo(Type type) {
       Method method = type.getDefaultConstructor();
       return method == null || method.isEmpty();
     }
-    return AstUtils.isAnnotatedWithAutoValue(type.getDeclaration());
+    return isOptimizableAutoValue(type.getDeclaration());
+  }
+
+  private static boolean isOptimizableAutoValue(TypeDeclaration typeDeclaration) {
+    return AstUtils.isAnnotatedWithAutoValue(typeDeclaration)
+        // Do not optimize @AutoValue types that have array properties since ValueType does not
+        // support array valued @AutoValue property semantics.
+        && typeDeclaration.toDescriptor().getPolymorphicMethods().stream()
+            .filter(MethodDescriptor::isAbstract)
+            .map(MethodDescriptor::getReturnTypeDescriptor)
+            .noneMatch(TypeDescriptor::isArray);
   }
 
   private static void inlineMembers(Type from, Type to) {
@@ -381,6 +395,8 @@ private void optimizeAsValueTypes(Library library) {
               int mask = removeJavaLangObjectMethods(autoValue);
               if (mask == 0) {
                 // No method removed/needs optimization. Leave the type alone.
+                // Note that this also applies when the type can not be inlined to as it wouldn't
+                // have any of the generated `j.l.Object` methods.
                 return;
               }
 

transpiler/javatests/com/google/j2cl/autovalue/AutoValueTest.java

@@ -193,23 +193,20 @@ public void testStrangeGetters() {
   abstract static class GettersAndConcreteNonGetters {
     abstract int getFoo();
 
-    @SuppressWarnings("mutable")
-    abstract byte[] getBytes();
-
-    boolean hasNoBytes() {
-      return getBytes().length == 0;
+    boolean fooIsZero() {
+      return getFoo() == 0;
     }
 
-    static GettersAndConcreteNonGetters create(int foo, byte[] bytes) {
-      return new AutoValue_AutoValueTest_GettersAndConcreteNonGetters(foo, bytes);
+    static GettersAndConcreteNonGetters create(int foo) {
+      return new AutoValue_AutoValueTest_GettersAndConcreteNonGetters(foo);
     }
   }
 
   @Test
   public void testGettersAndConcreteNonGetters() {
-    GettersAndConcreteNonGetters instance = GettersAndConcreteNonGetters.create(23, new byte[] {1});
-    assertFalse(instance.hasNoBytes());
-    String expectedString = "GettersAndConcreteNonGetters{foo=23, bytes=[1]}";
+    GettersAndConcreteNonGetters instance = GettersAndConcreteNonGetters.create(23);
+    assertFalse(instance.fooIsZero());
+    String expectedString = "GettersAndConcreteNonGetters{foo=23}";
     assertThat(instance.toString()).isEqualTo(expectedString);
   }
 
@@ -1036,8 +1033,7 @@ public void testPrimitiveArrays() {
     // EqualsTester also exercises hashCode(). We clone the arrays above to ensure that using the
     // default Object.hashCode() will fail.
 
-    String expectedString =
-        "PrimitiveArrays{booleans=[" + booleans + "], " + "ints=[" + ints + "]}";
+    String expectedString = "{" + Arrays.toString(booleans) + ", " + Arrays.toString(ints) + "}";
     assertThat(object1.toString()).isEqualTo(expectedString);
     assertThat(object1.ints()).isSameInstanceAs(object1.ints());
   }
@@ -1052,7 +1048,7 @@ public void testNullablePrimitiveArrays() {
         PrimitiveArrays.create(Arrays.copyOf(booleans, booleans.length), null);
     new EqualsTester().addEqualityGroup(object1, object2).addEqualityGroup(object0).testEquals();
 
-    String expectedString = "PrimitiveArrays{booleans=[" + booleans + "], " + "ints=null}";
+    String expectedString = "{" + Arrays.toString(booleans) + ", " + "null}";
     assertThat(object1.toString()).isEqualTo(expectedString);
 
     assertThat(object1.booleans()).isSameInstanceAs(object1.booleans());
@@ -1862,9 +1858,6 @@ public abstract static class BuilderWithUnprefixedGetters<T extends Comparable<T
     @Nullable
     public abstract T t();
 
-    @SuppressWarnings("mutable")
-    public abstract int[] ints();
-
     public abstract int noGetter();
 
     public abstract String oAuth();
@@ -1881,8 +1874,6 @@ public interface Builder<T extends Comparable<T>> {
 
       Builder<T> setT(T t);
 
-      Builder<T> setInts(int[] ints);
-
       Builder<T> setNoGetter(int x);
 
       Builder<T> setoAuth(String x); // this ugly spelling is for compatibility
@@ -1893,8 +1884,6 @@ public interface Builder<T extends Comparable<T>> {
 
       T t();
 
-      int[] ints();
-
       String oAuth();
 
       String oBrien();
@@ -1906,7 +1895,6 @@ public interface Builder<T extends Comparable<T>> {
   @Test
   public void testBuilderWithUnprefixedGetter() {
     ImmutableList<String> names = ImmutableList.of("fred", "jim");
-    int[] ints = {6, 28, 496, 8128, 33550336};
     int noGetter = -1;
 
     BuilderWithUnprefixedGetters.Builder<String> builder = BuilderWithUnprefixedGetters.builder();
@@ -1917,30 +1905,17 @@ public void testBuilderWithUnprefixedGetter() {
     } catch (IllegalStateException e) {
       assertThat(e).hasMessageThat().isNull();
     }
-    try {
-      builder.ints();
-      fail("Attempt to retrieve unset ints property should have failed");
-    } catch (IllegalStateException e) {
-      assertThat(e).hasMessageThat().isNull();
-    }
 
     builder.setList(names);
     assertThat(builder.list()).isSameInstanceAs(names);
-    builder.setInts(ints);
-    assertThat(builder.ints()).isEqualTo(ints);
     builder.setoAuth("OAuth");
     assertThat(builder.oAuth()).isEqualTo("OAuth");
     builder.setOBrien("Flann");
     assertThat(builder.oBrien()).isEqualTo("Flann");
-    // The array is not cloned by the getter, so the client can modify it (but shouldn't).
-    ints[0] = 0;
-    assertThat(builder.ints()[0]).isEqualTo(0);
-    ints[0] = 6;
 
     BuilderWithUnprefixedGetters<String> instance = builder.setNoGetter(noGetter).build();
     assertThat(instance.list()).isSameInstanceAs(names);
     assertThat(instance.t()).isNull();
-    assertThat(instance.ints()).isEqualTo(ints);
     assertThat(instance.noGetter()).isEqualTo(noGetter);
     assertThat(instance.oAuth()).isEqualTo("OAuth");
     assertThat(instance.oBrien()).isEqualTo("Flann");

transpiler/javatests/com/google/j2cl/readable/java/autovalue/output_closure/ArrayField$Builder.impl.java.js.txt

@@ -4,42 +4,23 @@ const j_l_Object = goog.require('java.lang.Object$impl');
 const $Util = goog.require('nativebootstrap.Util$impl');
 
 let ArrayField = goog.forwardDeclare('autovalue.ArrayField$impl');
-let IllegalStateException = goog.forwardDeclare('java.lang.IllegalStateException$impl');
-let Objects = goog.forwardDeclare('java.util.Objects$impl');
-let $Equality = goog.forwardDeclare('nativebootstrap.Equality$impl');
-let $Exceptions = goog.forwardDeclare('vmbootstrap.Exceptions$impl');
 
+/**
+ * @abstract
+ */
 class Builder extends j_l_Object {
  /** @protected @nodts */
  constructor() {
   super();
-  /**@type {Array<number>} @nodts*/
-  this.f_arrayField__autovalue_AutoValue_ArrayField_Builder_;
- }
- /** @nodts @return {!Builder} */
- static $create__() {
-  Builder.$clinit();
-  let $instance = new Builder();
-  $instance.$ctor__autovalue_ArrayField_Builder__void();
-  return $instance;
  }
  /** @nodts */
  $ctor__autovalue_ArrayField_Builder__void() {
   this.$ctor__java_lang_Object__void();
  }
- /** @nodts @return {Builder} */
- m_setArrayField__arrayOf_int__autovalue_ArrayField_Builder(/** Array<number> */ arrayField) {
-  Objects.m_requireNonNull__java_lang_Object__java_lang_Object(arrayField);
-  this.f_arrayField__autovalue_AutoValue_ArrayField_Builder_ = arrayField;
-  return this;
- }
- /** @nodts @return {ArrayField} */
- m_build__autovalue_ArrayField_$pp_autovalue() {
-  if ($Equality.$same(this.f_arrayField__autovalue_AutoValue_ArrayField_Builder_, null)) {
-   throw $Exceptions.toJs(IllegalStateException.$create__());
-  }
-  return ArrayField.$create__arrayOf_int(this.f_arrayField__autovalue_AutoValue_ArrayField_Builder_);
- }
+ /** @abstract @nodts @return {Builder} */
+ m_setArrayField__arrayOf_int__autovalue_ArrayField_Builder(/** Array<number> */ arrayField) {}
+ /** @abstract @nodts @return {ArrayField} */
+ m_build__autovalue_ArrayField_$pp_autovalue() {}
  /** @nodts */
  static $clinit() {
   Builder.$clinit = () =>{};
@@ -52,13 +33,7 @@ class Builder extends j_l_Object {
  }
 
  /** @nodts */
- static $loadModules() {
-  ArrayField = goog.module.get('autovalue.ArrayField$impl');
-  IllegalStateException = goog.module.get('java.lang.IllegalStateException$impl');
-  Objects = goog.module.get('java.util.Objects$impl');
-  $Equality = goog.module.get('nativebootstrap.Equality$impl');
-  $Exceptions = goog.module.get('vmbootstrap.Exceptions$impl');
- }
+ static $loadModules() {}
 }
 $Util.$setClassMetadata(Builder, 'autovalue.ArrayField$Builder');
 

transpiler/javatests/com/google/j2cl/readable/java/autovalue/output_closure/ArrayField$Builder.java.js.txt

@@ -1,12 +1,8 @@
 goog.module('autovalue.ArrayField.Builder');
 
 goog.require('autovalue.ArrayField');
-goog.require('java.lang.IllegalStateException');
 goog.require('java.lang.Object');
-goog.require('java.util.Objects');
-goog.require('nativebootstrap.Equality');
 goog.require('nativebootstrap.Util');
-goog.require('vmbootstrap.Exceptions');
 
 const Builder = goog.require('autovalue.ArrayField.Builder$impl');
 exports = Builder;

transpiler/javatests/com/google/j2cl/readable/java/autovalue/output_closure/ArrayField.impl.java.js.txt

@@ -1,37 +1,27 @@
 goog.module('autovalue.ArrayField$impl');
 
-const ValueType = goog.require('javaemul.internal.ValueType$impl');
+const j_l_Object = goog.require('java.lang.Object$impl');
 const $Util = goog.require('nativebootstrap.Util$impl');
 
-class ArrayField extends ValueType {
+/**
+ * @abstract
+ */
+class ArrayField extends j_l_Object {
  /** @protected @nodts */
  constructor() {
   super();
-  /**@type {Array<number>} @nodts*/
-  this.f_arrayField__autovalue_AutoValue_ArrayField_;
- }
- /** @nodts @return {!ArrayField} */
- static $create__arrayOf_int(/** Array<number> */ arrayField) {
-  ArrayField.$clinit();
-  let $instance = new ArrayField();
-  $instance.$ctor__autovalue_ArrayField__arrayOf_int__void(arrayField);
-  return $instance;
  }
  /** @nodts */
- $ctor__autovalue_ArrayField__arrayOf_int__void(/** Array<number> */ arrayField) {
-  this.$ctor__javaemul_internal_ValueType__void();
-  this.f_arrayField__autovalue_AutoValue_ArrayField_ = arrayField;
-  $J2CL_PRESERVE$(this.f_arrayField__autovalue_AutoValue_ArrayField_);
- }
- /** @nodts @return {Array<number>} */
- m_getArrayField__arrayOf_int() {
-  return this.f_arrayField__autovalue_AutoValue_ArrayField_;
+ $ctor__autovalue_ArrayField__void() {
+  this.$ctor__java_lang_Object__void();
  }
+ /** @abstract @nodts @return {Array<number>} */
+ m_getArrayField__arrayOf_int() {}
  /** @nodts */
  static $clinit() {
   ArrayField.$clinit = () =>{};
   ArrayField.$loadModules();
-  ValueType.$clinit();
+  j_l_Object.$clinit();
  }
  /** @nodts @return {boolean} */
  static $isInstance(/** ? */ instance) {

transpiler/javatests/com/google/j2cl/readable/java/autovalue/output_closure/ArrayField.java.js.txt

@@ -1,6 +1,6 @@
 goog.module('autovalue.ArrayField');
 
-goog.require('javaemul.internal.ValueType');
+goog.require('java.lang.Object');
 goog.require('nativebootstrap.Util');
 
 const ArrayField = goog.require('autovalue.ArrayField$impl');