From 8890b0a81e2f4b1de4a33cc6d81aba07655bde1a Mon Sep 17 00:00:00 2001
From: wangxiaochao <[email protected]>
Date: Thu, 24 Mar 2022 20:54:36 +0800
Subject: [PATCH] Improve performance of parsing unknown fields in Java

Signed-off-by: wangxiaochao <[email protected]>

---
 Makefile.am                                   |   1 +
 .../com/google/protobuf/UnknownFieldSet.java  | 427 +++++++++---------
 .../UnknownFieldSetPerformanceTest.java       |  78 ++++
 .../google/protobuf/UnknownFieldSetTest.java  | 171 ++++++-
 java/lite/pom.xml                             |   3 +-
 5 files changed, 466 insertions(+), 214 deletions(-)
 create mode 100644 java/core/src/test/java/com/google/protobuf/UnknownFieldSetPerformanceTest.java

diff --git a/Makefile.am b/Makefile.am
index 4fc706b..908c2d2 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -487,6 +487,7 @@ java_EXTRA_DIST=
   java/core/src/test/java/com/google/protobuf/TypeRegistryTest.java                \
   java/core/src/test/java/com/google/protobuf/UnknownEnumValueTest.java            \
   java/core/src/test/java/com/google/protobuf/UnknownFieldSetTest.java             \
+  java/core/src/test/java/com/google/protobuf/UnknownFieldSetPerformanceTest.java             \
   java/core/src/test/java/com/google/protobuf/UnmodifiableLazyStringListTest.java  \
   java/core/src/test/java/com/google/protobuf/Utf8Test.java                        \
   java/core/src/test/java/com/google/protobuf/Utf8Utils.java                       \
diff --git a/java/core/src/main/java/com/google/protobuf/UnknownFieldSet.java b/java/core/src/main/java/com/google/protobuf/UnknownFieldSet.java
index ba2f9df..5c482d6 100644
--- a/java/core/src/main/java/com/google/protobuf/UnknownFieldSet.java
+++ b/java/core/src/main/java/com/google/protobuf/UnknownFieldSet.java
@@ -43,13 +43,13 @@ import java.util.Map;
 import java.util.TreeMap;
 
 /**
- * {@code UnknownFieldSet} is used to keep track of fields which were seen when parsing a protocol
+ * {@code UnknownFieldSet} keeps track of fields which were seen when parsing a protocol
  * message but whose field numbers or types are unrecognized. This most frequently occurs when new
  * fields are added to a message type and then messages containing those fields are read by old
  * software that was compiled before the new types were added.
  *
  * <p>Every {@link Message} contains an {@code UnknownFieldSet} (and every {@link Message.Builder}
- * contains an {@link Builder}).
+ * contains a {@link Builder}).
  *
  * <p>Most users will never need to use this class.
  *
@@ -57,9 +57,13 @@ import java.util.TreeMap;
  */
 public final class UnknownFieldSet implements MessageLite {
 
-  private UnknownFieldSet() {
-    fields = null;
-    fieldsDescending = null;
+  private final TreeMap<Integer, Field> fields;
+
+  /**
+   * Construct an {@code UnknownFieldSet} around the given map.
+   */
+  UnknownFieldSet(TreeMap<Integer, Field> fields) {
+    this.fields = fields;
   }
 
   /** Create a new {@link Builder}. */
@@ -68,7 +72,7 @@ public final class UnknownFieldSet implements MessageLite {
   }
 
   /** Create a new {@link Builder} and initialize it to be a copy of {@code copyFrom}. */
-  public static Builder newBuilder(final UnknownFieldSet copyFrom) {
+  public static Builder newBuilder(UnknownFieldSet copyFrom) {
     return newBuilder().mergeFrom(copyFrom);
   }
 
@@ -83,25 +87,11 @@ public final class UnknownFieldSet implements MessageLite {
   }
 
   private static final UnknownFieldSet defaultInstance =
-      new UnknownFieldSet(
-          Collections.<Integer, Field>emptyMap(), Collections.<Integer, Field>emptyMap());
-
-  /**
-   * Construct an {@code UnknownFieldSet} around the given map. The map is expected to be immutable.
-   */
-  UnknownFieldSet(final Map<Integer, Field> fields, final Map<Integer, Field> fieldsDescending) {
-    this.fields = fields;
-    this.fieldsDescending = fieldsDescending;
-  }
-
-  private final Map<Integer, Field> fields;
-
-  /** A copy of {@link #fields} who's iterator order is reversed. */
-  private final Map<Integer, Field> fieldsDescending;
+      new UnknownFieldSet(new TreeMap<Integer, Field>());
 
 
   @Override
-  public boolean equals(final Object other) {
+  public boolean equals(Object other) {
     if (this == other) {
       return true;
     }
@@ -110,29 +100,33 @@ public final class UnknownFieldSet implements MessageLite {
 
   @Override
   public int hashCode() {
+    if (fields.isEmpty()) { // avoid allocation of iterator.
+      // This optimization may not be helpful but it is needed for the allocation tests to pass.
+      return 0;
+    }
     return fields.hashCode();
   }
 
   /** Get a map of fields in the set by number. */
   public Map<Integer, Field> asMap() {
-    return fields;
+    return (Map<Integer, Field>) fields.clone();
   }
 
   /** Check if the given field number is present in the set. */
-  public boolean hasField(final int number) {
+  public boolean hasField(int number) {
     return fields.containsKey(number);
   }
 
   /** Get a field by number. Returns an empty field if not present. Never returns {@code null}. */
-  public Field getField(final int number) {
-    final Field result = fields.get(number);
+  public Field getField(int number) {
+    Field result = fields.get(number);
     return (result == null) ? Field.getDefaultInstance() : result;
   }
 
   /** Serializes the set and writes it to {@code output}. */
   @Override
-  public void writeTo(final CodedOutputStream output) throws IOException {
-    for (final Map.Entry<Integer, Field> entry : fields.entrySet()) {
+  public void writeTo(CodedOutputStream output) throws IOException {
+    for (Map.Entry<Integer, Field> entry : fields.entrySet()) {
       Field field = entry.getValue();
       field.writeTo(entry.getKey(), output);
     }
@@ -154,10 +148,10 @@ public final class UnknownFieldSet implements MessageLite {
   @Override
   public ByteString toByteString() {
     try {
-      final ByteString.CodedBuilder out = ByteString.newCodedBuilder(getSerializedSize());
+      ByteString.CodedBuilder out = ByteString.newCodedBuilder(getSerializedSize());
       writeTo(out.getCodedOutput());
       return out.build();
-    } catch (final IOException e) {
+    } catch (IOException e) {
       throw new RuntimeException(
           "Serializing to a ByteString threw an IOException (should never happen).", e);
     }
@@ -170,12 +164,12 @@ public final class UnknownFieldSet implements MessageLite {
   @Override
   public byte[] toByteArray() {
     try {
-      final byte[] result = new byte[getSerializedSize()];
-      final CodedOutputStream output = CodedOutputStream.newInstance(result);
+      byte[] result = new byte[getSerializedSize()];
+      CodedOutputStream output = CodedOutputStream.newInstance(result);
       writeTo(output);
       output.checkNoSpaceLeft();
       return result;
-    } catch (final IOException e) {
+    } catch (IOException e) {
       throw new RuntimeException(
           "Serializing to a byte array threw an IOException (should never happen).", e);
     }
@@ -186,16 +180,16 @@ public final class UnknownFieldSet implements MessageLite {
    * {@link #writeTo(CodedOutputStream)}.
    */
   @Override
-  public void writeTo(final OutputStream output) throws IOException {
-    final CodedOutputStream codedOutput = CodedOutputStream.newInstance(output);
+  public void writeTo(OutputStream output) throws IOException {
+    CodedOutputStream codedOutput = CodedOutputStream.newInstance(output);
     writeTo(codedOutput);
     codedOutput.flush();
   }
 
   @Override
   public void writeDelimitedTo(OutputStream output) throws IOException {
-    final CodedOutputStream codedOutput = CodedOutputStream.newInstance(output);
-    codedOutput.writeRawVarint32(getSerializedSize());
+    CodedOutputStream codedOutput = CodedOutputStream.newInstance(output);
+    codedOutput.writeUInt32NoTag(getSerializedSize());
     writeTo(codedOutput);
     codedOutput.flush();
   }
@@ -204,15 +198,17 @@ public final class UnknownFieldSet implements MessageLite {
   @Override
   public int getSerializedSize() {
     int result = 0;
-    for (final Map.Entry<Integer, Field> entry : fields.entrySet()) {
-      result += entry.getValue().getSerializedSize(entry.getKey());
+    if (!fields.isEmpty()) {
+      for (Map.Entry<Integer, Field> entry : fields.entrySet()) {
+        result += entry.getValue().getSerializedSize(entry.getKey());
+      }
     }
     return result;
   }
 
   /** Serializes the set and writes it to {@code output} using {@code MessageSet} wire format. */
-  public void writeAsMessageSetTo(final CodedOutputStream output) throws IOException {
-    for (final Map.Entry<Integer, Field> entry : fields.entrySet()) {
+  public void writeAsMessageSetTo(CodedOutputStream output) throws IOException {
+    for (Map.Entry<Integer, Field> entry : fields.entrySet()) {
       entry.getValue().writeAsMessageSetExtensionTo(entry.getKey(), output);
     }
   }
@@ -221,7 +217,7 @@ public final class UnknownFieldSet implements MessageLite {
   void writeTo(Writer writer) throws IOException {
     if (writer.fieldOrder() == Writer.FieldOrder.DESCENDING) {
       // Write fields in descending order.
-      for (Map.Entry<Integer, Field> entry : fieldsDescending.entrySet()) {
+      for (Map.Entry<Integer, Field> entry : fields.descendingMap().entrySet()) {
         entry.getValue().writeTo(entry.getKey(), writer);
       }
     } else {
@@ -233,15 +229,15 @@ public final class UnknownFieldSet implements MessageLite {
   }
 
   /** Serializes the set and writes it to {@code writer} using {@code MessageSet} wire format. */
-  void writeAsMessageSetTo(final Writer writer) throws IOException {
+  void writeAsMessageSetTo(Writer writer) throws IOException {
     if (writer.fieldOrder() == Writer.FieldOrder.DESCENDING) {
       // Write fields in descending order.
-      for (final Map.Entry<Integer, Field> entry : fieldsDescending.entrySet()) {
+      for (Map.Entry<Integer, Field> entry : fields.descendingMap().entrySet()) {
         entry.getValue().writeAsMessageSetExtensionTo(entry.getKey(), writer);
       }
     } else {
       // Write fields in ascending order.
-      for (final Map.Entry<Integer, Field> entry : fields.entrySet()) {
+      for (Map.Entry<Integer, Field> entry : fields.entrySet()) {
         entry.getValue().writeAsMessageSetExtensionTo(entry.getKey(), writer);
       }
     }
@@ -250,7 +246,7 @@ public final class UnknownFieldSet implements MessageLite {
   /** Get the number of bytes required to encode this set using {@code MessageSet} wire format. */
   public int getSerializedSizeAsMessageSet() {
     int result = 0;
-    for (final Map.Entry<Integer, Field> entry : fields.entrySet()) {
+    for (Map.Entry<Integer, Field> entry : fields.entrySet()) {
       result += entry.getValue().getSerializedSizeAsMessageSetExtension(entry.getKey());
     }
     return result;
@@ -264,23 +260,23 @@ public final class UnknownFieldSet implements MessageLite {
   }
 
   /** Parse an {@code UnknownFieldSet} from the given input stream. */
-  public static UnknownFieldSet parseFrom(final CodedInputStream input) throws IOException {
+  public static UnknownFieldSet parseFrom(CodedInputStream input) throws IOException {
     return newBuilder().mergeFrom(input).build();
   }
 
   /** Parse {@code data} as an {@code UnknownFieldSet} and return it. */
-  public static UnknownFieldSet parseFrom(final ByteString data)
+  public static UnknownFieldSet parseFrom(ByteString data)
       throws InvalidProtocolBufferException {
     return newBuilder().mergeFrom(data).build();
   }
 
   /** Parse {@code data} as an {@code UnknownFieldSet} and return it. */
-  public static UnknownFieldSet parseFrom(final byte[] data) throws InvalidProtocolBufferException {
+  public static UnknownFieldSet parseFrom(byte[] data) throws InvalidProtocolBufferException {
     return newBuilder().mergeFrom(data).build();
   }
 
   /** Parse an {@code UnknownFieldSet} from {@code input} and return it. */
-  public static UnknownFieldSet parseFrom(final InputStream input) throws IOException {
+  public static UnknownFieldSet parseFrom(InputStream input) throws IOException {
     return newBuilder().mergeFrom(input).build();
   }
 
@@ -309,64 +305,43 @@ public final class UnknownFieldSet implements MessageLite {
     // This constructor should never be called directly (except from 'create').
     private Builder() {}
 
-    private Map<Integer, Field> fields;
-
-    // Optimization:  We keep around a builder for the last field that was
-    //   modified so that we can efficiently add to it multiple times in a
-    //   row (important when parsing an unknown repeated field).
-    private int lastFieldNumber;
-    private Field.Builder lastField;
+    private TreeMap<Integer, Field.Builder> fieldBuilders = new TreeMap<>();
 
     private static Builder create() {
-      Builder builder = new Builder();
-      builder.reinitialize();
-      return builder;
+      return new Builder();
     }
 
     /**
      * Get a field builder for the given field number which includes any values that already exist.
      */
-    private Field.Builder getFieldBuilder(final int number) {
-      if (lastField != null) {
-        if (number == lastFieldNumber) {
-          return lastField;
-        }
-        // Note:  addField() will reset lastField and lastFieldNumber.
-        addField(lastFieldNumber, lastField.build());
-      }
+    private Field.Builder getFieldBuilder(int number) {
       if (number == 0) {
         return null;
       } else {
-        final Field existing = fields.get(number);
-        lastFieldNumber = number;
-        lastField = Field.newBuilder();
-        if (existing != null) {
-          lastField.mergeFrom(existing);
+        Field.Builder builder = fieldBuilders.get(number);
+        if (builder == null) {
+          builder = Field.newBuilder();
+          fieldBuilders.put(number, builder);
         }
-        return lastField;
+        return builder;
       }
     }
 
     /**
      * Build the {@link UnknownFieldSet} and return it.
-     *
-     * <p>Once {@code build()} has been called, the {@code Builder} will no longer be usable.
-     * Calling any method after {@code build()} will result in undefined behavior and can cause a
-     * {@code NullPointerException} to be thrown.
      */
     @Override
     public UnknownFieldSet build() {
-      getFieldBuilder(0); // Force lastField to be built.
-      final UnknownFieldSet result;
-      if (fields.isEmpty()) {
+      UnknownFieldSet result;
+      if (fieldBuilders.isEmpty()) {
         result = getDefaultInstance();
       } else {
-        Map<Integer, Field> descendingFields = null;
-        descendingFields =
-            Collections.unmodifiableMap(((TreeMap<Integer, Field>) fields).descendingMap());
-        result = new UnknownFieldSet(Collections.unmodifiableMap(fields), descendingFields);
+        TreeMap<Integer, Field> fields = new TreeMap<>();
+        for (Map.Entry<Integer, Field.Builder> entry : fieldBuilders.entrySet()) {
+          fields.put(entry.getKey(), entry.getValue().build());
+        }
+        result = new UnknownFieldSet(fields);
       }
-      fields = null;
       return result;
     }
 
@@ -378,11 +353,13 @@ public final class UnknownFieldSet implements MessageLite {
 
     @Override
     public Builder clone() {
-      getFieldBuilder(0); // Force lastField to be built.
-      Map<Integer, Field> descendingFields = null;
-      descendingFields =
-          Collections.unmodifiableMap(((TreeMap<Integer, Field>) fields).descendingMap());
-      return UnknownFieldSet.newBuilder().mergeFrom(new UnknownFieldSet(fields, descendingFields));
+      Builder clone = UnknownFieldSet.newBuilder();
+      for (Map.Entry<Integer, Field.Builder> entry : fieldBuilders.entrySet()) {
+        Integer key = entry.getKey();
+        Field.Builder value = entry.getValue();
+        clone.fieldBuilders.put(key, value.clone());
+      }
+      return clone;
     }
 
     @Override
@@ -390,31 +367,24 @@ public final class UnknownFieldSet implements MessageLite {
       return UnknownFieldSet.getDefaultInstance();
     }
 
-    private void reinitialize() {
-      fields = Collections.emptyMap();
-      lastFieldNumber = 0;
-      lastField = null;
-    }
-
     /** Reset the builder to an empty set. */
     @Override
     public Builder clear() {
-      reinitialize();
+      fieldBuilders = new TreeMap<>();
       return this;
     }
 
-    /** Clear fields from the set with a given field number. */
-    public Builder clearField(final int number) {
-      if (number == 0) {
-        throw new IllegalArgumentException("Zero is not a valid field number.");
-      }
-      if (lastField != null && lastFieldNumber == number) {
-        // Discard this.
-        lastField = null;
-        lastFieldNumber = 0;
+    /**
+     * Clear fields from the set with a given field number.
+     *
+     * @throws IllegalArgumentException if number is not positive
+     */
+    public Builder clearField(int number) {
+      if (number <= 0) {
+        throw new IllegalArgumentException(number + " is not a valid field number.");
       }
-      if (fields.containsKey(number)) {
-        fields.remove(number);
+      if (fieldBuilders.containsKey(number)) {
+        fieldBuilders.remove(number);
       }
       return this;
     }
@@ -423,9 +393,9 @@ public final class UnknownFieldSet implements MessageLite {
      * Merge the fields from {@code other} into this set. If a field number exists in both sets,
      * {@code other}'s values for that field will be appended to the values in this set.
      */
-    public Builder mergeFrom(final UnknownFieldSet other) {
+    public Builder mergeFrom(UnknownFieldSet other) {
       if (other != getDefaultInstance()) {
-        for (final Map.Entry<Integer, Field> entry : other.fields.entrySet()) {
+        for (Map.Entry<Integer, Field> entry : other.fields.entrySet()) {
           mergeField(entry.getKey(), entry.getValue());
         }
       }
@@ -435,10 +405,12 @@ public final class UnknownFieldSet implements MessageLite {
     /**
      * Add a field to the {@code UnknownFieldSet}. If a field with the same number already exists,
      * the two are merged.
+     *
+     * @throws IllegalArgumentException if number is not positive
      */
-    public Builder mergeField(final int number, final Field field) {
-      if (number == 0) {
-        throw new IllegalArgumentException("Zero is not a valid field number.");
+    public Builder mergeField(int number, final Field field) {
+      if (number <= 0) {
+        throw new IllegalArgumentException(number + " is not a valid field number.");
       }
       if (hasField(number)) {
         getFieldBuilder(number).mergeFrom(field);
@@ -454,10 +426,12 @@ public final class UnknownFieldSet implements MessageLite {
     /**
      * Convenience method for merging a new field containing a single varint value. This is used in
      * particular when an unknown enum value is encountered.
+     *
+     * @throws IllegalArgumentException if number is not positive
      */
-    public Builder mergeVarintField(final int number, final int value) {
-      if (number == 0) {
-        throw new IllegalArgumentException("Zero is not a valid field number.");
+    public Builder mergeVarintField(int number, int value) {
+      if (number <= 0) {
+        throw new IllegalArgumentException(number + " is not a valid field number.");
       }
       getFieldBuilder(number).addVarint(value);
       return this;
@@ -467,40 +441,33 @@ public final class UnknownFieldSet implements MessageLite {
      * Convenience method for merging a length-delimited field.
      *
      * <p>For use by generated code only.
+     *
+     * @throws IllegalArgumentException if number is not positive
      */
-    public Builder mergeLengthDelimitedField(final int number, final ByteString value) {
-      if (number == 0) {
-        throw new IllegalArgumentException("Zero is not a valid field number.");
+    public Builder mergeLengthDelimitedField(int number, ByteString value) {
+      if (number <= 0) {
+        throw new IllegalArgumentException(number + " is not a valid field number.");
       }
       getFieldBuilder(number).addLengthDelimited(value);
       return this;
     }
 
     /** Check if the given field number is present in the set. */
-    public boolean hasField(final int number) {
-      if (number == 0) {
-        throw new IllegalArgumentException("Zero is not a valid field number.");
-      }
-      return number == lastFieldNumber || fields.containsKey(number);
+    public boolean hasField(int number) {
+      return fieldBuilders.containsKey(number);
     }
 
     /**
      * Add a field to the {@code UnknownFieldSet}. If a field with the same number already exists,
      * it is removed.
+     *
+     * @throws IllegalArgumentException if number is not positive
      */
-    public Builder addField(final int number, final Field field) {
-      if (number == 0) {
-        throw new IllegalArgumentException("Zero is not a valid field number.");
-      }
-      if (lastField != null && lastFieldNumber == number) {
-        // Discard this.
-        lastField = null;
-        lastFieldNumber = 0;
+    public Builder addField(int number, Field field) {
+      if (number <= 0) {
+        throw new IllegalArgumentException(number + " is not a valid field number.");
       }
-      if (fields.isEmpty()) {
-        fields = new TreeMap<Integer, Field>();
-      }
-      fields.put(number, field);
+      fieldBuilders.put(number, Field.newBuilder(field));
       return this;
     }
 
@@ -509,15 +476,18 @@ public final class UnknownFieldSet implements MessageLite {
      * changes may or may not be reflected in this map.
      */
     public Map<Integer, Field> asMap() {
-      getFieldBuilder(0); // Force lastField to be built.
+      TreeMap<Integer, Field> fields = new TreeMap<>();
+      for (Map.Entry<Integer, Field.Builder> entry : fieldBuilders.entrySet()) {
+        fields.put(entry.getKey(), entry.getValue().build());
+      }
       return Collections.unmodifiableMap(fields);
     }
 
     /** Parse an entire message from {@code input} and merge its fields into this set. */
     @Override
-    public Builder mergeFrom(final CodedInputStream input) throws IOException {
+    public Builder mergeFrom(CodedInputStream input) throws IOException {
       while (true) {
-        final int tag = input.readTag();
+        int tag = input.readTag();
         if (tag == 0 || !mergeFieldFrom(tag, input)) {
           break;
         }
@@ -531,8 +501,8 @@ public final class UnknownFieldSet implements MessageLite {
      * @param tag The field's tag number, which was already parsed.
      * @return {@code false} if the tag is an end group tag.
      */
-    public boolean mergeFieldFrom(final int tag, final CodedInputStream input) throws IOException {
-      final int number = WireFormat.getTagFieldNumber(tag);
+    public boolean mergeFieldFrom(int tag, CodedInputStream input) throws IOException {
+      int number = WireFormat.getTagFieldNumber(tag);
       switch (WireFormat.getTagWireType(tag)) {
         case WireFormat.WIRETYPE_VARINT:
           getFieldBuilder(number).addVarint(input.readInt64());
@@ -544,7 +514,7 @@ public final class UnknownFieldSet implements MessageLite {
           getFieldBuilder(number).addLengthDelimited(input.readBytes());
           return true;
         case WireFormat.WIRETYPE_START_GROUP:
-          final Builder subBuilder = newBuilder();
+          Builder subBuilder = newBuilder();
           input.readGroup(number, subBuilder, ExtensionRegistry.getEmptyRegistry());
           getFieldBuilder(number).addGroup(subBuilder.build());
           return true;
@@ -563,15 +533,15 @@ public final class UnknownFieldSet implements MessageLite {
      * is just a small wrapper around {@link #mergeFrom(CodedInputStream)}.
      */
     @Override
-    public Builder mergeFrom(final ByteString data) throws InvalidProtocolBufferException {
+    public Builder mergeFrom(ByteString data) throws InvalidProtocolBufferException {
       try {
-        final CodedInputStream input = data.newCodedInput();
+        CodedInputStream input = data.newCodedInput();
         mergeFrom(input);
         input.checkLastTagWas(0);
         return this;
-      } catch (final InvalidProtocolBufferException e) {
+      } catch (InvalidProtocolBufferException e) {
         throw e;
-      } catch (final IOException e) {
+      } catch (IOException e) {
         throw new RuntimeException(
             "Reading from a ByteString threw an IOException (should never happen).", e);
       }
@@ -582,15 +552,15 @@ public final class UnknownFieldSet implements MessageLite {
      * is just a small wrapper around {@link #mergeFrom(CodedInputStream)}.
      */
     @Override
-    public Builder mergeFrom(final byte[] data) throws InvalidProtocolBufferException {
+    public Builder mergeFrom(byte[] data) throws InvalidProtocolBufferException {
       try {
-        final CodedInputStream input = CodedInputStream.newInstance(data);
+        CodedInputStream input = CodedInputStream.newInstance(data);
         mergeFrom(input);
         input.checkLastTagWas(0);
         return this;
-      } catch (final InvalidProtocolBufferException e) {
+      } catch (InvalidProtocolBufferException e) {
         throw e;
-      } catch (final IOException e) {
+      } catch (IOException e) {
         throw new RuntimeException(
             "Reading from a byte array threw an IOException (should never happen).", e);
       }
@@ -601,8 +571,8 @@ public final class UnknownFieldSet implements MessageLite {
      * This is just a small wrapper around {@link #mergeFrom(CodedInputStream)}.
      */
     @Override
-    public Builder mergeFrom(final InputStream input) throws IOException {
-      final CodedInputStream codedInput = CodedInputStream.newInstance(input);
+    public Builder mergeFrom(InputStream input) throws IOException {
+      CodedInputStream codedInput = CodedInputStream.newInstance(input);
       mergeFrom(codedInput);
       codedInput.checkLastTagWas(0);
       return this;
@@ -610,12 +580,12 @@ public final class UnknownFieldSet implements MessageLite {
 
     @Override
     public boolean mergeDelimitedFrom(InputStream input) throws IOException {
-      final int firstByte = input.read();
+      int firstByte = input.read();
       if (firstByte == -1) {
         return false;
       }
-      final int size = CodedInputStream.readRawVarint32(firstByte, input);
-      final InputStream limitedInput = new LimitedInputStream(input, size);
+      int size = CodedInputStream.readRawVarint32(firstByte, input);
+      InputStream limitedInput = new LimitedInputStream(input, size);
       mergeFrom(limitedInput);
       return true;
     }
@@ -644,7 +614,7 @@ public final class UnknownFieldSet implements MessageLite {
     @Override
     public Builder mergeFrom(byte[] data, int off, int len) throws InvalidProtocolBufferException {
       try {
-        final CodedInputStream input = CodedInputStream.newInstance(data, off, len);
+        CodedInputStream input = CodedInputStream.newInstance(data, off, len);
         mergeFrom(input);
         input.checkLastTagWas(0);
         return this;
@@ -718,7 +688,7 @@ public final class UnknownFieldSet implements MessageLite {
     }
 
     /** Construct a new {@link Builder} and initialize it to a copy of {@code copyFrom}. */
-    public static Builder newBuilder(final Field copyFrom) {
+    public static Builder newBuilder(Field copyFrom) {
       return newBuilder().mergeFrom(copyFrom);
     }
 
@@ -758,7 +728,7 @@ public final class UnknownFieldSet implements MessageLite {
     }
 
     @Override
-    public boolean equals(final Object other) {
+    public boolean equals(Object other) {
       if (this == other) {
         return true;
       }
@@ -785,7 +755,7 @@ public final class UnknownFieldSet implements MessageLite {
     public ByteString toByteString(int fieldNumber) {
       try {
         // TODO(lukes): consider caching serialized size in a volatile long
-        final ByteString.CodedBuilder out =
+        ByteString.CodedBuilder out =
             ByteString.newCodedBuilder(getSerializedSize(fieldNumber));
         writeTo(fieldNumber, out.getCodedOutput());
         return out.build();
@@ -796,40 +766,40 @@ public final class UnknownFieldSet implements MessageLite {
     }
 
     /** Serializes the field, including field number, and writes it to {@code output}. */
-    public void writeTo(final int fieldNumber, final CodedOutputStream output) throws IOException {
-      for (final long value : varint) {
+    public void writeTo(int fieldNumber, CodedOutputStream output) throws IOException {
+      for (long value : varint) {
         output.writeUInt64(fieldNumber, value);
       }
-      for (final int value : fixed32) {
+      for (int value : fixed32) {
         output.writeFixed32(fieldNumber, value);
       }
-      for (final long value : fixed64) {
+      for (long value : fixed64) {
         output.writeFixed64(fieldNumber, value);
       }
-      for (final ByteString value : lengthDelimited) {
+      for (ByteString value : lengthDelimited) {
         output.writeBytes(fieldNumber, value);
       }
-      for (final UnknownFieldSet value : group) {
+      for (UnknownFieldSet value : group) {
         output.writeGroup(fieldNumber, value);
       }
     }
 
     /** Get the number of bytes required to encode this field, including field number. */
-    public int getSerializedSize(final int fieldNumber) {
+    public int getSerializedSize(int fieldNumber) {
       int result = 0;
-      for (final long value : varint) {
+      for (long value : varint) {
         result += CodedOutputStream.computeUInt64Size(fieldNumber, value);
       }
-      for (final int value : fixed32) {
+      for (int value : fixed32) {
         result += CodedOutputStream.computeFixed32Size(fieldNumber, value);
       }
-      for (final long value : fixed64) {
+      for (long value : fixed64) {
         result += CodedOutputStream.computeFixed64Size(fieldNumber, value);
       }
-      for (final ByteString value : lengthDelimited) {
+      for (ByteString value : lengthDelimited) {
         result += CodedOutputStream.computeBytesSize(fieldNumber, value);
       }
-      for (final UnknownFieldSet value : group) {
+      for (UnknownFieldSet value : group) {
         result += CodedOutputStream.computeGroupSize(fieldNumber, value);
       }
       return result;
@@ -839,15 +809,15 @@ public final class UnknownFieldSet implements MessageLite {
      * Serializes the field, including field number, and writes it to {@code output}, using {@code
      * MessageSet} wire format.
      */
-    public void writeAsMessageSetExtensionTo(final int fieldNumber, final CodedOutputStream output)
+    public void writeAsMessageSetExtensionTo(int fieldNumber, CodedOutputStream output)
         throws IOException {
-      for (final ByteString value : lengthDelimited) {
+      for (ByteString value : lengthDelimited) {
         output.writeRawMessageSetExtension(fieldNumber, value);
       }
     }
 
     /** Serializes the field, including field number, and writes it to {@code writer}. */
-    void writeTo(final int fieldNumber, final Writer writer) throws IOException {
+    void writeTo(int fieldNumber, Writer writer) throws IOException {
       writer.writeInt64List(fieldNumber, varint, false);
       writer.writeFixed32List(fieldNumber, fixed32, false);
       writer.writeFixed64List(fieldNumber, fixed64, false);
@@ -872,7 +842,7 @@ public final class UnknownFieldSet implements MessageLite {
      * Serializes the field, including field number, and writes it to {@code writer}, using {@code
      * MessageSet} wire format.
      */
-    private void writeAsMessageSetExtensionTo(final int fieldNumber, final Writer writer)
+    private void writeAsMessageSetExtensionTo(int fieldNumber, Writer writer)
         throws IOException {
       if (writer.fieldOrder() == Writer.FieldOrder.DESCENDING) {
         // Write in descending field order.
@@ -882,7 +852,7 @@ public final class UnknownFieldSet implements MessageLite {
         }
       } else {
         // Write in ascending field order.
-        for (final ByteString value : lengthDelimited) {
+        for (ByteString value : lengthDelimited) {
           writer.writeMessageSetItem(fieldNumber, value);
         }
       }
@@ -892,9 +862,9 @@ public final class UnknownFieldSet implements MessageLite {
      * Get the number of bytes required to encode this field, including field number, using {@code
      * MessageSet} wire format.
      */
-    public int getSerializedSizeAsMessageSetExtension(final int fieldNumber) {
+    public int getSerializedSizeAsMessageSetExtension(int fieldNumber) {
       int result = 0;
-      for (final ByteString value : lengthDelimited) {
+      for (ByteString value : lengthDelimited) {
         result += CodedOutputStream.computeRawMessageSetExtensionSize(fieldNumber, value);
       }
       return result;
@@ -912,52 +882,85 @@ public final class UnknownFieldSet implements MessageLite {
      * <p>Use {@link Field#newBuilder()} to construct a {@code Builder}.
      */
     public static final class Builder {
-      // This constructor should never be called directly (except from 'create').
-      private Builder() {}
+      // This constructor should only be called directly from 'create' and 'clone'.
+      private Builder() {
+        result = new Field();
+      }
 
       private static Builder create() {
         Builder builder = new Builder();
-        builder.result = new Field();
         return builder;
       }
 
       private Field result;
 
+      @Override
+      public Builder clone() {
+        Field copy = new Field();
+        if (result.varint == null) {
+          copy.varint = null;
+        } else {
+          copy.varint = new ArrayList<>(result.varint);
+        }
+        if (result.fixed32 == null) {
+          copy.fixed32 = null;
+        } else {
+          copy.fixed32 = new ArrayList<>(result.fixed32);
+        }
+        if (result.fixed64 == null) {
+          copy.fixed64 = null;
+        } else {
+          copy.fixed64 = new ArrayList<>(result.fixed64);
+        }
+        if (result.lengthDelimited == null) {
+          copy.lengthDelimited = null;
+        } else {
+          copy.lengthDelimited = new ArrayList<>(result.lengthDelimited);
+        }
+        if (result.group == null) {
+          copy.group = null;
+        } else {
+          copy.group = new ArrayList<>(result.group);
+        }
+
+        Builder clone = new Builder();
+        clone.result = copy;
+        return clone;
+      }
+
       /**
-       * Build the field. After {@code build()} has been called, the {@code Builder} is no longer
-       * usable. Calling any other method will result in undefined behavior and can cause a {@code
-       * NullPointerException} to be thrown.
+       * Build the field.
        */
       public Field build() {
+        Field built = new Field();
         if (result.varint == null) {
-          result.varint = Collections.emptyList();
+          built.varint = Collections.emptyList();
         } else {
-          result.varint = Collections.unmodifiableList(result.varint);
+          built.varint = Collections.unmodifiableList(new ArrayList<>(result.varint));
         }
         if (result.fixed32 == null) {
-          result.fixed32 = Collections.emptyList();
+          built.fixed32 = Collections.emptyList();
         } else {
-          result.fixed32 = Collections.unmodifiableList(result.fixed32);
+          built.fixed32 = Collections.unmodifiableList(new ArrayList<>(result.fixed32));
         }
         if (result.fixed64 == null) {
-          result.fixed64 = Collections.emptyList();
+          built.fixed64 = Collections.emptyList();
         } else {
-          result.fixed64 = Collections.unmodifiableList(result.fixed64);
+          built.fixed64 = Collections.unmodifiableList(new ArrayList<>(result.fixed64));
         }
         if (result.lengthDelimited == null) {
-          result.lengthDelimited = Collections.emptyList();
+          built.lengthDelimited = Collections.emptyList();
         } else {
-          result.lengthDelimited = Collections.unmodifiableList(result.lengthDelimited);
+          built.lengthDelimited = Collections.unmodifiableList(
+              new ArrayList<>(result.lengthDelimited));
         }
         if (result.group == null) {
-          result.group = Collections.emptyList();
+          built.group = Collections.emptyList();
         } else {
-          result.group = Collections.unmodifiableList(result.group);
+          built.group = Collections.unmodifiableList(new ArrayList<>(result.group));
         }
 
-        final Field returnMe = result;
-        result = null;
-        return returnMe;
+        return built;
       }
 
       /** Discard the field's contents. */
@@ -970,7 +973,7 @@ public final class UnknownFieldSet implements MessageLite {
        * Merge the values in {@code other} into this field. For each list of values, {@code other}'s
        * values are append to the ones in this field.
        */
-      public Builder mergeFrom(final Field other) {
+      public Builder mergeFrom(Field other) {
         if (!other.varint.isEmpty()) {
           if (result.varint == null) {
             result.varint = new ArrayList<Long>();
@@ -985,19 +988,19 @@ public final class UnknownFieldSet implements MessageLite {
         }
         if (!other.fixed64.isEmpty()) {
           if (result.fixed64 == null) {
-            result.fixed64 = new ArrayList<Long>();
+            result.fixed64 = new ArrayList<>();
           }
           result.fixed64.addAll(other.fixed64);
         }
         if (!other.lengthDelimited.isEmpty()) {
           if (result.lengthDelimited == null) {
-            result.lengthDelimited = new ArrayList<ByteString>();
+            result.lengthDelimited = new ArrayList<>();
           }
           result.lengthDelimited.addAll(other.lengthDelimited);
         }
         if (!other.group.isEmpty()) {
           if (result.group == null) {
-            result.group = new ArrayList<UnknownFieldSet>();
+            result.group = new ArrayList<>();
           }
           result.group.addAll(other.group);
         }
@@ -1005,45 +1008,45 @@ public final class UnknownFieldSet implements MessageLite {
       }
 
       /** Add a varint value. */
-      public Builder addVarint(final long value) {
+      public Builder addVarint(long value) {
         if (result.varint == null) {
-          result.varint = new ArrayList<Long>();
+          result.varint = new ArrayList<>();
         }
         result.varint.add(value);
         return this;
       }
 
       /** Add a fixed32 value. */
-      public Builder addFixed32(final int value) {
+      public Builder addFixed32(int value) {
         if (result.fixed32 == null) {
-          result.fixed32 = new ArrayList<Integer>();
+          result.fixed32 = new ArrayList<>();
         }
         result.fixed32.add(value);
         return this;
       }
 
       /** Add a fixed64 value. */
-      public Builder addFixed64(final long value) {
+      public Builder addFixed64(long value) {
         if (result.fixed64 == null) {
-          result.fixed64 = new ArrayList<Long>();
+          result.fixed64 = new ArrayList<>();
         }
         result.fixed64.add(value);
         return this;
       }
 
       /** Add a length-delimited value. */
-      public Builder addLengthDelimited(final ByteString value) {
+      public Builder addLengthDelimited(ByteString value) {
         if (result.lengthDelimited == null) {
-          result.lengthDelimited = new ArrayList<ByteString>();
+          result.lengthDelimited = new ArrayList<>();
         }
         result.lengthDelimited.add(value);
         return this;
       }
 
       /** Add an embedded group. */
-      public Builder addGroup(final UnknownFieldSet value) {
+      public Builder addGroup(UnknownFieldSet value) {
         if (result.group == null) {
-          result.group = new ArrayList<UnknownFieldSet>();
+          result.group = new ArrayList<>();
         }
         result.group.add(value);
         return this;
diff --git a/java/core/src/test/java/com/google/protobuf/UnknownFieldSetPerformanceTest.java b/java/core/src/test/java/com/google/protobuf/UnknownFieldSetPerformanceTest.java
new file mode 100644
index 0000000..6ce0fc7
--- /dev/null
+++ b/java/core/src/test/java/com/google/protobuf/UnknownFieldSetPerformanceTest.java
@@ -0,0 +1,78 @@
+// Protocol Buffers - Google's data interchange format
+// Copyright 2008 Google Inc.  All rights reserved.
+// https://developers.google.com/protocol-buffers/
+//
+// Redistribution and use in source and binary forms, with or without
+// modification, are permitted provided that the following conditions are
+// met:
+//
+//     * Redistributions of source code must retain the above copyright
+// notice, this list of conditions and the following disclaimer.
+//     * Redistributions in binary form must reproduce the above
+// copyright notice, this list of conditions and the following disclaimer
+// in the documentation and/or other materials provided with the
+// distribution.
+//     * Neither the name of Google Inc. nor the names of its
+// contributors may be used to endorse or promote products derived from
+// this software without specific prior written permission.
+//
+// THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+// "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+// LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+// A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+// OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+// SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+// LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+// DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+// THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+// (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+// OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+
+package com.google.protobuf;
+
+import static com.google.common.truth.Truth.assertThat;
+
+import java.io.ByteArrayInputStream;
+import java.io.IOException;
+import java.io.InputStream;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.JUnit4;
+
+@RunWith(JUnit4.class)
+public final class UnknownFieldSetPerformanceTest {
+
+  private static byte[] generateBytes(int length) {
+    assertThat(length % 4).isEqualTo(0);
+    byte[] input = new byte[length];
+    for (int i = 0; i < length; i += 4) {
+        input[i] =     (byte) 0x08; // field 1, wiretype 0
+        input[i + 1] = (byte) 0x08; // field 1, payload 8
+        input[i + 2] = (byte) 0x20; // field 4, wiretype 0
+        input[i + 3] = (byte) 0x20; // field 4, payload 32
+    }
+    return input;
+  }
+
+  @Test
+  // This is a performance test. Failure here is a timeout.
+  public void testAlternatingFieldNumbers() throws IOException {
+    byte[] input = generateBytes(800000);
+    InputStream in = new ByteArrayInputStream(input);
+    UnknownFieldSet.Builder builder = UnknownFieldSet.newBuilder();
+    CodedInputStream codedInput = CodedInputStream.newInstance(in);
+    builder.mergeFrom(codedInput);
+  }
+
+  @Test
+  // This is a performance test. Failure here is a timeout.
+  public void testAddField() {
+    UnknownFieldSet.Builder builder = UnknownFieldSet.newBuilder();
+    for (int i = 1; i <= 100000; i++) {
+      UnknownFieldSet.Field field = UnknownFieldSet.Field.newBuilder().addFixed32(i).build();
+      builder.addField(i, field);
+    }
+    UnknownFieldSet fieldSet = builder.build();
+    assertThat(fieldSet.getField(100000).getFixed32List().get(0)).isEqualTo(100000);
+  }
+}
diff --git a/java/core/src/test/java/com/google/protobuf/UnknownFieldSetTest.java b/java/core/src/test/java/com/google/protobuf/UnknownFieldSetTest.java
index c7eb57c..3e1e928 100644
--- a/java/core/src/test/java/com/google/protobuf/UnknownFieldSetTest.java
+++ b/java/core/src/test/java/com/google/protobuf/UnknownFieldSetTest.java
@@ -30,6 +30,9 @@
 
 package com.google.protobuf;
 
+import static com.google.common.truth.Truth.assertThat;
+import static com.google.common.truth.Truth.assertWithMessage;
+
 import protobuf_unittest.UnittestProto;
 import protobuf_unittest.UnittestProto.ForeignEnum;
 import protobuf_unittest.UnittestProto.TestAllExtensions;
@@ -39,8 +42,10 @@ import protobuf_unittest.UnittestProto.TestEmptyMessageWithExtensions;
 import protobuf_unittest.UnittestProto.TestPackedExtensions;
 import protobuf_unittest.UnittestProto.TestPackedTypes;
 import proto3_unittest.UnittestProto3;
+import java.util.List;
 import java.util.Arrays;
 import java.util.Map;
+import org.junit.Assert;
 import junit.framework.TestCase;
 
 /**
@@ -58,7 +63,7 @@ public class UnknownFieldSetTest extends TestCase {
     unknownFields = emptyMessage.getUnknownFields();
   }
 
-  UnknownFieldSet.Field getField(String name) {
+  private UnknownFieldSet.Field getField(String name) {
     Descriptors.FieldDescriptor field = descriptor.findFieldByName(name);
     assertNotNull(field);
     return unknownFields.getField(field.getNumber());
@@ -97,6 +102,161 @@ public class UnknownFieldSetTest extends TestCase {
 
   // =================================================================
 
+  public void testFieldBuildersAreReusable() {
+    UnknownFieldSet.Field.Builder fieldBuilder = UnknownFieldSet.Field.newBuilder();
+    fieldBuilder.addFixed32(10);
+    UnknownFieldSet.Field first = fieldBuilder.build();
+    UnknownFieldSet.Field second = fieldBuilder.build();
+    fieldBuilder.addFixed32(11);
+    UnknownFieldSet.Field third = fieldBuilder.build();
+
+    assertThat(first).isEqualTo(second);
+    assertThat(first).isNotEqualTo(third);
+  }
+
+  public void testClone() {
+    UnknownFieldSet.Builder unknownSetBuilder = UnknownFieldSet.newBuilder();
+    UnknownFieldSet.Field.Builder fieldBuilder = UnknownFieldSet.Field.newBuilder();
+    fieldBuilder.addFixed32(10);
+    unknownSetBuilder.addField(8, fieldBuilder.build());
+    // necessary to call clone twice to expose the bug
+    UnknownFieldSet.Builder clone1 = unknownSetBuilder.clone();
+    UnknownFieldSet.Builder clone2 = unknownSetBuilder.clone(); // failure is a NullPointerException
+    assertThat(clone1).isNotSameInstanceAs(clone2);
+  }
+
+  public void testClone_lengthDelimited() {
+    UnknownFieldSet.Builder destUnknownFieldSet =
+        UnknownFieldSet.newBuilder()
+            .addField(997, UnknownFieldSet.Field.newBuilder().addVarint(99).build())
+            .addField(
+                999,
+                UnknownFieldSet.Field.newBuilder()
+                    .addLengthDelimited(ByteString.copyFromUtf8("some data"))
+                    .addLengthDelimited(ByteString.copyFromUtf8("some more data"))
+                    .build());
+    UnknownFieldSet clone = destUnknownFieldSet.clone().build();
+    assertThat(clone.getField(997)).isNotNull();
+    UnknownFieldSet.Field field999 = clone.getField(999);
+    List<ByteString> lengthDelimited = field999.getLengthDelimitedList();
+    assertThat(lengthDelimited.get(0).toStringUtf8()).isEqualTo("some data");
+    assertThat(lengthDelimited.get(1).toStringUtf8()).isEqualTo("some more data");
+
+    UnknownFieldSet clone2 = destUnknownFieldSet.clone().build();
+    assertThat(clone2.getField(997)).isNotNull();
+    UnknownFieldSet.Field secondField = clone2.getField(999);
+    List<ByteString> lengthDelimited2 = secondField.getLengthDelimitedList();
+    assertThat(lengthDelimited2.get(0).toStringUtf8()).isEqualTo("some data");
+    assertThat(lengthDelimited2.get(1).toStringUtf8()).isEqualTo("some more data");
+  }
+
+  public void testReuse() {
+    UnknownFieldSet.Builder builder =
+        UnknownFieldSet.newBuilder()
+            .addField(997, UnknownFieldSet.Field.newBuilder().addVarint(99).build())
+            .addField(
+                999,
+                UnknownFieldSet.Field.newBuilder()
+                    .addLengthDelimited(ByteString.copyFromUtf8("some data"))
+                    .addLengthDelimited(ByteString.copyFromUtf8("some more data"))
+                    .build());
+
+    UnknownFieldSet fieldSet1 = builder.build();
+    UnknownFieldSet fieldSet2 = builder.build();
+    builder.addField(1000, UnknownFieldSet.Field.newBuilder().addVarint(-90).build());
+    UnknownFieldSet fieldSet3 = builder.build();
+
+    assertThat(fieldSet1).isEqualTo(fieldSet2);
+    assertThat(fieldSet1).isNotEqualTo(fieldSet3);
+  }
+
+  @SuppressWarnings("ModifiedButNotUsed")
+  public void testAddField_zero() {
+    UnknownFieldSet.Field field = getField("optional_int32");
+    try {
+      UnknownFieldSet.newBuilder().addField(0, field);
+      Assert.fail();
+    } catch (IllegalArgumentException expected) {
+      assertThat(expected).hasMessageThat().isEqualTo("0 is not a valid field number.");
+    }
+  }
+
+  @SuppressWarnings("ModifiedButNotUsed")
+  public void testAddField_negative() {
+    UnknownFieldSet.Field field = getField("optional_int32");
+    try {
+      UnknownFieldSet.newBuilder().addField(-2, field);
+      Assert.fail();
+    } catch (IllegalArgumentException expected) {
+      assertThat(expected).hasMessageThat().isEqualTo("-2 is not a valid field number.");
+    }
+  }
+
+  @SuppressWarnings("ModifiedButNotUsed")
+  public void testClearField_negative() {
+    try {
+      UnknownFieldSet.newBuilder().clearField(-28);
+      Assert.fail();
+    } catch (IllegalArgumentException expected) {
+      assertThat(expected).hasMessageThat().isEqualTo("-28 is not a valid field number.");
+    }
+  }
+
+  @SuppressWarnings("ModifiedButNotUsed")
+  public void testMergeField_negative() {
+    UnknownFieldSet.Field field = getField("optional_int32");
+    try {
+      UnknownFieldSet.newBuilder().mergeField(-2, field);
+      Assert.fail();
+    } catch (IllegalArgumentException expected) {
+      assertThat(expected).hasMessageThat().isEqualTo("-2 is not a valid field number.");
+    }
+  }
+
+  @SuppressWarnings("ModifiedButNotUsed")
+  public void testMergeVarintField_negative() {
+    try {
+      UnknownFieldSet.newBuilder().mergeVarintField(-2, 78);
+      Assert.fail();
+    } catch (IllegalArgumentException expected) {
+      assertThat(expected).hasMessageThat().isEqualTo("-2 is not a valid field number.");
+    }
+  }
+
+  @SuppressWarnings("ModifiedButNotUsed")
+  public void testHasField_negative() {
+    assertThat(UnknownFieldSet.newBuilder().hasField(-2)).isFalse();
+  }
+
+  @SuppressWarnings("ModifiedButNotUsed")
+  public void testMergeLengthDelimitedField_negative() {
+    ByteString byteString = ByteString.copyFromUtf8("some data");
+    try {
+      UnknownFieldSet.newBuilder().mergeLengthDelimitedField(-2, byteString);
+      Assert.fail();
+    } catch (IllegalArgumentException expected) {
+      assertThat(expected).hasMessageThat().isEqualTo("-2 is not a valid field number.");
+    }
+  }
+
+  public void testAddField() {
+    UnknownFieldSet.Field field = getField("optional_int32");
+    UnknownFieldSet fieldSet = UnknownFieldSet.newBuilder().addField(1, field).build();
+    assertThat(fieldSet.getField(1)).isEqualTo(field);
+  }
+
+  public void testAddField_withReplacement() {
+    UnknownFieldSet.Field first = UnknownFieldSet.Field.newBuilder().addFixed32(56).build();
+    UnknownFieldSet.Field second = UnknownFieldSet.Field.newBuilder().addFixed32(25).build();
+    UnknownFieldSet fieldSet = UnknownFieldSet.newBuilder()
+        .addField(1, first)
+        .addField(1, second)
+        .build();
+    List<Integer> list = fieldSet.getField(1).getFixed32List();
+    assertThat(list).hasSize(1);
+    assertThat(list.get(0)).isEqualTo(25);
+  }
+
   public void testVarint() throws Exception {
     UnknownFieldSet.Field field = getField("optional_int32");
     assertEquals(1, field.getVarintList().size());
@@ -173,6 +333,15 @@ public class UnknownFieldSetTest extends TestCase {
     assertEquals("1: 1\n2: 2\n3: 3\n3: 4\n", destination.toString());
   }
 
+  public void testAsMap() throws Exception {
+    UnknownFieldSet.Builder builder = UnknownFieldSet.newBuilder().mergeFrom(unknownFields);
+    Map<Integer, UnknownFieldSet.Field> mapFromBuilder = builder.asMap();
+    assertThat(mapFromBuilder).isNotEmpty();
+    UnknownFieldSet fields = builder.build();
+    Map<Integer, UnknownFieldSet.Field> mapFromFieldSet = fields.asMap();
+    assertThat(mapFromFieldSet).containsExactlyEntriesIn(mapFromBuilder);
+  }
+
   public void testClear() throws Exception {
     UnknownFieldSet fields = UnknownFieldSet.newBuilder().mergeFrom(unknownFields).clear().build();
     assertTrue(fields.asMap().isEmpty());
diff --git a/java/lite/pom.xml b/java/lite/pom.xml
index 104c5c1..d095483 100644
--- a/java/lite/pom.xml
+++ b/java/lite/pom.xml
@@ -232,7 +232,8 @@
                     <exclude>TestUtil.java</exclude>
                     <exclude>TypeRegistryTest.java</exclude>
                     <exclude>UnknownEnumValueTest.java</exclude>
-                    <exclude>UnknownFieldSetLiteTest.java</exclude>
+		    <exclude>UnknownFieldSetLiteTest.java</exclude>
+                    <exclude>UnknownFieldSetPerformanceTest.java</exclude>
                     <exclude>UnknownFieldSetTest.java</exclude>
                     <exclude>WellKnownTypesTest.java</exclude>
                     <exclude>WireFormatTest.java</exclude>
-- 
2.30.0