Details
-
Type:
Improvement
-
Status:
Closed
-
Priority:
Major
-
Resolution: Fixed
-
Affects Version/s: 7.0.0.pre5, 6.1.14
-
Fix Version/s: 6.1.15.rc3
-
Component/s: None
-
Labels:None
-
Number of attachments :
Description
From Joshua Ball:
Tell me if there is another place where I should send patches to. But
this one probably needs to be discussed anyway before it is committed.
When profiling a server under heavy traffic load, I noticed that a lot
of lock contention was occurring on the ArrayList class. I traced it
down to locks being acquired in the AbstractBuffers class.
For some reason ArrayList is being used as a stack. I think a queue
makes much more sense, since then items can be put onto the queue and
removed simultaneously.
The following patch replaces the use of ArrayList with a
ConcurrentLinkedQueue. Profiling the server with the patch applied
makes the contention go away.
Josh "Ua" Ball
diff -ur jetty-6.1.14_theirs/modules/jetty/src/main/java/org/mortbay/jetty/AbstractBuffers.java
jetty-6.1.14/modules/jetty/src/main/java/org/mortbay/jetty/AbstractBuffers.java
— jetty-6.1.14_theirs/modules/jetty/src/main/java/org/mortbay/jetty/AbstractBuffers.java 2008-11-13
08:19:44.000000000 -0800
+++ jetty-6.1.14/modules/jetty/src/main/java/org/mortbay/jetty/AbstractBuffers.java 2009-01-15
18:36:33.234375000 -0800
@@ -19,6 +19,7 @@
import org.mortbay.component.AbstractLifeCycle;
import org.mortbay.io.Buffer;
import org.mortbay.io.Buffers;
+import java.util.concurrent.ConcurrentLinkedQueue;
/* ------------------------------------------------------------ */
/** Abstract Buffer pool.
@@ -32,11 +33,15 @@
private int _requestBufferSize=8*1024;
private int _responseBufferSize=24*1024;
+ // extending ConcurrentLinkedQueue makes it easier to spot the
monitors in a profiling tool
+
+ class AbstractBuffersQueue extends ConcurrentLinkedQueue<Buffer> { }
+
// Use and array of buffers to avoid contention
- private transient ArrayList _headerBuffers=new ArrayList();
+ private transient AbstractBuffersQueue _headerBuffers=new
AbstractBuffersQueue();
protected transient int _loss; - private transient ArrayList _requestBuffers;
- private transient ArrayList _responseBuffers;
+ private transient AbstractBuffersQueue _requestBuffers;
+ private transient AbstractBuffersQueue _responseBuffers;
/* ------------------------------------------------------------ */
/**
@@ -101,31 +106,19 @@
public Buffer getBuffer(int size)
{
if (size==_headerBufferSize)
- {
- synchronized(_headerBuffers)
- { - if (_headerBuffers.size()>0) - return (Buffer) _headerBuffers.remove(_headerBuffers.size()-1); - }
- return newBuffer(size);
+ { + Buffer ret = _headerBuffers.poll(); + return ret == null ? newBuffer(size) : ret; }else if (size==_responseBufferSize)
{ - synchronized(_responseBuffers)
- {
- if (_responseBuffers.size()==0)
- return newBuffer(size);
- return (Buffer)
_responseBuffers.remove(_responseBuffers.size()-1);
- }
+ Buffer ret = _responseBuffers.poll();
+ return ret == null ? newBuffer(size) : ret;
}
else if (size==_requestBufferSize)
{ - synchronized(_requestBuffers)
- {
- if (_requestBuffers.size()==0)
- return newBuffer(size);
- return (Buffer)
_requestBuffers.remove(_requestBuffers.size()-1);
- }
+ Buffer ret = _requestBuffers.poll();
+ return ret == null ? newBuffer(size) : ret;
}
return newBuffer(size);
@@ -141,24 +134,15 @@
int c=buffer.capacity();
if (c==_headerBufferSize)
{
- synchronized(_headerBuffers)
- {
- _headerBuffers.add(buffer);
- }
+ _headerBuffers.offer(buffer);
}
else if (c==_responseBufferSize)
{ - synchronized(_responseBuffers)
- {
- _responseBuffers.add(buffer);
- }
+ _responseBuffers.offer(buffer);
}
else if (c==_requestBufferSize)
{ - synchronized(_requestBuffers)
- {
- _requestBuffers.add(buffer);
- }
+ _requestBuffers.offer(buffer);
{ super.doStart(); - if (_headerBuffers!=null) - _headerBuffers.clear(); - else - _headerBuffers=new ArrayList(); - - if (_requestBuffers!=null) - _requestBuffers.clear(); - else - _requestBuffers=new ArrayList(); + _headerBuffers = new AbstractBuffersQueue(); + + _requestBuffers=new AbstractBuffersQueue(); - if (_responseBuffers!=null) - _responseBuffers.clear(); - else - _responseBuffers=new ArrayList(); + _responseBuffers=new AbstractBuffersQueue(); }
}
}
}
@@ -168,20 +152,11 @@
I have some concerns (other than java 1.5 dependency for jetty 6), with
the object creation that concurrent link queue does.
I may investigate applying it's non blocking algorithms to our ArrayQueue
class.