Cleaner fall-back stack capture for --enable-heap-profiling=native.

base/trace_event/memory_dump_manager.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 "base/trace_event/memory_dump_manager.h"
 
 #include <algorithm>
 #include <utility>
 
 #include "base/allocator/features.h"
@@ skipping 15 matching lines @@
 #include "base/trace_event/heap_profiler_type_name_deduplicator.h"
 #include "base/trace_event/malloc_dump_provider.h"
 #include "base/trace_event/memory_dump_provider.h"
 #include "base/trace_event/memory_dump_scheduler.h"
 #include "base/trace_event/memory_dump_session_state.h"
 #include "base/trace_event/memory_infra_background_whitelist.h"
 #include "base/trace_event/process_memory_dump.h"
 #include "base/trace_event/trace_event.h"
 #include "base/trace_event/trace_event_argument.h"
 #include "build/build_config.h"
+#include "build/buildflag.h"
 
 #if defined(OS_ANDROID)
 #include "base/trace_event/java_heap_dump_provider_android.h"
 #endif
 
 namespace base {
 namespace trace_event {
 
 namespace {
 
@@ skipping 134 matching lines @@
   if (!CommandLine::InitializedForCurrentProcess() ||
       !CommandLine::ForCurrentProcess()->HasSwitch(
           switches::kEnableHeapProfiling))
     return;
 
   std::string profiling_mode = CommandLine::ForCurrentProcess()
       ->GetSwitchValueASCII(switches::kEnableHeapProfiling);
   if (profiling_mode == "") {
     AllocationContextTracker::SetCaptureMode(
         AllocationContextTracker::CaptureMode::PSEUDO_STACK);
-#if HAVE_TRACE_STACK_FRAME_POINTERS && \
-    (BUILDFLAG(ENABLE_PROFILING) || !defined(NDEBUG))
   } else if (profiling_mode == switches::kEnableHeapProfilingModeNative) {

[Primiano Tucci (use gerrit), 2017/03/22 10:38:35]

should we have an #if defined(COMPONENT_BUILD) here to just die with a "don't
even try, we can't support component builds, too much trouble sorry."?
Not sure what is the state of component build on various platforms. If it works
I am happy. But if it doesn't (or is just too painful to maintain) I am very
happy if we just give a clear signal saying "nope, static build only".

I'd just avoid mixed signals, where we pretend that it works, but then at the
end it doesn't. finding that after some spending some hours of attempts can be
quite frustrating for people.

[DmitrySkiba, 2017/03/23 19:10:24]

Component builds *should* be working on Linux and Android, not sure about other
platforms. I'll double check. 

[Wez, 2017/03/27 03:59:37]

That would equally apply to the pseudo-stack case, above, since its a question
of whether we have working allocator shims, I believe.

Rather than an explicit check for component build here, I'd prefer to test a
macro indicating whether or not we have working allocator shims, and have that
set alongside the shimming code (or in the GN if necessary) appropriately to the
build type. 
Agreed - ideally I'd prefer that we pop up a helpful message as well, rather
than just CHECK()ing to generate a stack trace!

[Wez, 2017/03/27 03:59:37]

Definitely not on Windows, since the shim impl is relies on statically-linked
MSVCRT.

[Primiano Tucci (use gerrit), 2017/03/27 09:42:47]

Ah, I was thinking more to the "can this unwinder cope with multiple .so(s)".
but you make another good point.
+1 I like this. To be clear doesn't  have to happen in this CL, but I'd just
kindly ask that this happens at some point in the near future.
Filed crbug.com/705461 to keep track of this.

-    // We need frame pointers for native tracing to work, and they are
-    // enabled in profiling and debug builds.
+    // If we don't have frame pointers then native tracing falls-back to
+    // using base::debug::StackTrace, which may be slow.
     AllocationContextTracker::SetCaptureMode(
         AllocationContextTracker::CaptureMode::NATIVE_STACK);
-#endif
 #if BUILDFLAG(ENABLE_MEMORY_TASK_PROFILER)
   } else if (profiling_mode == switches::kEnableHeapProfilingTaskProfiler) {
     // Enable heap tracking, which in turn enables capture of heap usage
     // tracking in tracked_objects.cc.
     if (!base::debug::ThreadHeapUsageTracker::IsHeapTrackingEnabled())
       base::debug::ThreadHeapUsageTracker::EnableHeapTracking();
 #endif
   } else {
     CHECK(false) << "Invalid mode '" << profiling_mode << "' for "
                << switches::kEnableHeapProfiling << " flag.";
@@ skipping 753 matching lines @@
   if (iter == process_dumps.end()) {
     std::unique_ptr<ProcessMemoryDump> new_pmd(
         new ProcessMemoryDump(session_state, dump_args));
     iter = process_dumps.insert(std::make_pair(pid, std::move(new_pmd))).first;
   }
   return iter->second.get();
 }
 
 }  // namespace trace_event
 }  // namespace base