SDCH: add TimeWeightedMemoryUse

net/sdch/sdch_owner.cc

 // Copyright 2014 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "net/sdch/sdch_owner.h"
 
 #include "base/bind.h"
 #include "base/debug/alias.h"
 #include "base/logging.h"
 #include "base/metrics/histogram_macros.h"
 #include "base/prefs/persistent_pref_store.h"
 #include "base/prefs/value_map_pref_store.h"
+#include "base/process/process_info.h"
 #include "base/strings/string_util.h"
 #include "base/time/default_clock.h"
 #include "base/values.h"
 #include "net/base/sdch_manager.h"
 #include "net/base/sdch_net_log_params.h"
 
 namespace net {
 
 namespace {
 
-enum DictionaryFate {
-  // A Get-Dictionary header wasn't acted on.
-  DICTIONARY_FATE_GET_IGNORED = 1,
-
-  // A fetch was attempted, but failed.
-  // TODO(rdsmith): Actually record this case.
-  DICTIONARY_FATE_FETCH_FAILED = 2,
-
-  // A successful fetch was dropped on the floor, no space.
-  DICTIONARY_FATE_FETCH_IGNORED_NO_SPACE = 3,
-
-  // A successful fetch was refused by the SdchManager.
-  DICTIONARY_FATE_FETCH_MANAGER_REFUSED = 4,
-
-  // A dictionary was successfully added based on
-  // a Get-Dictionary header in a response.
-  DICTIONARY_FATE_ADD_RESPONSE_TRIGGERED = 5,
-
-  // A dictionary was evicted by an incoming dict.
-  DICTIONARY_FATE_EVICT_FOR_DICT = 6,
-
-  // A dictionary was evicted by memory pressure.
-  DICTIONARY_FATE_EVICT_FOR_MEMORY = 7,
-
-  // A dictionary was evicted on destruction.
-  DICTIONARY_FATE_EVICT_FOR_DESTRUCTION = 8,
-
-  // A dictionary was successfully added based on
-  // persistence from a previous browser revision.
-  DICTIONARY_FATE_ADD_PERSISTENCE_TRIGGERED = 9,
-
-  // A dictionary was unloaded on destruction, but is still present on disk.
-  DICTIONARY_FATE_UNLOAD_FOR_DESTRUCTION = 10,
-
-  DICTIONARY_FATE_MAX = 11
-};
-
 enum PersistenceFailureReason {
   // File didn't exist; is being created.
   PERSISTENCE_FAILURE_REASON_NO_FILE = 1,
 
   // Error reading in information, but should be able to write.
   PERSISTENCE_FAILURE_REASON_READ_FAILED = 2,
 
   // Error leading to abort on attempted persistence.
   PERSISTENCE_FAILURE_REASON_WRITE_FAILED = 3,
 
   PERSISTENCE_FAILURE_REASON_MAX = 4
 };
 
 // Dictionaries that haven't been touched in 24 hours may be evicted
 // to make room for new dictionaries.
 const int kFreshnessLifetimeHours = 24;
 
 // Dictionaries that have never been used only stay fresh for one hour.
 const int kNeverUsedFreshnessLifetimeHours = 1;
 
-void RecordDictionaryFate(enum DictionaryFate fate) {
-  UMA_HISTOGRAM_ENUMERATION("Sdch3.DictionaryFate", fate, DICTIONARY_FATE_MAX);
-}
-
 void RecordPersistenceFailure(PersistenceFailureReason failure_reason) {
   UMA_HISTOGRAM_ENUMERATION("Sdch3.PersistenceFailureReason", failure_reason,
                             PERSISTENCE_FAILURE_REASON_MAX);
 }
 
-void RecordDictionaryEvictionOrUnload(int use_count, DictionaryFate fate) {
-  DCHECK(fate == DICTIONARY_FATE_EVICT_FOR_DICT ||
-         fate == DICTIONARY_FATE_EVICT_FOR_MEMORY ||
-         fate == DICTIONARY_FATE_EVICT_FOR_DESTRUCTION ||
-         fate == DICTIONARY_FATE_UNLOAD_FOR_DESTRUCTION);
-
-  UMA_HISTOGRAM_COUNTS_100("Sdch3.DictionaryUseCount", use_count);
-  RecordDictionaryFate(fate);
-}
-
 // Schema specifications and access routines.
 
 // The persistent prefs store is conceptually shared with any other network
 // stack systems that want to persist data over browser restarts, and so
 // use of it must be namespace restricted.
 // Schema:
 //      pref_store_->GetValue(kPreferenceName) -> Dictionary {
 //          'version' -> 1 [int]
 //          'dictionaries' -> Dictionary {
 //              server_hash -> {
@@ skipping 150 matching lines @@
 // static
 const size_t SdchOwner::kMaxTotalDictionarySize = 20 * 1000 * 1000;
 #endif
 
 // Somewhat arbitrary, but we assume a dictionary smaller than
 // 50K isn't going to do anyone any good.  Note that this still doesn't
 // prevent download and addition unless there is less than this
 // amount of space available in storage.
 const size_t SdchOwner::kMinSpaceForDictionaryFetch = 50 * 1000;
 
+void SdchOwner::RecordDictionaryFate(enum DictionaryFate fate) {
+  UMA_HISTOGRAM_ENUMERATION("Sdch3.DictionaryFate", fate, DICTIONARY_FATE_MAX);
+}
+
+void SdchOwner::RecordDictionaryEvictionOrUnload(const std::string& server_hash,
+                                                 size_t size,
+                                                 int use_count,
+                                                 DictionaryFate fate) {
+  DCHECK(fate == DICTIONARY_FATE_EVICT_FOR_DICT ||
+         fate == DICTIONARY_FATE_EVICT_FOR_MEMORY ||
+         fate == DICTIONARY_FATE_EVICT_FOR_DESTRUCTION ||
+         fate == DICTIONARY_FATE_UNLOAD_FOR_DESTRUCTION);
+
+  UMA_HISTOGRAM_COUNTS_100("Sdch3.DictionaryUseCount", use_count);
+  RecordDictionaryFate(fate);
+
+  DCHECK(load_times_.count(server_hash) == 1);

[Randy Smith (Not in Mondays), 2015/04/03 20:36:51]

If you're going to have this DCHECK, that suggests to me that you should also
delete the entry in load_times_ at the end of this function.  Otherwise the
DCHECK() isn't checking anything if a dictionary is re-loaded.

(Just a suggestion, may be irrelevant because SdchDictionaryFetcher won't
re-fetch the same dictionary without a clear, but I'd consider that a bug if
it's true and that behavior isn't one SdchOwner should count on anyway.  But
it's just a DCHECK.)

+  base::Time now = clock_->Now();
+  base::TimeDelta dict_lifetime = now - load_times_[server_hash];
+  consumed_byte_seconds_.push_back(size * dict_lifetime.InMilliseconds());
+}
+
 SdchOwner::SdchOwner(SdchManager* sdch_manager, URLRequestContext* context)
     : manager_(sdch_manager->GetWeakPtr()),
       fetcher_(new SdchDictionaryFetcher(context)),
       total_dictionary_bytes_(0),
       clock_(new base::DefaultClock),
       max_total_dictionary_size_(kMaxTotalDictionarySize),
       min_space_for_dictionary_fetch_(kMinSpaceForDictionaryFetch),
 #if defined(OS_CHROMEOS)
       // For debugging http://crbug.com/454198; remove when resolved.
       destroyed_(0),
@@ skipping 23 matching lines @@
   clock_.reset();
   CHECK(manager_.get());
 #endif
 
   for (DictionaryPreferenceIterator it(pref_store_); !it.IsAtEnd();
        it.Advance()) {
     int new_uses = it.use_count() - use_counts_at_load_[it.server_hash()];
     DictionaryFate fate = IsPersistingDictionaries() ?
                           DICTIONARY_FATE_UNLOAD_FOR_DESTRUCTION :
                           DICTIONARY_FATE_EVICT_FOR_DESTRUCTION;
-    RecordDictionaryEvictionOrUnload(new_uses, fate);
+    RecordDictionaryEvictionOrUnload(it.server_hash(), it.size(), new_uses,
+                                     fate);
   }
   manager_->RemoveObserver(this);
 
   // This object only observes the external store during loading,
   // i.e. before it's made the default preferences store.
   if (external_pref_store_)
     external_pref_store_->RemoveObserver(this);
 
+  base::Time creation_time = base::CurrentProcessInfo::CreationTime();

[Randy Smith (Not in Mondays), 2015/04/03 20:36:51]

I'm still inclined to think this should just be the lifetime of the SdchOwner;
that seems strictly better than process lifetime.  WDYT?

+  int64 process_lifetime =
+      (clock_->Now() - creation_time).InMilliseconds();
+  for (auto& val : consumed_byte_seconds_) {
+    UMA_HISTOGRAM_COUNTS("Sdch3.TimeWeightedMemoryUse",
+                         val / process_lifetime);
+  }
+
 #if defined(OS_CHROMEOS)
   destroyed_ = 0xdeadbeef;
 #endif
 }
 
 void SdchOwner::EnablePersistentStorage(PersistentPrefStore* pref_store) {
   DCHECK(!external_pref_store_);
   external_pref_store_ = pref_store;
   external_pref_store_->AddObserver(this);
 
@@ skipping 112 matching lines @@
 
     DCHECK(pref_dictionary_map->HasKey(stale_it->server_hash));
     bool success = pref_dictionary_map->RemoveWithoutPathExpansion(
         stale_it->server_hash, nullptr);
     DCHECK(success);
 
     avail_bytes += stale_it->dictionary_size;
 
     int new_uses = stale_it->use_count -
         use_counts_at_load_[stale_it->server_hash];
-    RecordDictionaryEvictionOrUnload(new_uses,
+    RecordDictionaryEvictionOrUnload(stale_it->server_hash,
+                                     stale_it->dictionary_size,
+                                     new_uses,
                                      DICTIONARY_FATE_EVICT_FOR_DICT);
 
     ++stale_it;
   }
   DCHECK_GE(avail_bytes, dictionary_text.size());
 
   RecordDictionaryFate(
       // Distinguish between loads triggered by network responses and
       // loads triggered by persistence.
       last_used.is_null() ? DICTIONARY_FATE_ADD_RESPONSE_TRIGGERED
@@ skipping 16 matching lines @@
   // Record the addition in the pref store.
   scoped_ptr<base::DictionaryValue> dictionary_description(
       new base::DictionaryValue());
   dictionary_description->SetString(kDictionaryUrlKey, dictionary_url.spec());
   dictionary_description->SetDouble(kDictionaryLastUsedKey,
                                     last_used.ToDoubleT());
   dictionary_description->SetInteger(kDictionaryUseCountKey, use_count);
   dictionary_description->SetInteger(kDictionarySizeKey,
                                      dictionary_text.size());
   pref_dictionary_map->Set(server_hash, dictionary_description.Pass());
+  load_times_[server_hash] = clock_->Now();
 }
 
 void SdchOwner::OnDictionaryUsed(SdchManager* manager,
                                  const std::string& server_hash) {
   base::Time now(clock_->Now());
   base::DictionaryValue* pref_dictionary_map =
       GetPersistentStoreDictionaryMap(pref_store_);
   ScopedPrefNotifier scoped_pref_notifier(pref_store_);
 
   base::Value* value = nullptr;
@@ skipping 212 matching lines @@
   fetcher_.reset(fetcher.release());
 }
 
 void SdchOwner::OnMemoryPressure(
     base::MemoryPressureListener::MemoryPressureLevel level) {
   DCHECK_NE(base::MemoryPressureListener::MEMORY_PRESSURE_LEVEL_NONE, level);
 
   for (DictionaryPreferenceIterator it(pref_store_); !it.IsAtEnd();
        it.Advance()) {
     int new_uses = it.use_count() - use_counts_at_load_[it.server_hash()];
-    RecordDictionaryEvictionOrUnload(new_uses,
+    RecordDictionaryEvictionOrUnload(it.server_hash(),
+                                     it.size(),
+                                     new_uses,
                                      DICTIONARY_FATE_EVICT_FOR_MEMORY);
   }
 
   // TODO(rdsmith): Make a distinction between moderate and critical
   // memory pressure.
   manager_->ClearData();
 }
 
 bool SdchOwner::SchedulePersistedDictionaryLoads(
     const base::DictionaryValue& persisted_info) {
@@ skipping 41 matching lines @@
   }
 
   return true;
 }
 
 bool SdchOwner::IsPersistingDictionaries() const {
   return in_memory_pref_store_.get() != nullptr;
 }
 
 }  // namespace net