1
0
mirror of https://github.com/square/okhttp.git synced 2026-01-25 16:01:38 +03:00

Improve connection reuse.

Instead of trying to read the last bytes of a content body
when the input stream is read, do it when the connection is
closed. This approach is less elegant, since it requires
a connection to do work on behalf of a potentially nonexistent
follow up connection. But it should result in more reuse in
practice.

http://code.google.com/p/android/issues/detail?id=38817
This commit is contained in:
Jesse Wilson
2012-12-06 21:06:45 -05:00
parent f3d956e25b
commit 04d742bd3d
3 changed files with 77 additions and 29 deletions

View File

@@ -24,6 +24,7 @@ import java.io.OutputStream;
import java.net.CacheRequest;
import java.net.CookieHandler;
import java.net.ProtocolException;
import java.net.Socket;
import libcore.io.Streams;
import libcore.util.Libcore;
@@ -36,6 +37,13 @@ final class HttpTransport implements Transport {
*/
private static final int MAX_REQUEST_BUFFER_LENGTH = 32768;
/**
* The timeout to use while discarding a stream of input data. Since this is
* used for connection reuse, this timeout should be significantly less than
* the time it takes to establish a new connection.
*/
private static final int DISCARD_STREAM_TIMEOUT_MILLIS = 30;
private final HttpEngine httpEngine;
private final InputStream socketIn;
private final OutputStream socketOut;
@@ -169,17 +177,36 @@ final class HttpTransport implements Transport {
}
if (responseBodyIn != null) {
// Discard the response body before the connection can be reused.
try {
Streams.skipAll(responseBodyIn);
} catch (IOException e) {
return false;
}
return discardStream(httpEngine, responseBodyIn);
}
return true;
}
/**
* Discards the response body so that the connection can be reused. This
* needs to be done judiciously, since it delays the current request in
* order to speed up a potential future request that may never occur.
*/
private static boolean discardStream(HttpEngine httpEngine, InputStream responseBodyIn) {
HttpConnection connection = httpEngine.connection;
if (connection == null) return false;
Socket socket = connection.getSocket();
if (socket == null) return false;
try {
int socketTimeout = socket.getSoTimeout();
socket.setSoTimeout(DISCARD_STREAM_TIMEOUT_MILLIS);
try {
Streams.skipAll(responseBodyIn);
return true;
} finally {
socket.setSoTimeout(socketTimeout);
}
} catch (IOException e) {
return false;
}
}
@Override public InputStream getTransferStream(CacheRequest cacheRequest) throws IOException {
if (!httpEngine.hasResponseBody()) {
return new FixedLengthInputStream(socketIn, cacheRequest, httpEngine, 0);
@@ -396,10 +423,10 @@ final class HttpTransport implements Transport {
if (closed) {
return;
}
closed = true;
if (bytesRemaining != 0) {
if (bytesRemaining != 0 && !discardStream(httpEngine, this)) {
unexpectedEndOfInput();
}
closed = true;
}
}
@@ -407,7 +434,6 @@ final class HttpTransport implements Transport {
* An HTTP body with alternating chunk sizes and chunk bodies.
*/
private static class ChunkedInputStream extends AbstractHttpInputStream {
private static final int MIN_LAST_CHUNK_LENGTH = "\r\n0\r\n\r\n".length();
private static final int NO_CHUNK_YET = -1;
private final HttpTransport transport;
private int bytesRemainingInChunk = NO_CHUNK_YET;
@@ -439,18 +465,6 @@ final class HttpTransport implements Transport {
}
bytesRemainingInChunk -= read;
cacheWrite(buffer, offset, read);
/*
* If we're at the end of a chunk and the next chunk size is readable,
* read it! Reading the last chunk causes the underlying connection to
* be recycled and we want to do that as early as possible. Otherwise
* self-delimiting streams like gzip will never be recycled.
* http://code.google.com/p/android/issues/detail?id=7059
*/
if (bytesRemainingInChunk == 0 && in.available() >= MIN_LAST_CHUNK_LENGTH) {
readChunkSize();
}
return read;
}
@@ -490,11 +504,10 @@ final class HttpTransport implements Transport {
if (closed) {
return;
}
closed = true;
if (hasMoreChunks) {
if (hasMoreChunks && !discardStream(httpEngine, this)) {
unexpectedEndOfInput();
}
closed = true;
}
}

View File

@@ -506,7 +506,8 @@ public final class HttpResponseCacheTest extends TestCase {
}
private void testClientPrematureDisconnect(TransferKind transferKind) throws IOException {
MockResponse response = new MockResponse();
// Setting a low transfer speed ensures that stream discarding will time out.
MockResponse response = new MockResponse().setBytesPerSecond(6);
transferKind.setBody(response, "ABCDE\nFGHIJKLMNOPQRSTUVWXYZ", 1024);
server.enqueue(response);
server.enqueue(new MockResponse().setBody("Request #2"));

View File

@@ -20,10 +20,6 @@ import com.google.mockwebserver.MockResponse;
import com.google.mockwebserver.MockWebServer;
import com.google.mockwebserver.RecordedRequest;
import com.google.mockwebserver.SocketPolicy;
import static com.google.mockwebserver.SocketPolicy.DISCONNECT_AT_END;
import static com.google.mockwebserver.SocketPolicy.DISCONNECT_AT_START;
import static com.google.mockwebserver.SocketPolicy.SHUTDOWN_INPUT_AT_END;
import static com.google.mockwebserver.SocketPolicy.SHUTDOWN_OUTPUT_AT_END;
import com.squareup.okhttp.OkHttpConnection;
import com.squareup.okhttp.OkHttpsConnection;
import java.io.ByteArrayOutputStream;
@@ -75,6 +71,11 @@ import javax.net.ssl.X509TrustManager;
import junit.framework.TestCase;
import libcore.net.ssl.SslContextBuilder;
import static com.google.mockwebserver.SocketPolicy.DISCONNECT_AT_END;
import static com.google.mockwebserver.SocketPolicy.DISCONNECT_AT_START;
import static com.google.mockwebserver.SocketPolicy.SHUTDOWN_INPUT_AT_END;
import static com.google.mockwebserver.SocketPolicy.SHUTDOWN_OUTPUT_AT_END;
/**
* Android's URLConnectionTest.
*/
@@ -1181,6 +1182,39 @@ public final class URLConnectionTest extends TestCase {
assertEquals(1, server.takeRequest().getSequenceNumber());
}
public void testEarlyDisconnectDoesntHarmPoolingWithChunkedEncoding() throws Exception {
testEarlyDisconnectDoesntHarmPooling(TransferKind.CHUNKED);
}
public void testEarlyDisconnectDoesntHarmPoolingWithFixedLengthEncoding() throws Exception {
testEarlyDisconnectDoesntHarmPooling(TransferKind.FIXED_LENGTH);
}
private void testEarlyDisconnectDoesntHarmPooling(TransferKind transferKind) throws Exception {
MockResponse response1 = new MockResponse();
transferKind.setBody(response1, "ABCDEFGHIJK", 1024);
server.enqueue(response1);
MockResponse response2 = new MockResponse();
transferKind.setBody(response2, "LMNOPQRSTUV", 1024);
server.enqueue(response2);
server.play();
URLConnection connection1 = openConnection(server.getUrl("/"));
InputStream in1 = connection1.getInputStream();
assertEquals("ABCDE", readAscii(in1, 5));
in1.close();
OkHttpConnection connection2 = openConnection(server.getUrl("/"));
InputStream in2 = connection2.getInputStream();
assertEquals("LMNOP", readAscii(in2, 5));
in2.close();
assertEquals(0, server.takeRequest().getSequenceNumber());
assertEquals(1, server.takeRequest().getSequenceNumber()); // Connection is pooled!
}
/**
* Obnoxiously test that the chunk sizes transmitted exactly equal the
* requested data+chunk header size. Although setChunkedStreamingMode()