Skip to content

Commit

Permalink
[Sampling profiler] Provide unwinder in stack_unwinder DFM
Browse files Browse the repository at this point in the history
Implements the Android native/Java stack unwinder within the stack_unwinder
dynamic feature module.

Adds visibility/assert_no_deps constraints to //base/BUILD.gn to avoid
leaking module implementation types into general Chrome code, which
would defeat the purpose of using the module to avoid Chrome binary size
increase.

Adds required stub declarations/implementation for MemoryRegionsMap.

Bug: 1083530, 1004855
Change-Id: If9d31c4e1b9d214bb0baabe3afa8475ba7adef4a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2209297
Commit-Queue: kylechar <kylechar@chromium.org>
Reviewed-by: kylechar <kylechar@chromium.org>
Reviewed-by: Etienne Pierre-Doray <etiennep@chromium.org>
Auto-Submit: Mike Wittman <wittman@chromium.org>
Cr-Commit-Position: refs/heads/master@{#771018}
  • Loading branch information
Mike Wittman authored and Commit Bot committed May 21, 2020
1 parent a392f87 commit 01b9624
Show file tree
Hide file tree
Showing 7 changed files with 63 additions and 7 deletions.
14 changes: 13 additions & 1 deletion base/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -1311,6 +1311,10 @@ jumbo_component("base") {
"//third_party/modp_b64",
]

# native_unwinder_android is intended for use solely via a dynamic feature
# module, to avoid increasing Chrome's executable size.
assert_no_deps = [ ":native_unwinder_android" ]

public_deps = [
":anchor_functions_buildflags",
":base_static",
Expand Down Expand Up @@ -2420,8 +2424,16 @@ if ((is_win && (current_cpu == "x64" || current_cpu == "arm64")) || is_mac ||
}
}

if (is_android && (current_cpu == "arm" || current_cpu == "arm64")) {
if (is_android) {
source_set("native_unwinder_android") {
# This target is intended to be used only within the stack_unwinder dynamic
# feature module, to avoid binary size increase in Chrome due to the
# libunwindstack dependency. The additional :* visibility is needed to allow
# use by base test targets.
visibility = [
":*",
"//chrome/android/modules/stack_unwinder/internal:*",
]
sources = [
"profiler/native_unwinder_android.cc",
"profiler/native_unwinder_android.h",
Expand Down
1 change: 1 addition & 0 deletions chrome/android/features/stack_unwinder/public/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -4,4 +4,5 @@

source_set("memory_regions_map") {
public = [ "memory_regions_map.h" ]
sources = [ "memory_regions_map.cc" ]
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
// Copyright 2020 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 "chrome/android/features/stack_unwinder/public/memory_regions_map.h"

namespace stack_unwinder {

MemoryRegionsMap::MemoryRegionsMap() = default;
MemoryRegionsMap::~MemoryRegionsMap() = default;

} // namespace stack_unwinder
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,11 @@ namespace stack_unwinder {
// It must only be subclassed within the module implementation.
class MemoryRegionsMap {
public:
MemoryRegionsMap();
virtual ~MemoryRegionsMap() = 0;

MemoryRegionsMap(const MemoryRegionsMap&) = delete;
MemoryRegionsMap& operator=(const MemoryRegionsMap&) = delete;
};

} // namespace stack_unwinder
Expand Down
1 change: 1 addition & 0 deletions chrome/android/modules/stack_unwinder/internal/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ component("stack_unwinder") {
":jni_headers",
":jni_registration",
"//base",
"//base:native_unwinder_android",
"//chrome/android/features/stack_unwinder/public:memory_regions_map",
]

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,20 +3,47 @@
// found in the LICENSE file.

#include <memory>
#include <utility>

#include "base/profiler/unwinder.h"
#include "base/profiler/native_unwinder_android.h"
#include "chrome/android/features/stack_unwinder/public/memory_regions_map.h"
#include "chrome/android/modules/stack_unwinder/internal/jni_headers/StackUnwinderModuleContentsImpl_jni.h"

namespace {

class MemoryRegionsMap : public stack_unwinder::MemoryRegionsMap {
public:
MemoryRegionsMap(std::unique_ptr<unwindstack::Maps> maps,
std::unique_ptr<unwindstack::Memory> memory)
: maps_(std::move(maps)), memory_(std::move(memory)) {}

unwindstack::Maps* maps() { return maps_.get(); }
unwindstack::Memory* memory() { return memory_.get(); }

private:
std::unique_ptr<unwindstack::Maps> maps_;
std::unique_ptr<unwindstack::Memory> memory_;
};

} // namespace

std::unique_ptr<stack_unwinder::MemoryRegionsMap> CreateMemoryRegionsMap() {
// TODO(etiennep): Implement.
return nullptr;
return std::make_unique<MemoryRegionsMap>(
base::NativeUnwinderAndroid::CreateMaps(),
base::NativeUnwinderAndroid::CreateProcessMemory());
}

std::unique_ptr<base::Unwinder> CreateNativeUnwinder(
stack_unwinder::MemoryRegionsMap* memory_regions_map) {
// TODO(etiennep): Implement.
return nullptr;
// The user is expected to only pass the subclass generated by
// CreateMemoryRegionsMap().
MemoryRegionsMap* concrete_memory_regions_map =
static_cast<MemoryRegionsMap*>(memory_regions_map);
// TODO(https://crbug.com/1004855): Provide the address of the Chrome module
// to as the |exclude_module_with_base_address| parameter.
return std::make_unique<base::NativeUnwinderAndroid>(
concrete_memory_regions_map->maps(),
concrete_memory_regions_map->memory());
}

static jlong
Expand Down
1 change: 0 additions & 1 deletion chrome/android/modules/stack_unwinder/public/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -23,5 +23,4 @@ source_set("module") {
"//chrome/android/features/stack_unwinder/public:memory_regions_map",
"//chrome/android/modules/stack_unwinder/provider:jni_headers",
]
libs = [ "dl" ]
}

0 comments on commit 01b9624

Please sign in to comment.