Automated rollback of commit 9998c63.

*** Reason for rollback ***

Suspected cause of b/217386541

*** Original change description ***

Minor DynamicCodec simplifications.

Serialization order is now based on field offset instead of field name. It's
possible to create dependencies on serialization order, but it seems to be
accidental.

PiperOrigin-RevId: 425683231

src/main/java/com/google/devtools/build/lib/skyframe/serialization/DynamicCodec.java

@@ -14,29 +14,33 @@
 
 package com.google.devtools.build.lib.skyframe.serialization;
 
-import static com.google.devtools.build.lib.unsafe.UnsafeProvider.unsafe;
-
 import com.google.common.flogger.GoogleLogger;
+import com.google.devtools.build.lib.unsafe.UnsafeProvider;
 import com.google.protobuf.CodedInputStream;
 import com.google.protobuf.CodedOutputStream;
 import java.io.IOException;
 import java.lang.reflect.Array;
+import java.lang.reflect.Constructor;
 import java.lang.reflect.Field;
 import java.lang.reflect.Modifier;
 import java.nio.ByteBuffer;
+import java.util.Comparator;
 import java.util.Map;
 import java.util.TreeMap;
+import sun.reflect.ReflectionFactory;
 
 /** A codec that serializes arbitrary types. */
 public final class DynamicCodec implements ObjectCodec<Object> {
 
   private static final GoogleLogger logger = GoogleLogger.forEnclosingClass();
 
   private final Class<?> type;
+  private final Constructor<?> constructor;
   private final TypeAndOffset[] offsets;
 
-  public DynamicCodec(Class<?> type) {
+  public DynamicCodec(Class<?> type) throws ReflectiveOperationException {
     this.type = type;
+    this.constructor = getConstructor(type);
     this.offsets = getOffsets(type);
   }
 
@@ -53,8 +57,8 @@ public MemoizationStrategy getStrategy() {
   @Override
   public void serialize(SerializationContext context, Object obj, CodedOutputStream codedOut)
       throws SerializationException, IOException {
-    for (TypeAndOffset fieldInfo : offsets) {
-      serializeField(context, codedOut, obj, fieldInfo.type, fieldInfo.offset);
+    for (int i = 0; i < offsets.length; ++i) {
+      serializeField(context, codedOut, obj, offsets[i].type, offsets[i].offset);
     }
   }
 
@@ -75,30 +79,32 @@ private void serializeField(
       throws SerializationException, IOException {
     if (type.isPrimitive()) {
       if (type.equals(boolean.class)) {
-        codedOut.writeBoolNoTag(unsafe().getBoolean(obj, offset));
+        codedOut.writeBoolNoTag(UnsafeProvider.getInstance().getBoolean(obj, offset));
       } else if (type.equals(byte.class)) {
-        codedOut.writeRawByte(unsafe().getByte(obj, offset));
+        codedOut.writeRawByte(UnsafeProvider.getInstance().getByte(obj, offset));
       } else if (type.equals(short.class)) {
-        ByteBuffer buffer = ByteBuffer.allocate(2).putShort(unsafe().getShort(obj, offset));
+        ByteBuffer buffer =
+            ByteBuffer.allocate(2).putShort(UnsafeProvider.getInstance().getShort(obj, offset));
         codedOut.writeRawBytes(buffer);
       } else if (type.equals(char.class)) {
-        ByteBuffer buffer = ByteBuffer.allocate(2).putChar(unsafe().getChar(obj, offset));
+        ByteBuffer buffer =
+            ByteBuffer.allocate(2).putChar(UnsafeProvider.getInstance().getChar(obj, offset));
         codedOut.writeRawBytes(buffer);
       } else if (type.equals(int.class)) {
-        codedOut.writeInt32NoTag(unsafe().getInt(obj, offset));
+        codedOut.writeInt32NoTag(UnsafeProvider.getInstance().getInt(obj, offset));
       } else if (type.equals(long.class)) {
-        codedOut.writeInt64NoTag(unsafe().getLong(obj, offset));
+        codedOut.writeInt64NoTag(UnsafeProvider.getInstance().getLong(obj, offset));
       } else if (type.equals(float.class)) {
-        codedOut.writeFloatNoTag(unsafe().getFloat(obj, offset));
+        codedOut.writeFloatNoTag(UnsafeProvider.getInstance().getFloat(obj, offset));
       } else if (type.equals(double.class)) {
-        codedOut.writeDoubleNoTag(unsafe().getDouble(obj, offset));
+        codedOut.writeDoubleNoTag(UnsafeProvider.getInstance().getDouble(obj, offset));
       } else if (type.equals(void.class)) {
         // Does nothing for void type.
       } else {
         throw new UnsupportedOperationException("Unknown primitive type: " + type);
       }
     } else if (type.isArray()) {
-      Object arr = unsafe().getObject(obj, offset);
+      Object arr = UnsafeProvider.getInstance().getObject(obj, offset);
       if (type.getComponentType().equals(byte.class)) {
         if (arr == null) {
           codedOut.writeBoolNoTag(false);
@@ -114,8 +120,8 @@ private void serializeField(
       }
       int length = Array.getLength(arr);
       codedOut.writeInt32NoTag(length);
-      int base = unsafe().arrayBaseOffset(type);
-      int scale = unsafe().arrayIndexScale(type);
+      int base = UnsafeProvider.getInstance().arrayBaseOffset(type);
+      int scale = UnsafeProvider.getInstance().arrayIndexScale(type);
       if (scale == 0) {
         throw new SerializationException("Failed to get index scale for type: " + type);
       }
@@ -125,7 +131,7 @@ private void serializeField(
       }
     } else {
       try {
-        context.serialize(unsafe().getObject(obj, offset), codedOut);
+        context.serialize(UnsafeProvider.getInstance().getObject(obj, offset), codedOut);
       } catch (SerializationException e) {
         logger.atSevere().withCause(e).log(
             "Unserializable object and superclass: %s %s", obj, obj.getClass().getSuperclass());
@@ -140,13 +146,13 @@ public Object deserialize(DeserializationContext context, CodedInputStream coded
       throws SerializationException, IOException {
     Object instance;
     try {
-      instance = unsafe().allocateInstance(type);
+      instance = constructor.newInstance();
     } catch (ReflectiveOperationException e) {
       throw new SerializationException("Could not instantiate object of type: " + type, e);
     }
     context.registerInitialValue(instance);
-    for (TypeAndOffset fieldInfo : offsets) {
-      deserializeField(context, codedIn, instance, fieldInfo.type, fieldInfo.offset);
+    for (int i = 0; i < offsets.length; ++i) {
+      deserializeField(context, codedIn, instance, offsets[i].type, offsets[i].offset);
     }
     return instance;
   }
@@ -168,23 +174,23 @@ private void deserializeField(
       throws SerializationException, IOException {
     if (fieldType.isPrimitive()) {
       if (fieldType.equals(boolean.class)) {
-        unsafe().putBoolean(obj, offset, codedIn.readBool());
+        UnsafeProvider.getInstance().putBoolean(obj, offset, codedIn.readBool());
       } else if (fieldType.equals(byte.class)) {
-        unsafe().putByte(obj, offset, codedIn.readRawByte());
+        UnsafeProvider.getInstance().putByte(obj, offset, codedIn.readRawByte());
       } else if (fieldType.equals(short.class)) {
         ByteBuffer buffer = ByteBuffer.allocate(2).put(codedIn.readRawBytes(2));
-        unsafe().putShort(obj, offset, buffer.getShort(0));
+        UnsafeProvider.getInstance().putShort(obj, offset, buffer.getShort(0));
       } else if (fieldType.equals(char.class)) {
         ByteBuffer buffer = ByteBuffer.allocate(2).put(codedIn.readRawBytes(2));
-        unsafe().putChar(obj, offset, buffer.getChar(0));
+        UnsafeProvider.getInstance().putChar(obj, offset, buffer.getChar(0));
       } else if (fieldType.equals(int.class)) {
-        unsafe().putInt(obj, offset, codedIn.readInt32());
+        UnsafeProvider.getInstance().putInt(obj, offset, codedIn.readInt32());
       } else if (fieldType.equals(long.class)) {
-        unsafe().putLong(obj, offset, codedIn.readInt64());
+        UnsafeProvider.getInstance().putLong(obj, offset, codedIn.readInt64());
       } else if (fieldType.equals(float.class)) {
-        unsafe().putFloat(obj, offset, codedIn.readFloat());
+        UnsafeProvider.getInstance().putFloat(obj, offset, codedIn.readFloat());
       } else if (fieldType.equals(double.class)) {
-        unsafe().putDouble(obj, offset, codedIn.readDouble());
+        UnsafeProvider.getInstance().putDouble(obj, offset, codedIn.readDouble());
       } else if (fieldType.equals(void.class)) {
         // Does nothing for void type.
       } else {
@@ -193,19 +199,20 @@ private void deserializeField(
       }
     } else if (fieldType.isArray()) {
       if (fieldType.getComponentType().equals(byte.class)) {
-        if (codedIn.readBool()) {
-          unsafe().putObject(obj, offset, codedIn.readByteArray());
-        } // Otherwise, it was null.
+        boolean isNonNull = codedIn.readBool();
+        UnsafeProvider.getInstance()
+            .putObject(obj, offset, isNonNull ? codedIn.readByteArray() : null);
         return;
       }
       int length = codedIn.readInt32();
       if (length < 0) {
+        UnsafeProvider.getInstance().putObject(obj, offset, null);
         return;
       }
       Object arr = Array.newInstance(fieldType.getComponentType(), length);
-      unsafe().putObject(obj, offset, arr);
-      int base = unsafe().arrayBaseOffset(fieldType);
-      int scale = unsafe().arrayIndexScale(fieldType);
+      UnsafeProvider.getInstance().putObject(obj, offset, arr);
+      int base = UnsafeProvider.getInstance().arrayBaseOffset(fieldType);
+      int scale = UnsafeProvider.getInstance().arrayIndexScale(fieldType);
       if (scale == 0) {
         throw new SerializationException(
             "Failed to get index scale for field type " + fieldType + " for " + type);
@@ -225,10 +232,7 @@ private void deserializeField(
         e.addTrail(this.type);
         throw e;
       }
-      if (fieldValue == null) {
-        return;
-      }
-      if (!fieldType.isInstance(fieldValue)) {
+      if (fieldValue != null && !fieldType.isInstance(fieldValue)) {
         throw new SerializationException(
             "Field "
                 + fieldValue
@@ -239,38 +243,59 @@ private void deserializeField(
                 + ") for "
                 + type);
       }
-      unsafe().putObject(obj, offset, fieldValue);
+      UnsafeProvider.getInstance().putObject(obj, offset, fieldValue);
     }
   }
 
   private static <T> TypeAndOffset[] getOffsets(Class<T> type) {
-    TreeMap<Long, Class<?>> offsets = new TreeMap<>();
+    TreeMap<Field, Long> offsets = new TreeMap<>(new FieldComparator());
     for (Class<? super T> next = type; next != null; next = next.getSuperclass()) {
       for (Field field : next.getDeclaredFields()) {
         if ((field.getModifiers() & (Modifier.STATIC | Modifier.TRANSIENT)) != 0) {
           continue; // Skips static or transient fields.
         }
-        if (offsets.put(unsafe().objectFieldOffset(field), field.getType()) != null) {
-          throw new IllegalStateException("Field number collision in: " + type);
-        }
+        field.setAccessible(true);
+        offsets.put(field, UnsafeProvider.getInstance().objectFieldOffset(field));
       }
     }
-    // Converts to an array to avoid iterator allocation when iterating.
+    // Converts to an array to make it easy to avoid the use of iterators.
     TypeAndOffset[] offsetsArr = new TypeAndOffset[offsets.size()];
     int i = 0;
-    for (Map.Entry<Long, Class<?>> entry : offsets.entrySet()) {
-      offsetsArr[i++] = new TypeAndOffset(entry.getValue(), entry.getKey());
+    for (Map.Entry<Field, Long> entry : offsets.entrySet()) {
+      offsetsArr[i] = new TypeAndOffset(entry.getKey().getType(), entry.getValue());
+      ++i;
     }
     return offsetsArr;
   }
 
   private static final class TypeAndOffset {
-    private final Class<?> type;
-    private final long offset;
+    public final Class<?> type;
+    public final long offset;
 
-    private TypeAndOffset(Class<?> type, long offset) {
+    TypeAndOffset(Class<?> type, long offset) {
       this.type = type;
       this.offset = offset;
     }
   }
+
+  private static Constructor<?> getConstructor(Class<?> type) throws ReflectiveOperationException {
+    Constructor<?> constructor =
+        ReflectionFactory.getReflectionFactory()
+            .newConstructorForSerialization(type, Object.class.getDeclaredConstructor());
+    constructor.setAccessible(true);
+    return constructor;
+  }
+
+  private static final class FieldComparator implements Comparator<Field> {
+
+    @Override
+    public int compare(Field f1, Field f2) {
+      int classCompare =
+          f1.getDeclaringClass().getName().compareTo(f2.getDeclaringClass().getName());
+      if (classCompare != 0) {
+        return classCompare;
+      }
+      return f1.getName().compareTo(f2.getName());
+    }
+  }
 }

src/main/java/com/google/devtools/build/lib/skyframe/serialization/EnumMapCodec.java

@@ -14,13 +14,13 @@
 
 package com.google.devtools.build.lib.skyframe.serialization;
 
-import static com.google.devtools.build.lib.unsafe.UnsafeProvider.unsafe;
-
+import com.google.devtools.build.lib.unsafe.UnsafeProvider;
 import com.google.protobuf.CodedInputStream;
 import com.google.protobuf.CodedOutputStream;
 import java.io.IOException;
 import java.util.EnumMap;
 import java.util.Map;
+import sun.misc.Unsafe;
 
 /**
  * Serialize {@link EnumMap}. Subclasses of {@link EnumMap} will crash at runtime because currently
@@ -33,7 +33,8 @@ class EnumMapCodec<E extends Enum<E>, V> implements ObjectCodec<EnumMap<E, V>> {
 
   EnumMapCodec() {
     try {
-      classTypeOffset = unsafe().objectFieldOffset(EnumMap.class.getDeclaredField("keyType"));
+      classTypeOffset =
+          UnsafeProvider.getInstance().objectFieldOffset(EnumMap.class.getDeclaredField("keyType"));
     } catch (NoSuchFieldException e) {
       throw new IllegalStateException("Couldn't get keyType field fron EnumMap", e);
     }
@@ -55,7 +56,8 @@ public void serialize(SerializationContext context, EnumMap<E, V> obj, CodedOutp
     codedOut.writeInt32NoTag(obj.size());
     if (obj.isEmpty()) {
       // Do gross hack to get key type of map, since we have no concrete element to examine.
-      context.serialize(unsafe().getObject(obj, classTypeOffset), codedOut);
+      Unsafe unsafe = UnsafeProvider.getInstance();
+      context.serialize(unsafe.getObject(obj, classTypeOffset), codedOut);
       return;
     }
     context.serialize(obj.keySet().iterator().next().getDeclaringClass(), codedOut);

src/main/java/com/google/devtools/build/lib/skyframe/serialization/autocodec/AutoCodecProcessor.java

@@ -508,7 +508,7 @@ private MethodSpec buildSerializeMethodWithInstantiator(
         }
         TypeKind typeKind = parameter.asType().getKind();
         serializeBuilder.addStatement(
-            "$T unsafe_$L = ($T) $T.unsafe().get$L(input, $L_offset)",
+            "$T unsafe_$L = ($T) $T.getInstance().get$L(input, $L_offset)",
             sanitizeTypeParameter(parameter.asType(), env),
             parameter.getSimpleName(),
             sanitizeTypeParameter(parameter.asType(), env),
@@ -676,7 +676,7 @@ private void initializeUnsafeOffsets(
           TypeName.LONG, param.getSimpleName() + "_offset", Modifier.PRIVATE, Modifier.FINAL);
       constructor.beginControlFlow("try");
       constructor.addStatement(
-          "this.$L_offset = $T.unsafe().objectFieldOffset($T.class.getDeclaredField(\"$L\"))",
+          "this.$L_offset = $T.getInstance().objectFieldOffset($T.class.getDeclaredField(\"$L\"))",
           param.getSimpleName(),
           UnsafeProvider.class,
           ClassName.get(field.get().declaringClassType),

src/main/java/com/google/devtools/build/lib/unsafe/StringUnsafe.java

@@ -13,11 +13,10 @@
 // limitations under the License.
 package com.google.devtools.build.lib.unsafe;
 
-import static com.google.devtools.build.lib.unsafe.UnsafeProvider.unsafe;
-
 import java.lang.reflect.Constructor;
 import java.lang.reflect.Field;
 import java.util.Arrays;
+import sun.misc.Unsafe;
 
 /**
  * Provides direct access to the string implementation used by JDK9.
@@ -35,6 +34,7 @@ public final class StringUnsafe {
 
   private static final StringUnsafe INSTANCE = new StringUnsafe();
 
+  private final Unsafe unsafe;
   private final Constructor<String> constructor;
   private final long valueOffset;
   private final long coderOffset;
@@ -44,6 +44,7 @@ public static StringUnsafe getInstance() {
   }
 
   private StringUnsafe() {
+    unsafe = UnsafeProvider.getInstance();
     Field valueField;
     Field coderField;
     try {
@@ -59,13 +60,15 @@ private StringUnsafe() {
           e);
     }
     this.constructor.setAccessible(true);
-    valueOffset = unsafe().objectFieldOffset(valueField);
-    coderOffset = unsafe().objectFieldOffset(coderField);
+    valueField.setAccessible(true);
+    valueOffset = UnsafeProvider.getInstance().objectFieldOffset(valueField);
+    coderField.setAccessible(true);
+    coderOffset = UnsafeProvider.getInstance().objectFieldOffset(coderField);
   }
 
   /** Returns the coder used for this string. See {@link #LATIN1} and {@link #UTF16}. */
   public byte getCoder(String obj) {
-    return unsafe().getByte(obj, coderOffset);
+    return unsafe.getByte(obj, coderOffset);
   }
 
   /**
@@ -75,7 +78,7 @@ public byte getCoder(String obj) {
    * Ensure you do not mutate this byte array in any way.
    */
   public byte[] getByteArray(String obj) {
-    return (byte[]) unsafe().getObject(obj, valueOffset);
+    return (byte[]) unsafe.getObject(obj, valueOffset);
   }
 
   /** Constructs a new string from a byte array and coder. */

src/main/java/com/google/devtools/build/lib/unsafe/UnsafeProvider.java

@@ -30,7 +30,7 @@ public class UnsafeProvider {
 
   private static final Unsafe UNSAFE = getUnsafe();
 
-  public static Unsafe unsafe() {
+  public static Unsafe getInstance() {
     return UNSAFE;
   }