Add a multiplier in tracking certificate memory allocation size

net/ssl/ssl_client_session_cache.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 "net/ssl/ssl_client_session_cache.h"
 
+#include <unordered_set>
 #include <utility>
 
 #include "base/memory/memory_coordinator_client_registry.h"
 #include "base/strings/stringprintf.h"
 #include "base/time/clock.h"
 #include "base/time/default_clock.h"
 #include "base/trace_event/process_memory_dump.h"
 #include "net/cert/x509_util_openssl.h"
 #include "third_party/boringssl/src/include/openssl/ssl.h"
 #include "third_party/boringssl/src/include/openssl/x509.h"
@@ skipping 98 matching lines @@
   std::string absolute_name = "net/ssl_session_cache";
   base::trace_event::MemoryAllocatorDump* cache_dump =
       pmd->GetAllocatorDump(absolute_name);
   // This method can be reached from different URLRequestContexts. Since this is
   // a singleton, only log memory stats once.
   // TODO(xunjieli): Change this once crbug.com/458365 is fixed.
   if (cache_dump)
     return;
   cache_dump = pmd->CreateAllocatorDump(absolute_name);
   base::AutoLock lock(lock_);
-  size_t total_serialized_cert_size = 0;
+  size_t total_cert_size = 0;
+  size_t undeduped_cert_size = 0;
   size_t total_cert_count = 0;
+  std::unordered_set<const CRYPTO_BUFFER*> crypto_buffer_set;

[DmitrySkiba, 2017/02/17 22:02:58]

I'm worried that this will slow things down. We've seen malloc() (which is
called in the end by operator new) taking very long, and what is worse, it's
unpredictable.

One way to speed things up is to count total number of certs and reserve() that
many elements in the set. That should preallocate buckets, but nodes will still
be allocated on demand.

Another alternative might be base::flat_set, which is backed by std::vector. So
reserve() guarantees that we allocate only once, but at the same time insertion
is n*logn.

BTW, here you are doing find / insert, while you could just always do insert()
and examine resulting pair.

To decide we need some stats on total number of certs / unique number of certs -
can you get that? I would just browse ~10 sites and see (btw,
undeduped_cert_count is not reported).

[davidben, 2017/02/17 22:09:46]

In the session cache, I would expect it to not do much *for now*. (Though it may
still do something for intermediates... depends a LOT on which 10 sites.) In the
future, with TLS 1.3's single-use sessions, it will be important.

In the socket pools, it would depend heavily on which 10 sites you picked.
HTTP/1.1 you can expect it to be valuable. HTTP/2 probably less so.

As always, this is the sort of thing where the kind of instrumentation you want
to do is very dependent on how the net stack happens to work right now (with how
it happens to work being a constantly changing thing as we improve it) and also
very dependent on what question you're trying to answer.

[xunjieli, 2017/02/17 22:22:01]

I don't think we can know the factor of total number of undeduped certs/unique
number of certs. Every browsing session will be different. 
This "cert_size" is a best effort. In the trace linked 
https://drive.google.com/file/d/0Bw0C8-TgzJfsYlYwV3hwTTJUcVE/view?pli=1,  you
can see that the "undeduped_cert_size" is about 1.5 more than "cert_size" in
session cache. Again, this is one run. 
If allocating a std::unordered_set<> is slow, how about I get rid of
"undeduped_cert_size" and reduce the multiplier from 4 to 2? This is sort of
arbitrary anyway.

I don't know a way to estimate the number of deduped certs.

   for (const auto& pair : cache_) {
     const SSL_SESSION* session = pair.second.session.get();
     size_t cert_count = sk_CRYPTO_BUFFER_num(session->certs);
     total_cert_count += cert_count;
     for (size_t i = 0; i < cert_count; ++i) {
       const CRYPTO_BUFFER* cert = sk_CRYPTO_BUFFER_value(session->certs, i);
-      total_serialized_cert_size += CRYPTO_BUFFER_len(cert);
+      // TODO(xunjieli): The multipler is added to account for the difference
+      // between the serialized form and real cert allocation. Remove after
+      // crbug.com/671420 is done.
+      size_t cert_size = 4 * CRYPTO_BUFFER_len(cert);
+      undeduped_cert_size += cert_size;
+      if (crypto_buffer_set.find(cert) != crypto_buffer_set.end())
+        continue;
+      total_cert_size += cert_size;
+      crypto_buffer_set.insert(cert);
     }
   }
   // This measures the lower bound of the serialized certificate. It doesn't
   // measure the actual memory used, which is 4x this amount (see
   // crbug.com/671420 for more details).
-  cache_dump->AddScalar("serialized_cert_size",
+  cache_dump->AddScalar("cert_size",
                         base::trace_event::MemoryAllocatorDump::kUnitsBytes,
-                        total_serialized_cert_size);
+                        total_cert_size);
+  cache_dump->AddScalar("undeduped_cert_size",
+                        base::trace_event::MemoryAllocatorDump::kUnitsBytes,
+                        undeduped_cert_size);
   cache_dump->AddScalar("cert_count",
                         base::trace_event::MemoryAllocatorDump::kUnitsObjects,
                         total_cert_count);
   cache_dump->AddScalar(base::trace_event::MemoryAllocatorDump::kNameSize,
                         base::trace_event::MemoryAllocatorDump::kUnitsBytes,
-                        total_serialized_cert_size);
+                        total_cert_size);
 }
 
 SSLClientSessionCache::Entry::Entry() : lookups(0) {}
 SSLClientSessionCache::Entry::Entry(Entry&&) = default;
 SSLClientSessionCache::Entry::~Entry() = default;
 
 void SSLClientSessionCache::FlushExpiredSessions() {
   time_t now = clock_->Now().ToTimeT();
   auto iter = cache_.begin();
   while (iter != cache_.end()) {
@@ skipping 17 matching lines @@
       Flush();
       break;
   }
 }
 
 void SSLClientSessionCache::OnPurgeMemory() {
   Flush();
 }
 
 }  // namespace net