Track QuicChromiumClientSession in net/ MemoryDumpProvider

net/quic/chromium/quic_stream_factory.cc

Index: net/quic/chromium/quic_stream_factory.cc
diff --git a/net/quic/chromium/quic_stream_factory.cc b/net/quic/chromium/quic_stream_factory.cc
index bee0b0ef2f7976f5bc3893aeadf3f1a700bd4da5..45cbcd8b79f621c8f3de4c78d04c398efd0d9a36 100644
--- a/net/quic/chromium/quic_stream_factory.cc
+++ b/net/quic/chromium/quic_stream_factory.cc
@@ -19,6 +19,9 @@
 #include "base/strings/string_util.h"
 #include "base/strings/stringprintf.h"
 #include "base/threading/thread_task_runner_handle.h"
+#include "base/trace_event/memory_allocator_dump.h"
+#include "base/trace_event/memory_usage_estimator.h"
+#include "base/trace_event/process_memory_dump.h"
 #include "base/trace_event/trace_event.h"
 #include "base/values.h"
 #include "crypto/openssl_util.h"
@@ -185,6 +188,11 @@ class ServerIdOriginFilter : public QuicCryptoClientConfig::ServerIdFilter {
   const base::Callback<bool(const GURL&)> origin_filter_;
 };
 
+// Returns the estimate of dynamically allocated memory of |server_id|.
+size_t EstimateServerIdMemoryUsage(const QuicServerId& server_id) {
+  return HostPortPair::EstimateMemoryUsage(server_id.host_port_pair());
+}
+
 }  // namespace
 
 // Responsible for verifying the certificates saved in
@@ -897,6 +905,24 @@ void QuicStreamFactory::set_quic_server_info_factory(
   quic_server_info_factory_.reset(quic_server_info_factory);
 }
 
+void QuicStreamFactory::DumpMemoryStats(
+    base::trace_event::ProcessMemoryDump* pmd,
+    const std::string& parent_absolute_name) const {
+  if (all_sessions_.empty())
+    return;
+  base::trace_event::MemoryAllocatorDump* factory_dump =
+      pmd->CreateAllocatorDump(parent_absolute_name + "/quic_stream_factory");
+  size_t memory_estimate =
+      base::trace_event::EstimateMemoryUsage(all_sessions_);
+  factory_dump->AddScalar(base::trace_event::MemoryAllocatorDump::kNameSize,
+                          base::trace_event::MemoryAllocatorDump::kUnitsBytes,
+                          memory_estimate);
+  factory_dump->AddScalar(
+      base::trace_event::MemoryAllocatorDump::kNameObjectCount,
+      base::trace_event::MemoryAllocatorDump::kUnitsObjects,
+      all_sessions_.size());
+}
+
 bool QuicStreamFactory::CanUseExistingSession(const QuicServerId& server_id,
                                               const HostPortPair& destination) {
   // TODO(zhongyi): delete active_sessions_.empty() checks once the
@@ -1047,6 +1073,11 @@ bool QuicStreamFactory::QuicSessionKey::operator==(
          server_id_ == other.server_id_;
 }
 
+size_t QuicStreamFactory::QuicSessionKey::EstimateMemoryUsage() const {
+  return HostPortPair::EstimateMemoryUsage(destination_) +
+         EstimateServerIdMemoryUsage(server_id_);

[ssid, 2017/01/26 22:43:59]

I think this should just return 0 here instead of trying to estimate it. I
actually looked at the string from running chrome and it is 25bytes large. Even
if we have 1000 QuicSessionKey objects around, we will still see 20KiB in total.
I do not think the instrumentation discussion is worth the time given the
marginal improvement in accuracy of the measurement. I have tried opening a lot
of tabs and still don't see more than 25 instances of the QuicSessionKey at any
time.

I think if we are going to show 100 bytes after adding these 3 functions along
with a downstream change, it just feels useless.

[xunjieli, 2017/01/26 23:10:00]

Dmitry suggested to track all allocations regardless size. But I agree with you
that adding 2 extra methods here probably outweighs the benefits. Dmitry do you
feel strongly about this? Should we go back to return 0 for this one?

+}
+
 void QuicStreamFactory::CreateAuxilaryJob(const QuicSessionKey& key,
                                           int cert_verify_flags,
                                           const NetLogWithSource& net_log) {