Bug 591881 - Suspend pruning of idle connections when possible. r=dwitte a=blocking-fennec

netwerk/protocol/http/nsHttpConnection.cpp

@@ -206,6 +206,15 @@ nsHttpConnection::CanReuse()
                          && IsAlive();
 }
 
+PRUint32 nsHttpConnection::TimeToLive()
+{
+    PRInt32 tmp = mIdleTimeout - (NowInSeconds() - mLastReadTime);
+    if (0 > tmp)
+        tmp = 0;
+
+    return tmp;
+}
+
 PRBool
 nsHttpConnection::IsAlive()
 {

netwerk/protocol/http/nsHttpConnection.h

@@ -99,6 +99,10 @@ public:
     PRBool   SupportsPipelining() { return mSupportsPipelining; }
     PRBool   IsKeepAlive() { return mKeepAliveMask && mKeepAlive; }
     PRBool   CanReuse();   // can this connection be reused?
+
+    // Returns time in seconds for how long connection can be reused.
+    PRUint32 TimeToLive();
+
     void     DontReuse()   { mKeepAliveMask = PR_FALSE;
                              mKeepAlive = PR_FALSE;
                              mIdleTimeout = 0; }

netwerk/protocol/http/nsHttpConnectionMgr.cpp

@@ -46,6 +46,8 @@
 
 #include "nsIServiceManager.h"
 
+#include "nsIObserverService.h"
+
 // defined by the socket transport service while active
 extern PRThread *gSocketThread;
 
@@ -53,6 +55,9 @@ static NS_DEFINE_CID(kSocketTransportServiceCID, NS_SOCKETTRANSPORTSERVICE_CID);
 
 //-----------------------------------------------------------------------------
 
+
+NS_IMPL_THREADSAFE_ISUPPORTS1(nsHttpConnectionMgr, nsIObserver)
+
 static void
 InsertTransactionSorted(nsTArray<nsHttpTransaction*> &pendingQ, nsHttpTransaction *trans)
 {
@@ -82,6 +87,7 @@ nsHttpConnectionMgr::nsHttpConnectionMgr()
     , mMaxPersistConnsPerProxy(0)
     , mNumActiveConns(0)
     , mNumIdleConns(0)
+    , mTimeOfNextWakeUp(LL_MAXUINT)
 {
     LOG(("Creating nsHttpConnectionMgr @%x\n", this));
 }
@@ -177,6 +183,59 @@ nsHttpConnectionMgr::PostEvent(nsConnEventHandler handler, PRInt32 iparam, void
     return rv;
 }
 
+void
+nsHttpConnectionMgr::PruneDeadConnectionsAfter(PRUint32 timeInSeconds)
+{
+    LOG(("nsHttpConnectionMgr::PruneDeadConnectionsAfter\n"));
+
+    if(!mTimer)
+        mTimer = do_CreateInstance("@mozilla.org/timer;1");
+
+    // failure to create a timer is not a fatal error, but idle connections
+    // will not be cleaned up until we try to use them.
+    if (mTimer) {
+        mTimeOfNextWakeUp = timeInSeconds + NowInSeconds();
+        mTimer->Init(this, timeInSeconds*1000, nsITimer::TYPE_ONE_SHOT);
+    } else {
+        NS_WARNING("failed to create: timer for pruning the dead connections!");
+    }
+}
+
+void
+nsHttpConnectionMgr::StopPruneDeadConnectionsTimer()
+{
+    LOG(("nsHttpConnectionMgr::StopPruneDeadConnectionsTimer\n"));
+
+    if (mTimer) {
+        mTimer->Cancel();
+        mTimer = NULL;
+    }
+}
+
+//-----------------------------------------------------------------------------
+// nsHttpConnectionMgr::nsIObserver
+//-----------------------------------------------------------------------------
+
+NS_IMETHODIMP
+nsHttpConnectionMgr::Observe(nsISupports *subject,
+                             const char *topic,
+                             const PRUnichar *data)
+{
+    LOG(("nsHttpConnectionMgr::Observe [topic=\"%s\"]\n", topic));
+
+    if (0 == strcmp(topic, "timer-callback")) {
+        // prune dead connections
+        PruneDeadConnections();
+#ifdef DEBUG
+        nsCOMPtr<nsITimer> timer = do_QueryInterface(subject);
+        NS_ASSERTION(timer == mTimer, "unexpected timer-callback");
+#endif
+    }
+
+    return NS_OK;
+}
+
+
 //-----------------------------------------------------------------------------
 
 nsresult
@@ -317,6 +376,8 @@ nsHttpConnectionMgr::PurgeOneIdleConnectionCB(nsHashKey *key, void *data, void *
         conn->Close(NS_ERROR_ABORT);
         NS_RELEASE(conn);
         self->mNumIdleConns--;
+        if (0 == self->mNumIdleConns)
+            self->StopPruneDeadConnectionsTimer();
         return kHashEnumerateStop;
     }
 
@@ -331,6 +392,9 @@ nsHttpConnectionMgr::PruneDeadConnectionsCB(nsHashKey *key, void *data, void *cl
 
     LOG(("  pruning [ci=%s]\n", ent->mConnInfo->HashKey().get()));
 
+    // Find out how long it will take for next idle connection to not be reusable
+    // anymore.
+    PRUint32 timeToNextExpire = PR_UINT32_MAX;
     PRInt32 count = ent->mIdleConns.Length();
     if (count > 0) {
         for (PRInt32 i=count-1; i>=0; --i) {
@@ -340,10 +404,27 @@ nsHttpConnectionMgr::PruneDeadConnectionsCB(nsHashKey *key, void *data, void *cl
                 conn->Close(NS_ERROR_ABORT);
                 NS_RELEASE(conn);
                 self->mNumIdleConns--;
+            } else {
+                timeToNextExpire = PR_MIN(timeToNextExpire, conn->TimeToLive());
             }
         }
     }
 
+    // If time to next expire found is shorter than time to next wake-up, we need to
+    // change the time for next wake-up.
+    if (0 < ent->mIdleConns.Length()) {
+        PRUint32 now = NowInSeconds();
+        PRUint64 timeOfNextExpire = now + timeToNextExpire;
+        // If pruning of dead connections is not already scheduled to happen
+        // or time found for next connection to expire is is before
+        // mTimeOfNextWakeUp, we need to schedule the pruning to happen
+        // after timeToNextExpire.
+        if (!self->mTimer || timeOfNextExpire < self->mTimeOfNextWakeUp) {
+            self->PruneDeadConnectionsAfter(timeToNextExpire);
+        }
+    } else if (0 == self->mNumIdleConns) {
+        self->StopPruneDeadConnectionsTimer();
+    }
 #ifdef DEBUG
     count = ent->mActiveConns.Length();
     if (count > 0) {
@@ -401,6 +482,10 @@ nsHttpConnectionMgr::ShutdownPassCB(nsHashKey *key, void *data, void *closure)
         conn->Close(NS_ERROR_ABORT);
         NS_RELEASE(conn);
     }
+    // If all idle connections are removed,
+    // we can stop pruning dead connections.
+    if (0 == self->mNumIdleConns)
+        self->StopPruneDeadConnectionsTimer();
 
     // close all pending transactions
     while (ent->mPendingQ.Length()) {
@@ -543,6 +628,12 @@ nsHttpConnectionMgr::GetConnection(nsConnectionEntry *ent, PRUint8 caps,
     }
 
     if (!conn) {
+        // No reusable idle connection found for this entry. If there are no
+        // idle connections left at all, we need to make sure that we are not
+        // pruning dead connections anymore.
+        if (0 == mNumIdleConns)
+            StopPruneDeadConnectionsTimer();
+
         conn = new nsHttpConnection();
         if (!conn)
             return;
@@ -556,7 +647,8 @@ nsHttpConnectionMgr::GetConnection(nsConnectionEntry *ent, PRUint8 caps,
         
         // We created a new connection that will become active, purge the
         // oldest idle connection if we've reached the upper limit.
-        if (mNumIdleConns + mNumActiveConns + 1 > mMaxConns)
+        // This only needs to be done if there is a idle connection.
+        if (0 < mNumIdleConns && mNumIdleConns + mNumActiveConns + 1 > mMaxConns)
             mCT.Enumerate(PurgeOneIdleConnectionCB, this);
 
         // XXX this just purges a random idle connection.  we should instead
@@ -874,6 +966,12 @@ nsHttpConnectionMgr::OnMsgReclaimConnection(PRInt32, void *param)
             // connections first before getting to this one.
             ent->mIdleConns.AppendElement(conn);
             mNumIdleConns++;
+            // If the added connection was first idle connection or has shortest
+            // time to live among the idle connections, pruning dead
+            // connections needs to be done when it can't be reused anymore.
+            PRUint32 timeToLive = conn->TimeToLive();
+            if(!mTimer || NowInSeconds() + timeToLive < mTimeOfNextWakeUp)
+                PruneDeadConnectionsAfter(timeToLive);
         }
         else {
             LOG(("  connection cannot be reused; closing connection\n"));
@@ -990,3 +1088,4 @@ nsHttpConnectionMgr::nsConnectionHandle::PushBack(const char *buf, PRUint32 bufL
 {
     return mConn->PushBack(buf, bufLen);
 }
+

netwerk/protocol/http/nsHttpConnectionMgr.h

@@ -48,13 +48,18 @@
 #include "nsAutoPtr.h"
 #include "prmon.h"
 
+#include "nsIObserver.h"
+#include "nsITimer.h"
+
 class nsHttpPipeline;
 
 //-----------------------------------------------------------------------------
 
-class nsHttpConnectionMgr
+class nsHttpConnectionMgr : public nsIObserver
 {
 public:
+    NS_DECL_ISUPPORTS
+    NS_DECL_NSIOBSERVER
 
     // parameter names
     enum nsParamName {
@@ -86,18 +91,12 @@ public:
     // NOTE: functions below may be called on any thread.
     //-------------------------------------------------------------------------
 
-    nsrefcnt AddRef()
+    // Schedules next pruning of dead connection to happen after
-    {
+    // given time.
-        return PR_AtomicIncrement(&mRef);
+    void PruneDeadConnectionsAfter(PRUint32 time);
-    }
 
-    nsrefcnt Release()
+    // Stops timer scheduled for next pruning of dead connections.
-    {
+    void StopPruneDeadConnectionsTimer();
-        nsrefcnt n = PR_AtomicDecrement(&mRef);
-        if (n == 0)
-            delete this;
-        return n;
-    }
 
     // adds a transaction to the list of managed transactions.
     nsresult AddTransaction(nsHttpTransaction *, PRInt32 priority);
@@ -274,10 +273,18 @@ private:
     void OnMsgReclaimConnection    (PRInt32, void *);
     void OnMsgUpdateParam          (PRInt32, void *);
 
-    // counters
+    // Total number of active connections in all of the ConnectionEntry objects
+    // that are accessed from mCT connection table.
     PRUint16 mNumActiveConns;
+    // Total number of idle connections in all of the ConnectionEntry objects
+    // that are accessed from mCT connection table.
     PRUint16 mNumIdleConns;
 
+    // Holds time in seconds for next wake-up to prune dead connections. 
+    PRUint64 mTimeOfNextWakeUp;
+    // Timer for next pruning of dead connections.
+    nsCOMPtr<nsITimer> mTimer;
+
     //
     // the connection table
     //

netwerk/protocol/http/nsHttpHandler.cpp

@@ -210,9 +210,6 @@ nsHttpHandler::nsHttpHandler()
 
 nsHttpHandler::~nsHttpHandler()
 {
-    // We do not deal with the timer cancellation in the destructor since
-    // it is taken care of in xpcom shutdown event in the Observe method.
-
     LOG(("Deleting nsHttpHandler [this=%x]\n", this));
 
     // make sure the connection manager is shutdown
@@ -335,7 +332,6 @@ nsHttpHandler::Init()
         mObserverService->AddObserver(this, NS_PRIVATE_BROWSING_SWITCH_TOPIC, PR_TRUE);
     }
  
-    StartPruneDeadConnectionsTimer();
     return NS_OK;
 }
 
@@ -363,31 +359,6 @@ nsHttpHandler::InitConnectionMgr()
     return rv;
 }
 
-void
-nsHttpHandler::StartPruneDeadConnectionsTimer()
-{
-    LOG(("nsHttpHandler::StartPruneDeadConnectionsTimer\n"));
-
-    mTimer = do_CreateInstance("@mozilla.org/timer;1");
-    NS_ASSERTION(mTimer, "no timer");
-    // failure to create a timer is not a fatal error, but idle connections
-    // will not be cleaned up until we try to use them.
-    if (mTimer)
-        mTimer->Init(this, 15*1000, // every 15 seconds
-                     nsITimer::TYPE_REPEATING_SLACK);
-}
-
-void
-nsHttpHandler::StopPruneDeadConnectionsTimer()
-{
-    LOG(("nsHttpHandler::StopPruneDeadConnectionsTimer\n"));
-
-    if (mTimer) {
-        mTimer->Cancel();
-        mTimer = 0;
-    }
-}
-
 nsresult
 nsHttpHandler::AddStandardRequestHeaders(nsHttpHeaderArray *request,
                                          PRUint8 caps,
@@ -1649,9 +1620,6 @@ nsHttpHandler::Observe(nsISupports *subject,
     else if (strcmp(topic, "profile-change-net-teardown")    == 0 ||
              strcmp(topic, NS_XPCOM_SHUTDOWN_OBSERVER_ID)    == 0) {
 
-        // kill off the "prune dead connections" timer
-        StopPruneDeadConnectionsTimer();
-
         // clear cache of all authentication credentials.
         mAuthCache.ClearAll();
 
@@ -1666,18 +1634,6 @@ nsHttpHandler::Observe(nsISupports *subject,
     else if (strcmp(topic, "profile-change-net-restore") == 0) {
         // initialize connection manager
         InitConnectionMgr();
-
-        // restart the "prune dead connections" timer
-        StartPruneDeadConnectionsTimer();
-    }
-    else if (strcmp(topic, "timer-callback") == 0) {
-        // prune dead connections
-#ifdef DEBUG
-        nsCOMPtr<nsITimer> timer = do_QueryInterface(subject);
-        NS_ASSERTION(timer == mTimer, "unexpected timer-callback");
-#endif
-        if (mConnMgr)
-            mConnMgr->PruneDeadConnections();
     }
     else if (strcmp(topic, "net:clear-active-logins") == 0) {
         mAuthCache.ClearAll();

netwerk/protocol/http/nsHttpHandler.h

@@ -234,8 +234,6 @@ private:
     nsresult SetAcceptCharsets(const char *);
 
     nsresult InitConnectionMgr();
-    void     StartPruneDeadConnectionsTimer();
-    void     StopPruneDeadConnectionsTimer();
 
     void     NotifyObservers(nsIHttpChannel *chan, const char *event);
 
@@ -247,7 +245,6 @@ private:
     nsCOMPtr<nsIObserverService>        mObserverService;
     nsCOMPtr<nsICookieService>          mCookieService;
     nsCOMPtr<nsIIDNService>             mIDNConverter;
-    nsCOMPtr<nsITimer>                  mTimer;
     nsCOMPtr<nsIStrictTransportSecurityService> mSTSService;
 
     // the authentication credentials cache