Fix range download: use real seek instead of read-and-discard (#3044)
* perf(rest): fix range download to use real seek FileStreamingUtil.copyStreams called IOUtils.skipFully(from, start), which reads start bytes through a 2KB scratch buffer. Combined with ArtifactStream not overriding skip(long), a Range request at offset 600MB on an 800MB artifact made the server read+discard 600MB before serving any payload. With 80 concurrent devices this saturated CPU. Fix: - ArtifactStream.skip(long) now delegates to the wrapped stream so a FileInputStream can lseek(2). Non-seekable backends (CipherInputStream for encrypted artifacts, S3 streams) keep their existing behaviour. - FileStreamingUtil.copyStreams uses InputStream.skipNBytes(start) instead of IOUtils.skipFully so the call chain reaches the underlying skip(). JMH (single thread, 600MB offset, 1MB read): 27.21 ms -> 0.034 ms (800x). Real stack (80 parallel curl, 1MB range at 600MB offset): avg 728 ms -> 28 ms (26x), p99 966 ms -> 54 ms. Adds JMH test-scope dep and FileStreamingBenchmark/BufferSizeBenchmark for regression detection. Both gated on -Dperf=true so default test runs stay fast. * perf(rest): drop JMH benchmarks per upstream review Eclipse hawkBit minimizes dependencies. Drop jmh-core / jmh-generator-annprocess test-scope deps (also GPL-2.0 — not EPL-2.0 compatible) and the two JMH benchmarks added with the seek fix. Move the BUFFER_SIZE rationale into an inline comment in FileStreamingUtil so the empirical reasoning behind keeping the 8 KiB constant stays discoverable. The benchmarks may be reintroduced as a separate PR if upstream wants a perf-regression harness later.
This commit is contained in:
@@ -44,6 +44,13 @@ public class ArtifactStream extends InputStream {
|
|||||||
return inputStream.read(b, off, len);
|
return inputStream.read(b, off, len);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// Delegate so wrappers backed by a seekable stream (e.g. FileInputStream) actually seek
|
||||||
|
// instead of reading bytes through the default InputStream.skip implementation.
|
||||||
|
@Override
|
||||||
|
public long skip(final long n) throws IOException {
|
||||||
|
return inputStream.skip(n);
|
||||||
|
}
|
||||||
|
|
||||||
@Override
|
@Override
|
||||||
public void close() throws IOException {
|
public void close() throws IOException {
|
||||||
inputStream.close();
|
inputStream.close();
|
||||||
|
|||||||
@@ -37,7 +37,6 @@ import lombok.AccessLevel;
|
|||||||
import lombok.NoArgsConstructor;
|
import lombok.NoArgsConstructor;
|
||||||
import lombok.Value;
|
import lombok.Value;
|
||||||
import lombok.extern.slf4j.Slf4j;
|
import lombok.extern.slf4j.Slf4j;
|
||||||
import org.apache.commons.io.IOUtils;
|
|
||||||
import org.eclipse.hawkbit.artifact.model.ArtifactStream;
|
import org.eclipse.hawkbit.artifact.model.ArtifactStream;
|
||||||
import org.eclipse.hawkbit.rest.exception.FileStreamingFailedException;
|
import org.eclipse.hawkbit.rest.exception.FileStreamingFailedException;
|
||||||
import org.jspecify.annotations.NonNull;
|
import org.jspecify.annotations.NonNull;
|
||||||
@@ -54,7 +53,11 @@ import org.springframework.http.ResponseEntity;
|
|||||||
@Slf4j
|
@Slf4j
|
||||||
public final class FileStreamingUtil {
|
public final class FileStreamingUtil {
|
||||||
|
|
||||||
private static final int BUFFER_SIZE = 0x2000; // 8k
|
// 8 KiB: matches the Tomcat output buffer size set via response.setBufferSize(BUFFER_SIZE) below, so each read
|
||||||
|
// corresponds to a single downstream flush. Larger read buffers split into multiple Tomcat flushes and add
|
||||||
|
// per-request heap pressure under concurrency without improving throughput (end-to-end tests with 80 parallel
|
||||||
|
// 100 MiB range requests measured ~6x worse latency at 256 KiB versus 8 KiB).
|
||||||
|
private static final int BUFFER_SIZE = 0x2000;
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* <p>
|
* <p>
|
||||||
@@ -260,7 +263,9 @@ public final class FileStreamingUtil {
|
|||||||
long total = 0;
|
long total = 0;
|
||||||
int progressPercent = 1;
|
int progressPercent = 1;
|
||||||
|
|
||||||
IOUtils.skipFully(from, start);
|
// Use InputStream.skipNBytes so seekable backends (FileInputStream → lseek) advance in O(1)
|
||||||
|
// instead of reading and discarding 'start' bytes through a 2KB scratch buffer.
|
||||||
|
from.skipNBytes(start);
|
||||||
|
|
||||||
long toRead = length;
|
long toRead = length;
|
||||||
boolean toContinue = true;
|
boolean toContinue = true;
|
||||||
|
|||||||
Reference in New Issue
Block a user