Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(580)

Unified Diff: services/resource_coordinator/memory/coordinator/coordinator_impl.cc

Issue 2871223002: memory-infra: add ProcessType and expose data in RequestGlobalDump() (Closed)
Patch Set: review comments Created 3 years, 7 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: services/resource_coordinator/memory/coordinator/coordinator_impl.cc
diff --git a/services/resource_coordinator/memory/coordinator/coordinator_impl.cc b/services/resource_coordinator/memory/coordinator/coordinator_impl.cc
index 55d08e9e086ecda227628abb8c22ef4d27004a82..4cc1840ba72ab45b937cd767787ea13942f9bf9a 100644
--- a/services/resource_coordinator/memory/coordinator/coordinator_impl.cc
+++ b/services/resource_coordinator/memory/coordinator/coordinator_impl.cc
@@ -34,8 +34,11 @@ CoordinatorImpl::CoordinatorImpl(bool initialize_memory_dump_manager)
: failed_memory_dump_count_(0),
initialize_memory_dump_manager_(initialize_memory_dump_manager) {
if (initialize_memory_dump_manager) {
+ // TODO(primiano): the current state where the coordinator also creates a
+ // client (ProcessLocalDumpManagerImpl) is contra-intuitive. BrowserMainLoop
+ // should be doing this.
ProcessLocalDumpManagerImpl::CreateInstance(
- ProcessLocalDumpManagerImpl::Config(this));
+ ProcessLocalDumpManagerImpl::Config(this, mojom::ProcessType::BROWSER));
base::trace_event::MemoryDumpManager::GetInstance()->set_tracing_process_id(
mojom::kServiceTracingProcessId);
}
@@ -81,7 +84,8 @@ void CoordinatorImpl::RequestGlobalMemoryDump(
<< base::trace_event::MemoryDumpLevelOfDetailToString(
args.level_of_detail)
<< ") is already in the queue";
- callback.Run(args.dump_guid, false /* success */);
+ callback.Run(args.dump_guid, false /* success */,
+ nullptr /* global_memory_dump */);
return;
}
}
@@ -121,8 +125,7 @@ void CoordinatorImpl::UnregisterProcessLocalDumpManager(
DCHECK(!queued_memory_dump_requests_.empty());
OnProcessMemoryDumpResponse(
process_manager, queued_memory_dump_requests_.front().args.dump_guid,
- false /* success */,
- base::Optional<base::trace_event::MemoryDumpCallbackResult>());
+ false /* success */, nullptr /* process_memory_dump */);
}
}
@@ -150,15 +153,20 @@ void CoordinatorImpl::OnProcessMemoryDumpResponse(
mojom::ProcessLocalDumpManager* process_manager,
uint64_t dump_guid,
bool success,
- const base::Optional<base::trace_event::MemoryDumpCallbackResult>& result) {
+ mojom::ProcessMemoryDumpPtr process_memory_dump) {
auto it = pending_process_managers_.find(process_manager);
- DCHECK(!queued_memory_dump_requests_.empty());
- if (queued_memory_dump_requests_.front().args.dump_guid != dump_guid ||
+ if (queued_memory_dump_requests_.empty() ||
+ queued_memory_dump_requests_.front().args.dump_guid != dump_guid ||
it == pending_process_managers_.end()) {
VLOG(1) << "Received unexpected memory dump response: " << dump_guid;
return;
}
+ if (process_memory_dump) {
+ queued_memory_dump_requests_.front().process_memory_dumps.push_back(
+ std::move(process_memory_dump));
+ }
+
pending_process_managers_.erase(it);
if (!success) {
@@ -166,6 +174,7 @@ void CoordinatorImpl::OnProcessMemoryDumpResponse(
VLOG(1) << base::trace_event::MemoryDumpManager::kLogPrefix
<< " failed because of NACK from provider";
}
+
FinalizeGlobalMemoryDumpIfAllManagersReplied();
}
@@ -173,13 +182,20 @@ void CoordinatorImpl::FinalizeGlobalMemoryDumpIfAllManagersReplied() {
if (pending_process_managers_.size() > 0)
return;
- DCHECK(!queued_memory_dump_requests_.empty());
- {
- const auto& callback = queued_memory_dump_requests_.front().callback;
- const bool global_success = failed_memory_dump_count_ == 0;
- callback.Run(queued_memory_dump_requests_.front().args.dump_guid,
- global_success);
+ if (queued_memory_dump_requests_.empty()) {
+ NOTREACHED();
+ return;
}
+
+ // TODO(hjd,fmeawad): At this point the |process_memory_dumps| accumulated in
+ // queued_memory_dump_requests_.front() should be normalized (merge
+ // |extra_process_dump|, compute CMM) into a GlobalMemoryDumpPtr and passed
+ // below.
+
+ const auto& callback = queued_memory_dump_requests_.front().callback;
+ const bool global_success = failed_memory_dump_count_ == 0;
+ callback.Run(queued_memory_dump_requests_.front().args.dump_guid,
+ global_success, nullptr /* global_memory_dump */);
queued_memory_dump_requests_.pop_front();
// Schedule the next queued dump (if applicable).

Powered by Google App Engine
This is Rietveld 408576698