Skip to content

Commit ee3cf1d

Browse files
Read InputStream bodies straight into the buffer, no staging array (#2233)
Motivation: `InputStreamBodyGenerator.Body.transferTo` allocated a new `byte[]` on every invocation, sized to the target `ByteBuf`'s writable region, then read the `InputStream` into that array before copying the bytes into the `ByteBuf`. This incurred both a per-chunk allocation and an unnecessary copy. Additionally, the implementation reserved a 10-byte margin (`writableBytes() - 10`), which could cause `NegativeArraySizeException` or prematurely stop transfers when the target had 10 or fewer writable bytes. Modification: `transferTo` now reads directly from the `InputStream` into the target `ByteBuf` using `ByteBuf.writeBytes(InputStream, int)`, eliminating the intermediate staging array, the extra copy, and the per-instance chunk field. The state machine is unchanged: each call performs a single stream read, returns `CONTINUE` while data remains, and returns `STOP` at EOF or on an I/O error (with logging unchanged). The legacy 10-byte writable margin has been removed, allowing each transfer to use the full writable capacity and correctly handle small writable regions. Although AsyncHttpClient's internal request path uses `NettyInputStreamBody`, this implementation remains part of the public `InputStreamBodyGenerator.createBody()` API and is used by external callers. Result: Reduces allocation and copy overhead during transfers, correctly supports small writable buffers, preserves existing transfer semantics, and adds tests covering multi-read transfers, single-read completion, empty streams, and transfers through a 10-byte buffer. No public API changes.
1 parent 1a73921 commit ee3cf1d

2 files changed

Lines changed: 130 additions & 14 deletions

File tree

‎client/src/main/java/org/asynchttpclient/request/body/generator/InputStreamBodyGenerator.java‎

Lines changed: 8 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,6 @@ private static class InputStreamBody implements Body {
5858

5959
private final InputStream inputStream;
6060
private final long contentLength;
61-
private byte[] chunk;
6261

6362
private InputStreamBody(InputStream inputStream, long contentLength) {
6463
this.inputStream = inputStream;
@@ -72,23 +71,18 @@ public long getContentLength() {
7271

7372
@Override
7473
public BodyState transferTo(ByteBuf target) {
75-
76-
// To be safe.
77-
chunk = new byte[target.writableBytes() - 10];
78-
79-
int read = -1;
80-
boolean write = false;
74+
// Read straight from the stream into the target buffer instead of staging through a per-call byte[].
75+
// For heap target buffers this drops both the staging array and the copy; for direct buffers Netty
76+
// still stages through a temporary heap array internally (InputStream can only read into a byte[]),
77+
// so there the win is smaller. Mirrors InputStreamMultipartPart, which writes the full writable region.
78+
int read;
8179
try {
82-
read = inputStream.read(chunk);
80+
read = target.writeBytes(inputStream, target.writableBytes());
8381
} catch (IOException ex) {
8482
LOGGER.warn("Unable to read", ex);
83+
return BodyState.STOP;
8584
}
86-
87-
if (read > 0) {
88-
target.writeBytes(chunk, 0, read);
89-
write = true;
90-
}
91-
return write ? BodyState.CONTINUE : BodyState.STOP;
85+
return read > 0 ? BodyState.CONTINUE : BodyState.STOP;
9286
}
9387

9488
@Override
Lines changed: 122 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,122 @@
1+
/*
2+
* Copyright (c) 2026 AsyncHttpClient Project. All rights reserved.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* http://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
package org.asynchttpclient.request.body.generator;
17+
18+
import io.github.artsok.RepeatedIfExceptionsTest;
19+
import io.netty.buffer.ByteBuf;
20+
import io.netty.buffer.Unpooled;
21+
import org.asynchttpclient.request.body.Body;
22+
import org.asynchttpclient.request.body.Body.BodyState;
23+
24+
import java.io.ByteArrayInputStream;
25+
import java.io.ByteArrayOutputStream;
26+
import java.io.IOException;
27+
import java.util.Random;
28+
29+
import static org.junit.jupiter.api.Assertions.assertArrayEquals;
30+
import static org.junit.jupiter.api.Assertions.assertEquals;
31+
32+
/**
33+
* Covers {@link InputStreamBodyGenerator}'s {@link Body#transferTo(ByteBuf)}, which now reads straight from
34+
* the stream into the target buffer (dropping the per-call staging {@code byte[]} and copy for heap buffers).
35+
* The whole stream must still be transferred byte-for-byte, CONTINUE while data remains and STOP at EOF.
36+
*/
37+
public class InputStreamBodyGeneratorTest {
38+
39+
private static final int CHUNK_SIZE = 1024 * 8;
40+
41+
@RepeatedIfExceptionsTest(repeats = 5)
42+
public void streamsAllBytesAcrossMultipleReads() throws IOException {
43+
final byte[] src = new byte[3 * CHUNK_SIZE + 42];
44+
new Random().nextBytes(src);
45+
46+
Body body = new InputStreamBodyGenerator(new ByteArrayInputStream(src)).createBody();
47+
ByteBuf chunkBuffer = Unpooled.buffer(CHUNK_SIZE);
48+
ByteArrayOutputStream collected = new ByteArrayOutputStream();
49+
try {
50+
BodyState state;
51+
while ((state = body.transferTo(chunkBuffer)) != BodyState.STOP) {
52+
assertEquals(BodyState.CONTINUE, state, "a stream with data left must report CONTINUE");
53+
byte[] b = new byte[chunkBuffer.readableBytes()];
54+
chunkBuffer.readBytes(b);
55+
collected.write(b);
56+
chunkBuffer.clear();
57+
}
58+
assertArrayEquals(src, collected.toByteArray(), "the whole stream must be transferred unchanged");
59+
} finally {
60+
chunkBuffer.release();
61+
body.close();
62+
}
63+
}
64+
65+
@RepeatedIfExceptionsTest(repeats = 5)
66+
public void singleReadDrainsASmallStream() throws IOException {
67+
final byte[] src = new byte[CHUNK_SIZE - 100]; // fits in one writable region, so one read drains it
68+
new Random().nextBytes(src);
69+
70+
Body body = new InputStreamBodyGenerator(new ByteArrayInputStream(src)).createBody();
71+
ByteBuf chunkBuffer = Unpooled.buffer(CHUNK_SIZE);
72+
try {
73+
assertEquals(BodyState.CONTINUE, body.transferTo(chunkBuffer));
74+
assertEquals(src.length, chunkBuffer.readableBytes(), "one read should drain a small stream");
75+
chunkBuffer.clear();
76+
assertEquals(BodyState.STOP, body.transferTo(chunkBuffer), "body at EOF");
77+
} finally {
78+
chunkBuffer.release();
79+
body.close();
80+
}
81+
}
82+
83+
@RepeatedIfExceptionsTest(repeats = 5)
84+
public void emptyStreamStopsImmediately() throws IOException {
85+
Body body = new InputStreamBodyGenerator(new ByteArrayInputStream(new byte[0])).createBody();
86+
ByteBuf chunkBuffer = Unpooled.buffer(CHUNK_SIZE);
87+
try {
88+
assertEquals(BodyState.STOP, body.transferTo(chunkBuffer), "an empty stream must STOP immediately");
89+
assertEquals(0, chunkBuffer.readableBytes(), "nothing should be written for an empty stream");
90+
} finally {
91+
chunkBuffer.release();
92+
body.close();
93+
}
94+
}
95+
96+
// Locks the removal of the old "writableBytes() - 10" margin: with a writable region of 10 or fewer bytes the
97+
// margin made the transfer length 0 (or negative), so it silently STOPped without writing / threw. The stream
98+
// must now still be drained through a tiny target buffer.
99+
@RepeatedIfExceptionsTest(repeats = 5)
100+
public void smallWritableRegionStillTransfers() throws IOException {
101+
final byte[] src = new byte[25];
102+
new Random().nextBytes(src);
103+
104+
Body body = new InputStreamBodyGenerator(new ByteArrayInputStream(src)).createBody();
105+
ByteBuf chunkBuffer = Unpooled.buffer(10, 10); // writableBytes() == 10, the old margin's boundary
106+
ByteArrayOutputStream collected = new ByteArrayOutputStream();
107+
try {
108+
BodyState state;
109+
while ((state = body.transferTo(chunkBuffer)) != BodyState.STOP) {
110+
assertEquals(BodyState.CONTINUE, state, "a stream with data left must report CONTINUE");
111+
byte[] b = new byte[chunkBuffer.readableBytes()];
112+
chunkBuffer.readBytes(b);
113+
collected.write(b);
114+
chunkBuffer.clear();
115+
}
116+
assertArrayEquals(src, collected.toByteArray(), "the whole stream must drain through a tiny buffer");
117+
} finally {
118+
chunkBuffer.release();
119+
body.close();
120+
}
121+
}
122+
}

0 commit comments

Comments
 (0)