Type profiler by intercepting 'new' and 'delete' expressions.

third_party/tcmalloc/chromium/src/allocated_type_map.cc

Index: third_party/tcmalloc/chromium/src/allocated_type_map.cc
diff --git a/third_party/tcmalloc/chromium/src/allocated_type_map.cc b/third_party/tcmalloc/chromium/src/allocated_type_map.cc
new file mode 100644
index 0000000000000000000000000000000000000000..9b8b4107b024bb6735c4b00e98b59d2003e47b67
--- /dev/null
+++ b/third_party/tcmalloc/chromium/src/allocated_type_map.cc
@@ -0,0 +1,106 @@
+// Copyright (c) 2012 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 <config.h>
+
+#include <new>
+#include <typeinfo>
+
+#include <gperftools/allocated_type_map.h>
+
+#include "addressmap-inl.h"
+#include "tcmalloc_guard.h"
+#include "base/spinlock.h"

[Alexander Potapenko, 2012/08/03 09:57:07]

Mind the order of includes, please.

[Dai Mikurube (NOT FULLTIME), 2012/08/07 10:39:18]

Done.

+#include "base/low_level_alloc.h"
+
+
+//----------------------------------------------------------------------
+// Locking
+//----------------------------------------------------------------------
+
+static SpinLock allocated_type_lock(SpinLock::LINKER_INITIALIZED);
+
+//----------------------------------------------------------------------
+// Simple allocator for allocated type map's internal memory
+//----------------------------------------------------------------------
+
+static LowLevelAlloc::Arena* allocated_type_map_memory = NULL;
+
+static void* AllocatedTypeMalloc(size_t bytes) {
+  return LowLevelAlloc::AllocWithArena(bytes, allocated_type_map_memory);
+}
+static void AllocatedTypeFree(void* p) {
+  LowLevelAlloc::Free(p);
+}
+
+//----------------------------------------------------------------------
+// Profiling control/state data
+//----------------------------------------------------------------------
+
+static AddressMap<AllocatedObject>* allocated_type_map = NULL;

[Alexander Potapenko, 2012/08/02 14:57:12]

How about keeping a per-thread allocated_type_map. It will be a bit harder to
aggregate the data, but you'll have less problems with locking.

[Dai Mikurube (NOT FULLTIME), 2012/08/02 16:14:35]

I thought about that.  But in my thought, the problem is that an object may be
deleted by another thread than it is newed.  (I think it hardly happens, but not
never.)

[jar (doing other things), 2012/08/02 23:38:30]

Allocation on one thread, and deletion on another thread, is very common.  Task
objects follow this model commonly, as tasks are constructed on one thread, then
run and deleted on a second thread.

On 2012/08/02 16:14:35, Dai Mikurube wrote:

[Dai Mikurube (NOT FULLTIME), 2012/08/03 10:01:50]

Thanks.  We cannot use per-thread storage for this case. :(

+
+//----------------------------------------------------------------------
+// Manage allocated type map
+//----------------------------------------------------------------------
+
+static void InitializeAllocatedTypeMemory() {
+  if (allocated_type_map_memory == NULL) {
+    allocated_type_map_memory =
+        LowLevelAlloc::NewArena(0, LowLevelAlloc::DefaultArena());
+  }
+
+  if (allocated_type_map == NULL) {
+    allocated_type_map =
+        new(AllocatedTypeMalloc(sizeof(AddressMap<AllocatedObject>)))
+            AddressMap<AllocatedObject>(AllocatedTypeMalloc, AllocatedTypeFree);
+  }
+}
+
+extern "C" void InsertAllocatedType(
+    void* address, unsigned int size, const std::type_info& type) {

[jar (doing other things), 2012/08/02 23:38:30]

nit: align parameters, one per line, indented to match the paren.

[Dai Mikurube (NOT FULLTIME), 2012/08/03 10:01:50]

Done.

+  if (allocated_type_lock.IsHeld()) {

[jar (doing other things), 2012/08/02 23:38:30]

I suspect you're trying to use this as recursion protection... but this function
is only guaranteed correct when the lock is held by *this* thread.  It can be
wrong when the lock is held by another thread (indicating true or false, with no
correlation to reality(?).

Do you, for instance, really mean to exit if some other thread is holding the
lock?  Do you mind that you can be told "false" in this call, and then spin
because the lock *is* held by the time you reach the next line?

I'm not clear on the logic here.


The same logic exists in all the functions below... and I have the same
questions.

[Dai Mikurube (NOT FULLTIME), 2012/08/03 10:01:50]

They were just-in-case code when I was doing heavy try-and-error.  Removed them
since they're actually not required.

+    return;
+  }
+  SpinLockHolder l(&allocated_type_lock);
+  InitializeAllocatedTypeMemory();
+
+  allocated_type_map->Insert(address, AllocatedObject(size, &type));
+}
+
+extern "C" void EraseAllocatedType(void* address) {
+  if (allocated_type_lock.IsHeld()) {
+    return;
+  }
+  SpinLockHolder l(&allocated_type_lock);
+  InitializeAllocatedTypeMemory();
+
+  AllocatedObject obj;
+  allocated_type_map->FindAndRemove(address, &obj);
+}
+
+extern "C" const std::type_info* LookupAllocatedType(const void* address) {
+  if (allocated_type_lock.IsHeld()) {
+    return NULL;
+  }
+  SpinLockHolder l(&allocated_type_lock);
+  InitializeAllocatedTypeMemory();
+
+  const AllocatedObject* found = allocated_type_map->Find(address);
+  if (found == NULL)
+    return NULL;
+  return found->type;
+}
+
+extern "C" void IterateAllocatedType(
+    void (*callback)(const void*, AllocatedObject*, void*), void* arg) {

[jar (doing other things), 2012/08/02 23:38:30]

This class of API is always frightening to me.  You are going to callback into
arbitrary code, with a lock held.

It is possible for this to work, but it is common to have subtle undetected
error.  

IMO, general locking philosophy suggests: you want to only lock during access to
data, and not lock to prevent other code from "running."  In addition, you want
to hold the lock for the briefest possible period, and be assured that you never
call another function that might block (or wait on a second lock... leading to a
deadlock).

Most commonly, when code holding a lock calls into other code, the function name
even states that a lock is held, so there is less confusion.  With a callback,
you have no such guarantees whatever. :-(.

[Dai Mikurube (NOT FULLTIME), 2012/08/03 10:01:50]

In fact, I was a little worried about that.  The user of this function is
actually only one in another patch, I remove this function for now, and consider
providing another limited function to the user.

+  if (allocated_type_lock.IsHeld()) {
+    return;
+  }
+  SpinLockHolder l(&allocated_type_lock);
+  InitializeAllocatedTypeMemory();
+
+  allocated_type_map->Iterate(callback, arg);
+}
+
+static const TCMallocGuard tcmalloc_initializer;