You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by ma...@apache.org on 2015/01/08 14:09:47 UTC
svn commit: r1650268 -
/tomcat/trunk/java/org/apache/coyote/http11/InternalNio2OutputBuffer.java
Author: markt
Date: Thu Jan 8 13:09:47 2015
New Revision: 1650268
URL: http://svn.apache.org/r1650268
Log:
Deque<ByteBufferHolder> is a little more complex than List<ByteBuffer>
but using the same structure for all connectors will improve code re-use
and thereby improve maintainability.
Modified:
tomcat/trunk/java/org/apache/coyote/http11/InternalNio2OutputBuffer.java
Modified: tomcat/trunk/java/org/apache/coyote/http11/InternalNio2OutputBuffer.java
URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/coyote/http11/InternalNio2OutputBuffer.java?rev=1650268&r1=1650267&r2=1650268&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/coyote/http11/InternalNio2OutputBuffer.java (original)
+++ tomcat/trunk/java/org/apache/coyote/http11/InternalNio2OutputBuffer.java Thu Jan 8 13:09:47 2015
@@ -31,6 +31,7 @@ import java.util.concurrent.TimeoutExcep
import javax.servlet.RequestDispatcher;
import org.apache.coyote.Response;
+import org.apache.tomcat.util.buf.ByteBufferHolder;
import org.apache.tomcat.util.net.AbstractEndpoint;
import org.apache.tomcat.util.net.Nio2Channel;
import org.apache.tomcat.util.net.Nio2Endpoint;
@@ -79,11 +80,6 @@ public class InternalNio2OutputBuffer ex
protected AbstractEndpoint<Nio2Channel> endpoint = null;
/**
- * Used instead of the deque since it looks equivalent and simpler.
- */
- protected ArrayList<ByteBuffer> bufferedWrites = new ArrayList<>();
-
- /**
* Exception that occurred during writing.
*/
protected IOException e = null;
@@ -109,9 +105,9 @@ public class InternalNio2OutputBuffer ex
if (attachment.hasRemaining()) {
arrayList.add(attachment);
}
- for (ByteBuffer buffer : bufferedWrites) {
+ for (ByteBufferHolder buffer : bufferedWrites) {
buffer.flip();
- arrayList.add(buffer);
+ arrayList.add(buffer.getBuf());
}
bufferedWrites.clear();
ByteBuffer[] array = arrayList.toArray(EMPTY_BUF_ARRAY);
@@ -164,9 +160,9 @@ public class InternalNio2OutputBuffer ex
arrayList.add(buffer);
}
}
- for (ByteBuffer buffer : bufferedWrites) {
+ for (ByteBufferHolder buffer : bufferedWrites) {
buffer.flip();
- arrayList.add(buffer);
+ arrayList.add(buffer.getBuf());
}
bufferedWrites.clear();
ByteBuffer[] array = arrayList.toArray(EMPTY_BUF_ARRAY);
@@ -303,7 +299,7 @@ public class InternalNio2OutputBuffer ex
private void addToBuffers(byte[] buf, int offset, int length) {
ByteBuffer buffer = ByteBuffer.allocate(length);
buffer.put(buf, offset, length);
- bufferedWrites.add(buffer);
+ bufferedWrites.add(new ByteBufferHolder(buffer, false));
}
@@ -336,8 +332,9 @@ public class InternalNio2OutputBuffer ex
}
try {
if (bufferedWrites.size() > 0) {
- for (ByteBuffer buffer : bufferedWrites) {
- buffer.flip();
+ for (ByteBufferHolder holder : bufferedWrites) {
+ holder.flip();
+ ByteBuffer buffer = holder.getBuf();
while (buffer.hasRemaining()) {
if (socketWrapper.getSocket().write(buffer).get(socketWrapper.getTimeout(), TimeUnit.MILLISECONDS).intValue() < 0) {
throw new EOFException(sm.getString("iob.failedwrite"));
@@ -383,9 +380,9 @@ public class InternalNio2OutputBuffer ex
if (socketWriteBuffer.hasRemaining()) {
arrayList.add(socketWriteBuffer);
}
- for (ByteBuffer buffer : bufferedWrites) {
+ for (ByteBufferHolder buffer : bufferedWrites) {
buffer.flip();
- arrayList.add(buffer);
+ arrayList.add(buffer.getBuf());
}
bufferedWrites.clear();
ByteBuffer[] array = arrayList.toArray(EMPTY_BUF_ARRAY);
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: svn commit: r1650268 - /tomcat/trunk/java/org/apache/coyote/http11/InternalNio2OutputBuffer.java
Posted by Rémy Maucherat <re...@apache.org>.
2015-01-08 14:46 GMT+01:00 Mark Thomas <ma...@apache.org>:
> On 08/01/2015 13:30, Rémy Maucherat wrote:
> > 2015-01-08 14:09 GMT+01:00 <ma...@apache.org>:
> >
> >> Author: markt
> >> Date: Thu Jan 8 13:09:47 2015
> >> New Revision: 1650268
> >>
> >> URL: http://svn.apache.org/r1650268
> >> Log:
> >> Deque<ByteBufferHolder> is a little more complex than List<ByteBuffer>
> >> but using the same structure for all connectors will improve code re-use
> >> and thereby improve maintainability.
> >>
> > When I tried that originally in this connector, I ran into corruption
> > problems. Maybe it is fine now, but it is worth running the relevant
> > testsuite sections repeatedly just in case.
>
> I've been running the unit tests over this for several days any haven't
> seen any issues in this area. I'll keep an eye on the CI system in case
> it finds issues that didn't crop up on my dev machine.
>
Well, ok, it's kind of obvious the structure itself cannot create
corruption. Things back then worked well with a simple list and not this
deque.
>
> > Also it adds an additional layer of objects without providing additional
> reuse > I could see, so this structure should be removed IMO rather than
> generalized.
>
> Removing it is on the TODO list along with a handful of other things I
> spotted while I was working on this refactoring. Having got to a point
> where I had a single Http11OutputBuffer and passing unit tests I wanted
> to get it into svn before continuing.
>
Good. It would be fun to have but is probably dramatically risky if at all
possible.
Rémy
Re: svn commit: r1650268 - /tomcat/trunk/java/org/apache/coyote/http11/InternalNio2OutputBuffer.java
Posted by Mark Thomas <ma...@apache.org>.
On 08/01/2015 13:30, Rémy Maucherat wrote:
> 2015-01-08 14:09 GMT+01:00 <ma...@apache.org>:
>
>> Author: markt
>> Date: Thu Jan 8 13:09:47 2015
>> New Revision: 1650268
>>
>> URL: http://svn.apache.org/r1650268
>> Log:
>> Deque<ByteBufferHolder> is a little more complex than List<ByteBuffer>
>> but using the same structure for all connectors will improve code re-use
>> and thereby improve maintainability.
>>
> When I tried that originally in this connector, I ran into corruption
> problems. Maybe it is fine now, but it is worth running the relevant
> testsuite sections repeatedly just in case.
I've been running the unit tests over this for several days any haven't
seen any issues in this area. I'll keep an eye on the CI system in case
it finds issues that didn't crop up on my dev machine.
> Also it adds an additional layer of objects without providing additional reuse > I could see, so this structure should be removed IMO rather than
generalized.
Removing it is on the TODO list along with a handful of other things I
spotted while I was working on this refactoring. Having got to a point
where I had a single Http11OutputBuffer and passing unit tests I wanted
to get it into svn before continuing.
Mark
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: svn commit: r1650268 - /tomcat/trunk/java/org/apache/coyote/http11/InternalNio2OutputBuffer.java
Posted by Rémy Maucherat <re...@apache.org>.
2015-01-08 14:09 GMT+01:00 <ma...@apache.org>:
> Author: markt
> Date: Thu Jan 8 13:09:47 2015
> New Revision: 1650268
>
> URL: http://svn.apache.org/r1650268
> Log:
> Deque<ByteBufferHolder> is a little more complex than List<ByteBuffer>
> but using the same structure for all connectors will improve code re-use
> and thereby improve maintainability.
>
> When I tried that originally in this connector, I ran into corruption
problems. Maybe it is fine now, but it is worth running the relevant
testsuite sections repeatedly just in case. Also it adds an additional
layer of objects without providing additional reuse I could see, so this
structure should be removed IMO rather than generalized.
Rémy