[tracing] Adding SkGlyphCache detailed statistics to skia dump provider

skia/ext/skia_memory_dump_provider.cc

 // Copyright 2015 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 "skia_memory_dump_provider.h"
 
+#include "base/strings/stringprintf.h"
 #include "base/trace_event/memory_allocator_dump.h"
 #include "base/trace_event/memory_dump_manager.h"
 #include "base/trace_event/process_memory_dump.h"
 #include "third_party/skia/include/core/SkGraphics.h"
+#include "third_party/skia/include/core/SkGlyphCacheStats.h"

[petrcermak, 2015/07/28 19:40:29]

nit: I think this should come before the include on line 11 (unless case is
taken into order).

[ssid, 2015/07/28 20:19:11]

I would expect upload to give me error. Fixed it, thanks

 #include "third_party/skia/src/core/SkResourceCache.h"
 
 namespace skia {
 
+namespace {
+
+const char kFontCacheDumpName[] = "skia/sk_font_cache";
+
+class SkiaGlyphCacheDumper : public SkGlyphCacheStatsDumper {
+ public:
+  SkiaGlyphCacheDumper(
+      base::trace_event::ProcessMemoryDump* process_memory_dump);
+
+  // SkGlyphCacheStatsDumper implementation:
+  void DumpStats(const char* uniqueName, const size_t memoryUsed) override;
+
+ private:
+  base::trace_event::ProcessMemoryDump* process_memory_dump_;
+};
+
+SkiaGlyphCacheDumper::SkiaGlyphCacheDumper(
+    base::trace_event::ProcessMemoryDump* process_memory_dump)
+    : process_memory_dump_(process_memory_dump) {}
+
+void SkiaGlyphCacheDumper::DumpStats(const char* uniqueName,
+                                     const size_t memoryUsed) {
+  auto cache_mad = process_memory_dump_->CreateAllocatorDump(
+      base::StringPrintf("%s/%s", kFontCacheDumpName, uniqueName));
+
+  cache_mad->AddScalar(base::trace_event::MemoryAllocatorDump::kNameSize,

[petrcermak, 2015/07/28 19:40:29]

nit: I'd find this more readable if both calls had the same format:

cache_mad->AddScalar(
    base::trace_event::MemoryAllocatorDump::kNameSize,
    base::trace_event::MemoryAllocatorDump::kUnitsBytes,
    memoryUsed);
cache_mad->AddScalar(
    base::trace_event::MemoryAllocatorDump::kNameObjectsCount,
    base::trace_event::MemoryAllocatorDump::kUnitsObjects,
    1u);

[ssid, 2015/07/28 20:19:11]

Hm, it was git cl format. I don't want to touch this because next time someone
changes this file, it will rearrange back to this.

+                       base::trace_event::MemoryAllocatorDump::kUnitsBytes,
+                       memoryUsed);
+  cache_mad->AddScalar(
+      base::trace_event::MemoryAllocatorDump::kNameObjectsCount,
+      base::trace_event::MemoryAllocatorDump::kUnitsObjects, 1u);
+}
+
+}  // namespace
+
 // static
 SkiaMemoryDumpProvider* SkiaMemoryDumpProvider::GetInstance() {
   return Singleton<SkiaMemoryDumpProvider,
                    LeakySingletonTraits<SkiaMemoryDumpProvider>>::get();
 }
 
 SkiaMemoryDumpProvider::SkiaMemoryDumpProvider() {}
 
 SkiaMemoryDumpProvider::~SkiaMemoryDumpProvider() {}
 
 bool SkiaMemoryDumpProvider::OnMemoryDump(
     base::trace_event::ProcessMemoryDump* process_memory_dump) {
-  auto font_mad =
-      process_memory_dump->CreateAllocatorDump("skia/sk_font_cache");
-  font_mad->AddScalar("size", "bytes", SkGraphics::GetFontCacheUsed());
-  font_mad->AddScalar("count", "objects", SkGraphics::GetFontCacheCountUsed());
+  auto font_mad = process_memory_dump->CreateAllocatorDump(kFontCacheDumpName);
+  font_mad->AddScalar(base::trace_event::MemoryAllocatorDump::kNameSize,
+                      base::trace_event::MemoryAllocatorDump::kUnitsBytes,
+                      SkGraphics::GetFontCacheUsed());
+  font_mad->AddScalar(base::trace_event::MemoryAllocatorDump::kNameObjectsCount,
+                      base::trace_event::MemoryAllocatorDump::kUnitsObjects,
+                      SkGraphics::GetFontCacheCountUsed());
+
+  SkiaGlyphCacheDumper dumper(process_memory_dump);
+  DumpSkGlyphCacheStats(&dumper);
 
   auto resource_mad =
       process_memory_dump->CreateAllocatorDump("skia/sk_resource_cache");

[petrcermak, 2015/07/28 19:40:29]

nit: I'd factor this dump name out into a constant as well (to be consistent
with kFontCacheDumpName).

[ssid, 2015/07/28 20:19:11]

Done.

-  resource_mad->AddScalar("size", "bytes",
+  resource_mad->AddScalar(base::trace_event::MemoryAllocatorDump::kNameSize,
+                          base::trace_event::MemoryAllocatorDump::kUnitsBytes,
                           SkResourceCache::GetTotalBytesUsed());
   // TODO(ssid): crbug.com/503168. Add sub-allocation edges from discardable or
   // malloc memory dumps to avoid double counting.
 
   return true;
 }
 
 }  // namespace skia