Refine UMA stats of QuotaTemporaryStorageEvictor

webkit/quota/quota_temporary_storage_evictor.cc

Index: webkit/quota/quota_temporary_storage_evictor.cc
diff --git a/webkit/quota/quota_temporary_storage_evictor.cc b/webkit/quota/quota_temporary_storage_evictor.cc
index d9e9fec0b33739aff562d48e31ae6e5e00561d3f..ea52868fa6aa0ee20cf99f48ebcfc110ffadae6e 100644
--- a/webkit/quota/quota_temporary_storage_evictor.cc
+++ b/webkit/quota/quota_temporary_storage_evictor.cc
@@ -64,19 +64,62 @@ void QuotaTemporaryStorageEvictor::GetStatistics(
       statistics_.num_skipped_eviction_rounds;
 }
 
-void QuotaTemporaryStorageEvictor::ReportHistogram() {
-  UMA_HISTOGRAM_COUNTS("Quota.ErrorsOnEvictingOrigin",
-                       statistics_.num_errors_on_evicting_origin);
-  UMA_HISTOGRAM_COUNTS("Quota.ErrorsOnGettingUsageAndQuota",
-                       statistics_.num_errors_on_getting_usage_and_quota);
-  UMA_HISTOGRAM_COUNTS("Quota.EvictedOrigins",
-                       statistics_.num_evicted_origins);
-  UMA_HISTOGRAM_COUNTS("Quota.EvictionRounds",
-                       statistics_.num_eviction_rounds);
-  UMA_HISTOGRAM_COUNTS("Quota.SkippedEvictionRounds",
-                       statistics_.num_skipped_eviction_rounds);
-
-  statistics_ = Statistics();
+void QuotaTemporaryStorageEvictor::ReportPerRoundHistogram() {
+  DCHECK(round_statistics_.in_round);
+  DCHECK(round_statistics_.is_initialized);
+
+  base::Time now = base::Time::Now();
+  UMA_HISTOGRAM_TIMES("Quota.TimeSpentToAEvictionRound",
+                      now - round_statistics_.start_time);
+  if (!time_of_end_of_last_round_.is_null())
+    UMA_HISTOGRAM_MINUTES("Quota.TimeDeltaOfEvictionRounds",
+                          now - time_of_end_of_last_round_);
+  UMA_HISTOGRAM_MBYTES("Quota.UsageOverageOfTemporaryGlobalStorage",
+                       round_statistics_.usage_overage_at_round);
+  UMA_HISTOGRAM_MBYTES("Quota.DiskspaceShortage",
+                       round_statistics_.diskspace_shortage_at_round);
+  UMA_HISTOGRAM_MBYTES("Quota.EvictedBytesPerRound",
+                       round_statistics_.usage_on_beginning_of_round -
+                       round_statistics_.usage_on_end_of_round);
+  UMA_HISTOGRAM_COUNTS("Quota.NumberOfEvictedOriginsPerRound",
+                       round_statistics_.num_evicted_origins_in_round);
+}
+
+void QuotaTemporaryStorageEvictor::ReportPerHourHistogram() {
+  Statistics stats(statistics_);

[Dai Mikurube (NOT FULLTIME), 2011/08/12 07:32:39]

Can we have more appropriate name for this variable?  (like delta, difference,
or in hour...)

[tzik, 2011/08/16 04:31:54]

Done.

+  stats.subtract_assign(previous_statistics_);
+  previous_statistics_ = statistics_;
+
+  UMA_HISTOGRAM_COUNTS("Quota.ErrorsOnEvictingOriginPerHour",
+                       stats.num_errors_on_evicting_origin);
+  UMA_HISTOGRAM_COUNTS("Quota.ErrorsOnGettingUsageAndQuotaPerHour",
+                       stats.num_errors_on_getting_usage_and_quota);
+  UMA_HISTOGRAM_COUNTS("Quota.EvictedOriginsPerHour",
+                       stats.num_evicted_origins);
+  UMA_HISTOGRAM_COUNTS("Quota.EvictionRoundsPerHour",
+                       stats.num_eviction_rounds);
+  UMA_HISTOGRAM_COUNTS("Quota.SkippedEvictionRoundsPerHour",
+                       stats.num_skipped_eviction_rounds);
+}
+
+void QuotaTemporaryStorageEvictor::OnEvictionRoundStarted() {
+  if (round_statistics_.in_round)
+    return;
+  round_statistics_.in_round = true;
+  round_statistics_.start_time = base::Time::Now();
+  ++statistics_.num_eviction_rounds;
+}
+
+void QuotaTemporaryStorageEvictor::OnEvictionRoundFinished() {
+  // Check if skipped round
+  if (round_statistics_.num_evicted_origins_in_round) {
+    ReportPerRoundHistogram();
+    time_of_end_of_last_nonskipped_round_ = base::Time::Now();
+  } else {
+    ++statistics_.num_skipped_eviction_rounds;
+  }
+  // Reset stats for next round.
+  round_statistics_ = EvictionRoundStatistics();
 }
 
 void QuotaTemporaryStorageEvictor::Start() {
@@ -86,21 +129,19 @@ void QuotaTemporaryStorageEvictor::Start() {
   if (histogram_timer_.IsRunning())
     return;
   histogram_timer_.Start(kHistogramReportInterval, this,
-                         &QuotaTemporaryStorageEvictor::ReportHistogram);
+                         &QuotaTemporaryStorageEvictor::ReportPerHourHistogram);
 }
 
 void QuotaTemporaryStorageEvictor::StartEvictionTimerWithDelay(int delay_ms) {
   if (eviction_timer_.IsRunning())
     return;
-  num_evicted_origins_in_round_ = 0;
-  usage_on_beginning_of_round_ = -1;
-  time_of_beginning_of_round_ = base::Time::Now();
-  ++statistics_.num_eviction_rounds;
   eviction_timer_.Start(base::TimeDelta::FromMilliseconds(delay_ms), this,
                         &QuotaTemporaryStorageEvictor::ConsiderEviction);
 }
 
 void QuotaTemporaryStorageEvictor::ConsiderEviction() {
+  OnEvictionRoundStarted();
+
   // Get usage and disk space, then continue.
   quota_eviction_handler_->GetUsageAndQuotaForEviction(callback_factory_.
       NewCallback(
@@ -121,49 +162,43 @@ void QuotaTemporaryStorageEvictor::OnGotUsageAndQuotaForEviction(
   if (status != kQuotaStatusOk)
     ++statistics_.num_errors_on_getting_usage_and_quota;
 
-  int64 amount_to_evict = std::max(
-      usage - static_cast<int64>(quota * kUsageRatioToStartEviction),
+  int64 usage_overage = std::max(
+      static_cast<int64>(0),
+      usage - static_cast<int64>(quota * kUsageRatioToStartEviction));
+
+  int64 diskspace_shortage = std::max(
+      static_cast<int64>(0),
       min_available_disk_space_to_start_eviction_ - available_disk_space);
+
+  if (!round_statistics_.is_initialized) {
+    round_statistics_.usage_overage_at_round = usage_overage;
+    round_statistics_.diskspace_shortage_at_round = diskspace_shortage;
+    round_statistics_.usage_on_beginning_of_round = usage;
+    round_statistics_.is_initialized = true;
+  }
+  round_statistics_.usage_on_end_of_round = usage;
+
+  int64 amount_to_evict = std::max(usage_overage, diskspace_shortage);
   if (status == kQuotaStatusOk && amount_to_evict > 0) {
     // Space is getting tight. Get the least recently used origin and continue.
     // TODO(michaeln): if the reason for eviction is low physical disk space,
     // make 'unlimited' origins subject to eviction too.
-
-    if (usage_on_beginning_of_round_ < 0) {
-      usage_on_beginning_of_round_ = usage;
-      UMA_HISTOGRAM_MBYTES("Quota.TemporaryStorageSizeToEvict",
-                           amount_to_evict);
-    }
-
     quota_eviction_handler_->GetLRUOrigin(kStorageTypeTemporary,
         callback_factory_.NewCallback(
             &QuotaTemporaryStorageEvictor::OnGotLRUOrigin));
-  } else if (repeated_eviction_) {
-    // No action required, sleep for a while and check again later.
-    if (num_evicted_origins_in_round_ == 0) {
-      ++statistics_.num_skipped_eviction_rounds;
-    } else if (usage_on_beginning_of_round_ >= 0) {
-      int64 evicted_bytes = usage_on_beginning_of_round_ - usage;
-      base::Time now = base::Time::Now();
-      UMA_HISTOGRAM_MBYTES("Quota.EvictedBytesPerRound", evicted_bytes);
-      UMA_HISTOGRAM_COUNTS("Quota.NumberOfEvictedOriginsPerRound",
-                          num_evicted_origins_in_round_);
-      UMA_HISTOGRAM_TIMES("Quota.TimeSpentToAEvictionRound",
-                          now - time_of_beginning_of_round_);
-      if (!time_of_end_of_last_round_.is_null()) {
-        UMA_HISTOGRAM_MINUTES("Quota.TimeDeltaOfEvictionRounds",
-                            now - time_of_end_of_last_round_);
+  } else {
+    if (repeated_eviction_) {
+      // No action required, sleep for a while and check again later.
+      if (statistics_.num_errors_on_getting_usage_and_quota <
+          kThresholdOfErrorsToStopEviction) {
+        StartEvictionTimerWithDelay(interval_ms_);
+      } else {
+        // TODO(dmikurube): Try restarting eviction after a while.
+        LOG(WARNING) << "Stopped eviction of temporary storage due to errors "
+                        "in GetUsageAndQuotaForEviction.";
       }
-      time_of_end_of_last_round_ = now;
-    }
-    if (statistics_.num_errors_on_getting_usage_and_quota <
-        kThresholdOfErrorsToStopEviction) {
-      StartEvictionTimerWithDelay(interval_ms_);
-    } else {
-      // TODO(dmikurube): Try restarting eviction after a while.
-      LOG(WARNING) << "Stopped eviction of temporary storage due to errors "
-                      "in GetUsageAndQuotaForEviction.";
     }
+    OnEvictionRoundFinished();
   }
 
   // TODO(dmikurube): Add error handling for the case status != kQuotaStatusOk.
@@ -175,6 +210,7 @@ void QuotaTemporaryStorageEvictor::OnGotLRUOrigin(const GURL& origin) {
   if (origin.is_empty()) {
     if (repeated_eviction_)
       StartEvictionTimerWithDelay(interval_ms_);
+    OnEvictionRoundFinished();
     return;
   }
 
@@ -195,7 +231,7 @@ void QuotaTemporaryStorageEvictor::OnEvictionComplete(
 
   if (status == kQuotaStatusOk) {
     ++statistics_.num_evicted_origins;
-    ++num_evicted_origins_in_round_;
+    ++round_statistics_.num_evicted_origins_in_round;
     // We many need to get rid of more space so reconsider immediately.
     ConsiderEviction();
   } else {
@@ -204,6 +240,7 @@ void QuotaTemporaryStorageEvictor::OnEvictionComplete(
       // Sleep for a while and retry again until we see too many errors.
       StartEvictionTimerWithDelay(interval_ms_);
     }
+    OnEvictionRoundFinished();
   }
 }