Upgraded ByteStreams#copy(InputStream, OutputStream) to use the faste…

…r FileChannel if possible. See also https://medium.com/@xunnan.xu/its-all-about-buffers-zero-copy-mmap-and-java-nio-50f2a1bfc05c for some background. RELNOTES=`io`: Upgraded `ByteStreams#copy(InputStream, OutputStream)` to use the faster `FileChannel` if possible. ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=316177487
google · Jun 13, 2020 · a1e9a0b · a1e9a0b
1 parent 14dd8fe
commit a1e9a0b
Show file tree

Hide file tree

Showing 6 changed files with 202 additions and 24 deletions.
diff --git a/android/guava-tests/test/com/google/common/io/ByteStreamsTest.java b/android/guava-tests/test/com/google/common/io/ByteStreamsTest.java
@@ -23,6 +23,7 @@
 import java.io.ByteArrayOutputStream;
 import java.io.EOFException;
 import java.io.File;
+import java.io.FileInputStream;
 import java.io.FileOutputStream;
 import java.io.FilterInputStream;
 import java.io.IOException;
@@ -41,7 +42,7 @@
  */
 public class ByteStreamsTest extends IoTestCase {
 
-  public void testCopyChannel() throws IOException {
+  public void testCopy_channel() throws IOException {
     byte[] expected = newPreFilledByteArray(100);
     ByteArrayOutputStream out = new ByteArrayOutputStream();
     WritableByteChannel outChannel = Channels.newChannel(out);
@@ -51,7 +52,7 @@ public void testCopyChannel() throws IOException {
     assertThat(out.toByteArray()).isEqualTo(expected);
   }
 
-  public void testCopyFileChannel() throws IOException {
+  public void testCopy_channel_fromFile() throws IOException {
     final int chunkSize = 14407; // Random prime, unlikely to match any internal chunk size
     ByteArrayOutputStream out = new ByteArrayOutputStream();
     WritableByteChannel outChannel = Channels.newChannel(out);
@@ -72,6 +73,68 @@ public void testCopyFileChannel() throws IOException {
     }
   }
 
+  public void testCopy_stream() throws IOException {
+    byte[] expected = newPreFilledByteArray(100);
+    ByteArrayOutputStream out = new ByteArrayOutputStream();
+
+    ByteStreams.copy(new ByteArrayInputStream(expected), out);
+
+    assertThat(out.toByteArray()).isEqualTo(expected);
+  }
+
+  public void testCopy_stream_files_emptyDestination() throws IOException {
+    byte[] expected = new byte[] {0, 1, 2};
+    File inputFile = createTempFile(expected);
+    File outputFile = createTempFile();
+
+    try (FileInputStream inputStream = new FileInputStream(inputFile);
+        FileOutputStream outputStream = new FileOutputStream(outputFile)) {
+      ByteStreams.copy(inputStream, outputStream);
+    }
+
+    assertThat(Files.asByteSource(outputFile).read()).isEqualTo(expected);
+  }
+
+  public void testCopy_stream_files_appendDestination() throws IOException {
+    File inputFile = createTempFile(new byte[] {3, 4, 5});
+    File outputFile = createTempFile(new byte[] {0, 1, 2});
+
+    try (FileInputStream inputStream = new FileInputStream(inputFile);
+        FileOutputStream outputStream = new FileOutputStream(outputFile, /* append= */ true)) {
+      ByteStreams.copy(inputStream, outputStream);
+    }
+
+    assertThat(Files.asByteSource(outputFile).read()).isEqualTo(new byte[] {0, 1, 2, 3, 4, 5});
+  }
+
+  public void testCopy_stream_files_additionalWrites_emptyDestination() throws IOException {
+    File inputFile = createTempFile(new byte[] {0, 1, 2});
+    File outputFile = createTempFile();
+
+    try (FileInputStream inputStream = new FileInputStream(inputFile);
+        FileOutputStream outputStream = new FileOutputStream(outputFile)) {
+      outputStream.write(new byte[] {0, 0});
+      ByteStreams.copy(inputStream, outputStream);
+      outputStream.write(new byte[] {2, 2});
+    }
+
+    assertThat(Files.asByteSource(outputFile).read()).isEqualTo(new byte[] {0, 0, 0, 1, 2, 2, 2});
+  }
+
+  public void testCopy_stream_files_additionalWrites_appendDestination() throws IOException {
+    File inputFile = createTempFile(new byte[] {0, 1, 2});
+    File outputFile = createTempFile(new byte[] {0});
+
+    try (FileInputStream inputStream = new FileInputStream(inputFile);
+        FileOutputStream outputStream = new FileOutputStream(outputFile, /* append= */ true)) {
+      outputStream.write(new byte[] {0});
+      ByteStreams.copy(inputStream, outputStream);
+      outputStream.write(new byte[] {2, 2});
+    }
+
+    assertThat(Files.asByteSource(outputFile).read()).isEqualTo(new byte[] {0, 0, 0, 1, 2, 2, 2});
+  }
+
   public void testReadFully() throws IOException {
     byte[] b = new byte[10];
 

diff --git a/android/guava-tests/test/com/google/common/io/IoTestCase.java b/android/guava-tests/test/com/google/common/io/IoTestCase.java
@@ -139,6 +139,18 @@ protected final File createTempFile() throws IOException {
     return File.createTempFile("test", null, getTempDir());
   }
 
+  /**
+   * Creates a new temp file in the temp directory returned by {@link #getTempDir()}. The file will
+   * be deleted in the tear-down for this test.
+   *
+   * @param content which should be written to the file
+   */
+  protected final File createTempFile(byte[] content) throws IOException {
+    File file = File.createTempFile("test", null, getTempDir());
+    Files.write(content, file);
+    return file;
+  }
+
   /** Returns a byte array of length size that has values 0 .. size - 1. */
   static byte[] newPreFilledByteArray(int size) {
     return newPreFilledByteArray(0, size);

diff --git a/android/guava/src/com/google/common/io/ByteStreams.java b/android/guava/src/com/google/common/io/ByteStreams.java
@@ -30,6 +30,8 @@
 import java.io.DataOutput;
 import java.io.DataOutputStream;
 import java.io.EOFException;
+import java.io.FileInputStream;
+import java.io.FileOutputStream;
 import java.io.FilterInputStream;
 import java.io.IOException;
 import java.io.InputStream;
@@ -103,6 +105,15 @@ private ByteStreams() {}
   public static long copy(InputStream from, OutputStream to) throws IOException {
     checkNotNull(from);
     checkNotNull(to);
+
+    // Use java.nio.channels in case we're copying from file to file.
+    // Copying through channels happens ideally in the kernel space and therefore faster.
+    if (from instanceof FileInputStream && to instanceof FileOutputStream) {
+      FileChannel fromChannel = ((FileInputStream) from).getChannel();
+      FileChannel toChannel = ((FileOutputStream) to).getChannel();
+      return copyFileChannel(fromChannel, toChannel);
+    }
+
     byte[] buf = createBuffer();
     long total = 0;
     while (true) {
@@ -130,16 +141,7 @@ public static long copy(ReadableByteChannel from, WritableByteChannel to) throws
     checkNotNull(from);
     checkNotNull(to);
     if (from instanceof FileChannel) {
-      FileChannel sourceChannel = (FileChannel) from;
-      long oldPosition = sourceChannel.position();
-      long position = oldPosition;
-      long copied;
-      do {
-        copied = sourceChannel.transferTo(position, ZERO_COPY_CHUNK_SIZE, to);
-        position += copied;
-        sourceChannel.position(position);
-      } while (copied > 0 || position < sourceChannel.size());
-      return position - oldPosition;
+      return copyFileChannel((FileChannel) from, to);
     }
 
     ByteBuffer buf = ByteBuffer.wrap(createBuffer());
@@ -154,6 +156,18 @@ public static long copy(ReadableByteChannel from, WritableByteChannel to) throws
     return total;
   }
 
+  private static long copyFileChannel(FileChannel from, WritableByteChannel to) throws IOException {
+    long oldPosition = from.position();
+    long position = oldPosition;
+    long copied;
+    do {
+      copied = from.transferTo(position, ZERO_COPY_CHUNK_SIZE, to);
+      position += copied;
+      from.position(position);
+    } while (copied > 0 || position < from.size());
+    return position - oldPosition;
+  }
+
   /** Max array length on JVM. */
   private static final int MAX_ARRAY_LEN = Integer.MAX_VALUE - 8;
 

diff --git a/guava-tests/test/com/google/common/io/ByteStreamsTest.java b/guava-tests/test/com/google/common/io/ByteStreamsTest.java
@@ -23,6 +23,7 @@
 import java.io.ByteArrayOutputStream;
 import java.io.EOFException;
 import java.io.File;
+import java.io.FileInputStream;
 import java.io.FileOutputStream;
 import java.io.FilterInputStream;
 import java.io.IOException;
@@ -41,7 +42,7 @@
  */
 public class ByteStreamsTest extends IoTestCase {
 
-  public void testCopyChannel() throws IOException {
+  public void testCopy_channel() throws IOException {
     byte[] expected = newPreFilledByteArray(100);
     ByteArrayOutputStream out = new ByteArrayOutputStream();
     WritableByteChannel outChannel = Channels.newChannel(out);
@@ -51,7 +52,7 @@ public void testCopyChannel() throws IOException {
     assertThat(out.toByteArray()).isEqualTo(expected);
   }
 
-  public void testCopyFileChannel() throws IOException {
+  public void testCopy_channel_fromFile() throws IOException {
     final int chunkSize = 14407; // Random prime, unlikely to match any internal chunk size
     ByteArrayOutputStream out = new ByteArrayOutputStream();
     WritableByteChannel outChannel = Channels.newChannel(out);
@@ -72,6 +73,68 @@ public void testCopyFileChannel() throws IOException {
     }
   }
 
+  public void testCopy_stream() throws IOException {
+    byte[] expected = newPreFilledByteArray(100);
+    ByteArrayOutputStream out = new ByteArrayOutputStream();
+
+    ByteStreams.copy(new ByteArrayInputStream(expected), out);
+
+    assertThat(out.toByteArray()).isEqualTo(expected);
+  }
+
+  public void testCopy_stream_files_emptyDestination() throws IOException {
+    byte[] expected = new byte[] {0, 1, 2};
+    File inputFile = createTempFile(expected);
+    File outputFile = createTempFile();
+
+    try (FileInputStream inputStream = new FileInputStream(inputFile);
+        FileOutputStream outputStream = new FileOutputStream(outputFile)) {
+      ByteStreams.copy(inputStream, outputStream);
+    }
+
+    assertThat(Files.asByteSource(outputFile).read()).isEqualTo(expected);
+  }
+
+  public void testCopy_stream_files_appendDestination() throws IOException {
+    File inputFile = createTempFile(new byte[] {3, 4, 5});
+    File outputFile = createTempFile(new byte[] {0, 1, 2});
+
+    try (FileInputStream inputStream = new FileInputStream(inputFile);
+        FileOutputStream outputStream = new FileOutputStream(outputFile, /* append= */ true)) {
+      ByteStreams.copy(inputStream, outputStream);
+    }
+
+    assertThat(Files.asByteSource(outputFile).read()).isEqualTo(new byte[] {0, 1, 2, 3, 4, 5});
+  }
+
+  public void testCopy_stream_files_additionalWrites_emptyDestination() throws IOException {
+    File inputFile = createTempFile(new byte[] {0, 1, 2});
+    File outputFile = createTempFile();
+
+    try (FileInputStream inputStream = new FileInputStream(inputFile);
+        FileOutputStream outputStream = new FileOutputStream(outputFile)) {
+      outputStream.write(new byte[] {0, 0});
+      ByteStreams.copy(inputStream, outputStream);
+      outputStream.write(new byte[] {2, 2});
+    }
+
+    assertThat(Files.asByteSource(outputFile).read()).isEqualTo(new byte[] {0, 0, 0, 1, 2, 2, 2});
+  }
+
+  public void testCopy_stream_files_additionalWrites_appendDestination() throws IOException {
+    File inputFile = createTempFile(new byte[] {0, 1, 2});
+    File outputFile = createTempFile(new byte[] {0});
+
+    try (FileInputStream inputStream = new FileInputStream(inputFile);
+        FileOutputStream outputStream = new FileOutputStream(outputFile, /* append= */ true)) {
+      outputStream.write(new byte[] {0});
+      ByteStreams.copy(inputStream, outputStream);
+      outputStream.write(new byte[] {2, 2});
+    }
+
+    assertThat(Files.asByteSource(outputFile).read()).isEqualTo(new byte[] {0, 0, 0, 1, 2, 2, 2});
+  }
+
   public void testReadFully() throws IOException {
     byte[] b = new byte[10];
 

diff --git a/guava-tests/test/com/google/common/io/IoTestCase.java b/guava-tests/test/com/google/common/io/IoTestCase.java
@@ -139,6 +139,18 @@ protected final File createTempFile() throws IOException {
     return File.createTempFile("test", null, getTempDir());
   }
 
+  /**
+   * Creates a new temp file in the temp directory returned by {@link #getTempDir()}. The file will
+   * be deleted in the tear-down for this test.
+   *
+   * @param content which should be written to the file
+   */
+  protected final File createTempFile(byte[] content) throws IOException {
+    File file = File.createTempFile("test", null, getTempDir());
+    Files.write(content, file);
+    return file;
+  }
+
   /** Returns a byte array of length size that has values 0 .. size - 1. */
   static byte[] newPreFilledByteArray(int size) {
     return newPreFilledByteArray(0, size);

diff --git a/guava/src/com/google/common/io/ByteStreams.java b/guava/src/com/google/common/io/ByteStreams.java
@@ -30,6 +30,8 @@
 import java.io.DataOutput;
 import java.io.DataOutputStream;
 import java.io.EOFException;
+import java.io.FileInputStream;
+import java.io.FileOutputStream;
 import java.io.FilterInputStream;
 import java.io.IOException;
 import java.io.InputStream;
@@ -103,6 +105,15 @@ private ByteStreams() {}
   public static long copy(InputStream from, OutputStream to) throws IOException {
     checkNotNull(from);
     checkNotNull(to);
+
+    // Use java.nio.channels in case we're copying from file to file.
+    // Copying through channels happens ideally in the kernel space and therefore faster.
+    if (from instanceof FileInputStream && to instanceof FileOutputStream) {
+      FileChannel fromChannel = ((FileInputStream) from).getChannel();
+      FileChannel toChannel = ((FileOutputStream) to).getChannel();
+      return copyFileChannel(fromChannel, toChannel);
+    }
+
     byte[] buf = createBuffer();
     long total = 0;
     while (true) {
@@ -130,16 +141,7 @@ public static long copy(ReadableByteChannel from, WritableByteChannel to) throws
     checkNotNull(from);
     checkNotNull(to);
     if (from instanceof FileChannel) {
-      FileChannel sourceChannel = (FileChannel) from;
-      long oldPosition = sourceChannel.position();
-      long position = oldPosition;
-      long copied;
-      do {
-        copied = sourceChannel.transferTo(position, ZERO_COPY_CHUNK_SIZE, to);
-        position += copied;
-        sourceChannel.position(position);
-      } while (copied > 0 || position < sourceChannel.size());
-      return position - oldPosition;
+      return copyFileChannel((FileChannel) from, to);
     }
 
     ByteBuffer buf = ByteBuffer.wrap(createBuffer());
@@ -154,6 +156,18 @@ public static long copy(ReadableByteChannel from, WritableByteChannel to) throws
     return total;
   }
 
+  private static long copyFileChannel(FileChannel from, WritableByteChannel to) throws IOException {
+    long oldPosition = from.position();
+    long position = oldPosition;
+    long copied;
+    do {
+      copied = from.transferTo(position, ZERO_COPY_CHUNK_SIZE, to);
+      position += copied;
+      from.position(position);
+    } while (copied > 0 || position < from.size());
+    return position - oldPosition;
+  }
+
   /** Max array length on JVM. */
   private static final int MAX_ARRAY_LEN = Integer.MAX_VALUE - 8;