Bug 787576 - Refine telemetry data for how much cache corruption reduction plan would help. r=michal
authorBrian R. Bondy <netzen@gmail.com>
Tue, 04 Sep 2012 15:05:19 -0400
changeset 104332 a20c53ec062ae6383299ccd3cdebdda70fbc1583
parent 104331 bc866177911f21f4216da86af46a0a65603e22e4
child 104333 a6f040934c99e68f5cba671ea6bafd5c5f76ee5d
push id1
push userroot
push dateMon, 20 Oct 2014 17:29:22 +0000
reviewersmichal
bugs787576
milestone18.0a1
Bug 787576 - Refine telemetry data for how much cache corruption reduction plan would help. r=michal
netwerk/cache/nsDiskCacheDevice.cpp
netwerk/cache/nsDiskCacheMap.cpp
netwerk/cache/nsDiskCacheMap.h
toolkit/components/telemetry/Histograms.json
--- a/netwerk/cache/nsDiskCacheDevice.cpp
+++ b/netwerk/cache/nsDiskCacheDevice.cpp
@@ -982,17 +982,17 @@ nsDiskCacheDevice::OpenDiskCache()
     bool exists;
     nsresult rv = mCacheDirectory->Exists(&exists);
     if (NS_FAILED(rv))
         return rv;
 
     if (exists) {
         // Try opening cache map file.
         nsDiskCache::CorruptCacheInfo corruptInfo;
-        rv = mCacheMap.Open(mCacheDirectory, &corruptInfo);
+        rv = mCacheMap.Open(mCacheDirectory, &corruptInfo, true);
 
         if (NS_SUCCEEDED(rv)) {
             Telemetry::Accumulate(Telemetry::DISK_CACHE_CORRUPT_DETAILS,
                                   corruptInfo);
         } else if (rv == NS_ERROR_ALREADY_INITIALIZED) {
           NS_WARNING("nsDiskCacheDevice::OpenDiskCache: already open!");
         } else {
             // Consider cache corrupt: delete it
@@ -1012,17 +1012,17 @@ nsDiskCacheDevice::OpenDiskCache()
         rv = mCacheDirectory->Create(nsIFile::DIRECTORY_TYPE, 0777);
         CACHE_LOG_PATH(PR_LOG_ALWAYS, "\ncreate cache directory: %s\n", mCacheDirectory);
         CACHE_LOG_ALWAYS(("mCacheDirectory->Create() = %x\n", rv));
         if (NS_FAILED(rv))
             return rv;
     
         // reopen the cache map     
         nsDiskCache::CorruptCacheInfo corruptInfo;
-        rv = mCacheMap.Open(mCacheDirectory, &corruptInfo);
+        rv = mCacheMap.Open(mCacheDirectory, &corruptInfo, false);
         if (NS_FAILED(rv))
             return rv;
     }
 
     return NS_OK;
 }
 
 
--- a/netwerk/cache/nsDiskCacheMap.cpp
+++ b/netwerk/cache/nsDiskCacheMap.cpp
@@ -27,17 +27,18 @@ using namespace mozilla;
  *****************************************************************************/
 
 /**
  *  File operations
  */
 
 nsresult
 nsDiskCacheMap::Open(nsIFile *  cacheDirectory,
-                     nsDiskCache::CorruptCacheInfo *  corruptInfo)
+                     nsDiskCache::CorruptCacheInfo *  corruptInfo,
+                     bool reportCacheCleanTelemetryData)
 {
     NS_ENSURE_ARG_POINTER(corruptInfo);
 
     // Assume we have an unexpected error until we find otherwise.
     *corruptInfo = nsDiskCache::kUnexpectedError;
     NS_ENSURE_ARG_POINTER(cacheDirectory);
     if (mMapFD)  return NS_ERROR_ALREADY_INITIALIZED;
 
@@ -57,17 +58,19 @@ nsDiskCacheMap::Open(nsIFile *  cacheDir
         NS_WARNING("Could not open cache map file");
         return NS_ERROR_FILE_CORRUPTED;
     }
 
     bool cacheFilesExist = CacheFilesExist();
     rv = NS_ERROR_FILE_CORRUPTED;  // presume the worst
     uint32_t mapSize = PR_Available(mMapFD);    
 
-    if (NS_FAILED(InitCacheClean(cacheDirectory, corruptInfo))) {
+    if (NS_FAILED(InitCacheClean(cacheDirectory,
+                                 corruptInfo,
+                                 reportCacheCleanTelemetryData))) {
         // corruptInfo is set in the call to InitCacheClean
         goto error_exit;
     }
 
     // check size of map file
     if (mapSize == 0) {  // creating a new _CACHE_MAP_
 
         // block files shouldn't exist if we're creating the _CACHE_MAP_
@@ -251,17 +254,16 @@ nsDiskCacheMap::Trim()
     if (NS_FAILED(rv))  rv2 = rv;   // if one or more errors, report at least one
     return rv2;
 }
 
 
 nsresult
 nsDiskCacheMap::FlushHeader()
 {
-    RevalidateCache();
     if (!mMapFD)  return NS_ERROR_NOT_AVAILABLE;
     
     // seek to beginning of cache map
     int32_t filePos = PR_Seek(mMapFD, 0, PR_SEEK_SET);
     if (filePos != 0)  return NS_ERROR_UNEXPECTED;
     
     // write the header
     mHeader.Swap();
@@ -269,16 +271,21 @@ nsDiskCacheMap::FlushHeader()
     mHeader.Unswap();
     if (sizeof(nsDiskCacheHeader) != bytesWritten) {
         return NS_ERROR_UNEXPECTED;
     }
 
     PRStatus err = PR_Sync(mMapFD);
     if (err != PR_SUCCESS) return NS_ERROR_UNEXPECTED;
 
+    // If we have a clean header then revalidate the cache clean file
+    if (!mHeader.mIsDirty) {
+        RevalidateCache();
+    }
+
     return NS_OK;
 }
 
 
 nsresult
 nsDiskCacheMap::FlushRecords(bool unswap)
 {
     if (!mMapFD)  return NS_ERROR_NOT_AVAILABLE;
@@ -1207,17 +1214,18 @@ nsDiskCacheMap::NotifyCapacityChange(uin
   if (mMaxRecordCount < maxRecordCount) {
     // We can only grow
     mMaxRecordCount = maxRecordCount;
   }
 }
 
 nsresult
 nsDiskCacheMap::InitCacheClean(nsIFile *  cacheDirectory,
-                               nsDiskCache::CorruptCacheInfo *  corruptInfo)
+                               nsDiskCache::CorruptCacheInfo *  corruptInfo,
+                               bool reportCacheCleanTelemetryData)
 {
     // The _CACHE_CLEAN_ file will be used in the future to determine
     // if the cache is clean or not. 
     bool cacheCleanFileExists = false;
     nsCOMPtr<nsIFile> cacheCleanFile;
     nsresult rv = cacheDirectory->GetParent(getter_AddRefs(cacheCleanFile));
     if (NS_SUCCEEDED(rv)) {
         rv = cacheCleanFile->AppendNative(
@@ -1243,17 +1251,17 @@ nsDiskCacheMap::InitCacheClean(nsIFile *
         return rv;
     }
 
     if (cacheCleanFileExists) {
         char clean = '0';
         int32_t bytesRead = PR_Read(mCleanFD, &clean, 1);
         if (bytesRead != 1) {
             NS_WARNING("Could not read _CACHE_CLEAN_ file contents");
-        } else {
+        } else if (reportCacheCleanTelemetryData) {
             Telemetry::Accumulate(Telemetry::DISK_CACHE_REDUCTION_TRIAL,
                                   clean == '1' ? 1 : 0);
         }
     }
 
     // Create a timer that will be used to validate the cache
     // as long as an activity threshold was met
     mCleanCacheTimer = do_CreateInstance("@mozilla.org/timer;1", &rv);
@@ -1307,17 +1315,22 @@ nsresult
 nsDiskCacheMap::InvalidateCache()
 {
     nsCacheService::AssertOwnsLock();
     CACHE_LOG_DEBUG(("CACHE: InvalidateCache\n"));
     nsresult rv;
   
     if (!mIsDirtyCacheFlushed) {
         rv = WriteCacheClean(false);
-        NS_ENSURE_SUCCESS(rv, rv);
+        if (NS_FAILED(rv)) {
+          Telemetry::Accumulate(Telemetry::DISK_CACHE_INVALIDATION_SUCCESS, 0);
+          return rv;
+        }
+
+        Telemetry::Accumulate(Telemetry::DISK_CACHE_INVALIDATION_SUCCESS, 1);
         mIsDirtyCacheFlushed = true;
     }
 
     rv = ResetCacheTimer();
     NS_ENSURE_SUCCESS(rv, rv);
 
     return NS_OK;
 }
@@ -1377,25 +1390,34 @@ nsDiskCacheMap::IsCacheInSafeState()
 
 nsresult
 nsDiskCacheMap::RevalidateCache()
 {
     CACHE_LOG_DEBUG(("CACHE: RevalidateCache\n"));
     nsresult rv;
 
     if (!IsCacheInSafeState()) {
+        Telemetry::Accumulate(Telemetry::DISK_CACHE_REVALIDATION_SAFE, 0);
         CACHE_LOG_DEBUG(("CACHE: Revalidation not performed because "
                          "cache not in a safe state\n"));
         return NS_ERROR_FAILURE;
     }
 
+    Telemetry::Accumulate(Telemetry::DISK_CACHE_REVALIDATION_SAFE, 1);
+
     // We want this after the lock to prove that flushing a file isn't that expensive
     Telemetry::AutoTimer<Telemetry::NETWORK_DISK_CACHE_REVALIDATION> totalTimer;
 
     // If telemetry data shows it is worth it, we'll be flushing headers and
     // records before flushing the clean cache file.
   
     // Write out the _CACHE_CLEAN_ file with '1'
     rv = WriteCacheClean(true);
+    if (NS_FAILED(rv)) {
+        Telemetry::Accumulate(Telemetry::DISK_CACHE_REVALIDATION_SUCCESS, 0);
+        return rv;
+    }
+
+    Telemetry::Accumulate(Telemetry::DISK_CACHE_REVALIDATION_SUCCESS, 1);
     mIsDirtyCacheFlushed = false;
 
     return NS_OK;
 }
--- a/netwerk/cache/nsDiskCacheMap.h
+++ b/netwerk/cache/nsDiskCacheMap.h
@@ -76,17 +76,17 @@ struct nsDiskCacheEntry;
 // reasons. See bug #651100 comment #21.
 #define kMaxDataSizeK      0xFFFF
 
 // preallocate up to 1MB of separate cache file
 #define kPreallocateLimit  1 * 1024 * 1024
 
 // The minimum amount of milliseconds to wait before re-attempting to
 // revalidate the cache.
-#define kRevalidateCacheTimeout 5000
+#define kRevalidateCacheTimeout 3000
 #define kRevalidateCacheTimeoutTolerance 10
 #define kRevalidateCacheErrorTimeout 1000
 
 class nsDiskCacheRecord {
 
 private:
     uint32_t    mHashNumber;
     uint32_t    mEvictionRank;
@@ -403,17 +403,18 @@ public:
  *  File Operations
  *
  *  Open
  *
  *  Creates a new cache map file if one doesn't exist.
  *  Returns error if it detects change in format or cache wasn't closed.
  */
     nsresult  Open( nsIFile *  cacheDirectory,
-                    nsDiskCache::CorruptCacheInfo *  corruptInfo);
+                    nsDiskCache::CorruptCacheInfo *  corruptInfo,
+                    bool reportCacheCleanTelemetryData);
     nsresult  Close(bool flush);
     nsresult  Trim();
 
     nsresult  FlushHeader();
     nsresult  FlushRecords( bool unswap);
 
     void      NotifyCapacityChange(uint32_t capacity);
 
@@ -534,17 +535,18 @@ private:
 
     // The returned structure will point to the buffer owned by nsDiskCacheMap, 
     // so it must not be deleted by the caller.
     nsDiskCacheEntry *  CreateDiskCacheEntry(nsDiskCacheBinding *  binding,
                                              uint32_t * size);
 
     // Initializes the _CACHE_CLEAN_ related functionality
     nsresult InitCacheClean(nsIFile *  cacheDirectory,
-                            nsDiskCache::CorruptCacheInfo *  corruptInfo);
+                            nsDiskCache::CorruptCacheInfo *  corruptInfo,
+                            bool reportCacheCleanTelemetryData);
     // Writes out a value of '0' or '1' in the _CACHE_CLEAN_ file
     nsresult WriteCacheClean(bool clean);
     // Resets the timout for revalidating the cache
     nsresult ResetCacheTimer(int32_t timeout = kRevalidateCacheTimeout);
     // Invalidates the cache, calls WriteCacheClean and ResetCacheTimer
     nsresult InvalidateCache();
     // Determines if the cache is in a safe state
     bool IsCacheInSafeState();
--- a/toolkit/components/telemetry/Histograms.json
+++ b/toolkit/components/telemetry/Histograms.json
@@ -823,16 +823,28 @@
     "kind": "enumerated",
     "n_values": 50,
     "description": "Why the HTTP disk cache was corrupted at startup"
   },
   "DISK_CACHE_REDUCTION_TRIAL": {
     "kind": "boolean",
     "description": "Stores 1 if the cache would be clean with the disk cache corruption plan of Bug 105843"
   },
+  "DISK_CACHE_REVALIDATION_SAFE": {
+    "kind": "boolean",
+    "description": "Stores 1 if the cache clean file was revalidated, or 0 if a non empty doom list prevented revalidation"
+  },
+  "DISK_CACHE_INVALIDATION_SUCCESS": {
+    "kind": "boolean",
+    "description": "Stores 1 if writing '0' to the cache clean file succeeded, and 0 if it failed."
+  },
+  "DISK_CACHE_REVALIDATION_SUCCESS": {
+    "kind": "boolean",
+    "description": "Stores 1 if writing '1' to the cache clean file succeeded, and 0 if it failed."
+  },
   "HTTP_CACHE_DISPOSITION_2": {
     "kind": "enumerated",
     "n_values": 5,
     "description": "HTTP Cache Hit, Reval, Failed-Reval, Miss"
   },
   "HTTP_DISK_CACHE_DISPOSITION_2": {
     "kind": "enumerated",
     "n_values": 5,