Bug 1442486 - Mark LookupCacheV4 as primed after creating it. r=gcp
authorFrancois Marier <francois@mozilla.com>
Thu, 01 Mar 2018 18:09:58 -0800
changeset 461373 8e02f09249084029fec6d6bbdd2b9351398c3dee
parent 461372 476e0758eb1c2736ef1c442cd70a8df6d1113897
child 461374 90ea9e2402f67e48c928c7c5463a17a0c12bc4ef
push id1683
push usersfraser@mozilla.com
push dateThu, 26 Apr 2018 16:43:40 +0000
treeherdermozilla-release@5af6cb21869d [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersgcp
bugs1442486
milestone60.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 1442486 - Mark LookupCacheV4 as primed after creating it. r=gcp RegenActiveTables() relies on mPrimed being set correctly and so the V4 lookup cache should behave the same way as the V2 one. The V2 lookup cache on the other hand was unnecessarily setting mPrimed to true twice. MozReview-Commit-ID: LwNdI9DTqZ7
toolkit/components/url-classifier/Classifier.cpp
toolkit/components/url-classifier/LookupCache.cpp
toolkit/components/url-classifier/LookupCacheV4.cpp
toolkit/components/url-classifier/nsUrlClassifierDBService.cpp
--- a/toolkit/components/url-classifier/Classifier.cpp
+++ b/toolkit/components/url-classifier/Classifier.cpp
@@ -927,20 +927,22 @@ Classifier::RegenActiveTables()
   nsTArray<nsCString> foundTables;
   ScanStoreDir(mRootStoreDirectory, foundTables);
 
   for (uint32_t i = 0; i < foundTables.Length(); i++) {
     nsCString table(foundTables[i]);
 
     LookupCache *lookupCache = GetLookupCache(table);
     if (!lookupCache) {
+      LOG(("Inactive table (no cache): %s", table.get()));
       continue;
     }
 
     if (!lookupCache->IsPrimed()) {
+      LOG(("Inactive table (cache not primed): %s", table.get()));
       continue;
     }
 
     if (LookupCache::Cast<LookupCacheV4>(lookupCache)) {
       LOG(("Active v4 table: %s", table.get()));
     } else {
       HashStore store(table, GetProvider(table), mRootStoreDirectory);
 
--- a/toolkit/components/url-classifier/LookupCache.cpp
+++ b/toolkit/components/url-classifier/LookupCache.cpp
@@ -625,17 +625,16 @@ LookupCacheV2::Build(AddPrefixArray& aAd
   aAddCompletes.Clear();
   mUpdateCompletions.Sort();
 
   Telemetry::Accumulate(Telemetry::URLCLASSIFIER_LC_PREFIXES,
                         static_cast<uint32_t>(aAddPrefixes.Length()));
 
   nsresult rv = ConstructPrefixSet(aAddPrefixes);
   NS_ENSURE_SUCCESS(rv, rv);
-  mPrimed = true;
 
   return NS_OK;
 }
 
 nsresult
 LookupCacheV2::GetPrefixes(FallibleTArray<uint32_t>& aAddPrefixes)
 {
   if (!mPrimed) {
--- a/toolkit/components/url-classifier/LookupCacheV4.cpp
+++ b/toolkit/components/url-classifier/LookupCacheV4.cpp
@@ -124,22 +124,31 @@ LookupCacheV4::IsEmpty()
   return isEmpty;
 }
 
 nsresult
 LookupCacheV4::Build(PrefixStringMap& aPrefixMap)
 {
   Telemetry::AutoTimer<Telemetry::URLCLASSIFIER_VLPS_CONSTRUCT_TIME> timer;
 
-  return mVLPrefixSet->SetPrefixes(aPrefixMap);
+  nsresult rv = mVLPrefixSet->SetPrefixes(aPrefixMap);
+  NS_ENSURE_SUCCESS(rv, rv);
+  mPrimed = true;
+
+  return rv;
 }
 
 nsresult
 LookupCacheV4::GetPrefixes(PrefixStringMap& aPrefixMap)
 {
+  if (!mPrimed) {
+    // This can happen if its a new table, so no error.
+    LOG(("GetPrefixes from empty LookupCache"));
+    return NS_OK;
+  }
   return mVLPrefixSet->GetPrefixes(aPrefixMap);
 }
 
 nsresult
 LookupCacheV4::GetFixedLengthPrefixes(FallibleTArray<uint32_t>& aPrefixes)
 {
   return mVLPrefixSet->GetFixedLengthPrefixes(aPrefixes);
 }
--- a/toolkit/components/url-classifier/nsUrlClassifierDBService.cpp
+++ b/toolkit/components/url-classifier/nsUrlClassifierDBService.cpp
@@ -860,16 +860,26 @@ nsUrlClassifierDBServiceWorker::CacheCom
   }
 
   // Only cache results for tables that we have, don't take
   // in tables we might accidentally have hit during a completion.
   // This happens due to goog vs googpub lists existing.
   nsTArray<nsCString> tables;
   nsresult rv = mClassifier->ActiveTables(tables);
   NS_ENSURE_SUCCESS(rv, rv);
+  if (LOG_ENABLED()) {
+    nsCString s;
+    for (size_t i=0; i < tables.Length(); i++) {
+      if (!s.IsEmpty()) {
+        s += ",";
+      }
+      s += tables[i];
+    }
+    LOG(("Active tables: %s", s.get()));
+  }
 
   nsTArray<TableUpdate*> updates;
 
   for (uint32_t i = 0; i < resultsPtr->Length(); i++) {
     bool activeTable = false;
     CacheResult* result = resultsPtr->ElementAt(i).get();
 
     for (uint32_t table = 0; table < tables.Length(); table++) {
@@ -890,17 +900,18 @@ nsUrlClassifierDBServiceWorker::CacheCom
       if (NS_FAILED(rv)) {
         // We can bail without leaking here because ForgetTableUpdates
         // hasn't been called yet.
         return rv;
       }
       updates.AppendElement(tu);
       pParse->ForgetTableUpdates();
     } else {
-      LOG(("Completion received, but table is not active, so not caching."));
+      LOG(("Completion received, but table %s is not active, so not caching.",
+           result->table.get()));
     }
    }
 
   mClassifier->ApplyFullHashes(&updates);
   mLastResults = Move(resultsPtr);
   return NS_OK;
 }