Issue #3840 - Static resource byte-range support performance

+ Reverting toFile().getInputStream() on PathResource
+ Adding RangeWriter concept for managing open resource
  across multiple range writes
+ RangeWriter implementation delegates to HttpContent behaviors
  Lookup is :
  - Direct Buffer
  - Indirect Buffer
  - ReadableByteChannel (as SeekableByteChannel)
  - InputStream
+ Adding unit tests for all RangeWriter implementation to ensure
  that they behave the same way everywhere.
+ Making ResourceService use new RangeWriter implementation
+ Existing DefaultServletRangeTest still works as-is

Signed-off-by: Joakim Erdfelt <joakim.erdfelt@gmail.com>

jetty-server/src/main/java/org/eclipse/jetty/server/ResourceService.java

@@ -20,7 +20,6 @@ package org.eclipse.jetty.server;
 
 import java.io.FileNotFoundException;
 import java.io.IOException;
-import java.io.InputStream;
 import java.io.OutputStream;
 import java.nio.ByteBuffer;
 import java.nio.charset.StandardCharsets;
@@ -47,9 +46,10 @@ import org.eclipse.jetty.http.PreEncodedHttpField;
 import org.eclipse.jetty.http.QuotedCSV;
 import org.eclipse.jetty.http.QuotedQualityCSV;
 import org.eclipse.jetty.io.WriterOutputStream;
+import org.eclipse.jetty.server.resource.HttpContentRangeWriter;
+import org.eclipse.jetty.server.resource.RangeWriter;
 import org.eclipse.jetty.util.BufferUtil;
 import org.eclipse.jetty.util.Callback;
-import org.eclipse.jetty.util.IO;
 import org.eclipse.jetty.util.MultiPartOutputStream;
 import org.eclipse.jetty.util.URIUtil;
 import org.eclipse.jetty.util.log.Log;
@@ -779,9 +779,6 @@ public class ResourceService
                 ctp = "multipart/byteranges; boundary=";
             response.setContentType(ctp + multi.getBoundary());
 
-            InputStream in = content.getResource().getInputStream();
-            long pos = 0;
-
             // calculate the content-length
             int length = 0;
             String[] header = new String[ranges.size()];
@@ -801,6 +798,8 @@ public class ResourceService
             length += 2 + 2 + multi.getBoundary().length() + 2 + 2;
             response.setContentLength(length);
 
+            try (RangeWriter rangeWriter = HttpContentRangeWriter.newRangeWriter(content))
+            {
                 i = 0;
                 for (InclusiveByteRange ibr : ranges)
                 {
@@ -808,32 +807,12 @@ public class ResourceService
 
                     long start = ibr.getFirst();
                     long size = ibr.getSize();
-                if (in != null)
-                {
-                    // Handle non cached resource
-                    if (start < pos)
-                    {
-                        in.close();
-                        in = content.getResource().getInputStream();
-                        pos = 0;
-                    }
-                    if (pos < start)
-                    {
-                        in.skip(start - pos);
-                        pos = start;
-                    }
-
-                    IO.copy(in, multi, size);
-                    pos += size;
-                }
-                else
-                    // Handle cached resource
-                    content.getResource().writeTo(multi, start, size);
 
+                    rangeWriter.writeTo(multi, start, size);
                     i++;
                 }
-            if (in != null)
+            }
-                in.close();
+
             multi.close();
         }
         return true;

jetty-server/src/main/java/org/eclipse/jetty/server/resource/ByteBufferRangeWriter.java

@@ -0,0 +1,64 @@
+//
+//  ========================================================================
+//  Copyright (c) 1995-2019 Mort Bay Consulting Pty. Ltd.
+//  ------------------------------------------------------------------------
+//  All rights reserved. This program and the accompanying materials
+//  are made available under the terms of the Eclipse Public License v1.0
+//  and Apache License v2.0 which accompanies this distribution.
+//
+//      The Eclipse Public License is available at
+//      http://www.eclipse.org/legal/epl-v10.html
+//
+//      The Apache License v2.0 is available at
+//      http://www.opensource.org/licenses/apache2.0.php
+//
+//  You may elect to redistribute this code under either of these licenses.
+//  ========================================================================
+//
+
+package org.eclipse.jetty.server.resource;
+
+import java.io.IOException;
+import java.io.OutputStream;
+import java.nio.ByteBuffer;
+
+import org.eclipse.jetty.util.BufferUtil;
+
+/**
+ * ByteBuffer based RangeWriter
+ */
+public class ByteBufferRangeWriter implements RangeWriter
+{
+    private final ByteBuffer buffer;
+    private boolean closed = false;
+
+    public ByteBufferRangeWriter(ByteBuffer buffer)
+    {
+        this.buffer = buffer.asReadOnlyBuffer();
+    }
+
+    @Override
+    public void close() throws IOException
+    {
+        closed = true;
+    }
+
+    @Override
+    public void writeTo(OutputStream outputStream, long skipTo, long length) throws IOException
+    {
+        if (skipTo > Integer.MAX_VALUE)
+        {
+            throw new IllegalArgumentException("Unsupported skipTo " + skipTo + " > " + Integer.MAX_VALUE);
+        }
+
+        if (length > Integer.MAX_VALUE)
+        {
+            throw new IllegalArgumentException("Unsupported length " + skipTo + " > " + Integer.MAX_VALUE);
+        }
+
+        ByteBuffer src = buffer.slice();
+        src.position((int)skipTo);
+        src.limit(Math.addExact((int)skipTo, (int)length));
+        BufferUtil.writeTo(src, outputStream);
+    }
+}

jetty-server/src/main/java/org/eclipse/jetty/server/resource/HttpContentRangeWriter.java

@@ -0,0 +1,83 @@
+//
+//  ========================================================================
+//  Copyright (c) 1995-2019 Mort Bay Consulting Pty. Ltd.
+//  ------------------------------------------------------------------------
+//  All rights reserved. This program and the accompanying materials
+//  are made available under the terms of the Eclipse Public License v1.0
+//  and Apache License v2.0 which accompanies this distribution.
+//
+//      The Eclipse Public License is available at
+//      http://www.eclipse.org/legal/epl-v10.html
+//
+//      The Apache License v2.0 is available at
+//      http://www.opensource.org/licenses/apache2.0.php
+//
+//  You may elect to redistribute this code under either of these licenses.
+//  ========================================================================
+//
+
+package org.eclipse.jetty.server.resource;
+
+import java.io.IOException;
+import java.nio.ByteBuffer;
+import java.nio.channels.ReadableByteChannel;
+import java.nio.channels.SeekableByteChannel;
+import java.util.Objects;
+
+import org.eclipse.jetty.http.HttpContent;
+import org.eclipse.jetty.util.log.Log;
+import org.eclipse.jetty.util.log.Logger;
+
+/**
+ * Range Writer selection for HttpContent
+ */
+public class HttpContentRangeWriter
+{
+    private static final Logger LOG = Log.getLogger(HttpContentRangeWriter.class);
+
+    /**
+     * Obtain a new RangeWriter for the supplied HttpContent.
+     *
+     * @param content the HttpContent to base RangeWriter on
+     * @return the RangeWriter best suited for the supplied HttpContent
+     */
+    public static RangeWriter newRangeWriter(HttpContent content)
+    {
+        Objects.requireNonNull(content, "HttpContent");
+
+        // Try direct buffer
+        ByteBuffer buffer = content.getDirectBuffer();
+        if (buffer == null)
+        {
+            buffer = content.getIndirectBuffer();
+        }
+        if (buffer != null)
+        {
+            return new ByteBufferRangeWriter(buffer);
+        }
+
+        try
+        {
+            ReadableByteChannel channel = content.getReadableByteChannel();
+            if (channel != null)
+            {
+                if (channel instanceof SeekableByteChannel)
+                {
+                    SeekableByteChannel seekableByteChannel = (SeekableByteChannel)channel;
+                    return new SeekableByteChannelRangeWriter(seekableByteChannel);
+                }
+
+                if (LOG.isDebugEnabled())
+                    LOG.debug("Skipping non-SeekableByteChannel option " + channel + " from content " + content);
+                channel.close();
+            }
+        }
+        catch (IOException e)
+        {
+            if (LOG.isDebugEnabled())
+                LOG.debug("Skipping ReadableByteChannel option", e);
+        }
+
+        return new InputStreamRangeWriter(() -> content.getInputStream());
+    }
+}

jetty-server/src/main/java/org/eclipse/jetty/server/resource/InputStreamRangeWriter.java

@@ -0,0 +1,93 @@
+//
+//  ========================================================================
+//  Copyright (c) 1995-2019 Mort Bay Consulting Pty. Ltd.
+//  ------------------------------------------------------------------------
+//  All rights reserved. This program and the accompanying materials
+//  are made available under the terms of the Eclipse Public License v1.0
+//  and Apache License v2.0 which accompanies this distribution.
+//
+//      The Eclipse Public License is available at
+//      http://www.eclipse.org/legal/epl-v10.html
+//
+//      The Apache License v2.0 is available at
+//      http://www.opensource.org/licenses/apache2.0.php
+//
+//  You may elect to redistribute this code under either of these licenses.
+//  ========================================================================
+//
+
+package org.eclipse.jetty.server.resource;
+
+import java.io.IOException;
+import java.io.InputStream;
+import java.io.OutputStream;
+
+import org.eclipse.jetty.util.IO;
+
+/**
+ * Default Range Writer for InputStream
+ */
+public class InputStreamRangeWriter implements RangeWriter
+{
+    public interface InputStreamSupplier
+    {
+        InputStream newInputStream() throws IOException;
+    }
+
+    private final InputStreamSupplier inputStreamSupplier;
+    private boolean closed = false;
+    private InputStream inputStream;
+    private long pos;
+
+    /**
+     * Create InputStremRangeWriter
+     *
+     * @param inputStreamSupplier Supplier of the InputStream.  If the stream needs to be regenerated, such as the next
+     * requested range being before the current position, then the current InputStream is closed and a new one obtained
+     * from this supplier.
+     */
+    public InputStreamRangeWriter(InputStreamSupplier inputStreamSupplier)
+    {
+        this.inputStreamSupplier = inputStreamSupplier;
+    }
+
+    @Override
+    public void close() throws IOException
+    {
+        closed = true;
+        if (inputStream != null)
+        {
+            inputStream.close();
+        }
+    }
+
+    @Override
+    public void writeTo(OutputStream outputStream, long skipTo, long length) throws IOException
+    {
+        if (closed)
+        {
+            throw new IOException("RangeWriter is closed");
+        }
+
+        if (inputStream == null)
+        {
+            inputStream = inputStreamSupplier.newInputStream();
+            pos = 0;
+        }
+
+        if (skipTo < pos)
+        {
+            inputStream.close();
+            inputStream = inputStreamSupplier.newInputStream();
+            pos = 0;
+        }
+        if (pos < skipTo)
+        {
+            inputStream.skip(skipTo - pos);
+            pos = skipTo;
+        }
+
+        IO.copy(inputStream, outputStream, length);
+        pos += length;
+    }
+}

jetty-server/src/main/java/org/eclipse/jetty/server/resource/RangeWriter.java

@@ -0,0 +1,38 @@
+//
+//  ========================================================================
+//  Copyright (c) 1995-2019 Mort Bay Consulting Pty. Ltd.
+//  ------------------------------------------------------------------------
+//  All rights reserved. This program and the accompanying materials
+//  are made available under the terms of the Eclipse Public License v1.0
+//  and Apache License v2.0 which accompanies this distribution.
+//
+//      The Eclipse Public License is available at
+//      http://www.eclipse.org/legal/epl-v10.html
+//
+//      The Apache License v2.0 is available at
+//      http://www.opensource.org/licenses/apache2.0.php
+//
+//  You may elect to redistribute this code under either of these licenses.
+//  ========================================================================
+//
+
+package org.eclipse.jetty.server.resource;
+
+import java.io.Closeable;
+import java.io.IOException;
+import java.io.OutputStream;
+
+/**
+ * Interface for writing sections (ranges) of a single resource (SeekableByteChannel, Resource, etc) to an outputStream.
+ */
+public interface RangeWriter extends Closeable
+{
+    /**
+     * Write the specific range (start, size) to the outputStream.
+     *
+     * @param outputStream the stream to write to
+     * @param skipTo the offset / skip-to / seek-to / position in the resource to start the write from
+     * @param length the size of the section to write
+     */
+    void writeTo(OutputStream outputStream, long skipTo, long length) throws IOException;
+}

jetty-server/src/main/java/org/eclipse/jetty/server/resource/SeekableByteChannelRangeWriter.java

@@ -0,0 +1,66 @@
+//
+//  ========================================================================
+//  Copyright (c) 1995-2019 Mort Bay Consulting Pty. Ltd.
+//  ------------------------------------------------------------------------
+//  All rights reserved. This program and the accompanying materials
+//  are made available under the terms of the Eclipse Public License v1.0
+//  and Apache License v2.0 which accompanies this distribution.
+//
+//      The Eclipse Public License is available at
+//      http://www.eclipse.org/legal/epl-v10.html
+//
+//      The Apache License v2.0 is available at
+//      http://www.opensource.org/licenses/apache2.0.php
+//
+//  You may elect to redistribute this code under either of these licenses.
+//  ========================================================================
+//
+
+package org.eclipse.jetty.server.resource;
+
+import java.io.IOException;
+import java.io.OutputStream;
+import java.nio.ByteBuffer;
+import java.nio.channels.SeekableByteChannel;
+
+import org.eclipse.jetty.util.BufferUtil;
+import org.eclipse.jetty.util.IO;
+
+public class SeekableByteChannelRangeWriter implements RangeWriter
+{
+    private final SeekableByteChannel channel;
+    private final int bufSize;
+    private final ByteBuffer buffer;
+
+    public SeekableByteChannelRangeWriter(SeekableByteChannel seekableByteChannel)
+    {
+        this.channel = seekableByteChannel;
+        this.bufSize = IO.bufferSize;
+        this.buffer = BufferUtil.allocate(this.bufSize);
+    }
+
+    @Override
+    public void close() throws IOException
+    {
+        this.channel.close();
+    }
+
+    @Override
+    public void writeTo(OutputStream outputStream, long skipTo, long length) throws IOException
+    {
+        this.channel.position(skipTo);
+
+        // copy from channel to output stream
+        long readTotal = 0;
+        while (readTotal < length)
+        {
+            BufferUtil.flipToFill(buffer);
+            int size = (int)Math.min(bufSize, length - readTotal);
+            buffer.limit(size);
+            int readLen = channel.read(buffer);
+            BufferUtil.flipToFlush(buffer, 0);
+            BufferUtil.writeTo(buffer, outputStream);
+            readTotal += readLen;
+        }
+    }
+}

jetty-server/src/test/java/org/eclipse/jetty/server/resource/RangeWriterTest.java

@@ -0,0 +1,156 @@
+//
+//  ========================================================================
+//  Copyright (c) 1995-2019 Mort Bay Consulting Pty. Ltd.
+//  ------------------------------------------------------------------------
+//  All rights reserved. This program and the accompanying materials
+//  are made available under the terms of the Eclipse Public License v1.0
+//  and Apache License v2.0 which accompanies this distribution.
+//
+//      The Eclipse Public License is available at
+//      http://www.eclipse.org/legal/epl-v10.html
+//
+//      The Apache License v2.0 is available at
+//      http://www.opensource.org/licenses/apache2.0.php
+//
+//  You may elect to redistribute this code under either of these licenses.
+//  ========================================================================
+//
+
+package org.eclipse.jetty.server.resource;
+
+import java.io.BufferedWriter;
+import java.io.ByteArrayOutputStream;
+import java.io.IOException;
+import java.nio.channels.SeekableByteChannel;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.nio.file.StandardOpenOption;
+import java.util.stream.Stream;
+
+import org.eclipse.jetty.toolchain.test.FS;
+import org.eclipse.jetty.toolchain.test.MavenTestingUtils;
+import org.eclipse.jetty.util.BufferUtil;
+import org.eclipse.jetty.util.resource.PathResource;
+import org.eclipse.jetty.util.resource.Resource;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.Arguments;
+import org.junit.jupiter.params.provider.MethodSource;
+
+import static java.nio.charset.StandardCharsets.UTF_8;
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.is;
+
+public class RangeWriterTest
+{
+    public static final String DATA = "01234567890abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWZYZ!@#$%^&*()_+/.,[]";
+
+    public static Path initDataFile() throws IOException
+    {
+        Path testDir = MavenTestingUtils.getTargetTestingPath(RangeWriterTest.class.getSimpleName());
+        FS.ensureEmpty(testDir);
+
+        Path dataFile = testDir.resolve("data.dat");
+        try (BufferedWriter writer = Files.newBufferedWriter(dataFile, UTF_8, StandardOpenOption.WRITE, StandardOpenOption.CREATE, StandardOpenOption.TRUNCATE_EXISTING))
+        {
+            writer.write(DATA);
+            writer.flush();
+        }
+
+        return dataFile;
+    }
+
+    public static Stream<Arguments> impls() throws IOException
+    {
+        Resource resource = new PathResource(initDataFile());
+
+        return Stream.of(
+            Arguments.of(new ByteBufferRangeWriter(BufferUtil.toBuffer(resource, true))),
+            Arguments.of(new ByteBufferRangeWriter(BufferUtil.toBuffer(resource, false))),
+            Arguments.of(new SeekableByteChannelRangeWriter((SeekableByteChannel)resource.getReadableByteChannel())),
+            Arguments.of(new InputStreamRangeWriter(() -> resource.getInputStream()))
+        );
+    }
+
+    @ParameterizedTest
+    @MethodSource("impls")
+    public void testSimpleRange(RangeWriter rangeWriter) throws IOException
+    {
+        ByteArrayOutputStream outputStream;
+
+        outputStream = new ByteArrayOutputStream();
+        rangeWriter.writeTo(outputStream, 10, 50);
+        assertThat("Range: 10 (len=50)", new String(outputStream.toByteArray(), UTF_8), is(DATA.substring(10, 60)));
+    }
+
+    @ParameterizedTest
+    @MethodSource("impls")
+    public void testSameRange_MultipleTimes(RangeWriter rangeWriter) throws IOException
+    {
+        ByteArrayOutputStream outputStream;
+
+        outputStream = new ByteArrayOutputStream();
+        rangeWriter.writeTo(outputStream, 10, 50);
+        assertThat("Range(a): 10 (len=50)", new String(outputStream.toByteArray(), UTF_8), is(DATA.substring(10, 60)));
+
+        outputStream = new ByteArrayOutputStream();
+        rangeWriter.writeTo(outputStream, 10, 50);
+        assertThat("Range(b): 10 (len=50)", new String(outputStream.toByteArray(), UTF_8), is(DATA.substring(10, 60)));
+    }
+
+    @ParameterizedTest
+    @MethodSource("impls")
+    public void testMultipleRanges_Ordered(RangeWriter rangeWriter) throws IOException
+    {
+        ByteArrayOutputStream outputStream;
+
+        outputStream = new ByteArrayOutputStream();
+        rangeWriter.writeTo(outputStream, 10, 20);
+        assertThat("Range(a): 10 (len=20)", new String(outputStream.toByteArray(), UTF_8), is(DATA.substring(10, 10 + 20)));
+
+        outputStream = new ByteArrayOutputStream();
+        rangeWriter.writeTo(outputStream, 35, 10);
+        assertThat("Range(b): 35 (len=10)", new String(outputStream.toByteArray(), UTF_8), is(DATA.substring(35, 35 + 10)));
+
+        outputStream = new ByteArrayOutputStream();
+        rangeWriter.writeTo(outputStream, 55, 10);
+        assertThat("Range(b): 55 (len=10)", new String(outputStream.toByteArray(), UTF_8), is(DATA.substring(55, 55 + 10)));
+    }
+
+    @ParameterizedTest
+    @MethodSource("impls")
+    public void testMultipleRanges_Overlapping(RangeWriter rangeWriter) throws IOException
+    {
+        ByteArrayOutputStream outputStream;
+
+        outputStream = new ByteArrayOutputStream();
+        rangeWriter.writeTo(outputStream, 10, 20);
+        assertThat("Range(a): 10 (len=20)", new String(outputStream.toByteArray(), UTF_8), is(DATA.substring(10, 10 + 20)));
+
+        outputStream = new ByteArrayOutputStream();
+        rangeWriter.writeTo(outputStream, 15, 20);
+        assertThat("Range(b): 15 (len=20)", new String(outputStream.toByteArray(), UTF_8), is(DATA.substring(15, 15 + 20)));
+
+        outputStream = new ByteArrayOutputStream();
+        rangeWriter.writeTo(outputStream, 20, 20);
+        assertThat("Range(b): 20 (len=20)", new String(outputStream.toByteArray(), UTF_8), is(DATA.substring(20, 20 + 20)));
+    }
+
+    @ParameterizedTest
+    @MethodSource("impls")
+    public void testMultipleRanges_ReverseOrder(RangeWriter rangeWriter) throws IOException
+    {
+        ByteArrayOutputStream outputStream;
+
+        outputStream = new ByteArrayOutputStream();
+        rangeWriter.writeTo(outputStream, 55, 10);
+        assertThat("Range(b): 55 (len=10)", new String(outputStream.toByteArray(), UTF_8), is(DATA.substring(55, 55 + 10)));
+
+        outputStream = new ByteArrayOutputStream();
+        rangeWriter.writeTo(outputStream, 35, 10);
+        assertThat("Range(b): 35 (len=10)", new String(outputStream.toByteArray(), UTF_8), is(DATA.substring(35, 35 + 10)));
+
+        outputStream = new ByteArrayOutputStream();
+        rangeWriter.writeTo(outputStream, 10, 20);
+        assertThat("Range(a): 10 (len=20)", new String(outputStream.toByteArray(), UTF_8), is(DATA.substring(10, 10 + 20)));
+    }
+}

jetty-util/src/main/java/org/eclipse/jetty/util/resource/PathResource.java

@@ -19,14 +19,12 @@
 package org.eclipse.jetty.util.resource;
 
 import java.io.File;
-import java.io.FileInputStream;
 import java.io.IOException;
 import java.io.InputStream;
 import java.net.MalformedURLException;
 import java.net.URI;
 import java.net.URISyntaxException;
 import java.net.URL;
-import java.nio.channels.FileChannel;
 import java.nio.channels.ReadableByteChannel;
 import java.nio.file.DirectoryIteratorException;
 import java.nio.file.DirectoryStream;
@@ -379,9 +377,7 @@ public class PathResource extends Resource
     @Override
     public InputStream getInputStream() throws IOException
     {
-        // Use a FileInputStream rather than Files.newInputStream(path)
+        return Files.newInputStream(path, StandardOpenOption.READ, StandardOpenOption.SPARSE);
-        // since it produces a stream with a fast skip implementation
-        return new FileInputStream(getFile());
     }
 
     @Override
@@ -393,7 +389,7 @@ public class PathResource extends Resource
     @Override
     public ReadableByteChannel getReadableByteChannel() throws IOException
     {
-        return FileChannel.open(path, StandardOpenOption.READ);
+        return Files.newByteChannel(path, StandardOpenOption.READ, StandardOpenOption.SPARSE);
     }
 
     @Override