Bug 483764 Switch database caches from nsVoidArray to nsTArray. r=Neil,sr=bienvenu
authorMark Banner <bugzilla@standard8.plus.com>
Thu, 19 Mar 2009 19:49:55 +0000
changeset 2247 a4dcc4160c3fdc0d693ff65760695d9afa446695
parent 2246 0485638c089ab98eab427a8ede5dc4c73ec3f011
child 2248 1d0fcdefa375a657e3a81816fb6ac9117c67e123
push idunknown
push userunknown
push dateunknown
reviewersNeil, bienvenu
bugs483764
Bug 483764 Switch database caches from nsVoidArray to nsTArray. r=Neil,sr=bienvenu
mailnews/addrbook/src/nsAddrDatabase.cpp
mailnews/addrbook/src/nsAddrDatabase.h
mailnews/db/msgdb/public/nsMsgDatabase.h
mailnews/db/msgdb/src/nsMsgDatabase.cpp
--- a/mailnews/addrbook/src/nsAddrDatabase.cpp
+++ b/mailnews/addrbook/src/nsAddrDatabase.cpp
@@ -65,16 +65,21 @@
 #include "nsEmbedCID.h"
 #include "nsXPCOMCIDInternal.h"
 #include "nsIProperty.h"
 #include "nsIVariant.h"
 
 #define ID_PAB_TABLE            1
 #define ID_DELETEDCARDS_TABLE           2
 
+// There's two books by default, although Mac may have one more, so set this
+// to three. Its not going to affect much, but will save us a few reallocations
+// when the cache is allocated.
+const PRUint32 kInitialAddrDBCacheSize = 3;
+
 const PRInt32 kAddressBookDBVersion = 1;
 
 static const char kPabTableKind[] = "ns:addrbk:db:table:kind:pab";
 static const char kDeletedCardsTableKind[] = "ns:addrbk:db:table:kind:deleted"; // this table is used to keep the deleted cards
 
 static const char kCardRowScope[] = "ns:addrbk:db:row:scope:card:all";
 static const char kListRowScope[] = "ns:addrbk:db:row:scope:list:all";
 static const char kDataRowScope[] = "ns:addrbk:db:row:scope:data:all";
@@ -251,88 +256,64 @@ nsresult nsAddrDatabase::NotifyListEntry
 NS_IMETHODIMP nsAddrDatabase::NotifyAnnouncerGoingAway(void)
 {
   NS_OBSERVER_ARRAY_NOTIFY_OBSERVERS(m_ChangeListeners, nsIAddrDBListener,
                                      OnAnnouncerGoingAway, ());
   return NS_OK;
 }
 
 
-
-nsVoidArray *nsAddrDatabase::m_dbCache = NULL;
-
-//----------------------------------------------------------------------
-// GetDBCache
-//----------------------------------------------------------------------
-
-nsVoidArray/*<nsAddrDatabase>*/ *
+// Apparently its not good for nsTArray to be allocated as static. Don't know
+// why it isn't but its not, so don't think about making it a static variable.
+// Maybe bz knows.
+nsTArray<nsAddrDatabase*>* nsAddrDatabase::m_dbCache = nsnull;
+
+nsTArray<nsAddrDatabase*>*
 nsAddrDatabase::GetDBCache()
 {
-    if (!m_dbCache)
-        m_dbCache = new nsVoidArray();
-
-    return m_dbCache;
-
+  if (!m_dbCache)
+    m_dbCache = new nsAutoTArray<nsAddrDatabase*, kInitialAddrDBCacheSize>;
+
+  return m_dbCache;
 }
 
 void
 nsAddrDatabase::CleanupCache()
 {
-    if (m_dbCache) // clean up memory leak
+  if (m_dbCache)
+  {
+    for (PRInt32 i = m_dbCache->Length() - 1; i >= 0; --i)
     {
-        PRInt32 i;
-        for (i = 0; i < GetDBCache()->Count(); i++)
-        {
-            nsAddrDatabase* pAddrDB = static_cast<nsAddrDatabase*>(GetDBCache()->ElementAt(i));
-            if (pAddrDB)
-            {
-                pAddrDB->ForceClosed();
-                i--;    // back up array index, since closing removes db from cache.
-            }
-        }
-//        NS_ASSERTION(GetNumInCache() == 0, "some msg dbs left open");    // better not be any open db's.
-        delete m_dbCache;
+      nsAddrDatabase* pAddrDB = m_dbCache->ElementAt(i);
+      if (pAddrDB)
+        pAddrDB->ForceClosed();
     }
-    m_dbCache = nsnull; // Need to reset to NULL since it's a
-              // static global ptr and maybe referenced
-              // again in other places.
+    //        NS_ASSERTION(m_dbCache.Length() == 0, "some msg dbs left open");    // better not be any open db's.
+    delete m_dbCache;
+    m_dbCache = nsnull;
+  }
 }
 
 //----------------------------------------------------------------------
 // FindInCache - this addrefs the db it finds.
 //----------------------------------------------------------------------
 nsAddrDatabase* nsAddrDatabase::FindInCache(nsIFile *dbName)
 {
-    PRInt32 i;
-    for (i = 0; i < GetDBCache()->Count(); i++)
+  nsTArray<nsAddrDatabase*>* dbCache = GetDBCache();
+  PRUint32 length = dbCache->Length();
+  for (PRUint32 i = 0; i < length; ++i)
+  {
+    nsAddrDatabase* pAddrDB = dbCache->ElementAt(i);
+    if (pAddrDB->MatchDbName(dbName))
     {
-        nsAddrDatabase* pAddrDB = static_cast<nsAddrDatabase*>(GetDBCache()->ElementAt(i));
-        if (pAddrDB->MatchDbName(dbName))
-        {
-            NS_ADDREF(pAddrDB);
-            return pAddrDB;
-        }
+      NS_ADDREF(pAddrDB);
+      return pAddrDB;
     }
-    return nsnull;
-}
-
-//----------------------------------------------------------------------
-// FindInCache
-//----------------------------------------------------------------------
-PRInt32 nsAddrDatabase::FindInCache(nsAddrDatabase* pAddrDB)
-{
-    PRInt32 i;
-    for (i = 0; i < GetDBCache()->Count(); i++)
-    {
-        if (GetDBCache()->ElementAt(i) == pAddrDB)
-        {
-            return(i);
-        }
-    }
-    return(-1);
+  }
+  return nsnull;
 }
 
 PRBool nsAddrDatabase::MatchDbName(nsIFile* dbName)    // returns PR_TRUE if they match
 {
     PRBool dbMatches = PR_FALSE;
 
     nsresult rv = m_dbName->Equals(dbName, &dbMatches);
     if (NS_FAILED(rv))
@@ -341,21 +322,17 @@ PRBool nsAddrDatabase::MatchDbName(nsIFi
     return dbMatches;
 }
 
 //----------------------------------------------------------------------
 // RemoveFromCache
 //----------------------------------------------------------------------
 void nsAddrDatabase::RemoveFromCache(nsAddrDatabase* pAddrDB)
 {
-    PRInt32 i = FindInCache(pAddrDB);
-    if (i != -1)
-    {
-        GetDBCache()->RemoveElementAt(i);
-    }
+  GetDBCache()->RemoveElement(pAddrDB);
 }
 
 void nsAddrDatabase::GetMDBFactory(nsIMdbFactory ** aMdbFactory)
 {
   if (!mMdbFactory)
   {
     nsresult rv;
     nsCOMPtr <nsIMdbFactoryService> mdbFactoryService = do_GetService(NS_MORK_CONTRACTID, &rv);
--- a/mailnews/addrbook/src/nsAddrDatabase.h
+++ b/mailnews/addrbook/src/nsAddrDatabase.h
@@ -37,17 +37,16 @@
  *
  * ***** END LICENSE BLOCK ***** */
 
 #ifndef _nsAddrDatabase_H_
 #define _nsAddrDatabase_H_
 
 #include "nsIAddrDatabase.h"
 #include "mdb.h"
-#include "nsVoidArray.h"
 #include "nsStringGlue.h"
 #include "nsIAddrDBListener.h"
 #include "nsCOMPtr.h"
 #include "nsTObserverArray.h"
 #include "nsIEnumerator.h"
 
 typedef enum
 {
@@ -306,19 +305,17 @@ public:
 
     NS_IMETHOD AddListCardColumnsToRow(nsIAbCard *aPCard, nsIMdbRow *aPListRow, PRUint32 aPos, nsIAbCard** aPNewCard, PRBool aInMailingList, nsIAbDirectory *aParent, nsIAbDirectory *aRoot);
     NS_IMETHOD InitCardFromRow(nsIAbCard *aNewCard, nsIMdbRow* aCardRow);
     NS_IMETHOD SetListAddressTotal(nsIMdbRow* aListRow, PRUint32 aTotal);
     NS_IMETHOD FindRowByCard(nsIAbCard * card,nsIMdbRow **aRow);
 
 protected:
 
-  static void    AddToCache(nsAddrDatabase* pAddrDB) {GetDBCache()->AppendElement(pAddrDB);}
-  static void    RemoveFromCache(nsAddrDatabase* pAddrDB);
-  static PRInt32  FindInCache(nsAddrDatabase* pAddrDB);
+  static void RemoveFromCache(nsAddrDatabase* pAddrDB);
   PRBool MatchDbName(nsIFile *dbName); // returns TRUE if they match
 
   void YarnToUInt32(struct mdbYarn *yarn, PRUint32 *pResult);
   void GetCharStringYarn(char* str, struct mdbYarn* strYarn);
   void GetStringYarn(const nsAString & aStr, struct mdbYarn* strYarn);
   void GetIntYarn(PRUint32 nValue, struct mdbYarn* intYarn);
   nsresult AddCharStringColumn(nsIMdbRow* cardRow, mdb_column inColumn, const char* str);
   nsresult AddStringColumn(nsIMdbRow* aCardRow, mdb_column aInColumn, const nsAString & aStr);
@@ -338,18 +335,18 @@ protected:
   nsresult DeleteCardFromListRow(nsIMdbRow* pListRow, mdb_id cardRowID);
   void DeleteCardFromAllMailLists(mdb_id cardRowID);
   nsresult NotifyListEntryChange(PRUint32 abCode, nsIAbDirectory *dir);
 
   nsresult AddLowercaseColumn(nsIMdbRow * row, mdb_token columnToken, const char* utf8String);
   nsresult GetRowFromAttribute(const char *aName, const nsACString &aUTF8Value,
                                PRBool aCaseInsensitive, nsIMdbRow  **aCardRow);
 
-  static nsVoidArray/*<nsAddrDatabase>*/ * GetDBCache();
-  static nsVoidArray/*<nsAddrDatabase>*/ * m_dbCache;
+  static nsTArray<nsAddrDatabase*>* m_dbCache;
+  static nsTArray<nsAddrDatabase*>* GetDBCache();
 
   // mdb bookkeeping stuff
   nsresult      InitExistingDB();
   nsresult      InitNewDB();
   nsresult      InitMDBInfo();
   nsresult      InitPabTable();
   nsresult      InitDeletedCardsTable(PRBool aCreate);
   nsresult      AddRowToDeletedCardsTable(nsIAbCard *card, nsIMdbRow **pCardRow);
--- a/mailnews/db/msgdb/public/nsMsgDatabase.h
+++ b/mailnews/db/msgdb/public/nsMsgDatabase.h
@@ -150,17 +150,16 @@ public:
   // helper functions to copy an nsString to a yarn, int32 to yarn, and vice versa.
   static struct mdbYarn *nsStringToYarn(struct mdbYarn *yarn, const nsAString &str);
   static struct mdbYarn *UInt32ToYarn(struct mdbYarn *yarn, PRUint32 i);
   static void YarnTonsString(struct mdbYarn *yarn, nsAString &str);
   static void YarnTonsCString(struct mdbYarn *yarn, nsACString &str);
   static void YarnToUInt32(struct mdbYarn *yarn, PRUint32 *i);
   
   static void   CleanupCache();
-  static int    GetNumInCache(void) {return(GetDBCache()->Count());}
 #ifdef DEBUG
   static void   DumpCache();
   virtual nsresult DumpContents();
   nsresult DumpThread(nsMsgKey threadId);
   nsresult DumpMsgChildren(nsIMsgDBHdr *msgHdr);
 #endif
   
   friend class nsMsgHdr;  // use this to get access to cached tokens for hdr fields
@@ -184,46 +183,44 @@ protected:
   virtual PRBool  ThreadBySubjectWithoutRe();
   virtual PRBool  UseStrictThreading();
   virtual PRBool  UseCorrectThreading();
   virtual nsresult ThreadNewHdr(nsMsgHdr* hdr, PRBool &newThread);
   virtual nsresult AddNewThread(nsMsgHdr *msgHdr);
   virtual nsresult AddToThread(nsMsgHdr *newHdr, nsIMsgThread *thread, nsIMsgDBHdr *pMsgHdr, PRBool threadInThread);
   
   
-  // open db cache
+  static nsTArray<nsMsgDatabase*>* m_dbCache;
+  static nsTArray<nsMsgDatabase*>* GetDBCache();
+  
   static void    AddToCache(nsMsgDatabase* pMessageDB) 
   {
 #ifdef DEBUG_David_Bienvenu
-//    NS_ASSERTION(GetNumInCache() < 50, "50 or more open db's");
+//    NS_ASSERTION(GetDBCache()->Length() < 50, "50 or more open db's");
 #endif
-    GetDBCache()->AppendElement(pMessageDB);}
+    GetDBCache()->AppendElement(pMessageDB);
+  }
   static void    RemoveFromCache(nsMsgDatabase* pMessageDB);
-  static int    FindInCache(nsMsgDatabase* pMessageDB);
   PRBool  MatchDbName(nsILocalFile *dbName);  // returns TRUE if they match
   
   // Flag handling routines
   virtual nsresult SetKeyFlag(nsMsgKey key, PRBool set, PRUint32 flag,
                               nsIDBChangeListener *instigator = NULL);
   virtual nsresult SetMsgHdrFlag(nsIMsgDBHdr *msgHdr, PRBool set, PRUint32 flag, 
                                  nsIDBChangeListener *instigator);
   
   virtual PRBool  SetHdrFlag(nsIMsgDBHdr *, PRBool bSet, nsMsgMessageFlagType flag);
   virtual PRBool  SetHdrReadFlag(nsIMsgDBHdr *, PRBool pRead);
   virtual PRUint32 GetStatusFlags(nsIMsgDBHdr *msgHdr, PRUint32 origFlags);
   // helper function which doesn't involve thread object
   
   virtual nsresult RemoveHeaderFromDB(nsMsgHdr *msgHdr);
   virtual nsresult RemoveHeaderFromThread(nsMsgHdr *msgHdr);
   virtual nsresult AdjustExpungedBytesOnDelete(nsIMsgDBHdr *msgHdr);
-  
-  
-  static nsVoidArray/*<nsMsgDatabase>*/* GetDBCache();
-  static nsVoidArray/*<nsMsgDatabase>*/* m_dbCache;
-  
+
   nsCOMPtr <nsICollation> m_collationKeyGenerator;
   nsCOMPtr <nsIMimeConverter> m_mimeConverter;
   nsCOMPtr <nsIMsgRetentionSettings> m_retentionSettings;
   nsCOMPtr <nsIMsgDownloadSettings> m_downloadSettings;
 
   nsresult PurgeMessagesOlderThan(PRUint32 daysToKeepHdrs,
                                   PRBool keepUnreadMessagesOnly,
                                   PRBool applyToFlaggedMessages,
--- a/mailnews/db/msgdb/src/nsMsgDatabase.cpp
+++ b/mailnews/db/msgdb/src/nsMsgDatabase.cpp
@@ -76,17 +76,23 @@
 #include "nsIPrefBranch.h"
 
 #if defined(DEBUG_sspitzer_) || defined(DEBUG_seth_)
 #define DEBUG_MSGKEYSET 1
 #endif
 
 #define MSG_HASH_SIZE 512
 
-const PRInt32 kMaxHdrsInCache = 512;  // this will be used on discovery, since we don't know total
+// This will be used on discovery, since we don't know total.
+const PRInt32 kMaxHdrsInCache = 512;
+
+// Hopefully we're not opening up lots of databases at the same time, however
+// this will give us a buffer before we need to start reallocating the cache
+// array.
+const PRUint32 kInitialMsgDBCacheSize = 20;
 
 // special keys
 static const nsMsgKey kAllMsgHdrsTableKey = 1;
 static const nsMsgKey kTableKeyForThreadOne = 0xfffffffe;
 static const nsMsgKey kAllThreadsTableKey = 0xfffffffd;
 static const nsMsgKey kFirstPseudoKey = 0xfffffff0;
 static const nsMsgKey kIdStartOfFake = 0xffffff80;
 
@@ -695,77 +701,71 @@ NS_IMETHODIMP nsMsgDatabase::NotifyParen
 
 
 NS_IMETHODIMP nsMsgDatabase::NotifyAnnouncerGoingAway(void)
 {
   NOTIFY_LISTENERS(OnAnnouncerGoingAway, (this));
   return NS_OK;
 }
 
-
-
-nsVoidArray *nsMsgDatabase::m_dbCache = NULL;
-
-//----------------------------------------------------------------------
-// GetDBCache
-//----------------------------------------------------------------------
-
-nsVoidArray/*<nsMsgDatabase>*/*
+// Apparently its not good for nsTArray to be allocated as static. Don't know
+// why it isn't but its not, so don't think about making it a static variable.
+// Maybe bz knows.
+nsTArray<nsMsgDatabase*>* nsMsgDatabase::m_dbCache;
+
+nsTArray<nsMsgDatabase*>*
 nsMsgDatabase::GetDBCache()
 {
   if (!m_dbCache)
-    m_dbCache = new nsVoidArray();
+    m_dbCache = new nsAutoTArray<nsMsgDatabase*, kInitialMsgDBCacheSize>;
 
   return m_dbCache;
-
 }
 
 void
 nsMsgDatabase::CleanupCache()
 {
-  if (m_dbCache) // clean up memory leak (needed because some destructors
-                 // have user-visible effects, which they shouldn't)
+  if (m_dbCache)
   {
-    for (PRInt32 i = GetNumInCache() - 1;
-         i >= 0 && GetNumInCache() > 0;
-         // prior closes may have removed more than one element
-         i >= GetNumInCache() ? i = GetNumInCache() - 1 : i--)
+    for (PRUint32 i = m_dbCache->Length() - 1; i != 0 && m_dbCache->Length() > 0;
+     // prior closes may have removed more than one element
+         i >= m_dbCache->Length() ? i = m_dbCache->Length() - 1 : i--)
     {
-      nsMsgDatabase* pMessageDB = static_cast<nsMsgDatabase*>(GetDBCache()->ElementAt(i));
+      nsMsgDatabase* pMessageDB = m_dbCache->ElementAt(i);
       if (pMessageDB)
       {
         // hold onto the db until we're finished closing it.
         pMessageDB->AddRef();
         // break cycle with folder -> parse msg state -> db
         pMessageDB->m_folder = nsnull;
         pMessageDB->ForceClosed();
         pMessageDB->Release();
       }
     }
 #ifdef DEBUG
     // If you hit this warning, it means that ForceClosed() did not
     // successfully remove all references to the db so that it can
     // be shutdown.
-    NS_WARN_IF_FALSE(!GetNumInCache(), "some msg dbs left open");
+    NS_WARN_IF_FALSE(!m_dbCache->Length(), "some msg dbs left open");
 #endif
     delete m_dbCache;
+    m_dbCache = nsnull;
   }
-  m_dbCache = nsnull; // Need to reset to NULL since it's a
-  // static global ptr and maybe referenced
-  // again in other places.
 }
 
 //----------------------------------------------------------------------
 // FindInCache - this addrefs the db it finds.
 //----------------------------------------------------------------------
 nsMsgDatabase* nsMsgDatabase::FindInCache(nsILocalFile *dbName)
 {
-  for (PRInt32 i = 0; i < GetDBCache()->Count(); i++)
+  nsTArray<nsMsgDatabase*>* dbCache = GetDBCache();
+  PRUint32 length = dbCache->Length();
+  for (PRUint32 i = 0; i < length; i++)
   {
-    nsMsgDatabase* pMessageDB = static_cast<nsMsgDatabase*>(GetDBCache()->ElementAt(i));
+    nsMsgDatabase* pMessageDB = dbCache->ElementAt(i);
     if (pMessageDB->MatchDbName(dbName))
     {
       if (pMessageDB->m_mdbStore)  // don't return db without store
       {
         NS_ADDREF(pMessageDB);
         return pMessageDB;
       }
     }
@@ -785,55 +785,39 @@ nsIMsgDatabase* nsMsgDatabase::FindInCac
 
   nsCOMPtr <nsILocalFile> summaryFile;
   rv = GetSummaryFileLocation(folderPath, getter_AddRefs(summaryFile));
   NS_ENSURE_SUCCESS(rv, nsnull);
 
   return (nsIMsgDatabase *) FindInCache(summaryFile);
 }
 
-//----------------------------------------------------------------------
-// FindInCache
-//----------------------------------------------------------------------
-int nsMsgDatabase::FindInCache(nsMsgDatabase* pMessageDB)
-{
-  for (PRInt32 i = 0; i < GetDBCache()->Count(); i++)
-  {
-    if (GetDBCache()->ElementAt(i) == pMessageDB)
-      return(i);
-  }
-  return(-1);
-}
-
 PRBool nsMsgDatabase::MatchDbName(nsILocalFile *dbName)  // returns PR_TRUE if they match
 {
   nsCString dbPath;
   dbName->GetNativePath(dbPath);
   return dbPath.Equals(m_dbName);
 }
 
 //----------------------------------------------------------------------
 // RemoveFromCache
 //----------------------------------------------------------------------
 void nsMsgDatabase::RemoveFromCache(nsMsgDatabase* pMessageDB)
 {
-  int i = FindInCache(pMessageDB);
-  if (i != -1)
-    GetDBCache()->RemoveElementAt(i);
+  GetDBCache()->RemoveElement(pMessageDB);
 }
 
 
 #ifdef DEBUG
 void nsMsgDatabase::DumpCache()
 {
+  nsTArray<nsMsgDatabase*>* dbCache = GetDBCache();
   nsMsgDatabase* pMessageDB = nsnull;
-  for (PRInt32 i = 0; i < GetDBCache()->Count(); i++)
-  {
-    pMessageDB = static_cast<nsMsgDatabase*>(GetDBCache()->ElementAt(i));
-  }
+  for (PRUint32 i = 0; i < dbCache->Length(); i++)
+    pMessageDB = dbCache->ElementAt(i);
 }
 #endif /* DEBUG */
 
 nsMsgDatabase::nsMsgDatabase()
         : m_dbFolderInfo(nsnull),
         m_nextPseudoMsgKey(kFirstPseudoKey),
         m_mdbEnv(nsnull), m_mdbStore(nsnull),
         m_mdbAllMsgHeadersTable(nsnull), m_mdbAllThreadsTable(nsnull),