[tracing] Add SkResourceCache as suballocation to avoid double counting.

skia/ext/skia_memory_dump_provider.cc

Index: skia/ext/skia_memory_dump_provider.cc
diff --git a/skia/ext/skia_memory_dump_provider.cc b/skia/ext/skia_memory_dump_provider.cc
index a7c445fd5d4f1f9b34865ac3162a3b7bf0a1688e..bada1da4b4aac3f152abe202a12be3be1fb5002e 100644
--- a/skia/ext/skia_memory_dump_provider.cc
+++ b/skia/ext/skia_memory_dump_provider.cc
@@ -4,6 +4,8 @@
 
 #include "skia_memory_dump_provider.h"
 
+#include "base/memory/discardable_shared_memory.h"
+#include "base/trace_event/malloc_dump_provider.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"
@@ -26,15 +28,23 @@ 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());
+  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());
 
   auto resource_mad =
       process_memory_dump->CreateAllocatorDump("skia/sk_resource_cache");
-  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.
+  const char* source_name =

[reveman, 2015/07/30 16:00:59]

When discardable memory is not used, shouldn't this be set to the return value
of MemoryDumpManager::GetInstance()->system_allocator_pool_name() instead of
MallocDumpProvider::kAllocatedObjects?

[ssid, 2015/07/30 20:05:15]

Done.

+      SkResourceCache::GetDiscardableFactory()
+          ? base::DiscardableSharedMemory::kAllocatedObjectsDumpName
+          : base::trace_event::MallocDumpProvider::kAllocatedObjects;

[reveman, 2015/07/29 18:32:11]

Can we instead ask the SkResourceCache for the name and add plumbing all the way
down to the segment to retrieve the correct source name?

[ssid, 2015/07/29 19:36:22]

Currently the SkResourceCache dump does not include each allocation from
discardable. It might not be included in future also. If that is the case, The
allocations can practically be from any segment. That is why I create a common
allocated_objects node and dump it.

Also, is it might not be very useful for the user to see multiple skia
allocations from multiple segments, in sense that:
Suppose gpu wants to create a sub-allocation from discardable, it would also
make multiple gpu dumps under multiple segments. In this view it is not possible
to see how much of discardable is skia using and how much is gpu using. But if
it is kept under separate node, then It is easy to see that.

Moreover, the segment-dumps in discardable to understand the fragmentation and
efficiency of the discardable heap. So, keeping it as different entity won't
harm the use-case.

[reveman, 2015/07/29 19:59:23]

I'm not following this. What's not possible to see when forcing suballocations
to be children on segment allocations rather than a global discardable
allocation?
I think it would be quite useful to see exactly what type of usage is causing
fragmentation. I'm failing to see any negative sides with having suballocations
connect to segments. Afaict it just provides more detailed information and align
better with what we have to do for shared-memory/gpu-memory-buffers. Maybe VC to
discuss this more?

+  process_memory_dump->AddSuballocation(resource_mad->guid(), source_name);
 
   return true;
 }