[heap profiler] Replace SharedMemoryBuffer with mojo::BigString
BUG=923459
Change-Id: I008e2aa9fea0d5d7d674cb72e141d6d949b463db
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1575734
Reviewed-by: Tom Sepez <tsepez@chromium.org>
Reviewed-by: Primiano Tucci <primiano@chromium.org>
Reviewed-by: Dmitry Gozman <dgozman@chromium.org>
Reviewed-by: Erik Chen <erikchen@chromium.org>
Commit-Queue: Alexei Filippov <alph@chromium.org>
Cr-Commit-Position: refs/heads/master@{#654237}
diff --git a/components/services/heap_profiling/connection_manager.cc b/components/services/heap_profiling/connection_manager.cc
index 57c1b99..9f2cd9d 100644
--- a/components/services/heap_profiling/connection_manager.cc
+++ b/components/services/heap_profiling/connection_manager.cc
@@ -7,7 +7,6 @@
#include "base/bind.h"
#include "base/json/string_escape.h"
#include "base/metrics/histogram_macros.h"
-#include "base/task/post_task.h"
#include "components/services/heap_profiling/json_exporter.h"
#include "components/services/heap_profiling/public/cpp/client.h"
@@ -33,7 +32,7 @@
VmRegions vm_regions;
// Collects the results.
- std::vector<memory_instrumentation::mojom::SharedBufferWithSizePtr> results;
+ std::vector<memory_instrumentation::mojom::HeapProfileResultPtr> results;
private:
friend class base::RefCountedThreadSafe<DumpProcessesForTracingTracking>;
@@ -156,7 +155,7 @@
// Early out if there are no connections.
if (connections_.empty()) {
std::move(callback).Run(
- std::vector<memory_instrumentation::mojom::SharedBufferWithSizePtr>());
+ std::vector<memory_instrumentation::mojom::HeapProfileResultPtr>());
return;
}
@@ -176,26 +175,20 @@
}
}
-void ConnectionManager::HeapProfileRetrieved(
- scoped_refptr<DumpProcessesForTracingTracking> tracking,
- base::ProcessId pid,
- mojom::ProcessType process_type,
- bool strip_path_from_mapped_files,
+bool ConnectionManager::ConvertProfileToExportParams(
+ mojom::HeapProfilePtr profile,
uint32_t sampling_rate,
- mojom::HeapProfilePtr profile) {
- AllocationMap allocations;
+ ExportParams* params) {
+ AllocationMap allocs;
ContextMap context_map;
AddressToStringMap string_map;
- bool success = true;
for (const mojom::HeapProfileSamplePtr& sample : profile->samples) {
int context_id = 0;
if (sample->context_id) {
auto it = profile->strings.find(sample->context_id);
- if (it == profile->strings.end()) {
- success = false;
- break;
- }
+ if (it == profile->strings.end())
+ return false;
const std::string& context = it->second;
// Escape the strings early, to simplify exporting a heap dump.
std::string escaped_context;
@@ -235,7 +228,7 @@
std::vector<Address> stack(sample->stack.begin(), sample->stack.end());
AllocationMetrics& metrics =
- allocations
+ allocs
.emplace(std::piecewise_construct,
std::forward_as_tuple(sample->allocator, std::move(stack),
context_id),
@@ -253,87 +246,46 @@
string_map.emplace(str.first, std::move(quoted_string));
}
- DCHECK(success);
- DoDumpOneProcessForTracing(std::move(tracking), pid, process_type,
- strip_path_from_mapped_files, success,
- std::move(allocations), std::move(context_map),
- std::move(string_map));
+ params->allocs = std::move(allocs);
+ params->context_map = std::move(context_map);
+ params->mapped_strings = std::move(string_map);
+ return true;
}
-void ConnectionManager::DoDumpOneProcessForTracing(
+void ConnectionManager::HeapProfileRetrieved(
scoped_refptr<DumpProcessesForTracingTracking> tracking,
base::ProcessId pid,
mojom::ProcessType process_type,
bool strip_path_from_mapped_files,
- bool success,
- AllocationMap counts,
- ContextMap context,
- AddressToStringMap mapped_strings) {
+ uint32_t sampling_rate,
+ mojom::HeapProfilePtr profile) {
// All code paths through here must issue the callback when waiting_responses
// is 0 or the browser will wait forever for the dump.
DCHECK(tracking->waiting_responses > 0);
- if (!success) {
- tracking->waiting_responses--;
- if (tracking->waiting_responses == 0)
- std::move(tracking->callback).Run(std::move(tracking->results));
- return;
+ ExportParams params;
+ bool success =
+ ConvertProfileToExportParams(std::move(profile), sampling_rate, ¶ms);
+ if (success) {
+ params.process_type = process_type;
+ params.strip_path_from_mapped_files = strip_path_from_mapped_files;
+ params.next_id = next_id_;
+
+ auto it = tracking->vm_regions.find(pid);
+ if (it != tracking->vm_regions.end())
+ params.maps = std::move(it->second);
+
+ memory_instrumentation::mojom::HeapProfileResultPtr result =
+ memory_instrumentation::mojom::HeapProfileResult::New();
+ result->pid = pid;
+ result->json = ExportMemoryMapsAndV2StackTraceToJSON(¶ms);
+ tracking->results.push_back(std::move(result));
+ next_id_ = params.next_id;
}
- ExportParams params;
- params.allocs = std::move(counts);
- params.context_map = std::move(context);
- params.mapped_strings = std::move(mapped_strings);
- params.process_type = process_type;
- params.strip_path_from_mapped_files = strip_path_from_mapped_files;
- params.next_id = next_id_;
-
- auto it = tracking->vm_regions.find(pid);
- if (it != tracking->vm_regions.end())
- params.maps = std::move(it->second);
-
- std::string reply = ExportMemoryMapsAndV2StackTraceToJSON(¶ms);
- size_t reply_size = reply.size();
- next_id_ = params.next_id;
-
- base::PostTaskWithTraitsAndReplyWithResult(
- FROM_HERE,
- {base::TaskPriority::BEST_EFFORT, base::MayBlock(),
- base::TaskShutdownBehavior::SKIP_ON_SHUTDOWN},
- base::BindOnce(
- [](size_t size) {
- // This call sends a synchronous IPC to the browser process.
- return mojo::SharedBufferHandle::Create(size);
- },
- reply_size),
- base::BindOnce(
- [](std::string reply, base::ProcessId pid,
- scoped_refptr<DumpProcessesForTracingTracking> tracking,
- mojo::ScopedSharedBufferHandle buffer) {
- if (!buffer.is_valid()) {
- DLOG(ERROR) << "Could not create Mojo shared buffer";
- } else {
- mojo::ScopedSharedBufferMapping mapping =
- buffer->Map(reply.size());
- if (!mapping) {
- DLOG(ERROR) << "Could not map Mojo shared buffer";
- } else {
- memcpy(mapping.get(), reply.c_str(), reply.size());
-
- memory_instrumentation::mojom::SharedBufferWithSizePtr result =
- memory_instrumentation::mojom::SharedBufferWithSize::New();
- result->buffer = std::move(buffer);
- result->size = reply.size();
- result->pid = pid;
- tracking->results.push_back(std::move(result));
- }
- }
-
- // When all responses complete, issue done callback.
- if (--tracking->waiting_responses == 0)
- std::move(tracking->callback).Run(std::move(tracking->results));
- },
- std::move(reply), pid, std::move(tracking)));
+ // When all responses complete, issue done callback.
+ if (--tracking->waiting_responses == 0)
+ std::move(tracking->callback).Run(std::move(tracking->results));
}
} // namespace heap_profiling
diff --git a/components/services/heap_profiling/connection_manager.h b/components/services/heap_profiling/connection_manager.h
index 01d65b47..cb576e9 100644
--- a/components/services/heap_profiling/connection_manager.h
+++ b/components/services/heap_profiling/connection_manager.h
@@ -24,6 +24,8 @@
namespace heap_profiling {
+struct ExportParams;
+
using VmRegions =
base::flat_map<base::ProcessId,
std::vector<memory_instrumentation::mojom::VmRegionPtr>>;
@@ -73,15 +75,9 @@
uint32_t sampling_rate,
mojom::HeapProfilePtr profile);
- void DoDumpOneProcessForTracing(
- scoped_refptr<DumpProcessesForTracingTracking> tracking,
- base::ProcessId pid,
- mojom::ProcessType process_type,
- bool strip_path_from_mapped_files,
- bool success,
- AllocationMap counts,
- ContextMap context,
- AddressToStringMap mapped_strings);
+ bool ConvertProfileToExportParams(mojom::HeapProfilePtr profile,
+ uint32_t sampling_rate,
+ ExportParams* out_params);
// Notification that a connection is complete. Unlike OnNewConnection which
// is signaled by the pipe server, this is signaled by the allocation tracker
diff --git a/services/resource_coordinator/memory_instrumentation/coordinator_impl.cc b/services/resource_coordinator/memory_instrumentation/coordinator_impl.cc
index cf3d934..a158a453 100644
--- a/services/resource_coordinator/memory_instrumentation/coordinator_impl.cc
+++ b/services/resource_coordinator/memory_instrumentation/coordinator_impl.cc
@@ -423,9 +423,8 @@
.IsArgumentFilterEnabled();
heap_profiler_->DumpProcessesForTracing(
strip_path_from_mapped_files,
- base::BindRepeating(&CoordinatorImpl::OnDumpProcessesForTracing,
- weak_ptr_factory_.GetWeakPtr(),
- request->dump_guid));
+ base::BindOnce(&CoordinatorImpl::OnDumpProcessesForTracing,
+ weak_ptr_factory_.GetWeakPtr(), request->dump_guid));
base::SequencedTaskRunnerHandle::Get()->PostDelayedTask(
FROM_HERE,
@@ -538,7 +537,7 @@
void CoordinatorImpl::OnDumpProcessesForTracing(
uint64_t dump_guid,
- std::vector<mojom::SharedBufferWithSizePtr> buffers) {
+ std::vector<mojom::HeapProfileResultPtr> heap_profile_results) {
DCHECK_CALLED_ON_VALID_THREAD(thread_checker_);
QueuedRequest* request = GetCurrentRequest();
if (!request || request->dump_guid != dump_guid) {
@@ -547,23 +546,9 @@
request->heap_dump_in_progress = false;
- for (auto& buffer_ptr : buffers) {
- mojo::ScopedSharedBufferHandle& buffer = buffer_ptr->buffer;
- uint32_t size = buffer_ptr->size;
-
- if (!buffer->is_valid())
- continue;
-
- mojo::ScopedSharedBufferMapping mapping = buffer->Map(size);
- if (!mapping) {
- DLOG(ERROR) << "Failed to map buffer";
- continue;
- }
-
- const char* char_buffer = static_cast<const char*>(mapping.get());
- std::string json(char_buffer, char_buffer + size);
+ for (auto& result : heap_profile_results) {
base::trace_event::TraceArguments args(
- "dumps", std::make_unique<StringWrapper>(std::move(json)));
+ "dumps", std::make_unique<StringWrapper>(std::move(result->json)));
// Using the same id merges all of the heap dumps into a single detailed
// dump node in the UI.
@@ -572,7 +557,7 @@
base::trace_event::TraceLog::GetCategoryGroupEnabled(
base::trace_event::MemoryDumpManager::kTraceCategory),
"periodic_interval", trace_event_internal::kGlobalScope, dump_guid,
- buffer_ptr->pid, &args, TRACE_EVENT_FLAG_HAS_ID);
+ result->pid, &args, TRACE_EVENT_FLAG_HAS_ID);
}
FinalizeGlobalMemoryDumpIfAllManagersReplied();
diff --git a/services/resource_coordinator/memory_instrumentation/coordinator_impl.h b/services/resource_coordinator/memory_instrumentation/coordinator_impl.h
index d56cb08..c3432de 100644
--- a/services/resource_coordinator/memory_instrumentation/coordinator_impl.h
+++ b/services/resource_coordinator/memory_instrumentation/coordinator_impl.h
@@ -139,7 +139,7 @@
// Callback of DumpProcessesForTracing.
void OnDumpProcessesForTracing(
uint64_t dump_guid,
- std::vector<mojom::SharedBufferWithSizePtr> buffers);
+ std::vector<mojom::HeapProfileResultPtr> heap_profile_results);
void RemovePendingResponse(mojom::ClientProcess*,
QueuedRequest::PendingResponse::Type);
diff --git a/services/resource_coordinator/public/mojom/memory_instrumentation/memory_instrumentation.mojom b/services/resource_coordinator/public/mojom/memory_instrumentation/memory_instrumentation.mojom
index d6af2047..aab6920 100644
--- a/services/resource_coordinator/public/mojom/memory_instrumentation/memory_instrumentation.mojom
+++ b/services/resource_coordinator/public/mojom/memory_instrumentation/memory_instrumentation.mojom
@@ -4,6 +4,7 @@
module memory_instrumentation.mojom;
+import "mojo/public/mojom/base/big_string.mojom";
import "mojo/public/mojom/base/process_id.mojom";
import "mojo/public/mojom/base/time.mojom";
@@ -240,10 +241,9 @@
(bool success, map<mojo_base.mojom.ProcessId, RawOSMemDump> dumps);
};
-struct SharedBufferWithSize {
- handle<shared_buffer> buffer;
- uint32 size;
+struct HeapProfileResult {
mojo_base.mojom.ProcessId pid;
+ mojo_base.mojom.BigString json;
};
// HeapProfilers expose a single interface to memory_instrumentation, allowing
@@ -254,17 +254,17 @@
// with the Coordinator (see RegisterHeapProfiler) and is invoked when a memory
// dump is requested (via Coordinator::RequestGlobalMemoryDump).
interface HeapProfiler {
- // Dumps the memory log of all profiled processes into shared buffers. The
- // contents of each shared buffer is a JSON string compatible with
+ // Dumps the memory log of all profiled processes into BigString's.
+ // The contents of each BigString is a JSON string compatible with
// TRACE_EVENT* macros. Processes that fail to dump will be omitted from
- // |buffers|. When |strip_path_from_mapped_files| is true, only the base name
+ // |result|. When |strip_path_from_mapped_files| is true, only the base name
// of mapped files is emitted. This prevents usernames from sneaking into the
// trace.
// |strip_path_from_mapped_files| should only be true for traces that will be
// uploaded to the crash servers - this strips potential PII, but prevents
// symbolization of local builds.
DumpProcessesForTracing(bool strip_path_from_mapped_files) =>
- (array<SharedBufferWithSize> buffers);
+ (array<HeapProfileResult> results);
};
// Implemented by resource_coordinator to provide additional information needed
diff --git a/third_party/blink/renderer/platform/BUILD.gn b/third_party/blink/renderer/platform/BUILD.gn
index 9e48c00..8420081 100644
--- a/third_party/blink/renderer/platform/BUILD.gn
+++ b/third_party/blink/renderer/platform/BUILD.gn
@@ -1232,7 +1232,6 @@
"mhtml/serialized_resource.h",
"mhtml/shared_buffer_chunk_reader.cc",
"mhtml/shared_buffer_chunk_reader.h",
- "mojo/big_string_mojom_traits.cc",
"mojo/big_string_mojom_traits.h",
"mojo/bluetooth_struct_traits.cc",
"mojo/bluetooth_struct_traits.h",
diff --git a/third_party/blink/renderer/platform/mojo/big_string_mojom_traits.cc b/third_party/blink/renderer/platform/mojo/big_string_mojom_traits.cc
deleted file mode 100644
index a0c0200..0000000
--- a/third_party/blink/renderer/platform/mojo/big_string_mojom_traits.cc
+++ /dev/null
@@ -1,43 +0,0 @@
-// Copyright 2016 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 "third_party/blink/renderer/platform/mojo/big_string_mojom_traits.h"
-
-#include <cstring>
-
-#include "base/containers/span.h"
-#include "mojo/public/cpp/base/big_buffer.h"
-#include "mojo/public/cpp/base/big_buffer_mojom_traits.h"
-#include "third_party/blink/renderer/platform/wtf/text/string_utf8_adaptor.h"
-
-namespace mojo {
-
-// static
-mojo_base::BigBuffer StructTraits<mojo_base::mojom::BigStringDataView,
- WTF::String>::data(const WTF::String& input) {
- WTF::StringUTF8Adaptor adaptor(input);
- return mojo_base::BigBuffer(base::as_bytes(base::make_span(adaptor)));
-}
-
-// static
-bool StructTraits<mojo_base::mojom::BigStringDataView, WTF::String>::Read(
- mojo_base::mojom::BigStringDataView data,
- WTF::String* out) {
- mojo_base::BigBuffer buffer;
- if (!data.ReadData(&buffer))
- return false;
- size_t size = buffer.size();
- if (size % sizeof(char))
- return false;
- // An empty |mojo_base::BigBuffer| may have a null |data()| if empty.
- if (!size) {
- *out = g_empty_string;
- } else {
- *out = WTF::String::FromUTF8(reinterpret_cast<const char*>(buffer.data()),
- size / sizeof(char));
- }
- return true;
-}
-
-} // namespace mojo
diff --git a/third_party/blink/renderer/platform/mojo/big_string_mojom_traits.h b/third_party/blink/renderer/platform/mojo/big_string_mojom_traits.h
index 4ea78d8..d157f8e 100644
--- a/third_party/blink/renderer/platform/mojo/big_string_mojom_traits.h
+++ b/third_party/blink/renderer/platform/mojo/big_string_mojom_traits.h
@@ -8,10 +8,7 @@
#include "mojo/public/cpp/bindings/struct_traits.h"
#include "mojo/public/mojom/base/big_string.mojom-blink.h"
#include "third_party/blink/renderer/platform/platform_export.h"
-
-namespace mojo_base {
-class BigBuffer;
-}
+#include "third_party/blink/renderer/platform/wtf/text/string_utf8_adaptor.h"
namespace mojo {
@@ -21,8 +18,27 @@
static bool IsNull(const WTF::String& input) { return input.IsNull(); }
static void SetToNull(WTF::String* output) { *output = WTF::String(); }
- static mojo_base::BigBuffer data(const WTF::String& input);
- static bool Read(mojo_base::mojom::BigStringDataView, WTF::String* out);
+ static mojo_base::BigBuffer data(const WTF::String& input) {
+ WTF::StringUTF8Adaptor adaptor(input);
+ return mojo_base::BigBuffer(base::as_bytes(base::make_span(adaptor)));
+ }
+
+ static bool Read(mojo_base::mojom::BigStringDataView data, WTF::String* out) {
+ mojo_base::BigBuffer buffer;
+ if (!data.ReadData(&buffer))
+ return false;
+ size_t size = buffer.size();
+ if (size % sizeof(char))
+ return false;
+ // An empty |mojo_base::BigBuffer| may have a null |data()| if empty.
+ if (!size) {
+ *out = g_empty_string;
+ } else {
+ *out = WTF::String::FromUTF8(reinterpret_cast<const char*>(buffer.data()),
+ size / sizeof(char));
+ }
+ return true;
+ }
};
} // namespace mojo
