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

base/allocator/type_profiler_map_unittests.cc

Index: base/allocator/type_profiler_map_unittests.cc
diff --git a/base/allocator/type_profiler_map_unittests.cc b/base/allocator/type_profiler_map_unittests.cc
new file mode 100644
index 0000000000000000000000000000000000000000..f2720c662e54e717f7f904c3659951d19cd60174
--- /dev/null
+++ b/base/allocator/type_profiler_map_unittests.cc
@@ -0,0 +1,77 @@
+// Copyright 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.
+
+// This is a unittest set for TypeProfilerMap in third_party/tcmalloc.  It is
+// independent from other tests and executed manually like allocator_unittests
+// since TypeProfilerMap is a singleton (like tcmalloc's heap-profiler), and
+// it requires RTTI and different compiling/linking options from others.
+
+#include <cstddef>
+
+#include "testing/gtest/include/gtest/gtest.h"
+#include "third_party/tcmalloc/chromium/src/gperftools/type_profiler_map.h"
+
+#if defined(PROFILING_TYPE)

[jar (doing other things), 2012/08/30 16:02:10]

Please change this macro to:
TYPE_PROFILING

As currently written, it confusingly sounds like there is a special "TYPE" which
is the PROFILING_TYPE.  In this context it could be deduced what you are trying
to say, but in other contexts (e.g., what is this macro define used for?) it is
harder to understand.

[Dai Mikurube (NOT FULLTIME), 2012/08/31 06:10:30]

Done.

+static const void* const g_const_null = static_cast<const void*>(NULL);

[jar (doing other things), 2012/08/30 16:02:10]

Is this cast needed?

[Dai Mikurube (NOT FULLTIME), 2012/08/31 06:10:30]

Required.

Using NULL directly, clang++ says:
../../testing/gtest/include/gtest/gtest.h:1532:28: error: comparison between
pointer and integer ('const std::type_info *' and 'long')
GTEST_IMPL_CMP_HELPER_(NE, !=);
~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~

+
+TEST(TypeProfilerMapTest, NormalOperation) {
+  const std::type_info* type;
+
+  // Allocate an object just to get a valid address.
+  // This 'new' is not profiled by type_profiler.
+  int* dummy = new int(48);
+
+  type = TypeProfilerLookupType(dummy);
+  EXPECT_EQ(type, g_const_null);
+
+  TypeProfilerInsertType(dummy, 12, typeid(int));
+  type = TypeProfilerLookupType(dummy);
+  ASSERT_NE(type, g_const_null);
+  EXPECT_STREQ(type->name(), typeid(int).name());
+
+  TypeProfilerEraseType(dummy);
+  type = TypeProfilerLookupType(dummy);
+  EXPECT_EQ(type, g_const_null);
+
+  delete dummy;
+}
+
+TEST(TypeProfilerMapTest, EraseWithoutInsert) {
+  const std::type_info* type;
+
+  int* dummy = new int(48);

[jar (doing other things), 2012/08/30 16:02:10]

use a scoped ref ptr, so you don't have to worry about delete (and early exits).
 Same in line 57.

[Dai Mikurube (NOT FULLTIME), 2012/08/31 06:10:30]

scoped_ptr?  Done.

+
+  for (int i = 0; i < 10; ++i) {
+    TypeProfilerEraseType(dummy);
+    type = TypeProfilerLookupType(dummy);
+    EXPECT_EQ(type, g_const_null);
+  }
+
+  delete dummy;
+}
+
+TEST(TypeProfilerMapTest, InsertThenMultipleErase) {

[jar (doing other things), 2012/08/30 16:02:10]

Please also test multiple inserts of a single address, with varying sizes,
without corresponding deletes.  This is the sort of corruption that will result
from dropping intermediary delete() calls (due to asynchronous toggling of
enables and disables, and setting of intercepts), and is what JamesR and I were
concerned about.

You can probably achieve this result (with genuine allocated data) by making
several consecutive large allocations (over 32K).  TCMalloc will (usually) reuse
the same blocks immediately after they are freed (as spans).  IF your repeat
allocation differ by less than one page, they *should* reuse the same areas of
memory (assuming you do a delete between).

You could do teh same thing with "small" allocations, but you'd need to be
careful to have two classes that were close enough in sizes that they both were
part of the same "TCMalloc class size."  Using large allocations (over 32K
currently in TCMalloc) makes this easy.

[Dai Mikurube (NOT FULLTIME), 2012/08/31 06:10:30]

Done for TypeProfilerMapTest for Insert/EraseType.  A new test
"MultipleInsertWithoutErase" does it.  I think it is enough to test this issue.


It is impossible to reproduce it with the intercepted new/delete in
TypeProfilerTest.  The type profiler's status can change only as follows:
1) Non-initialized (disabled)
2) => Initialized (enabled)
3) => Stopped (disabled)

Once the profiler is stopped, it never gets re-enabled.  The type profiler
provides an initializer and a stopper.

If new <1> is recorded (== called after initialized), and
delete <1> is dropped (== called after stopped),
next new <2> must not be recorded (it cannot be re-enabled).


But, similar behavior has been already happening, and it doesn't fail.  It is
because of placement new.  Our compiler modification catches also placement new.
 But, objects allocated by placement new are sometimes removed by calling the
destructor and "operator delete" directly... not a 'delete' expression.  It can
cause double new for a single address.  (We're giving up such cases.)

I added a new test "MultipleNewWithDroppingDelete" in TypeProfilerTest for this
case.

[jar (doing other things), 2012/09/02 01:37:11]

Good answer: (re: this CL has changed so that now you are no longer turning this
on and off repeatedly).

What does it mean when you say your are "giving up such cases?"

On 2012/08/31 06:10:30, Dai Mikurube wrote:

[Dai Mikurube (NOT FULLTIME), 2012/09/03 08:16:30]

What I'd like to say was: we accept this kind of wrong data when "operator
delete" is called directly unless it causes a significant runtime failure.

+  const std::type_info* type;
+
+  int* dummy = new int(48);
+
+  TypeProfilerInsertType(dummy, 12, typeid(int));
+  type = TypeProfilerLookupType(dummy);
+  ASSERT_NE(type, g_const_null);
+  EXPECT_STREQ(type->name(), typeid(int).name());
+
+  for (int i = 0; i < 10; ++i) {
+    TypeProfilerEraseType(dummy);
+    type = TypeProfilerLookupType(dummy);
+    EXPECT_EQ(type, g_const_null);
+  }
+
+  delete dummy;
+}
+#endif  // defined(PROFILING_TYPE)
+
+int main(int argc, char** argv) {
+  testing::InitGoogleTest(&argc, argv);
+  return RUN_ALL_TESTS();
+}