Revert 48024 - Reland http://codereview.chromium.org/1735024/show to assess the performance.

Review URL: http://codereview.chromium.org/2164001
TBR=antonm,willchan

Review URL: http://codereview.chromium.org/2155002

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@48032 0039d316-1c4b-4281-b951-d872f2087c98

1 files changed, 46 insertions, 23 deletions

diff --git a/third_party/tcmalloc/chromium/src/stacktrace_generic-inl.h b/third_party/tcmalloc/chromium/src/stacktrace_generic-inl.h
index 0e72ee7..490cd9d 100644
--- a/third_party/tcmalloc/chromium/src/stacktrace_generic-inl.h
+++ b/third_party/tcmalloc/chromium/src/stacktrace_generic-inl.h
@@ -34,32 +34,57 @@
 //
 // Note:  The glibc implementation may cause a call to malloc.
 // This can cause a deadlock in HeapProfiler.
-
-#ifndef BASE_STACKTRACE_GENERIC_INL_H_
-#define BASE_STACKTRACE_GENERIC_INL_H_
-// Note: this file is included into stacktrace.cc more than once.
-// Anything that should only be defined once should be here:
-
 #include <execinfo.h>
 #include <string.h>
 #include "google/stacktrace.h"
-#endif  // BASE_STACKTRACE_GENERIC_INL_H_
 
-// Note: this part of the file is included several times.
-// Do not put globals below.
+// If you change this function, also change GetStackFrames below.
+int GetStackTrace(void** result, int max_depth, int skip_count) {
+  static const int kStackLength = 64;
+  void * stack[kStackLength];
+  int size;
+
+  size = backtrace(stack, kStackLength);
+  skip_count++;  // we want to skip the current frame as well
+  int result_count = size - skip_count;
+  if (result_count < 0)
+    result_count = 0;
+  if (result_count > max_depth)
+    result_count = max_depth;
+  for (int i = 0; i < result_count; i++)
+    result[i] = stack[i + skip_count];
+
+  return result_count;
+}
 
-// The following 4 functions are generated from the code below:
-//   GetStack{Trace,Frames}()
-//   GetStack{Trace,Frames}WithContext()
+// If you change this function, also change GetStackTrace above:
+//
+// This GetStackFrames routine shares a lot of code with GetStackTrace
+// above. This code could have been refactored into a common routine,
+// and then both GetStackTrace/GetStackFrames could call that routine.
+// There are two problems with that:
+//
+// (1) The performance of the refactored-code suffers substantially - the
+//     refactored needs to be able to record the stack trace when called
+//     from GetStackTrace, and both the stack trace and stack frame sizes,
+//     when called from GetStackFrames - this introduces enough new
+//     conditionals that GetStackTrace performance can degrade by as much
+//     as 50%.
 //
-// These functions take the following args:
-//   void** result: the stack-trace, as an array
-//   int* sizes: the size of each stack frame, as an array
-//               (GetStackFrames* only)
-//   int max_depth: the size of the result (and sizes) array(s)
-//   int skip_count: how many stack pointers to skip before storing in result
-//   void* ucp: a ucontext_t* (GetStack{Trace,Frames}WithContext only)
-int GET_STACK_TRACE_OR_FRAMES {
+// (2) Whether the refactored routine gets inlined into GetStackTrace and
+//     GetStackFrames depends on the compiler, and we can't guarantee the
+//     behavior either-way, even with "__attribute__ ((always_inline))"
+//     or "__attribute__ ((noinline))". But we need this guarantee or the
+//     frame counts may be off by one.
+//
+// Both (1) and (2) can be addressed without this code duplication, by
+// clever use of template functions, and by defining GetStackTrace and
+// GetStackFrames as macros that expand to these template functions.
+// However, this approach comes with its own set of problems - namely,
+// macros and  preprocessor trouble - for example,  if GetStackTrace
+// and/or GetStackFrames is ever defined as a member functions in some
+// class, we are in trouble.
+int GetStackFrames(void** pcs, int* sizes, int max_depth, int skip_count) {
   static const int kStackLength = 64;
   void * stack[kStackLength];
   int size;
@@ -72,12 +97,10 @@ int GET_STACK_TRACE_OR_FRAMES {
   if (result_count > max_depth)
     result_count = max_depth;
   for (int i = 0; i < result_count; i++)
-    result[i] = stack[i + skip_count];
+    pcs[i] = stack[i + skip_count];
 
-#if IS_STACK_FRAMES
   // No implementation for finding out the stack frame sizes yet.
   memset(sizes, 0, sizeof(*sizes) * result_count);
-#endif
 
   return result_count;
 }