Bug 1164518 - Avoid unnecessary DB updates when caching Safe Browsing results. r=gcp
authorFrancois Marier <francois@mozilla.com>
Fri, 04 Mar 2016 12:33:20 -0800
changeset 323156 7f5b604e1b5a1c45478c3376ede26a19b1636b2c
parent 323155 105eec7abaf09a9d4cd76bd23c50c6ef96dd8ebd
child 323157 c733bc1211ac32b13cb228d5e2ac146afd0acffc
push id5913
push userjlund@mozilla.com
push dateMon, 25 Apr 2016 16:57:49 +0000
treeherdermozilla-beta@dcaf0a6fa115 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersgcp
bugs1164518
milestone47.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 1164518 - Avoid unnecessary DB updates when caching Safe Browsing results. r=gcp MozReview-Commit-ID: HYNaTdCRohL
toolkit/components/url-classifier/Entries.h
toolkit/components/url-classifier/LookupCache.h
toolkit/components/url-classifier/nsIUrlClassifierDBService.idl
toolkit/components/url-classifier/nsUrlClassifierDBService.cpp
toolkit/components/url-classifier/nsUrlClassifierDBService.h
toolkit/components/url-classifier/nsUrlClassifierProxies.cpp
toolkit/components/url-classifier/nsUrlClassifierProxies.h
--- a/toolkit/components/url-classifier/Entries.h
+++ b/toolkit/components/url-classifier/Entries.h
@@ -176,16 +176,23 @@ struct AddComplete {
   template<class T>
   int Compare(const T& other) const {
     int cmp = complete.Compare(other.CompleteHash());
     if (cmp != 0) {
       return cmp;
     }
     return addChunk - other.addChunk;
   }
+
+  bool operator!=(const AddComplete& aOther) const {
+    if (addChunk != aOther.addChunk) {
+      return true;
+    }
+    return complete != aOther.complete;
+  }
 };
 
 struct SubPrefix {
   // The hash to subtract.
   Prefix prefix;
   // The chunk number of the add chunk to which the hash belonged.
   uint32_t addChunk;
   // The chunk number of this sub chunk.
--- a/toolkit/components/url-classifier/LookupCache.h
+++ b/toolkit/components/url-classifier/LookupCache.h
@@ -62,16 +62,23 @@ public:
   nsCString mTableName;
 };
 
 typedef nsTArray<LookupResult> LookupResultArray;
 
 struct CacheResult {
   AddComplete entry;
   nsCString table;
+
+  bool operator==(const CacheResult& aOther) const {
+    if (entry != aOther.entry) {
+      return false;
+    }
+    return table == aOther.table;
+  }
 };
 typedef nsTArray<CacheResult> CacheResultArray;
 
 class LookupCache {
 public:
   // Check for a canonicalized IP address.
   static bool IsCanonicalizedIP(const nsACString& aHost);
 
--- a/toolkit/components/url-classifier/nsIUrlClassifierDBService.idl
+++ b/toolkit/components/url-classifier/nsIUrlClassifierDBService.idl
@@ -102,16 +102,21 @@ interface nsIUrlClassifierDBService : ns
 
   /**
    * Set the last update time for the given table. We use this to
    * remember freshness past restarts. Time is in milliseconds since epoch.
    */
   void setLastUpdateTime(in ACString tableName,
                          in unsigned long long lastUpdateTime);
 
+  /**
+   * Forget the results that were used in the last DB update.
+   */
+  void clearLastResults();
+
   ////////////////////////////////////////////////////////////////////////////
   // Incremental update methods.
   //
   // An update to the database has the following steps:
   //
   // 1) The update process is started with beginUpdate().  The client
   //    passes an nsIUrlClassifierUpdateObserver object which will be
   //    notified as the update is processed by the dbservice.
--- a/toolkit/components/url-classifier/nsUrlClassifierDBService.cpp
+++ b/toolkit/components/url-classifier/nsUrlClassifierDBService.cpp
@@ -655,16 +655,21 @@ nsUrlClassifierDBServiceWorker::CacheCom
 {
   LOG(("nsUrlClassifierDBServiceWorker::CacheCompletions [%p]", this));
   if (!mClassifier)
     return NS_OK;
 
   // Ownership is transferred in to us
   nsAutoPtr<CacheResultArray> resultsPtr(results);
 
+  if (mLastResults == *resultsPtr) {
+    LOG(("Skipping completions that have just been cached already."));
+    return NS_OK;
+  }
+
   nsAutoPtr<ProtocolParser> pParse(new ProtocolParser());
   nsTArray<TableUpdate*> updates;
 
   // 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);
@@ -697,16 +702,17 @@ nsUrlClassifierDBServiceWorker::CacheCom
       updates.AppendElement(tu);
       pParse->ForgetTableUpdates();
     } else {
       LOG(("Completion received, but table is not active, so not caching."));
     }
    }
 
   mClassifier->ApplyUpdates(&updates);
+  mLastResults = *resultsPtr;
   return NS_OK;
 }
 
 NS_IMETHODIMP
 nsUrlClassifierDBServiceWorker::CacheMisses(PrefixArray *results)
 {
   LOG(("nsUrlClassifierDBServiceWorker::CacheMisses [%p] %d",
        this, results->Length()));
@@ -753,16 +759,25 @@ nsUrlClassifierDBServiceWorker::SetLastU
   MOZ_ASSERT(!NS_IsMainThread(), "Must be on the background thread");
   MOZ_ASSERT(mClassifier, "Classifier connection must be opened");
 
   mClassifier->SetLastUpdateTime(table, updateTime);
 
   return NS_OK;
 }
 
+NS_IMETHODIMP
+nsUrlClassifierDBServiceWorker::ClearLastResults()
+{
+  MOZ_ASSERT(!NS_IsMainThread(), "Must be on the background thread");
+  mLastResults.Clear();
+  return NS_OK;
+}
+
+
 // -------------------------------------------------------------------------
 // nsUrlClassifierLookupCallback
 //
 // This class takes the results of a lookup found on the worker thread
 // and handles any necessary partial hash expansions before calling
 // the client callback.
 
 class nsUrlClassifierLookupCallback final : public nsIUrlClassifierLookupCallback
@@ -1453,30 +1468,38 @@ NS_IMETHODIMP
 nsUrlClassifierDBService::SetHashCompleter(const nsACString &tableName,
                                            nsIUrlClassifierHashCompleter *completer)
 {
   if (completer) {
     mCompleters.Put(tableName, completer);
   } else {
     mCompleters.Remove(tableName);
   }
-
+  ClearLastResults();
   return NS_OK;
 }
 
 NS_IMETHODIMP
 nsUrlClassifierDBService::SetLastUpdateTime(const nsACString &tableName,
                                             uint64_t lastUpdateTime)
 {
   NS_ENSURE_TRUE(gDbBackgroundThread, NS_ERROR_NOT_INITIALIZED);
 
   return mWorkerProxy->SetLastUpdateTime(tableName, lastUpdateTime);
 }
 
 NS_IMETHODIMP
+nsUrlClassifierDBService::ClearLastResults()
+{
+  NS_ENSURE_TRUE(gDbBackgroundThread, NS_ERROR_NOT_INITIALIZED);
+
+  return mWorkerProxy->ClearLastResults();
+}
+
+NS_IMETHODIMP
 nsUrlClassifierDBService::BeginUpdate(nsIUrlClassifierUpdateObserver *observer,
                                       const nsACString &updateTables)
 {
   NS_ENSURE_TRUE(gDbBackgroundThread, NS_ERROR_NOT_INITIALIZED);
 
   if (mInUpdate) {
     LOG(("Already updating, not available"));
     return NS_ERROR_NOT_AVAILABLE;
--- a/toolkit/components/url-classifier/nsUrlClassifierDBService.h
+++ b/toolkit/components/url-classifier/nsUrlClassifierDBService.h
@@ -207,16 +207,19 @@ private:
   nsTArray<mozilla::safebrowsing::TableUpdate*> mTableUpdates;
 
   int32_t mUpdateWait;
 
   // Entries that cannot be completed. We expect them to die at
   // the next update
   PrefixArray mMissCache;
 
+  // Stores the last results that triggered a table update.
+  CacheResultArray mLastResults;
+
   nsresult mUpdateStatus;
   nsTArray<nsCString> mUpdateTables;
 
   nsCOMPtr<nsIUrlClassifierUpdateObserver> mUpdateObserver;
   bool mInStream;
 
   // The number of noise entries to add to the set of lookup results.
   uint32_t mGethashNoise;
--- a/toolkit/components/url-classifier/nsUrlClassifierProxies.cpp
+++ b/toolkit/components/url-classifier/nsUrlClassifierProxies.cpp
@@ -227,16 +227,29 @@ UrlClassifierDBServiceWorkerProxy::SetLa
 
 NS_IMETHODIMP
 UrlClassifierDBServiceWorkerProxy::SetLastUpdateTimeRunnable::Run()
 {
   mTarget->SetLastUpdateTime(mTable, mUpdateTime);
   return NS_OK;
 }
 
+NS_IMETHODIMP
+UrlClassifierDBServiceWorkerProxy::ClearLastResults()
+{
+  nsCOMPtr<nsIRunnable> r = new ClearLastResultsRunnable(mTarget);
+  return DispatchToWorkerThread(r);
+}
+
+NS_IMETHODIMP
+UrlClassifierDBServiceWorkerProxy::ClearLastResultsRunnable::Run()
+{
+  return mTarget->ClearLastResults();
+}
+
 NS_IMPL_ISUPPORTS(UrlClassifierLookupCallbackProxy,
                   nsIUrlClassifierLookupCallback)
 
 NS_IMETHODIMP
 UrlClassifierLookupCallbackProxy::LookupComplete
   (LookupResultArray * aResults)
 {
   nsCOMPtr<nsIRunnable> r = new LookupCompleteRunnable(mTarget, aResults);
--- a/toolkit/components/url-classifier/nsUrlClassifierProxies.h
+++ b/toolkit/components/url-classifier/nsUrlClassifierProxies.h
@@ -186,16 +186,28 @@ public:
 
     NS_DECL_NSIRUNNABLE
   private:
     RefPtr<nsUrlClassifierDBServiceWorker> mTarget;
     nsCString mTable;
     uint64_t mUpdateTime;
   };
 
+  class ClearLastResultsRunnable : public nsRunnable
+  {
+  public:
+    explicit ClearLastResultsRunnable(nsUrlClassifierDBServiceWorker* aTarget)
+      : mTarget(aTarget)
+    { }
+
+    NS_DECL_NSIRUNNABLE
+  private:
+    RefPtr<nsUrlClassifierDBServiceWorker> mTarget;
+  };
+
 public:
   nsresult DoLocalLookup(const nsACString& spec,
                          const nsACString& tables,
                          mozilla::safebrowsing::LookupResultArray* results);
 
 private:
   ~UrlClassifierDBServiceWorkerProxy() {}