Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(197)

Unified Diff: components/cronet/android/test/javatests/src/org/chromium/net/TestBidirectionalStreamCallback.java

Issue 1856073002: Coalesce small buffers in net::BidirectionalStream (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master
Patch Set: Address Andrei's comments Created 4 years, 8 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: components/cronet/android/test/javatests/src/org/chromium/net/TestBidirectionalStreamCallback.java
diff --git a/components/cronet/android/test/javatests/src/org/chromium/net/TestBidirectionalStreamCallback.java b/components/cronet/android/test/javatests/src/org/chromium/net/TestBidirectionalStreamCallback.java
index c7a630c85ead7a43b544b63d88380fbdf6ef8755..e8214eaefee88742d988c357282db8708a329f6a 100644
--- a/components/cronet/android/test/javatests/src/org/chromium/net/TestBidirectionalStreamCallback.java
+++ b/components/cronet/android/test/javatests/src/org/chromium/net/TestBidirectionalStreamCallback.java
@@ -13,6 +13,7 @@ import static junit.framework.Assert.assertTrue;
import java.nio.ByteBuffer;
import java.util.ArrayList;
+import java.util.Iterator;
import java.util.concurrent.Executor;
import java.util.concurrent.ExecutorService;
import java.util.concurrent.Executors;
@@ -39,6 +40,7 @@ public class TestBidirectionalStreamCallback extends BidirectionalStream.Callbac
private static final int READ_BUFFER_SIZE = 32 * 1024;
+ private boolean mHandleFlush;
// When false, the consumer is responsible for all calls into the stream
// that advance it.
private boolean mAutoAdvance = true;
@@ -63,7 +65,10 @@ public class TestBidirectionalStreamCallback extends BidirectionalStream.Callbac
private int mBufferPositionBeforeRead;
// Data to write.
- private ArrayList<ByteBuffer> mWriteBuffers = new ArrayList<ByteBuffer>();
+ private ArrayList<WriteBuffer> mWriteBuffers = new ArrayList<WriteBuffer>();
+
+ // Buffers that we yet to receive the corresponding onWriteCompleted callback.
+ private ArrayList<WriteBuffer> mWriteBuffersToBeAcked = new ArrayList<WriteBuffer>();
private class ExecutorThreadFactory implements ThreadFactory {
public Thread newThread(Runnable r) {
@@ -72,9 +77,18 @@ public class TestBidirectionalStreamCallback extends BidirectionalStream.Callbac
}
}
+ private static class WriteBuffer {
+ final ByteBuffer mBuffer;
+ final boolean mFlush;
+ public WriteBuffer(ByteBuffer buffer, boolean flush) {
+ mBuffer = buffer;
+ mFlush = flush;
+ }
+ }
+
public enum ResponseStep {
NOTHING,
- ON_REQUEST_HEADERS_SENT,
+ ON_STREAM_READY,
ON_RESPONSE_STARTED,
ON_READ_COMPLETED,
ON_WRITE_COMPLETED,
@@ -94,6 +108,10 @@ public class TestBidirectionalStreamCallback extends BidirectionalStream.Callbac
THROW_SYNC
}
+ public void setHandleFlush() {
+ mHandleFlush = true;
+ }
+
public void setAutoAdvance(boolean autoAdvance) {
mAutoAdvance = autoAdvance;
}
@@ -126,20 +144,28 @@ public class TestBidirectionalStreamCallback extends BidirectionalStream.Callbac
}
public void addWriteData(byte[] data) {
+ addWriteData(data, false);
+ }
+
+ public void addWriteData(byte[] data, boolean flush) {
ByteBuffer writeBuffer = ByteBuffer.allocateDirect(data.length);
writeBuffer.put(data);
writeBuffer.flip();
- mWriteBuffers.add(writeBuffer);
+ if (flush && !mHandleFlush) {
kapishnikov 2016/04/12 19:27:39 Related to the previous comment. I think that shou
xunjieli 2016/04/12 19:59:03 Done.
+ throw new IllegalArgumentException("cannot add flush when auto flush is on");
+ }
+ mWriteBuffers.add(new WriteBuffer(writeBuffer, flush));
+ mWriteBuffersToBeAcked.add(new WriteBuffer(writeBuffer, flush));
}
@Override
- public void onRequestHeadersSent(BidirectionalStream stream) {
+ public void onStreamReady(BidirectionalStream stream) {
assertEquals(mExecutorThread, Thread.currentThread());
assertFalse(stream.isDone());
assertEquals(ResponseStep.NOTHING, mResponseStep);
assertNull(mError);
- mResponseStep = ResponseStep.ON_REQUEST_HEADERS_SENT;
+ mResponseStep = ResponseStep.ON_STREAM_READY;
if (maybeThrowCancelOrPause(stream, mWriteStepBlock)) {
return;
}
@@ -151,7 +177,7 @@ public class TestBidirectionalStreamCallback extends BidirectionalStream.Callbac
assertEquals(mExecutorThread, Thread.currentThread());
assertFalse(stream.isDone());
assertTrue(mResponseStep == ResponseStep.NOTHING
- || mResponseStep == ResponseStep.ON_REQUEST_HEADERS_SENT
+ || mResponseStep == ResponseStep.ON_STREAM_READY
|| mResponseStep == ResponseStep.ON_WRITE_COMPLETED);
assertNull(mError);
@@ -200,9 +226,9 @@ public class TestBidirectionalStreamCallback extends BidirectionalStream.Callbac
assertFalse(stream.isDone());
assertNull(mError);
mResponseStep = ResponseStep.ON_WRITE_COMPLETED;
- if (!mWriteBuffers.isEmpty()) {
- assertEquals(buffer, mWriteBuffers.get(0));
- mWriteBuffers.remove(0);
+ if (!mWriteBuffersToBeAcked.isEmpty()) {
+ assertEquals(buffer, mWriteBuffersToBeAcked.get(0).mBuffer);
+ mWriteBuffersToBeAcked.remove(0);
}
if (maybeThrowCancelOrPause(stream, mWriteStepBlock)) {
return;
@@ -234,6 +260,8 @@ public class TestBidirectionalStreamCallback extends BidirectionalStream.Callbac
assertFalse(mOnErrorCalled);
assertFalse(mOnCanceledCalled);
assertNull(mError);
+ assertEquals(0, mWriteBuffers.size());
+ assertEquals(0, mWriteBuffersToBeAcked.size());
mResponseStep = ResponseStep.ON_SUCCEEDED;
mResponseInfo = info;
@@ -285,8 +313,22 @@ public class TestBidirectionalStreamCallback extends BidirectionalStream.Callbac
public void startNextWrite(BidirectionalStream stream) {
if (!mWriteBuffers.isEmpty()) {
- boolean isLastBuffer = mWriteBuffers.size() == 1;
- stream.write(mWriteBuffers.get(0), isLastBuffer);
+ if (mHandleFlush) {
+ Iterator<WriteBuffer> iterator = mWriteBuffers.iterator();
+ while (iterator.hasNext()) {
+ WriteBuffer b = iterator.next();
+ stream.write(b.mBuffer, !iterator.hasNext());
+ iterator.remove();
+ if (b.mFlush) {
+ stream.flush();
+ break;
+ }
+ }
+ } else {
+ boolean isLastBuffer = (mWriteBuffers.size() == 1);
+ stream.write(mWriteBuffers.get(0).mBuffer, isLastBuffer);
+ mWriteBuffers.remove(0);
+ }
}
}

Powered by Google App Engine
This is Rietveld 408576698