[Storage] Added histograms for blob storage

storage/browser/blob/blob_storage_context.cc

 // Copyright (c) 2013 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 "storage/browser/blob/blob_storage_context.h"
 
 #include "base/bind.h"
 #include "base/location.h"
 #include "base/logging.h"
 #include "base/message_loop/message_loop_proxy.h"
+#include "base/metrics/histogram.h"
 #include "storage/browser/blob/blob_data_handle.h"
 #include "storage/common/blob/blob_data.h"
 #include "url/gurl.h"
 
 namespace storage {
 
 namespace {
 
 // We can't use GURL directly for these hash fragment manipulations
 // since it doesn't have specific knowlege of the BlobURL format. GURL
@@ skipping 115 matching lines @@
   // list of Data, File, and FileSystem items. Aggregated TYPE_BLOB items
   // are expanded into the primitive constituent types.
   // 1) The Data item is denoted by the raw data and length.
   // 2) The File item is denoted by the file path, the range and the expected
   //    modification time.
   // 3) The FileSystem File item is denoted by the FileSystem URL, the range
   //    and the expected modification time.
   // 4) The Blob items are expanded.
   // TODO(michaeln): Would be nice to avoid copying Data items when expanding.
 
-  DCHECK(item.length() > 0);
+  uint64 length = item.length();
+  DCHECK(length > 0);

[Alexei Svitkine (slow), 2015/01/06 16:47:42]

Nit: DCHECK_GT

[dmurph, 2015/01/06 20:15:44]

Done.

+  UMA_HISTOGRAM_COUNTS("Storage.Blob.StorageSizeBeforeAppend",
+                       memory_usage_ / 1024);
   switch (item.type()) {
     case BlobData::Item::TYPE_BYTES:
+      UMA_HISTOGRAM_COUNTS("Storage.BlobItemSize.Bytes", length);
       DCHECK(!item.offset());
       exceeded_memory = !AppendBytesItem(target_blob_data,
                                          item.bytes(),
-                                         static_cast<int64>(item.length()));
+                                         static_cast<int64>(length));
       break;
     case BlobData::Item::TYPE_FILE:
+      UMA_HISTOGRAM_COUNTS("Storage.BlobItemSize.File", length);
       AppendFileItem(target_blob_data,
                      item.path(),
                      item.offset(),
-                     item.length(),
+                     length,
                      item.expected_modification_time());
       break;
     case BlobData::Item::TYPE_FILE_FILESYSTEM:
+      UMA_HISTOGRAM_COUNTS("Storage.BlobItemSize.FileSystem", length);
       AppendFileSystemFileItem(target_blob_data,
                                item.filesystem_url(),
                                item.offset(),
-                               item.length(),
+                               length,
                                item.expected_modification_time());
       break;
     case BlobData::Item::TYPE_BLOB: {
+      UMA_HISTOGRAM_COUNTS("Storage.BlobItemSize.Blob", length);
       scoped_ptr<BlobDataHandle> src = GetBlobDataFromUUID(item.blob_uuid());
       if (src)
         exceeded_memory = !ExpandStorageItems(target_blob_data,
                                               src->data(),
                                               item.offset(),
                                               item.length());
       break;
     }
     default:
       NOTREACHED();
       break;
   }
+  UMA_HISTOGRAM_COUNTS("Storage.Blob.StorageSizeAfterAppend",
+                       memory_usage_ / 1024);

[Alexei Svitkine (slow), 2015/01/06 16:47:41]

Just wondering why you actually need both the Before and After versions of this
histogram.

I suspect their shapes would look rather similar - what's the purpose of needing
both?

[dmurph, 2015/01/06 20:15:44]

I want to find the distribution of the final storage size (or maximum storage
size) in all machines.  I can do this by logging all of the 'befores' and
'afters', and then subtracting all of the befores from the afters, which gives
me the final 'after', which is the maximum (or final) storage size on that
machine.

If you know a better way to do this I would definitely be interested.

[Alexei Svitkine (slow), 2015/01/06 20:22:48]

Ah, I see. You'll have to deal with some skew (i.e. when the before was logged
on a previous day than the after and when the next after will be on the next
day, etc), but I don't have a better suggestion for this, unfortunately - so
this is OK.

 
   // If we're using too much memory, drop this blob's data.
   // TODO(michaeln): Blob memory storage does not yet spill over to disk,
   // as a stop gap, we'll prevent memory usage over a max amount.
+  UMA_HISTOGRAM_BOOLEAN("Storage.BlobItem.ExceededMemory", exceeded_memory);

[Alexei Svitkine (slow), 2015/01/06 16:47:42]

Since exceeded memory can only happen on TYPE_BLOB, would it make more sense to
log this in that block? That way, you get the ratio of TYPE_BLOB's that exceeded
memory rather than of everything...

I guess you can derive one from the other with the help of the other metrics,
but if you're actually interested in ratio of blobs that exceeded, would be
better to be in the case.

[dmurph, 2015/01/06 20:15:44]

It's set in both TYPE_BLOB and TYPE_BYTES
However, I believe this isn't the best stat for us, as we want memory exceeded
ratio for blobs, not blob items.  Changed.

   if (exceeded_memory) {
     memory_usage_ -= target_blob_data->GetMemoryUsage();
     found->second.flags |= EXCEEDED_MEMORY;
     found->second.data = new BlobData(uuid);
     return;
   }
 }
 
 void BlobStorageContext::FinishBuildingBlob(
     const std::string& uuid, const std::string& content_type) {
@@ skipping 121 matching lines @@
   if (found == blob_map_.end())
     return false;
   return found->second.flags & BEING_BUILT;
 }
 
 bool BlobStorageContext::IsUrlRegistered(const GURL& blob_url) {
   return public_blob_urls_.find(blob_url) != public_blob_urls_.end();
 }
 
 }  // namespace storage