Use unsafe String operations when writing parameter files.

When a LATIN-1 parameter file is requested, we can take advantage of the fact that JDK9 strings are (usually) stored as LATIN-1.

For UTF-8, we can still optimize for the common case where a LATIN-1 string contains only ASCII characters, as these are bit-identical between UTF-8 and LATIN-1. This would still be expected to be the vast majority of parameter file contents.

RELNOTES: None
PiperOrigin-RevId: 199816430

12 files changed, 227 insertions, 64 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/BUILD b/src/main/java/com/google/devtools/build/lib/BUILD
index 3e3c1b7984..21e9ab98f3 100644
--- a/src/main/java/com/google/devtools/build/lib/BUILD
+++ b/src/main/java/com/google/devtools/build/lib/BUILD
@@ -77,6 +77,7 @@ filegroup(
         "//src/main/java/com/google/devtools/build/lib/windows/runfiles:srcs",
         "//src/main/java/com/google/devtools/build/lib/windows:srcs",
         "//src/main/java/com/google/devtools/build/lib/worker:srcs",
+        "//src/main/java/com/google/devtools/build/lib/unsafe:srcs",
         "//src/main/java/com/google/devtools/build/skyframe:srcs",
         "//src/main/java/com/google/devtools/common/options:srcs",
     ],
diff --git a/src/main/java/com/google/devtools/build/lib/actions/BUILD b/src/main/java/com/google/devtools/build/lib/actions/BUILD
index a65817caf8..b46675ab0f 100644
--- a/src/main/java/com/google/devtools/build/lib/actions/BUILD
+++ b/src/main/java/com/google/devtools/build/lib/actions/BUILD
@@ -39,6 +39,7 @@ java_library(
         "//src/main/java/com/google/devtools/build/lib/shell",
         "//src/main/java/com/google/devtools/build/lib/skyframe/serialization/autocodec",
         "//src/main/java/com/google/devtools/build/lib/skylarkbuildapi",
+        "//src/main/java/com/google/devtools/build/lib/unsafe:string",
         "//src/main/java/com/google/devtools/build/lib/vfs",
         "//src/main/java/com/google/devtools/build/skyframe",
         "//src/main/java/com/google/devtools/build/skyframe:skyframe-objects",
diff --git a/src/main/java/com/google/devtools/build/lib/actions/ParameterFile.java b/src/main/java/com/google/devtools/build/lib/actions/ParameterFile.java
index 61449f24e1..6a91a281b8 100644
--- a/src/main/java/com/google/devtools/build/lib/actions/ParameterFile.java
+++ b/src/main/java/com/google/devtools/build/lib/actions/ParameterFile.java
@@ -14,13 +14,18 @@
 package com.google.devtools.build.lib.actions;
 
 import com.google.common.annotations.VisibleForTesting;
+import com.google.devtools.build.lib.unsafe.StringUnsafe;
 import com.google.devtools.build.lib.util.FileType;
 import com.google.devtools.build.lib.util.ShellEscaper;
 import com.google.devtools.build.lib.vfs.PathFragment;
 import java.io.IOException;
 import java.io.OutputStream;
 import java.io.OutputStreamWriter;
+import java.nio.ByteBuffer;
+import java.nio.CharBuffer;
 import java.nio.charset.Charset;
+import java.nio.charset.CharsetEncoder;
+import java.nio.charset.StandardCharsets;
 
 /**
  * Support for parameter file generation (as used by gcc and other tools, e.g.
@@ -87,35 +92,89 @@ public class ParameterFile {
       throws IOException {
     switch (type) {
       case SHELL_QUOTED:
-        writeContentQuoted(out, arguments, charset);
+        Iterable<String> quotedContent = ShellEscaper.escapeAll(arguments);
+        writeContent(out, quotedContent, charset);
         break;
       case UNQUOTED:
-        writeContentUnquoted(out, arguments, charset);
+        writeContent(out, arguments, charset);
         break;
     }
   }
 
-  /** Writes the arguments from the list into the parameter file. */
-  private static void writeContentUnquoted(
+  private static void writeContent(
       OutputStream outputStream, Iterable<String> arguments, Charset charset) throws IOException {
-    OutputStreamWriter out = new OutputStreamWriter(outputStream, charset);
+    if (charset.equals(StandardCharsets.ISO_8859_1) && StringUnsafe.canUse()) {
+      writeContentLatin1Jdk9(outputStream, arguments);
+    } else if (charset.equals(StandardCharsets.UTF_8) && StringUnsafe.canUse()) {
+      writeContentUtf8Jdk9(outputStream, arguments);
+    } else {
+      // Generic charset support
+      OutputStreamWriter out = new OutputStreamWriter(outputStream, charset);
+      for (String line : arguments) {
+        out.write(line);
+        out.write('\n');
+      }
+      out.flush();
+    }
+  }
+
+  /**
+   * Fast LATIN-1 path that avoids GC overhead. This takes advantage of the fact that strings are
+   * encoded as either LATIN-1 or UTF-16 under JDK9. When LATIN-1 we can simply copy the byte
+   * buffer, when UTF-16 we can fail loudly.
+   */
+  private static void writeContentLatin1Jdk9(OutputStream outputStream, Iterable<String> arguments)
+      throws IOException {
+    StringUnsafe stringUnsafe = StringUnsafe.getInstance();
     for (String line : arguments) {
-      out.write(line);
-      out.write('\n');
+      if (stringUnsafe.getCoder(line) == StringUnsafe.LATIN1) {
+        byte[] bytes = stringUnsafe.getByteArray(line);
+        outputStream.write(bytes);
+      } else {
+        // Error case, encode with '?' characters
+        ByteBuffer encodedBytes = StandardCharsets.ISO_8859_1.encode(CharBuffer.wrap(line));
+        outputStream.write(
+            encodedBytes.array(),
+            encodedBytes.arrayOffset(),
+            encodedBytes.arrayOffset() + encodedBytes.limit());
+      }
+      outputStream.write('\n');
     }
-    out.flush();
+    outputStream.flush();
   }
 
   /**
-   * Writes the arguments from the list into the parameter file with shell quoting (if required).
+   * Fast UTF-8 path that tries to coder GC overhead. This takes advantage of the fact that strings
+   * are encoded as either LATIN-1 or UTF-16 under JDK9. When LATIN-1 we can check if the buffer is
+   * ASCII and copy that directly (since this is both valid LATIN-1 and UTF-8), in all other cases
+   * we must re-encode.
    */
-  private static void writeContentQuoted(
-      OutputStream outputStream, Iterable<String> arguments, Charset charset) throws IOException {
-    OutputStreamWriter out = new OutputStreamWriter(outputStream, charset);
-    for (String line : ShellEscaper.escapeAll(arguments)) {
-      out.write(line);
-      out.write('\n');
+  private static void writeContentUtf8Jdk9(OutputStream outputStream, Iterable<String> arguments)
+      throws IOException {
+    CharsetEncoder encoder = StandardCharsets.UTF_8.newEncoder();
+    StringUnsafe stringUnsafe = StringUnsafe.getInstance();
+    for (String line : arguments) {
+      byte[] bytes = stringUnsafe.getByteArray(line);
+      if (stringUnsafe.getCoder(line) == StringUnsafe.LATIN1 && isAscii(bytes)) {
+        outputStream.write(bytes);
+      } else {
+        ByteBuffer encodedBytes = encoder.encode(CharBuffer.wrap(line));
+        outputStream.write(
+            encodedBytes.array(),
+            encodedBytes.arrayOffset(),
+            encodedBytes.arrayOffset() + encodedBytes.limit());
+      }
+      outputStream.write('\n');
+    }
+    outputStream.flush();
+  }
+
+  private static boolean isAscii(byte[] latin1Bytes) {
+    boolean hiBitSet = false;
+    int n = latin1Bytes.length;
+    for (int i = 0; i < n; ++i) {
+      hiBitSet |= ((latin1Bytes[i] & 0x80) != 0);
     }
-    out.flush();
+    return !hiBitSet;
   }
 }
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/serialization/BUILD b/src/main/java/com/google/devtools/build/lib/skyframe/serialization/BUILD
index 93412b31be..96f9a87cd8 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/serialization/BUILD
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/serialization/BUILD
@@ -17,7 +17,8 @@ java_library(
     deps = [
         "//src/main/java/com/google/devtools/build/lib:crash-utils",
         "//src/main/java/com/google/devtools/build/lib/skyframe/serialization/autocodec:registered-singleton",
-        "//src/main/java/com/google/devtools/build/lib/skyframe/serialization/autocodec:unsafe-provider",
+        "//src/main/java/com/google/devtools/build/lib/unsafe:string",
+        "//src/main/java/com/google/devtools/build/lib/unsafe:unsafe-provider",
         "//third_party:guava",
         "//third_party:jsr305",
         "//third_party/protobuf:protobuf_java",
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/serialization/DynamicCodec.java b/src/main/java/com/google/devtools/build/lib/skyframe/serialization/DynamicCodec.java
index 47e9a69e64..57bfe6a87c 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/serialization/DynamicCodec.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/serialization/DynamicCodec.java
@@ -14,7 +14,7 @@
 
 package com.google.devtools.build.lib.skyframe.serialization;
 
-import com.google.devtools.build.lib.skyframe.serialization.autocodec.UnsafeProvider;
+import com.google.devtools.build.lib.unsafe.UnsafeProvider;
 import com.google.protobuf.CodedInputStream;
 import com.google.protobuf.CodedOutputStream;
 import java.io.IOException;
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/serialization/EnumMapCodec.java b/src/main/java/com/google/devtools/build/lib/skyframe/serialization/EnumMapCodec.java
index 39dfd976be..fbc340668f 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/serialization/EnumMapCodec.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/serialization/EnumMapCodec.java
@@ -14,7 +14,7 @@
 
 package com.google.devtools.build.lib.skyframe.serialization;
 
-import com.google.devtools.build.lib.skyframe.serialization.autocodec.UnsafeProvider;
+import com.google.devtools.build.lib.unsafe.UnsafeProvider;
 import com.google.protobuf.CodedInputStream;
 import com.google.protobuf.CodedOutputStream;
 import java.io.IOException;
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/serialization/UnsafeJdk9StringCodec.java b/src/main/java/com/google/devtools/build/lib/skyframe/serialization/UnsafeJdk9StringCodec.java
index 325524e264..559de5d3f1 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/serialization/UnsafeJdk9StringCodec.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/serialization/UnsafeJdk9StringCodec.java
@@ -15,12 +15,10 @@
 package com.google.devtools.build.lib.skyframe.serialization;
 
 import com.google.common.annotations.VisibleForTesting;
-import com.google.devtools.build.lib.skyframe.serialization.autocodec.UnsafeProvider;
+import com.google.devtools.build.lib.unsafe.StringUnsafe;
 import com.google.protobuf.CodedInputStream;
 import com.google.protobuf.CodedOutputStream;
 import java.io.IOException;
-import java.lang.reflect.Constructor;
-import java.lang.reflect.Field;
 import java.util.Arrays;
 
 /**
@@ -31,40 +29,13 @@ import java.util.Arrays;
 public class UnsafeJdk9StringCodec implements ObjectCodec<String> {
   @VisibleForTesting
   public static boolean canUseUnsafeCodec() {
-    return getVersion() > 1.8;
+    return StringUnsafe.canUse();
   }
 
-  private static double getVersion() {
-    String version = System.getProperty("java.version");
-    int pos = version.indexOf('.');
-    pos = version.indexOf('.', pos + 1);
-    return Double.parseDouble(version.substring(0, pos));
-  }
-
-  private final Constructor<String> constructor;
-  private final long valueOffset;
-  private final long coderOffset;
+  private final StringUnsafe stringUnsafe;
 
   public UnsafeJdk9StringCodec() {
-    Field valueField;
-    Field coderField;
-    try {
-      this.constructor = String.class.getDeclaredConstructor(byte[].class, byte.class);
-      valueField = String.class.getDeclaredField("value");
-      coderField = String.class.getDeclaredField("coder");
-    } catch (ReflectiveOperationException e) {
-      throw new IllegalStateException(
-          "Bad fields/constructor: "
-              + Arrays.toString(String.class.getDeclaredConstructors())
-              + ", "
-              + Arrays.toString(String.class.getDeclaredFields()),
-          e);
-    }
-    this.constructor.setAccessible(true);
-    valueField.setAccessible(true);
-    valueOffset = UnsafeProvider.getInstance().objectFieldOffset(valueField);
-    coderField.setAccessible(true);
-    coderOffset = UnsafeProvider.getInstance().objectFieldOffset(coderField);
+    stringUnsafe = StringUnsafe.getInstance();
   }
 
   @Override
@@ -84,8 +55,8 @@ public class UnsafeJdk9StringCodec implements ObjectCodec<String> {
   @Override
   public void serialize(SerializationContext context, String obj, CodedOutputStream codedOut)
       throws SerializationException, IOException {
-    byte coder = UnsafeProvider.getInstance().getByte(obj, coderOffset);
-    byte[] value = (byte[]) UnsafeProvider.getInstance().getObject(obj, valueOffset);
+    byte coder = stringUnsafe.getCoder(obj);
+    byte[] value = stringUnsafe.getByteArray(obj);
     // Optimize for the case that coder == 0, in which case we can just write the length here,
     // potentially using just one byte. If coder != 0, we'll use 4 bytes, but that's vanishingly
     // rare.
@@ -112,7 +83,7 @@ public class UnsafeJdk9StringCodec implements ObjectCodec<String> {
     }
     byte[] value = codedIn.readRawBytes(length);
     try {
-      return constructor.newInstance(value, coder);
+      return stringUnsafe.newInstance(value, coder);
     } catch (ReflectiveOperationException e) {
       throw new SerializationException(
           "Could not instantiate string: " + Arrays.toString(value) + ", " + coder, e);
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/serialization/autocodec/AutoCodecProcessor.java b/src/main/java/com/google/devtools/build/lib/skyframe/serialization/autocodec/AutoCodecProcessor.java
index cb38568b36..41909261a9 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/serialization/autocodec/AutoCodecProcessor.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/serialization/autocodec/AutoCodecProcessor.java
@@ -26,6 +26,7 @@ import com.google.devtools.build.lib.skyframe.serialization.CodecScanningConstan
 import com.google.devtools.build.lib.skyframe.serialization.ObjectCodec;
 import com.google.devtools.build.lib.skyframe.serialization.SerializationException;
 import com.google.devtools.build.lib.skyframe.serialization.autocodec.SerializationCodeGenerator.Marshaller;
+import com.google.devtools.build.lib.unsafe.UnsafeProvider;
 import com.squareup.javapoet.ClassName;
 import com.squareup.javapoet.FieldSpec;
 import com.squareup.javapoet.JavaFile;
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/serialization/autocodec/BUILD b/src/main/java/com/google/devtools/build/lib/skyframe/serialization/autocodec/BUILD
index 8653eb5e46..783e0dc690 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/serialization/autocodec/BUILD
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/serialization/autocodec/BUILD
@@ -15,9 +15,9 @@ java_library(
         ":autocodec-annotation",
         # Generated classes have the following dependencies.
         ":registered-singleton",
-        ":unsafe-provider",
         "//third_party/protobuf:protobuf_java",
         "//src/main/java/com/google/devtools/build/lib/skyframe/serialization",
+        "//src/main/java/com/google/devtools/build/lib/unsafe:unsafe-provider",
     ],
 )
 
@@ -41,11 +41,6 @@ java_plugin(
     ],
 )
 
-java_library(
-    name = "unsafe-provider",
-    srcs = ["UnsafeProvider.java"],
-)
-
 # @AutoCodec annotation processor implementation.
 java_library(
     name = "autocodec-processor",
@@ -58,8 +53,8 @@ java_library(
     deps = [
         ":autocodec-annotation",
         ":registered-singleton",
-        ":unsafe-provider",
         "//src/main/java/com/google/devtools/build/lib/skyframe/serialization",
+        "//src/main/java/com/google/devtools/build/lib/unsafe:unsafe-provider",
         "//third_party:auto_service",
         "//third_party:auto_value",
         "//third_party:guava",
@@ -80,7 +75,7 @@ pkg_tar(
         ":libautocodec-annotation.jar": "third_party/bazel_bootstrap/libautocodec-annotation.jar",
         ":libautocodec-processor.jar": "third_party/bazel_bootstrap/libautocodec-processor.jar",
         ":libregistered-singleton.jar": "third_party/bazel_bootstrap/libregistered-singleton.jar",
-        ":libunsafe-provider.jar": "third_party/bazel_bootstrap/libunsafe-provider.jar",
+        "//src/main/java/com/google/devtools/build/lib/unsafe:libunsafe-provider.jar": "third_party/bazel_bootstrap/libunsafe-provider.jar",
         "//src/main/java/com/google/devtools/build/lib/skyframe/serialization:libserialization.jar": "third_party/bazel_bootstrap/libserialization.jar",
     },
     visibility = ["//visibility:public"],
diff --git a/src/main/java/com/google/devtools/build/lib/unsafe/BUILD b/src/main/java/com/google/devtools/build/lib/unsafe/BUILD
new file mode 100644
index 0000000000..7c2b20866b
--- /dev/null
+++ b/src/main/java/com/google/devtools/build/lib/unsafe/BUILD
@@ -0,0 +1,23 @@
+package(
+    default_visibility = ["//src:__subpackages__"],
+)
+
+filegroup(
+    name = "srcs",
+    srcs = glob(["**"]),
+)
+
+java_library(
+    name = "unsafe-provider",
+    srcs = ["UnsafeProvider.java"],
+)
+
+java_library(
+    name = "string",
+    srcs = ["StringUnsafe.java"],
+    deps = [
+        ":unsafe-provider",
+        "//third_party:guava",
+        "//third_party:jsr305",
+    ],
+)
diff --git a/src/main/java/com/google/devtools/build/lib/unsafe/StringUnsafe.java b/src/main/java/com/google/devtools/build/lib/unsafe/StringUnsafe.java
new file mode 100644
index 0000000000..8701e8657a
--- /dev/null
+++ b/src/main/java/com/google/devtools/build/lib/unsafe/StringUnsafe.java
@@ -0,0 +1,110 @@
+// Copyright 2018 The Bazel Authors. All rights reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//    http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+package com.google.devtools.build.lib.unsafe;
+
+import com.google.common.base.Preconditions;
+import java.lang.reflect.Constructor;
+import java.lang.reflect.Field;
+import java.util.Arrays;
+import javax.annotation.Nullable;
+import sun.misc.Unsafe;
+
+/**
+ * Provides direct access to the string implementation used by JDK9.
+ *
+ * <p>Under JDK9, a string is two fields: <code>byte coder</code>, and <code>byte[] value</code>.
+ * The <code>coder</code> field has value 0 if the encoding is LATIN-1, and 2 if the encoding is
+ * UTF-16 (the classic JDK8 encoding).
+ *
+ * <p>The <code>value</code> field contains the actual bytes.
+ */
+public class StringUnsafe {
+  private static final boolean CAN_USE = getVersion() > 1.8;
+
+  // Fields corresponding to the coder
+  public static final byte LATIN1 = 0;
+  public static final byte UTF16 = 1;
+
+  private static final StringUnsafe INSTANCE = initInstance();
+  private final Unsafe unsafe;
+  private final Constructor<String> constructor;
+  private final long valueOffset;
+  private final long coderOffset;
+
+  public static boolean canUse() {
+    return CAN_USE;
+  }
+
+  private static double getVersion() {
+    String version = System.getProperty("java.version");
+    int pos = version.indexOf('.');
+    pos = version.indexOf('.', pos + 1);
+    return Double.parseDouble(version.substring(0, pos));
+  }
+
+  @Nullable
+  public static StringUnsafe getInstance() {
+    return Preconditions.checkNotNull(INSTANCE);
+  }
+
+  private static StringUnsafe initInstance() {
+    if (!canUse()) {
+      return null;
+    }
+    return new StringUnsafe();
+  }
+
+  private StringUnsafe() {
+    unsafe = UnsafeProvider.getInstance();
+    Field valueField;
+    Field coderField;
+    try {
+      this.constructor = String.class.getDeclaredConstructor(byte[].class, byte.class);
+      valueField = String.class.getDeclaredField("value");
+      coderField = String.class.getDeclaredField("coder");
+    } catch (ReflectiveOperationException e) {
+      throw new IllegalStateException(
+          "Bad fields/constructor: "
+              + Arrays.toString(String.class.getDeclaredConstructors())
+              + ", "
+              + Arrays.toString(String.class.getDeclaredFields()),
+          e);
+    }
+    this.constructor.setAccessible(true);
+    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);
+  }
+
+  /**
+   * Returns the internal byte array, encoded according to {@link #getCoder}.
+   *
+   * <p>Use of this is unsafe. The representation may change from one JDK version to the next.
+   * Ensure you do not mutate this byte array in any way.
+   */
+  public byte[] getByteArray(String obj) {
+    return (byte[]) unsafe.getObject(obj, valueOffset);
+  }
+
+  /** Constructs a new string from a byte array and coder. */
+  public String newInstance(byte[] bytes, byte coder) throws ReflectiveOperationException {
+    return constructor.newInstance(bytes, coder);
+  }
+}
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/serialization/autocodec/UnsafeProvider.java b/src/main/java/com/google/devtools/build/lib/unsafe/UnsafeProvider.java
index 4a1fe6c389..c1e3f3b87f 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/serialization/autocodec/UnsafeProvider.java
+++ b/src/main/java/com/google/devtools/build/lib/unsafe/UnsafeProvider.java
@@ -12,7 +12,7 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-package com.google.devtools.build.lib.skyframe.serialization.autocodec;
+package com.google.devtools.build.lib.unsafe;
 
 import java.lang.reflect.Field;
 import java.security.AccessController;
@@ -27,6 +27,7 @@ import sun.misc.Unsafe;
  * access this.
  */
 public class UnsafeProvider {
+
   private static final Unsafe UNSAFE = getUnsafe();
 
   public static Unsafe getInstance() {