From 663737396c8acef302ef5e4c2c18f56d792dd17f Mon Sep 17 00:00:00 2001
From: clayly <31773799+clayly@users.noreply.github.com>
Date: Thu, 14 May 2026 11:17:32 +0300
Subject: [PATCH] Fix range download: use real seek instead of read-and-discard
 (#3044)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

* 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.
---
 .../hawkbit/artifact/model/ArtifactStream.java        |  7 +++++++
 .../eclipse/hawkbit/rest/util/FileStreamingUtil.java  | 11 ++++++++---
 2 files changed, 15 insertions(+), 3 deletions(-)

diff --git a/hawkbit-artifact/hawkbit-artifact-api/src/main/java/org/eclipse/hawkbit/artifact/model/ArtifactStream.java b/hawkbit-artifact/hawkbit-artifact-api/src/main/java/org/eclipse/hawkbit/artifact/model/ArtifactStream.java
index 2fcc0efdc..e69fdeb65 100644
--- a/hawkbit-artifact/hawkbit-artifact-api/src/main/java/org/eclipse/hawkbit/artifact/model/ArtifactStream.java
+++ b/hawkbit-artifact/hawkbit-artifact-api/src/main/java/org/eclipse/hawkbit/artifact/model/ArtifactStream.java
@@ -44,6 +44,13 @@ public class ArtifactStream extends InputStream {
         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
     public void close() throws IOException {
         inputStream.close();
diff --git a/hawkbit-rest/hawkbit-rest-core/src/main/java/org/eclipse/hawkbit/rest/util/FileStreamingUtil.java b/hawkbit-rest/hawkbit-rest-core/src/main/java/org/eclipse/hawkbit/rest/util/FileStreamingUtil.java
index 66349a513..9f64bb4b7 100644
--- a/hawkbit-rest/hawkbit-rest-core/src/main/java/org/eclipse/hawkbit/rest/util/FileStreamingUtil.java
+++ b/hawkbit-rest/hawkbit-rest-core/src/main/java/org/eclipse/hawkbit/rest/util/FileStreamingUtil.java
@@ -37,7 +37,6 @@ import lombok.AccessLevel;
 import lombok.NoArgsConstructor;
 import lombok.Value;
 import lombok.extern.slf4j.Slf4j;
-import org.apache.commons.io.IOUtils;
 import org.eclipse.hawkbit.artifact.model.ArtifactStream;
 import org.eclipse.hawkbit.rest.exception.FileStreamingFailedException;
 import org.jspecify.annotations.NonNull;
@@ -54,7 +53,11 @@ import org.springframework.http.ResponseEntity;
 @Slf4j
 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>
@@ -260,7 +263,9 @@ public final class FileStreamingUtil {
         long total = 0;
         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;
         boolean toContinue = true;