Skip to content

GitLab

T
tangled
Commit 5640bf19 authored by Primiano Tucci's avatar Primiano Tucci Committed by Commit Bot
Browse files
Options

memory-infra: get rid of MemoryDumpManager::RequestGlobalDump method. 

This CL gets rid of the MemoryDumpManager::RequestGlobalDump(),
and moves the existing code to use the service. This allows to
cleanup all the odd callback proxy required to match the base vs
mojo callbacks.
Also, this moves the dump id generation responsibility to the
service, which is now the only one dealing with global dumps.

At this point the only knowledge left in base::MemoryDumpManager
about "global" dumps is only the |request_dump_function|, which
is required to keep the periodic dump scheduler and peak detector
working. This can be removed once those two are moved to the service.

BUG=720352

Change-Id: I25c22157234e1e627c5aeafa189df01944354e7d
Reviewed-on: https://chromium-review.googlesource.com/536952
Commit-Queue: Primiano Tucci <primiano@chromium.org>
Reviewed-by: default avatarsiddhartha sivakumar <ssid@chromium.org>
Reviewed-by: default avatarHector Dearman <hjd@chromium.org>
Cr-Commit-Position: refs/heads/master@{#481678}
parent fed6d120
Hide whitespace changes
Inline Side-by-side
Showing with 115 additions and 218 deletions
base/trace_event/memory_dump_manager.cc
View file @ 5640bf19
......@@ -50,27 +50,8 @@ namespace trace_event {
namespace {
StaticAtomicSequenceNumber g_next_guid;
MemoryDumpManager* g_instance_for_testing = nullptr;
// Callback wrapper to hook upon the completion of RequestGlobalDump() and
// inject trace markers.
void OnGlobalDumpDone(GlobalMemoryDumpCallback wrapped_callback,
uint64_t dump_guid,
bool success) {
char guid_str[20];
sprintf(guid_str, "0x%" PRIx64, dump_guid);
TRACE_EVENT_NESTABLE_ASYNC_END2(MemoryDumpManager::kTraceCategory,
"GlobalMemoryDump", TRACE_ID_LOCAL(dump_guid),
"dump_guid", TRACE_STR_COPY(guid_str),
"success", success);
if (!wrapped_callback.is_null()) {
wrapped_callback.Run(dump_guid, success);
wrapped_callback.Reset();
}
}
void FillOsDumpFromProcessMemoryDump(
const ProcessMemoryDump* pmd,
MemoryDumpCallbackResult::OSMemDump* osDump) {
......@@ -81,14 +62,18 @@ void FillOsDumpFromProcessMemoryDump(
}
}
void OnPeakDetected(MemoryDumpLevelOfDetail level_of_detail) {
MemoryDumpManager::GetInstance()->RequestGlobalDump(
MemoryDumpType::PEAK_MEMORY_USAGE, level_of_detail);
}
void OnPeriodicSchedulerTick(MemoryDumpLevelOfDetail level_of_detail) {
MemoryDumpManager::GetInstance()->RequestGlobalDump(
MemoryDumpType::PERIODIC_INTERVAL, level_of_detail);
// Temporary (until peak detector and scheduler are moved outside of here)
// trampoline function to match the |request_dump_function| passed to Initialize
// to the callback expected by MemoryPeakDetector and MemoryDumpScheduler.
// TODO(primiano): remove this.
void DoGlobalDumpWithoutCallback(
MemoryDumpManager::RequestGlobalDumpFunction global_dump_fn,
MemoryDumpType dump_type,
MemoryDumpLevelOfDetail level_of_detail) {
// The actual dump_guid will be set by service. TODO(primiano): remove
// guid from the request args API.
MemoryDumpRequestArgs args{0 /* dump_guid */, dump_type, level_of_detail};
global_dump_fn.Run(args);
}
} // namespace
......@@ -134,8 +119,6 @@ MemoryDumpManager::MemoryDumpManager()
tracing_process_id_(kInvalidTracingProcessId),
dumper_registrations_ignored_for_testing_(false),
heap_profiling_enabled_(false) {
g_next_guid.GetNext(); // Make sure that first guid is not zero.
// At this point the command line may not be initialized but we try to
// enable the heap profiler to capture allocations as soon as possible.
EnableHeapProfilingIfNeeded();
......@@ -402,35 +385,6 @@ void MemoryDumpManager::UnregisterDumpProviderInternal(
dump_providers_.erase(mdp_iter);
}
void MemoryDumpManager::RequestGlobalDump(
MemoryDumpType dump_type,
MemoryDumpLevelOfDetail level_of_detail,
const GlobalMemoryDumpCallback& callback) {
if (!is_initialized()) {
VLOG(1) << "RequestGlobalDump() FAIL: MemoryDumpManager is not initialized";
if (!callback.is_null())
callback.Run(0u /* guid */, false /* success */);
return;
}
const uint64_t guid =
TraceLog::GetInstance()->MangleEventId(g_next_guid.GetNext());
// Creates an async event to keep track of the global dump evolution.
// The |wrapped_callback| will generate the ASYNC_END event and then invoke
// the real |callback| provided by the caller.
TRACE_EVENT_NESTABLE_ASYNC_BEGIN2(
kTraceCategory, "GlobalMemoryDump", TRACE_ID_LOCAL(guid), "dump_type",
MemoryDumpTypeToString(dump_type), "level_of_detail",
MemoryDumpLevelOfDetailToString(level_of_detail));
GlobalMemoryDumpCallback wrapped_callback = Bind(&OnGlobalDumpDone, callback);
// The embedder will coordinate the IPC broadcast and at some point invoke
// CreateProcessDump() to get a dump for the current process.
MemoryDumpRequestArgs args = {guid, dump_type, level_of_detail};
request_dump_function_.Run(args, wrapped_callback);
}
void MemoryDumpManager::GetDumpProvidersForPolling(
std::vector<scoped_refptr<MemoryDumpProviderInfo>>* providers) {
DCHECK(providers->empty());
......@@ -441,13 +395,6 @@ void MemoryDumpManager::GetDumpProvidersForPolling(
}
}
void MemoryDumpManager::RequestGlobalDump(
MemoryDumpType dump_type,
MemoryDumpLevelOfDetail level_of_detail) {
auto noop_callback = [](uint64_t dump_guid, bool success) {};
RequestGlobalDump(dump_type, level_of_detail, Bind(noop_callback));
}
bool MemoryDumpManager::IsDumpProviderRegisteredForTesting(
MemoryDumpProvider* provider) {
AutoLock lock(lock_);
......@@ -811,7 +758,9 @@ void MemoryDumpManager::SetupForTracing(
for (const auto& trigger : memory_dump_config.triggers) {
if (trigger.trigger_type == MemoryDumpType::PERIODIC_INTERVAL) {
if (periodic_config.triggers.empty()) {
periodic_config.callback = BindRepeating(&OnPeriodicSchedulerTick);
periodic_config.callback =
BindRepeating(&DoGlobalDumpWithoutCallback, request_dump_function_,
MemoryDumpType::PERIODIC_INTERVAL);
}
periodic_config.triggers.push_back(
{trigger.level_of_detail, trigger.min_time_between_dumps_ms});
......@@ -823,7 +772,9 @@ void MemoryDumpManager::SetupForTracing(
BindRepeating(&MemoryDumpManager::GetDumpProvidersForPolling,
Unretained(this)),
GetOrCreateBgTaskRunnerLocked(),
BindRepeating(&OnPeakDetected, trigger.level_of_detail));
BindRepeating(&DoGlobalDumpWithoutCallback, request_dump_function_,
MemoryDumpType::PEAK_MEMORY_USAGE,
trigger.level_of_detail));
MemoryPeakDetector::Config peak_config;
peak_config.polling_interval_ms = 10;
......@@ -836,7 +787,10 @@ void MemoryDumpManager::SetupForTracing(
// gives a good reference point for analyzing the trace.
if (is_coordinator_) {
GetOrCreateBgTaskRunnerLocked()->PostTask(
FROM_HERE, BindRepeating(&OnPeakDetected, trigger.level_of_detail));
FROM_HERE,
BindRepeating(&DoGlobalDumpWithoutCallback, request_dump_function_,
MemoryDumpType::PEAK_MEMORY_USAGE,
trigger.level_of_detail));
}
}
}
......
base/trace_event/memory_dump_manager.h
View file @ 5640bf19
......@@ -50,8 +50,7 @@ enum HeapProfilingMode {
class BASE_EXPORT MemoryDumpManager {
public:
using RequestGlobalDumpFunction =
RepeatingCallback<void(const MemoryDumpRequestArgs& args,
const GlobalMemoryDumpCallback& callback)>;
RepeatingCallback<void(const MemoryDumpRequestArgs& args)>;
static const char* const kTraceCategory;
......@@ -66,12 +65,15 @@ class BASE_EXPORT MemoryDumpManager {
// Initialization can happen after (Un)RegisterMemoryDumpProvider() calls
// and the MemoryDumpManager guarantees to support this.
// On the other side, the MemoryDumpManager will not be fully operational
// (i.e. will NACK any RequestGlobalMemoryDump()) until initialized.
// (any CreateProcessDump() will return a failure) until initialized.
// Arguments:
// request_dump_function: Function to invoke a global dump. Global dump
// involves embedder-specific behaviors like multiprocess handshaking.
// is_coordinator: True when current process coordinates the periodic dump
// triggering.
// request_dump_function: Function to invoke a global dump. Global dump
// involves embedder-specific behaviors like multiprocess handshaking.
// TODO(primiano): this is only required to trigger global dumps from
// the scheduler and the peak detector. Should be removed once they are
// both moved out of base.
void Initialize(RequestGlobalDumpFunction request_dump_function,
bool is_coordinator);
......@@ -112,36 +114,20 @@ class BASE_EXPORT MemoryDumpManager {
void UnregisterAndDeleteDumpProviderSoon(
std::unique_ptr<MemoryDumpProvider> mdp);
// Requests a memory dump. The dump might happen or not depending on the
// filters and categories specified when enabling tracing.
// A SUMMARY_ONLY dump can be requested at any time after initialization and
// other type of dumps can be requested only when MDM is enabled.
// The optional |callback| is executed asynchronously, on an arbitrary thread,
// to notify about the completion of the global dump (i.e. after all the
// processes have dumped) and its success (true iff all the dumps were
// successful).
void RequestGlobalDump(MemoryDumpType,
MemoryDumpLevelOfDetail,
const GlobalMemoryDumpCallback&);
// Same as above (still asynchronous), but without callback.
void RequestGlobalDump(MemoryDumpType, MemoryDumpLevelOfDetail);
// Prepare MemoryDumpManager for RequestGlobalMemoryDump calls for tracing
// related modes (non-SUMMARY_ONLY).
// Initializes the peak detector, scheduler and heap profiler with the given
// config.
// Prepare MemoryDumpManager for CreateProcessDump() calls for tracing-related
// modes (i.e. |level_of_detail| != SUMMARY_ONLY).
// Also initializes the peak detector, scheduler and heap profiler with the
// given config.
void SetupForTracing(const TraceConfig::MemoryDumpConfig&);
// Tear-down tracing related state.
// Non-tracing modes (e.g. SUMMARY_ONLY) will continue to work.
void TeardownForTracing();
// NOTE: Use RequestGlobalDump() to create memory dumps. Creates a memory dump
// for the current process and appends it to the trace. |callback| will be
// invoked asynchronously upon completion on the same thread on which
// CreateProcessDump() was called. This method should only be used by the
// embedder while creating a global memory dump.
// Creates a memory dump for the current process and appends it to the trace.
// |callback| will be invoked asynchronously upon completion on the same
// thread on which CreateProcessDump() was called. This method should only be
// used by the memory-infra service while creating a global memory dump.
void CreateProcessDump(const MemoryDumpRequestArgs& args,
const ProcessMemoryDumpCallback& callback);
......
base/trace_event/memory_dump_manager_test_utils.h
View file @ 5640bf19
......@@ -12,20 +12,9 @@
namespace base {
namespace trace_event {
// Short-circuit the RequestGlobalDump() calls to CreateProcessDump().
// Rationale: only the in-process logic is required for unittests.
void RequestGlobalDumpForInProcessTesting(
const MemoryDumpRequestArgs& args,
const GlobalMemoryDumpCallback& global_callback) {
// Turns a ProcessMemoryDumpCallback into a GlobalMemoryDumpCallback.
auto callback_adapter = [](const GlobalMemoryDumpCallback& global_callback,
uint64_t dump_guid, bool success,
const Optional<MemoryDumpCallbackResult>& result) {
if (!global_callback.is_null())
global_callback.Run(dump_guid, success);
};
void RequestGlobalDumpForInProcessTesting(const MemoryDumpRequestArgs& args) {
MemoryDumpManager::GetInstance()->CreateProcessDump(
args, Bind(callback_adapter, global_callback));
args, ProcessMemoryDumpCallback());
};
// Short circuits the RequestGlobalDumpFunction() to CreateProcessDump(),
......
base/trace_event/memory_dump_request_args.h
View file @ 5640bf19
......@@ -108,7 +108,7 @@ struct BASE_EXPORT MemoryDumpCallbackResult {
};
using GlobalMemoryDumpCallback =
Callback<void(uint64_t dump_guid, bool success)>;
Callback<void(bool success, uint64_t dump_guid)>;
using ProcessMemoryDumpCallback =
Callback<void(uint64_t dump_guid,
......
content/browser/tracing/memory_tracing_browsertest.cc
View file @ 5640bf19
......@@ -21,6 +21,7 @@
#include "content/public/test/content_browser_test.h"
#include "content/public/test/content_browser_test_utils.h"
#include "content/shell/browser/shell.h"
#include "services/resource_coordinator/public/cpp/memory_instrumentation/memory_instrumentation.h"
#include "testing/gmock/include/gmock/gmock.h"
using base::trace_event::MemoryDumpArgs;
......@@ -43,28 +44,20 @@ class MockDumpProvider : public base::trace_event::MemoryDumpProvider {
class MemoryTracingTest : public ContentBrowserTest {
public:
void DoRequestGlobalDump(
const MemoryDumpType& dump_type,
const MemoryDumpLevelOfDetail& level_of_detail,
const base::trace_event::GlobalMemoryDumpCallback& cb) {
MemoryDumpManager::GetInstance()->RequestGlobalDump(dump_type,
level_of_detail, cb);
}
// Used as callback argument for MemoryDumpManager::RequestGlobalDump():
void OnGlobalMemoryDumpDone(
scoped_refptr<base::SingleThreadTaskRunner> task_runner,
base::Closure closure,
uint32_t request_index,
uint64_t dump_guid,
bool success) {
bool success,
uint64_t dump_guid) {
// Make sure we run the RunLoop closure on the same thread that originated
// the run loop (which is the IN_PROC_BROWSER_TEST_F main thread).
if (!task_runner->RunsTasksInCurrentSequence()) {
task_runner->PostTask(
FROM_HERE, base::Bind(&MemoryTracingTest::OnGlobalMemoryDumpDone,
base::Unretained(this), task_runner, closure,
request_index, dump_guid, success));
request_index, success, dump_guid));
return;
}
if (success)
......@@ -85,10 +78,15 @@ class MemoryTracingTest : public ContentBrowserTest {
base::ThreadTaskRunnerHandle::Get(), closure, request_index);
if (from_renderer_thread) {
PostTaskToInProcessRendererAndWait(base::Bind(
&MemoryTracingTest::DoRequestGlobalDump, base::Unretained(this),
&memory_instrumentation::MemoryInstrumentation::
RequestGlobalDumpAndAppendToTrace,
base::Unretained(
memory_instrumentation::MemoryInstrumentation::GetInstance()),
dump_type, level_of_detail, callback));
} else {
DoRequestGlobalDump(dump_type, level_of_detail, callback);
memory_instrumentation::MemoryInstrumentation::GetInstance()
->RequestGlobalDumpAndAppendToTrace(dump_type, level_of_detail,
callback);
}
}
......
services/resource_coordinator/memory_instrumentation/coordinator_impl.cc
View file @ 5640bf19
......@@ -4,6 +4,9 @@
#include "services/resource_coordinator/memory_instrumentation/coordinator_impl.h"
#include <inttypes.h>
#include <stdio.h>
#include <utility>
#include "base/bind.h"
......@@ -14,6 +17,7 @@
#include "base/threading/thread_task_runner_handle.h"
#include "base/trace_event/memory_dump_manager.h"
#include "base/trace_event/memory_dump_request_args.h"
#include "base/trace_event/trace_event.h"
#include "services/resource_coordinator/public/cpp/memory_instrumentation/client_process_impl.h"
#include "services/resource_coordinator/public/interfaces/memory_instrumentation/constants.mojom.h"
#include "services/resource_coordinator/public/interfaces/memory_instrumentation/memory_instrumentation.mojom.h"
......@@ -80,7 +84,7 @@ CoordinatorImpl* CoordinatorImpl::GetInstance() {
}
CoordinatorImpl::CoordinatorImpl(service_manager::Connector* connector)
: failed_memory_dump_count_(0) {
: failed_memory_dump_count_(0), next_dump_id_(0) {
process_map_ = base::MakeUnique<ProcessMap>(connector);
DCHECK(!g_coordinator_impl);
g_coordinator_impl = this;
......@@ -105,11 +109,17 @@ void CoordinatorImpl::BindCoordinatorRequest(
}
void CoordinatorImpl::RequestGlobalMemoryDump(
const base::trace_event::MemoryDumpRequestArgs& args,
const base::trace_event::MemoryDumpRequestArgs& args_in,
const RequestGlobalMemoryDumpCallback& callback) {
DCHECK_CALLED_ON_VALID_THREAD(thread_checker_);
bool another_dump_already_in_progress = !queued_memory_dump_requests_.empty();
// TODO(primiano): remove dump_guid from the request. For the moment callers
// should just pass a zero |dump_guid| in input. It should be an out-only arg.
DCHECK_EQ(0u, args_in.dump_guid);
base::trace_event::MemoryDumpRequestArgs args = args_in;
args.dump_guid = ++next_dump_id_;
// If this is a periodic or peak memory dump request and there already is
// another request in the queue with the same level of detail, there's no
// point in enqueuing this request.
......@@ -176,9 +186,15 @@ void CoordinatorImpl::PerformNextQueuedGlobalMemoryDump() {
const base::trace_event::MemoryDumpRequestArgs& args =
queued_memory_dump_requests_.front().args;
// No need to treat the service process different than other processes. The
// service process will register itself as a ClientProcess and
// will be treated like other client processes.
TRACE_EVENT_NESTABLE_ASYNC_BEGIN2(
base::trace_event::MemoryDumpManager::kTraceCategory, "GlobalMemoryDump",
TRACE_ID_LOCAL(args.dump_guid), "dump_type",
base::trace_event::MemoryDumpTypeToString(args.dump_type),
"level_of_detail",
base::trace_event::MemoryDumpLevelOfDetailToString(args.level_of_detail));
// Note: the service process itself is registered as a ClientProcess and
// will be treated like any other process for the sake of memory dumps.
pending_clients_for_current_dump_.clear();
failed_memory_dump_count_ = 0;
for (const auto& kv : clients_) {
......@@ -250,7 +266,7 @@ void CoordinatorImpl::FinalizeGlobalMemoryDumpIfAllManagersReplied() {
return;
DCHECK(!queued_memory_dump_requests_.empty());
QueuedMemoryDumpRequest& request = queued_memory_dump_requests_.front();
QueuedMemoryDumpRequest* request = &queued_memory_dump_requests_.front();
// Reconstruct a map of pid -> ProcessMemoryDump by reassembling the responses
// received by the clients for this dump. In some cases the response coming
......@@ -260,7 +276,7 @@ void CoordinatorImpl::FinalizeGlobalMemoryDumpIfAllManagersReplied() {
// opening /proc pseudo files.
std::map<base::ProcessId, OSMemDump> os_dumps;
for (auto& result : request.process_memory_dumps) {
for (auto& result : request->process_memory_dumps) {
const base::ProcessId pid = result.first;
const OSMemDump dump = result.second->os_dump;
......@@ -279,7 +295,7 @@ void CoordinatorImpl::FinalizeGlobalMemoryDumpIfAllManagersReplied() {
}
std::map<base::ProcessId, mojom::ProcessMemoryDumpPtr> finalized_pmds;
for (auto& result : request.process_memory_dumps) {
for (auto& result : request->process_memory_dumps) {
const base::ProcessId pid = result.first;
mojom::ProcessMemoryDumpPtr& pmd = finalized_pmds[pid];
if (!pmd)
......@@ -303,10 +319,19 @@ void CoordinatorImpl::FinalizeGlobalMemoryDumpIfAllManagersReplied() {
global_dump->process_dumps.push_back(std::move(pmd));
}
const auto& callback = request.callback;
const auto& callback = request->callback;
const bool global_success = failed_memory_dump_count_ == 0;
callback.Run(request.args.dump_guid, global_success, std::move(global_dump));
callback.Run(request->args.dump_guid, global_success, std::move(global_dump));
char guid_str[20];
sprintf(guid_str, "0x%" PRIx64, request->args.dump_guid);
TRACE_EVENT_NESTABLE_ASYNC_END2(
base::trace_event::MemoryDumpManager::kTraceCategory, "GlobalMemoryDump",
TRACE_ID_LOCAL(request->args.dump_guid), "dump_guid",
TRACE_STR_COPY(guid_str), "success", global_success);
queued_memory_dump_requests_.pop_front();
request = nullptr;
// Schedule the next queued dump (if applicable).
if (!queued_memory_dump_requests_.empty()) {
......
services/resource_coordinator/memory_instrumentation/coordinator_impl.h
View file @ 5640bf19
......@@ -108,6 +108,8 @@ class CoordinatorImpl : public Coordinator, public mojom::Coordinator {
// Maintains a map of service_manager::Identity -> pid for registered clients.
std::unique_ptr<ProcessMap> process_map_;
uint64_t next_dump_id_;
THREAD_CHECKER(thread_checker_);
DISALLOW_COPY_AND_ASSIGN(CoordinatorImpl);
};
......
services/resource_coordinator/memory_instrumentation/coordinator_impl_unittest.cc
View file @ 5640bf19
......@@ -21,6 +21,7 @@ ACTION_P(RunClosure, closure) {
using ::testing::_;
using ::testing::Invoke;
using ::testing::Ne;
using ::testing::NotNull;
using ::testing::NiceMock;
......@@ -113,11 +114,11 @@ class MockGlobalMemoryDumpCallback {
// Tests that the global dump is acked even in absence of clients.
TEST_F(CoordinatorImplTest, NoClients) {
base::trace_event::MemoryDumpRequestArgs args = {
1234, base::trace_event::MemoryDumpType::EXPLICITLY_TRIGGERED,
0, base::trace_event::MemoryDumpType::EXPLICITLY_TRIGGERED,
base::trace_event::MemoryDumpLevelOfDetail::DETAILED};
MockGlobalMemoryDumpCallback callback;
EXPECT_CALL(callback, OnCall(1234U, true, NotNull()));
EXPECT_CALL(callback, OnCall(Ne(0u), true, NotNull()));
RequestGlobalMemoryDump(args, callback.Get());
}
......@@ -132,11 +133,11 @@ TEST_F(CoordinatorImplTest, SeveralClients) {
EXPECT_CALL(client_process_2, RequestProcessMemoryDump(_, _)).Times(1);
base::trace_event::MemoryDumpRequestArgs args = {
2345, base::trace_event::MemoryDumpType::EXPLICITLY_TRIGGERED,
0, base::trace_event::MemoryDumpType::EXPLICITLY_TRIGGERED,
base::trace_event::MemoryDumpLevelOfDetail::DETAILED};
MockGlobalMemoryDumpCallback callback;
EXPECT_CALL(callback, OnCall(2345U, true, NotNull()))
EXPECT_CALL(callback, OnCall(Ne(0u), true, NotNull()))
.WillOnce(RunClosure(run_loop.QuitClosure()));
RequestGlobalMemoryDump(args, callback.Get());
run_loop.Run();
......@@ -148,7 +149,7 @@ TEST_F(CoordinatorImplTest, ClientCrashDuringGlobalDump) {
base::RunLoop run_loop;
base::trace_event::MemoryDumpRequestArgs args = {
3456, base::trace_event::MemoryDumpType::EXPLICITLY_TRIGGERED,
0, base::trace_event::MemoryDumpType::EXPLICITLY_TRIGGERED,
base::trace_event::MemoryDumpLevelOfDetail::DETAILED};
auto client_process_1 = base::MakeUnique<NiceMock<MockClientProcess>>(this);
......@@ -180,7 +181,7 @@ TEST_F(CoordinatorImplTest, ClientCrashDuringGlobalDump) {
}));
MockGlobalMemoryDumpCallback callback;
EXPECT_CALL(callback, OnCall(3456U, false, NotNull()))
EXPECT_CALL(callback, OnCall(Ne(0u), false, NotNull()))
.WillOnce(RunClosure(run_loop.QuitClosure()));
RequestGlobalMemoryDump(args, callback.Get());
run_loop.Run();
......
services/resource_coordinator/public/cpp/memory_instrumentation/client_process_impl.cc
View file @ 5640bf19
......@@ -46,8 +46,9 @@ ClientProcessImpl::ClientProcessImpl(const Config& config)
bool is_coordinator_process =
config.process_type == mojom::ProcessType::BROWSER;
base::trace_event::MemoryDumpManager::GetInstance()->Initialize(
base::BindRepeating(&ClientProcessImpl::RequestGlobalMemoryDump,
base::Unretained(this)),
base::BindRepeating(
&ClientProcessImpl::RequestGlobalMemoryDump_NoCallback,
base::Unretained(this)),
is_coordinator_process);
}
......@@ -83,62 +84,17 @@ void ClientProcessImpl::OnProcessMemoryDumpDone(
callback.Run(dump_guid, success, std::move(process_memory_dump));
}
void ClientProcessImpl::RequestGlobalMemoryDump(
const base::trace_event::MemoryDumpRequestArgs& args,
const base::trace_event::GlobalMemoryDumpCallback& callback) {
// Note: This condition is here to match the old behavior. If the delegate is
// in the browser process, we do not drop parallel requests in the delegate
// and so they will be queued by the Coordinator service (see
// CoordinatorImpl::RequestGlobalMemoryDump). If the delegate is in a child
// process, parallel requests will be cancelled.
//
// TODO(primiano): Remove all this boilerplate. There should be no need of
// any lock, proxy, callback adaption or early out. The service is able to
// deal with queueing.
if (task_runner_) {
auto callback_proxy =
base::Bind(&ClientProcessImpl::MemoryDumpCallbackProxy,
base::Unretained(this), callback);
void ClientProcessImpl::RequestGlobalMemoryDump_NoCallback(
const base::trace_event::MemoryDumpRequestArgs& args) {
if (!task_runner_->RunsTasksInCurrentSequence()) {
task_runner_->PostTask(
FROM_HERE,
base::Bind(&mojom::Coordinator::RequestGlobalMemoryDump,
base::Unretained(coordinator_.get()), args, callback_proxy));
return;
}
bool early_out_because_of_another_dump_pending = false;
{
base::AutoLock lock(pending_memory_dump_guid_lock_);
if (pending_memory_dump_guid_)
early_out_because_of_another_dump_pending = true;
else
pending_memory_dump_guid_ = args.dump_guid;
}
if (early_out_because_of_another_dump_pending) {
callback.Run(args.dump_guid, false);
base::Bind(&ClientProcessImpl::RequestGlobalMemoryDump_NoCallback,
base::Unretained(this), args));
return;
}
auto callback_proxy = base::Bind(&ClientProcessImpl::MemoryDumpCallbackProxy,
base::Unretained(this), callback);
coordinator_->RequestGlobalMemoryDump(args, callback_proxy);
}
void ClientProcessImpl::MemoryDumpCallbackProxy(
const base::trace_event::GlobalMemoryDumpCallback& callback,
uint64_t dump_guid,
bool success,
mojom::GlobalMemoryDumpPtr) {
{
base::AutoLock lock(pending_memory_dump_guid_lock_);
pending_memory_dump_guid_ = 0;
}
// The GlobalMemoryDumpPtr argument is ignored. The actual data of the dump
// is exposed only through the service and is not passed back to base.
// TODO(primiano): All these roundtrips are transitional until we move all
// the clients of memory-infra to use directly the service. crbug.com/720352 .
callback.Run(dump_guid, success);
coordinator_->RequestGlobalMemoryDump(
args, mojom::Coordinator::RequestGlobalMemoryDumpCallback());
}
ClientProcessImpl::Config::Config(service_manager::Connector* connector,
......
services/resource_coordinator/public/cpp/memory_instrumentation/client_process_impl.h
View file @ 5640bf19
......@@ -43,12 +43,6 @@ class SERVICES_RESOURCE_COORDINATOR_PUBLIC_CPP_EXPORT ClientProcessImpl
static void CreateInstance(const Config& config);
// Implements base::trace_event::MemoryDumpManager::RequestGlobalDumpCallback.
// NOTE: Use MemoryDumpManager::RequestGlobalDump() to request gobal dump.
void RequestGlobalMemoryDump(
const base::trace_event::MemoryDumpRequestArgs& args,
const base::trace_event::GlobalMemoryDumpCallback& callback);
private:
friend std::default_delete<ClientProcessImpl>; // For testing
friend class ClientProcessImplTest;
......@@ -56,6 +50,12 @@ class SERVICES_RESOURCE_COORDINATOR_PUBLIC_CPP_EXPORT ClientProcessImpl
ClientProcessImpl(const Config& config);
~ClientProcessImpl() override;
// Implements base::trace_event::MemoryDumpManager::RequestGlobalDumpCallback.
// This function will be called by the MemoryDumpScheduler::OnTick and
// MemoryPeakDetector.
void RequestGlobalMemoryDump_NoCallback(
const base::trace_event::MemoryDumpRequestArgs&);
// mojom::ClientProcess implementation. The Coordinator calls this.
void RequestProcessMemoryDump(
const base::trace_event::MemoryDumpRequestArgs& args,
......@@ -68,20 +68,10 @@ class SERVICES_RESOURCE_COORDINATOR_PUBLIC_CPP_EXPORT ClientProcessImpl
bool success,
const base::Optional<base::trace_event::MemoryDumpCallbackResult>&);
// A proxy callback for updating |pending_memory_dump_guid_|.
void MemoryDumpCallbackProxy(
const base::trace_event::GlobalMemoryDumpCallback& callback,
uint64_t dump_guid,
bool success,
mojom::GlobalMemoryDumpPtr global_memory_dump);
mojom::CoordinatorPtr coordinator_;
mojo::Binding<mojom::ClientProcess> binding_;
const mojom::ProcessType process_type_;
scoped_refptr<base::SingleThreadTaskRunner> task_runner_;
uint64_t pending_memory_dump_guid_;
base::Lock pending_memory_dump_guid_lock_;
DISALLOW_COPY_AND_ASSIGN(ClientProcessImpl);
};
......
services/resource_coordinator/public/cpp/memory_instrumentation/memory_instrumentation.cc
View file @ 5640bf19
......@@ -36,8 +36,7 @@ MemoryInstrumentation::MemoryInstrumentation(
: connector_(connector),
connector_task_runner_(base::ThreadTaskRunnerHandle::Get()),
tls_coordinator_(&DestroyCoordinatorTLS),
service_name_(service_name),
dump_id_() {
service_name_(service_name) {
DCHECK(connector_task_runner_);
}
......@@ -56,8 +55,7 @@ void MemoryInstrumentation::RequestGlobalDumpAndAppendToTrace(
if (callback)
callback.Run(success, dump_id);
};
// TODO(primiano): get rid of dump_id. It should be a return-only argument.
base::trace_event::MemoryDumpRequestArgs args = {++dump_id_, dump_type,
base::trace_event::MemoryDumpRequestArgs args = {0, dump_type,
level_of_detail};
coordinator->RequestGlobalMemoryDump(args,
base::Bind(callback_adapter, callback));
......@@ -73,9 +71,8 @@ MemoryInstrumentation::GetCoordinatorBindingForCurrentThread() {
mojom::CoordinatorRequest coordinator_req = mojo::MakeRequest(coordinator);
// The connector is not thread safe and BindInterface must be called on its
// own thread. Thankfully, the binding can happen *after* having started
// invoking methos on the |coordinator| proxy objects, as requests will be
// internally queued by Mojo and flushed when the BindInterface() happens.
// own thread. Thankfully, the binding can happen _after_ having started
// invoking methods on the |coordinator| proxy objects.
connector_task_runner_->PostTask(
FROM_HERE,
base::Bind(
......
services/resource_coordinator/public/cpp/memory_instrumentation/memory_instrumentation.h
View file @ 5640bf19
......@@ -66,7 +66,6 @@ class SERVICES_RESOURCE_COORDINATOR_PUBLIC_CPP_EXPORT MemoryInstrumentation {
scoped_refptr<base::SingleThreadTaskRunner> connector_task_runner_;
base::ThreadLocalStorage::Slot tls_coordinator_;
const std::string service_name_;
uint64_t dump_id_ = 0;
DISALLOW_COPY_AND_ASSIGN(MemoryInstrumentation);
};
......
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