David Daney wrote:
Currently we keep a HTTP keep-alive connection open forever.
Although allowed by RFC 2616, this is forbidden by several other
specifications derived from it (DLNA and UPnP). In addition to being
incompatible with these other specification, it is a waste of resources
to leave these connections open. A further point of reference is Sun's
runtime which closes after 10 seconds of idle time.
I am using http://gcc.gnu.org/bugzilla/show_bug.cgi?id=26082 to track
the issue.
The patch factors the connection pooling code into a new class
(HTTPConnection.Pool) and adds a thread that closes the idle connections.
Also of note is the ability to pool more than one connection to a single
server. Having more than one pooled connection to the same server
should give higher throughput to programs that read from the same server
on multiple threads.
The patch and testing are against GCC-4.1. If no objections are raised,
I will retest on the classpath HEAD before committing.
This is what I committed after testing on the HEAD.
2006-02-24 David Daney <[EMAIL PROTECTED]>
PR classpath/26082
* gnu/java/net/protocol/http/HTTPConnection.java (pool): Changed to
type Pool.
(Pool): New inner class.
(timeLastUsed): New field.
(setPool): Changed parameter type to Pool.
(release): Moved pool management logic to new class Pool.
* gnu/java/net/protocol/http/HTTPURLConnection.java (connectionPool):
Removed.
(maxConnections) : Removed.
(GetHTTPPropertiesAction.run): Don't initialize maxConnections.
(getConnection): Moved pool management logic to HTTPConnection.Pool.
Index: gnu/java/net/protocol/http/HTTPConnection.java
===================================================================
RCS file:
/sources/classpath/classpath/gnu/java/net/protocol/http/HTTPConnection.java,v
retrieving revision 1.10
diff -u -r1.10 HTTPConnection.java
--- gnu/java/net/protocol/http/HTTPConnection.java 12 Oct 2005 19:48:25
-0000 1.10
+++ gnu/java/net/protocol/http/HTTPConnection.java 24 Feb 2006 20:36:01
-0000
@@ -1,5 +1,5 @@
/* HTTPConnection.java --
- Copyright (C) 2004, 2005 Free Software Foundation, Inc.
+ Copyright (C) 2004, 2005, 2006 Free Software Foundation, Inc.
This file is part of GNU Classpath.
@@ -53,7 +53,8 @@
import java.util.ArrayList;
import java.util.HashMap;
import java.util.Iterator;
-import java.util.LinkedHashMap;
+import java.util.LinkedList;
+import java.util.ListIterator;
import java.util.List;
import java.util.Map;
@@ -164,7 +165,7 @@
/**
* The pool that this connection is a member of (if any).
*/
- private LinkedHashMap pool;
+ private Pool pool;
/**
* Creates a new HTTP connection.
@@ -331,27 +332,226 @@
}
/**
+ * Manages a pool of HTTPConections. The pool will have a maximum
+ * size determined by the value of the maxConn parameter passed to
+ * the [EMAIL PROTECTED] #get} method. This value inevitably comes from the
+ * http.maxConnections system property. If the
+ * classpath.net.http.keepAliveTTL system property is set, that will
+ * be the maximum time (in seconds) that an idle connection will be
+ * maintained.
+ */
+ static class Pool
+ {
+ /**
+ * Singleton instance of the pool.
+ */
+ static Pool instance = new Pool();
+
+ /**
+ * The pool
+ */
+ final LinkedList connectionPool = new LinkedList();
+
+ /**
+ * Maximum size of the pool.
+ */
+ int maxConnections;
+
+ /**
+ * If greater than zero, the maximum time a connection will remain
+ * int the pool.
+ */
+ int connectionTTL;
+
+ /**
+ * A thread that removes connections older than connectionTTL.
+ */
+ class Reaper
+ implements Runnable
+ {
+ public void run()
+ {
+ synchronized (Pool.this)
+ {
+ try
+ {
+ do
+ {
+ while (connectionPool.size() > 0)
+ {
+ long currentTime = System.currentTimeMillis();
+
+ HTTPConnection c =
+ (HTTPConnection)connectionPool.getFirst();
+
+ long waitTime = c.timeLastUsed
+ + connectionTTL - currentTime;
+
+ if (waitTime <= 0)
+ removeOldest();
+ else
+ try
+ {
+ Pool.this.wait(waitTime);
+ }
+ catch (InterruptedException _)
+ {
+ // Ignore the interrupt.
+ }
+ }
+ // After the pool is empty, wait TTL to see if it
+ // is used again. This is because in the
+ // situation where a single thread is making HTTP
+ // requests to the same server it can remove the
+ // connection from the pool before the Reaper has
+ // a chance to start. This would cause the Reaper
+ // to exit if it were not for this extra delay.
+ // The result would be starting a Reaper thread
+ // for each HTTP request. With the delay we get
+ // at most one Reaper created each TTL.
+ try
+ {
+ Pool.this.wait(connectionTTL);
+ }
+ catch (InterruptedException _)
+ {
+ // Ignore the interrupt.
+ }
+ }
+ while (connectionPool.size() > 0);
+ }
+ finally
+ {
+ reaper = null;
+ }
+ }
+ }
+ }
+
+ Reaper reaper;
+
+ /**
+ * Private constructor to ensure singleton.
+ */
+ private Pool()
+ {
+ }
+
+ /**
+ * Tests for a matching connection.
+ *
+ * @param c connection to match.
+ * @param h the host name.
+ * @param p the port.
+ * @param sec true if using https.
+ *
+ * @return true if c matches h, p, and sec.
+ */
+ private static boolean matches(HTTPConnection c,
+ String h, int p, boolean sec)
+ {
+ return h.equals(c.hostname) && (p == c.port) && (sec == c.secure);
+ }
+
+ /**
+ * Get a pooled HTTPConnection. If there is an existing idle
+ * connection to the requested server it is returned. Otherwise a
+ * new connection is created.
+ *
+ * @param hostname the name of the host to connect to
+ * @param port the port on the host to connect to
+ * @param secure whether to use a secure connection
+ *
+ * @return the HTTPConnection.
+ */
+ synchronized HTTPConnection get(String host,
+ int port,
+ boolean secure)
+ {
+ String ttl =
+ SystemProperties.getProperty("classpath.net.http.keepAliveTTL");
+ connectionTTL = (ttl != null && ttl.length() > 0) ?
+ 1000 * Math.max(1, Integer.parseInt(ttl)) : 10000;
+
+ String mc = SystemProperties.getProperty("http.maxConnections");
+ maxConnections = (mc != null && mc.length() > 0) ?
+ Math.max(Integer.parseInt(mc), 1) : 5;
+ if (maxConnections < 1)
+ maxConnections = 1;
+
+ HTTPConnection c = null;
+
+ ListIterator it = connectionPool.listIterator(0);
+ while (it.hasNext())
+ {
+ HTTPConnection cc = (HTTPConnection)it.next();
+ if (matches(cc, host, port, secure))
+ {
+ c = cc;
+ break;
+ }
+ }
+ if (c == null)
+ {
+ c = new HTTPConnection(host, port, secure);
+ c.setPool(this);
+ }
+ return c;
+ }
+
+ /**
+ * Put an idle HTTPConnection back into the pool. If this causes
+ * the pool to be come too large, the oldest connection is removed
+ * and closed.
+ *
+ */
+ synchronized void put(HTTPConnection c)
+ {
+ c.timeLastUsed = System.currentTimeMillis();
+ connectionPool.addLast(c);
+
+ // maxConnections must always be >= 1
+ while (connectionPool.size() >= maxConnections)
+ removeOldest();
+
+ if (connectionTTL > 0 && null == reaper) {
+ // If there is a connectionTTL, then the reaper has removed
+ // any stale connections, so we don't have to check for stale
+ // now. We do have to start a reaper though, as there is not
+ // one running now.
+ reaper = new Reaper();
+ Thread t = new Thread(reaper, "HTTPConnection.Reaper");
+ t.setDaemon(true);
+ t.start();
+ }
+ }
+
+ /**
+ * Remove the oldest connection from the pool and close it.
+ */
+ void removeOldest()
+ {
+ HTTPConnection cx = (HTTPConnection)connectionPool.removeFirst();
+ try
+ {
+ cx.closeConnection();
+ }
+ catch (IOException ioe)
+ {
+ // Ignore it. We are just cleaning up.
+ }
+ }
+ }
+
+ /**
* The number of times this HTTPConnection has be used via keep-alive.
*/
int useCount;
/**
- * Generates a key for connections in the connection pool.
- *
- * @param h the host name.
- * @param p the port.
- * @param sec true if using https.
- *
- * @return the key.
+ * If this HTTPConnection is in the pool, the time it was put there.
*/
- static Object getPoolKey(String h, int p, boolean sec)
- {
- StringBuilder buf = new StringBuilder(sec ? "https://"; : "http://";);
- buf.append(h);
- buf.append(':');
- buf.append(p);
- return buf.toString();
- }
+ long timeLastUsed;
/**
* Set the connection pool that this HTTPConnection is a member of.
@@ -360,7 +560,7 @@
*
* @param p the pool.
*/
- void setPool(LinkedHashMap p)
+ void setPool(Pool p)
{
pool = p;
}
@@ -374,25 +574,20 @@
{
if (pool != null)
{
- synchronized (pool)
+ useCount++;
+ pool.put(this);
+
+ }
+ else
+ {
+ // If there is no pool, just close.
+ try
{
- useCount++;
- Object key = HTTPConnection.getPoolKey(hostname, port, secure);
- pool.put(key, this);
- while (pool.size() >= HTTPURLConnection.maxConnections)
- {
- // maxConnections must always be >= 1
- Object lru = pool.keySet().iterator().next();
- HTTPConnection c = (HTTPConnection)pool.remove(lru);
- try
- {
- c.closeConnection();
- }
- catch (IOException ioe)
- {
- // Ignore it. We are just cleaning up.
- }
- }
+ closeConnection();
+ }
+ catch (IOException ioe)
+ {
+ // Ignore it. We are just cleaning up.
}
}
}
Index: gnu/java/net/protocol/http/HTTPURLConnection.java
===================================================================
RCS file:
/sources/classpath/classpath/gnu/java/net/protocol/http/HTTPURLConnection.java,v
retrieving revision 1.19
diff -u -r1.19 HTTPURLConnection.java
--- gnu/java/net/protocol/http/HTTPURLConnection.java 9 Feb 2006 20:54:56
-0000 1.19
+++ gnu/java/net/protocol/http/HTTPURLConnection.java 24 Feb 2006 20:36:01
-0000
@@ -70,13 +70,6 @@
extends HttpsURLConnection
implements HandshakeCompletedListener
{
-
- /**
- * Pool of reusable connections, used if keepAlive is true.
- */
- private static final LinkedHashMap connectionPool = new LinkedHashMap();
- static int maxConnections;
-
/*
* The underlying connection.
*/
@@ -134,9 +127,6 @@
agent = System.getProperty("http.agent");
String ka = System.getProperty("http.keepAlive");
keepAlive = !(ka != null && "false".equals(ka));
- String mc = System.getProperty("http.maxConnections");
- maxConnections = (mc != null && mc.length() > 0) ?
- Math.max(Integer.parseInt(mc), 1) : 5;
return null;
}
@@ -366,16 +356,7 @@
HTTPConnection connection;
if (keepAlive)
{
- Object key = HTTPConnection.getPoolKey(host, port, secure);
- synchronized (connectionPool)
- {
- connection = (HTTPConnection) connectionPool.remove(key);
- if (connection == null)
- {
- connection = new HTTPConnection(host, port, secure);
- connection.setPool(connectionPool);
- }
- }
+ connection = HTTPConnection.Pool.instance.get(host, port, secure);
}
else
{
