Bug 1050108 - Avoid race condition during memory report collection. r=njn
authorGian-Carlo Pascutto <gpascutto@mozilla.com>
Tue, 14 Oct 2014 13:15:50 +0200
changeset 210342 b17fa4f3eae4b9488ce4e8e06b7a995db93d7d76
parent 210341 4414bcd4350e43118ecfc281ebd809ed1874a23a
child 210343 1f089acdfd64d596cea894d9967589d6430bd1a9
push id27651
push userkwierso@gmail.com
push dateWed, 15 Oct 2014 00:18:02 +0000
treeherdermozilla-central@62f0b771583c [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersnjn
bugs1050108
milestone36.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1050108 - Avoid race condition during memory report collection. r=njn
toolkit/components/url-classifier/LookupCache.cpp
toolkit/components/url-classifier/nsUrlClassifierPrefixSet.cpp
toolkit/components/url-classifier/nsUrlClassifierPrefixSet.h
--- a/toolkit/components/url-classifier/LookupCache.cpp
+++ b/toolkit/components/url-classifier/LookupCache.cpp
@@ -627,17 +627,17 @@ LookupCache::ConstructPrefixSet(AddPrefi
   // construct new one, replace old entries
   nsresult rv = mPrefixSet->SetPrefixes(array.Elements(), array.Length());
   if (NS_FAILED(rv)) {
     goto error_bailout;
   }
 
 #ifdef DEBUG
   uint32_t size;
-  size = mPrefixSet->SizeOfIncludingThis(moz_malloc_size_of);
+  size = mPrefixSet->SizeInMemory();
   LOG(("SB tree done, size = %d bytes\n", size));
 #endif
 
   mPrimed = true;
 
   return NS_OK;
 
  error_bailout:
@@ -670,17 +670,17 @@ LookupCache::LoadPrefixSet()
     }
     mPrimed = true;
   } else {
     LOG(("no (usable) stored PrefixSet found"));
   }
 
 #ifdef DEBUG
   if (mPrimed) {
-    uint32_t size = mPrefixSet->SizeOfIncludingThis(moz_malloc_size_of);
+    uint32_t size = mPrefixSet->SizeInMemory();
     LOG(("SB tree done, size = %d bytes\n", size));
   }
 #endif
 
   return NS_OK;
 }
 
 nsresult
--- a/toolkit/components/url-classifier/nsUrlClassifierPrefixSet.cpp
+++ b/toolkit/components/url-classifier/nsUrlClassifierPrefixSet.cpp
@@ -33,19 +33,22 @@ static const PRLogModuleInfo *gUrlClassi
 #else
 #define LOG(args)
 #define LOG_ENABLED() (false)
 #endif
 
 NS_IMPL_ISUPPORTS(
   nsUrlClassifierPrefixSet, nsIUrlClassifierPrefixSet, nsIMemoryReporter)
 
+MOZ_DEFINE_MALLOC_SIZE_OF(UrlClassifierMallocSizeOf)
+
 nsUrlClassifierPrefixSet::nsUrlClassifierPrefixSet()
   : mHasPrefixes(false)
   , mTotalPrefixes(0)
+  , mMemoryInUse(0)
   , mMemoryReportPath()
 {
 #if defined(PR_LOGGING)
   if (!gUrlClassifierPrefixSetLog)
     gUrlClassifierPrefixSetLog = PR_NewLogModule("UrlClassifierPrefixSet");
 #endif
 }
 
@@ -66,29 +69,33 @@ nsUrlClassifierPrefixSet::Init(const nsA
 nsUrlClassifierPrefixSet::~nsUrlClassifierPrefixSet()
 {
   UnregisterWeakMemoryReporter(this);
 }
 
 NS_IMETHODIMP
 nsUrlClassifierPrefixSet::SetPrefixes(const uint32_t* aArray, uint32_t aLength)
 {
+  nsresult rv = NS_OK;
+
   if (aLength <= 0) {
     if (mHasPrefixes) {
       LOG(("Clearing PrefixSet"));
       mIndexDeltas.Clear();
       mIndexPrefixes.Clear();
       mTotalPrefixes = 0;
       mHasPrefixes = false;
     }
   } else {
-    return MakePrefixSet(aArray, aLength);
+    rv = MakePrefixSet(aArray, aLength);
   }
 
-  return NS_OK;
+  mMemoryInUse = SizeOfIncludingThis(UrlClassifierMallocSizeOf);
+
+  return rv;
 }
 
 nsresult
 nsUrlClassifierPrefixSet::MakePrefixSet(const uint32_t* aPrefixes, uint32_t aLength)
 {
   if (aLength == 0) {
     return NS_OK;
   }
@@ -231,25 +238,23 @@ nsUrlClassifierPrefixSet::Contains(uint3
 
   if (diff == 0) {
     *aFound = true;
   }
 
   return NS_OK;
 }
 
-MOZ_DEFINE_MALLOC_SIZE_OF(UrlClassifierMallocSizeOf)
-
 NS_IMETHODIMP
 nsUrlClassifierPrefixSet::CollectReports(nsIHandleReportCallback* aHandleReport,
                                          nsISupports* aData, bool aAnonymize)
 {
   return aHandleReport->Callback(
     EmptyCString(), mMemoryReportPath, KIND_HEAP, UNITS_BYTES,
-    SizeOfIncludingThis(UrlClassifierMallocSizeOf),
+    mMemoryInUse,
     NS_LITERAL_CSTRING("Memory used by the prefix set for a URL classifier."),
     aData);
 }
 
 size_t
 nsUrlClassifierPrefixSet::SizeOfIncludingThis(mozilla::MallocSizeOf aMallocSizeOf)
 {
   size_t n = 0;
@@ -340,19 +345,22 @@ NS_IMETHODIMP
 nsUrlClassifierPrefixSet::LoadFromFile(nsIFile* aFile)
 {
   Telemetry::AutoTimer<Telemetry::URLCLASSIFIER_PS_FILELOAD_TIME> timer;
 
   nsresult rv;
   AutoFDClose fileFd;
   rv = aFile->OpenNSPRFileDesc(PR_RDONLY | nsIFile::OS_READAHEAD,
                                0, &fileFd.rwget());
-  NS_ENSURE_SUCCESS(rv, rv);
+  if (!NS_FAILED(rv)) {
+    rv = LoadFromFd(fileFd);
+    mMemoryInUse = SizeOfIncludingThis(UrlClassifierMallocSizeOf);
+  }
 
-  return LoadFromFd(fileFd);
+  return rv;
 }
 
 nsresult
 nsUrlClassifierPrefixSet::StoreToFd(AutoFDClose& fileFd)
 {
   {
       Telemetry::AutoTimer<Telemetry::URLCLASSIFIER_PS_FALLOCATE_TIME> timer;
       int64_t size = 4 * sizeof(uint32_t);
--- a/toolkit/components/url-classifier/nsUrlClassifierPrefixSet.h
+++ b/toolkit/components/url-classifier/nsUrlClassifierPrefixSet.h
@@ -14,60 +14,66 @@
 #include "nsIMutableArray.h"
 #include "nsIUrlClassifierPrefixSet.h"
 #include "nsTArray.h"
 #include "nsToolkitCompsCID.h"
 #include "mozilla/MemoryReporting.h"
 #include "mozilla/Mutex.h"
 #include "mozilla/CondVar.h"
 #include "mozilla/FileUtils.h"
+#include "mozilla/Atomics.h"
 
 class nsUrlClassifierPrefixSet MOZ_FINAL
   : public nsIUrlClassifierPrefixSet
   , public nsIMemoryReporter
 {
 public:
   nsUrlClassifierPrefixSet();
 
   NS_IMETHOD Init(const nsACString& aName);
   NS_IMETHOD SetPrefixes(const uint32_t* aArray, uint32_t aLength);
   NS_IMETHOD GetPrefixes(uint32_t* aCount, uint32_t** aPrefixes);
   NS_IMETHOD Contains(uint32_t aPrefix, bool* aFound);
   NS_IMETHOD IsEmpty(bool* aEmpty);
   NS_IMETHOD LoadFromFile(nsIFile* aFile);
   NS_IMETHOD StoreToFile(nsIFile* aFile);
 
+  size_t SizeInMemory() { return mMemoryInUse; };
+
   NS_DECL_THREADSAFE_ISUPPORTS
   NS_DECL_NSIMEMORYREPORTER
 
-  // Return the estimated size of the set on disk and in memory, in bytes.
-  size_t SizeOfIncludingThis(mozilla::MallocSizeOf mallocSizeOf);
-
 protected:
   virtual ~nsUrlClassifierPrefixSet();
 
   static const uint32_t DELTAS_LIMIT = 120;
   static const uint32_t MAX_INDEX_DIFF = (1 << 16);
   static const uint32_t PREFIXSET_VERSION_MAGIC = 1;
 
   nsresult MakePrefixSet(const uint32_t* aArray, uint32_t aLength);
   uint32_t BinSearch(uint32_t start, uint32_t end, uint32_t target);
   nsresult LoadFromFd(mozilla::AutoFDClose& fileFd);
   nsresult StoreToFd(mozilla::AutoFDClose& fileFd);
 
+  // Return the estimated size of the set on disk and in memory, in bytes.
+  size_t SizeOfIncludingThis(mozilla::MallocSizeOf mallocSizeOf);
+
   // boolean indicating whether |setPrefixes| has been
   // called with a non-empty array.
   bool mHasPrefixes;
   // list of fully stored prefixes, that also form the
   // start of a run of deltas in mIndexDeltas.
   nsTArray<uint32_t> mIndexPrefixes;
   // array containing arrays of deltas from indices.
   // Index to the place that matches the closest lower
   // prefix from mIndexPrefix. Then every "delta" corresponds
   // to a prefix in the PrefixSet.
   nsTArray<nsTArray<uint16_t> > mIndexDeltas;
   // how many prefixes we have.
   uint32_t mTotalPrefixes;
 
+  // memory report collection might happen while we're updating the prefixset
+  // on another thread, so pre-compute and remember the size (bug 1050108).
+  mozilla::Atomic<size_t> mMemoryInUse;
   nsCString mMemoryReportPath;
 };
 
 #endif