tracing: Report ResourceClients and the reason for not deletable for web-caches

third_party/WebKit/Source/core/fetch/Resource.cpp

 /*
     Copyright (C) 1998 Lars Knoll (knoll@mpi-hd.mpg.de)
     Copyright (C) 2001 Dirk Mueller (mueller@kde.org)
     Copyright (C) 2002 Waldo Bastian (bastian@kde.org)
     Copyright (C) 2006 Samuel Weinig (sam.weinig@gmail.com)
     Copyright (C) 2004, 2005, 2006, 2007, 2008, 2009, 2010, 2011 Apple Inc. All rights reserved.
 
     This library is free software; you can redistribute it and/or
     modify it under the terms of the GNU Library General Public
     License as published by the Free Software Foundation; either
@@ skipping 503 matching lines @@
     if (cacheType == CachedMetadataHandler::SendToPlatform)
         Platform::current()->cacheMetadata(m_response.url(), m_response.responseTime(), 0, 0);
 }
 
 bool Resource::canDelete() const
 {
     return !hasClients() && !m_loader && !m_preloadCount && hasRightHandleCountApartFromCache(0)
         && !m_protectorCount;
 }
 
+String Resource::reasonNotDeletable() const
+{
+    StringBuilder builder;
+    if (hasClients()) {
+        builder.append("hasClients(");
+        builder.appendNumber(m_clients.size());
+        if (!m_clientsAwaitingCallback.isEmpty()) {
+            builder.append(", AwaitingCallback=");
+            builder.appendNumber(m_clientsAwaitingCallback.size());
+        }
+        if (!m_finishedClients.isEmpty()) {
+            builder.append(", Finished=");
+            builder.appendNumber(m_finishedClients.size());
+        }
+        builder.append(")");
+    }
+    if (m_loader) {
+        if (!builder.isEmpty())
+            builder.append(' ');
+        builder.append("m_loader");

[ssid, 2015/10/30 07:00:31]

Can we get rid of m_ and display more useful info for common user. Maybe
"loader_exists"?

[hiroshige, 2015/10/30 07:34:33]

I made the names to directly corresponding to the variable names and
canDelete(), because I think
- it is clearer for Chromium developers
- it is hard to make the description fully understandable for "common users" --
Resource lifetime management is quite complex, and anyway requires reading
source code to understand it.

One possible way is to use class names where applicable:
  hasClients -> ResourceClient
  m_handleCount -> ResourcePtr
  m_protectorCount -> InternalResourcePtr
which will make the names and the column name of "ResourceClient" (L840)
consistent.

Added +loading-reviews@.
How do you think? Any good names?

[kinuko, 2015/11/02 07:41:57]

Either looks fine with me given that it seems difficult anyway for 'common
users' to understand the meaning of these values. If we're going to show this as
something like TRACE_EVENT_OBJECT_SNAPSHOT with json UI support that sounds good
to me too.

[ssid, 2015/11/04 12:12:57]

Sorry for the late reply. It is fine as it is, since it is difficult for others
to understand anyway and more useful for developers.

+    }
+    if (m_preloadCount) {
+        if (!builder.isEmpty())
+            builder.append(' ');
+        builder.append("m_preloadCount(");

[ssid, 2015/10/30 07:00:31]

preloaded_with_count or just preloaded, and similar names below..

+        builder.appendNumber(m_preloadCount);
+        builder.append(")");
+    }
+    if (!hasRightHandleCountApartFromCache(0)) {
+        if (!builder.isEmpty())
+            builder.append(' ');
+        builder.append("m_handleCount(");
+        builder.appendNumber(m_handleCount);
+        builder.append(")");
+    }
+    if (m_protectorCount) {
+        if (!builder.isEmpty())
+            builder.append(' ');
+        builder.append("m_protectorCount(");
+        builder.appendNumber(m_protectorCount);
+        builder.append(")");
+    }
+    if (memoryCache()->contains(this)) {
+        if (!builder.isEmpty())
+            builder.append(' ');
+        builder.append("memoryCache");

[ssid, 2015/10/30 07:00:31]

reason_not_deletable : memoryCache -> in_memory_cache?

[hiroshige, 2015/10/30 07:34:33]

Acknowledged.

[hiroshige, 2015/11/09 20:53:58]

Done.

+    }
+    return builder.toString();
+}
+
 bool Resource::hasOneHandle() const
 {
     return hasRightHandleCountApartFromCache(1);
 }
 
 CachedMetadata* Resource::cachedMetadata(unsigned dataTypeID) const
 {
     if (!m_cachedMetadata || m_cachedMetadata->dataTypeID() != dataTypeID)
         return nullptr;
     return m_cachedMetadata.get();
@@ skipping 197 matching lines @@
 
 void Resource::prune()
 {
     destroyDecodedDataIfPossible();
     unlock();
 }
 
 void Resource::onMemoryDump(WebMemoryDumpLevelOfDetail levelOfDetail, WebProcessMemoryDump* memoryDump) const
 {
     static const size_t kMaxURLReportLength = 128;
+    static const int kMaxResourceClientToShowInMemoryInfra = 10;
 
     const String dumpName = getMemoryDumpName();
     WebMemoryAllocatorDump* dump = memoryDump->createMemoryAllocatorDump(dumpName);
     dump->addScalar("encoded_size", "bytes", m_encodedSize);
     if (canDelete()) {
         dump->addScalar("dead_size", "bytes", m_encodedSize);
     } else {
         dump->addScalar("live_size", "bytes", m_encodedSize);
     }
 
     if (m_data) {
         dump->addScalar("purgeable_size", "bytes", isPurgeable() && !wasPurged() ? encodedSize() + overheadSize() : 0);
         m_data->onMemoryDump(dumpName, memoryDump);
     }
 
     if (levelOfDetail == WebMemoryDumpLevelOfDetail::Detailed) {
         String urlToReport = url().string();
         if (urlToReport.length() > kMaxURLReportLength) {
             urlToReport.truncate(kMaxURLReportLength);
             urlToReport = urlToReport + "...";
         }
         dump->addString("url", "", urlToReport);
+
+        dump->addString("not_deletable_because", "", reasonNotDeletable());

[ssid, 2015/10/30 07:00:31]

I think it is better to show "reason_not_deletable" or something like that,
instead of open phrase "not_deletable_because". What if that column is empty?

[hiroshige, 2015/11/09 20:53:58]

Done.

+
+        Vector<String> clientNames;
+        ResourceClientWalker<ResourceClient> w(m_clients);
+        while (ResourceClient* c = w.next())
+            clientNames.append(c->debugName());
+        ResourceClientWalker<ResourceClient> w2(m_clientsAwaitingCallback);
+        while (ResourceClient* c = w2.next())
+            clientNames.append("(awaiting) " + c->debugName());
+        ResourceClientWalker<ResourceClient> w3(m_finishedClients);
+        while (ResourceClient* c = w3.next())
+            clientNames.append("(finished) " + c->debugName());
+        std::sort(clientNames.begin(), clientNames.end(), codePointCompareLessThan);
+
+        StringBuilder builder;
+        for (size_t i = 0; i < clientNames.size() && i < kMaxResourceClientToShowInMemoryInfra; ++i) {
+            if (i > 0)
+                builder.append(" / ");
+            builder.append(clientNames[i]);
+        }
+        if (clientNames.size() > kMaxResourceClientToShowInMemoryInfra) {
+            builder.append(" / and ");
+            builder.appendNumber(clientNames.size() - kMaxResourceClientToShowInMemoryInfra);
+            builder.append(" more");
+        }
+        dump->addString("ResourceClient", "", builder.toString());
     }
 
     const String overheadName = dumpName + "/metadata";
     WebMemoryAllocatorDump* overheadDump = memoryDump->createMemoryAllocatorDump(overheadName);
     overheadDump->addScalar("size", "bytes", overheadSize());
     memoryDump->addSuballocation(overheadDump->guid(), String(WTF::Partitions::kAllocatedObjectPoolName));
 }
 
 String Resource::getMemoryDumpName() const
 {
@@ skipping 291 matching lines @@
         return "ImportResource";
     case Resource::Media:
         return "Media";
     }
     ASSERT_NOT_REACHED();
     return "Unknown";
 }
 #endif // !LOG_DISABLED
 
 } // namespace blink