Commit a2474b64 authored by agrieve's avatar agrieve Committed by Commit Bot

Revert "Enable extracting unwind table on official builds without channel"

This reverts commit eb07cea1.

Reason for revert: Windows tryjobs are failing.
https://logs.chromium.org/v/?s=chromium%2Fbb%2Ftryserver.chromium.win%2Fwin-msvc-rel%2F81420%2F%2B%2Frecipes%2Fsteps%2Fanalyze%2F0%2Fstdout for an example.

ERROR at //base/BUILD.gn:1802:33: Undefined identifier in string expansion.
    "CAN_UNWIND_WITH_CFI_TABLE=$can_unwind_with_cfi_table",
                                ^------------------------
"can_unwind_with_cfi_table" is not currently in scope.
See //BUILD.gn:65:5: which caused the file to be included.
    "//base:base_perftests",
    ^----------------------
GN gen failed: 1

Original change's description:
> Enable extracting unwind table on official builds without channel
> 
> This CL enables extraction of unwind tables for chrome when
> is_official_build is set to true and when android_channel is not set to
> a release channel. This affects build time only on official builds.
> Also, Set legacy heap profiler to use unwinder for profiling.
> 
> TBR=dpranke@chromium.org
> BUG=819888
> 
> Change-Id: If9732b5ac1f9dff49c58e88e2ffb6cb387715244
> Reviewed-on: https://chromium-review.googlesource.com/976944
> Reviewed-by: Siddhartha S <ssid@chromium.org>
> Reviewed-by: agrieve <agrieve@chromium.org>
> Commit-Queue: Siddhartha S <ssid@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#546425}

TBR=dpranke@chromium.org,ssid@chromium.org,agrieve@chromium.org

Change-Id: Id033094871dfcf74248f631390f15f379d9cd3c7
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 819888
Reviewed-on: https://chromium-review.googlesource.com/983893Reviewed-by: default avataragrieve <agrieve@chromium.org>
Commit-Queue: agrieve <agrieve@chromium.org>
Cr-Commit-Position: refs/heads/master@{#546478}
parent 5772174d
......@@ -1799,7 +1799,6 @@ buildflag_header("debugging_buildflags") {
"ENABLE_PROFILING=$enable_profiling",
"CAN_UNWIND_WITH_FRAME_POINTERS=$can_unwind_with_frame_pointers",
"UNSAFE_DEVELOPER_BUILD=$is_unsafe_developer_build",
"CAN_UNWIND_WITH_CFI_TABLE=$can_unwind_with_cfi_table",
]
}
......
......@@ -15,11 +15,6 @@
#include "base/threading/platform_thread.h"
#include "base/threading/thread_local_storage.h"
#include "base/trace_event/heap_profiler_allocation_context.h"
#include "build/build_config.h"
#if defined(OS_ANDROID) && BUILDFLAG(CAN_UNWIND_WITH_CFI_TABLE)
#include "base/trace_event/cfi_backtrace_android.h"
#endif
#if defined(OS_LINUX) || defined(OS_ANDROID)
#include <sys/prctl.h>
......@@ -219,26 +214,20 @@ bool AllocationContextTracker::GetContextSnapshot(AllocationContext* ctx) {
// kMaxFrameCount + 1 frames, so that we know if there are more frames
// than our backtrace capacity.
#if !defined(OS_NACL) // We don't build base/debug/stack_trace.cc for NaCl.
#if defined(OS_ANDROID) && BUILDFLAG(CAN_UNWIND_WITH_CFI_TABLE)
const void* frames[Backtrace::kMaxFrameCount + 1];
static_assert(arraysize(frames) >= Backtrace::kMaxFrameCount,
"not requesting enough frames to fill Backtrace");
size_t frame_count = CFIBacktraceAndroid::GetInstance()->Unwind(
frames, arraysize(frames));
#elif BUILDFLAG(CAN_UNWIND_WITH_FRAME_POINTERS)
#if BUILDFLAG(CAN_UNWIND_WITH_FRAME_POINTERS)
const void* frames[Backtrace::kMaxFrameCount + 1];
static_assert(arraysize(frames) >= Backtrace::kMaxFrameCount,
"not requesting enough frames to fill Backtrace");
size_t frame_count = debug::TraceStackFramePointers(
frames, arraysize(frames),
1 /* exclude this function from the trace */);
#else
#else // BUILDFLAG(CAN_UNWIND_WITH_FRAME_POINTERS)
// Fall-back to capturing the stack with base::debug::StackTrace,
// which is likely slower, but more reliable.
base::debug::StackTrace stack_trace(Backtrace::kMaxFrameCount + 1);
size_t frame_count = 0u;
const void* const* frames = stack_trace.Addresses(&frame_count);
#endif
#endif // BUILDFLAG(CAN_UNWIND_WITH_FRAME_POINTERS)
// If there are too many frames, keep the ones furthest from main().
size_t backtrace_capacity = backtrace_end - backtrace;
......
......@@ -41,13 +41,8 @@
#if defined(OS_ANDROID)
#include "base/trace_event/java_heap_dump_provider_android.h"
#if BUILDFLAG(CAN_UNWIND_WITH_CFI_TABLE)
#include "base/trace_event/cfi_backtrace_android.h"
#endif
#endif // defined(OS_ANDROID)
namespace base {
namespace trace_event {
......@@ -278,15 +273,8 @@ bool MemoryDumpManager::EnableHeapProfiling(HeapProfilingMode profiling_mode) {
break;
case kHeapProfilingModeNative:
#if defined(OS_ANDROID) && BUILDFLAG(CAN_UNWIND_WITH_CFI_TABLE)
{
bool can_unwind =
CFIBacktraceAndroid::GetInstance()->can_unwind_stack_frames();
DCHECK(can_unwind);
}
#endif
// If we don't have frame pointers and unwind tables then native tracing
// falls-back to using base::debug::StackTrace, which may be slow.
// 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);
break;
......
......@@ -7,20 +7,14 @@ import("//build/config/android/rules.gni")
template("unwind_table_asset") {
_unwind_action = "${target_name}__extract"
_asset_path = "${target_gen_dir}/${target_name}/unwind_cfi"
action(_unwind_action) {
if (defined(invoker.testonly)) {
testonly = invoker.testonly
}
script = "//build/android/gyp/extract_unwind_tables.py"
outputs = [
_asset_path,
]
inputs = [
"$root_out_dir/lib.unstripped/$shlib_prefix${invoker.library_target}$shlib_extension",
]
args = [
"--input_path",
rebase_path(
......@@ -31,11 +25,11 @@ template("unwind_table_asset") {
"--dump_syms_path",
rebase_path("$root_out_dir/dump_syms", root_build_dir),
]
deps = invoker.deps
deps += [ "//third_party/breakpad:dump_syms" ]
deps = [
":${invoker.library_target}",
"//third_party/breakpad:dump_syms",
]
}
android_assets(target_name) {
if (defined(invoker.testonly)) {
testonly = invoker.testonly
......
......@@ -21,7 +21,6 @@ import("//tools/v8_context_snapshot/v8_context_snapshot.gni")
import("channel.gni")
import("java_sources.gni")
import("static_initializers.gni")
import("//build/config/android/extract_unwind_tables.gni")
manifest_package = "org.chromium.chrome"
......@@ -731,22 +730,6 @@ java_group("chrome_public_non_pak_assets") {
}
}
# Enable stack unwinding only for local official builds. Enabling on all local
# builds would increase build time for developers. Choosing a release channel
# should be done based on the size of the unwind file and performance of
# unwinding.
_add_unwind_tables_in_apk = can_unwind_with_cfi_table && is_android &&
is_official_build && android_channel == "default"
if (_add_unwind_tables_in_apk) {
unwind_table_asset("chrome_public_unwind_assets") {
library_target = "chrome"
deps = [
":libchrome",
]
}
}
android_assets("chrome_public_pak_assets") {
sources = [
"$root_out_dir/chrome_100_percent.pak",
......@@ -758,9 +741,6 @@ android_assets("chrome_public_pak_assets") {
":chrome_public_locale_pak_assets",
"//chrome:packed_resources",
]
if (_add_unwind_tables_in_apk) {
deps += [ ":chrome_public_unwind_assets" ]
}
}
# This target is separate from chrome_public_pak_assets because it does not
......@@ -994,15 +974,6 @@ if (current_toolchain == default_toolchain) {
]
}
if (_add_unwind_tables_in_apk) {
unwind_table_asset("monochrome_unwind_assets") {
library_target = "monochrome"
deps = [
":monochrome",
]
}
}
# This target explicitly includes locale paks via deps.
android_assets("monochrome_pak_assets") {
sources = [
......@@ -1016,9 +987,6 @@ if (current_toolchain == default_toolchain) {
":monochrome_paks",
"//android_webview:locale_pak_assets",
]
if (_add_unwind_tables_in_apk) {
deps += [ ":monochrome_unwind_assets" ]
}
}
} # current_toolchain == host_toolchain
......
......@@ -110,9 +110,6 @@ template("test") {
unwind_table_asset(_unwind_table_asset_name) {
testonly = true
library_target = _library_target
deps = [
":$_library_target",
]
}
}
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment