Reland "[fuchsia] Publish memory usage details to Inspect."
This reverts commit 85681bfa15eb915b1d3b1d93d5e26306001b2eeb.
Reason for revert: Original CL was reverted speculatively to address Linux test failures, despite only affecting Fuchsia.
Original change's description:
> Revert "[fuchsia] Publish memory usage details to Inspect."
>
> This reverts commit 3fa3172fdddfeff8de140b8b81c1d27d6c14e19e.
>
> Reason for revert: The CL likely caused service_unittests failures.
> https://ci.chromium.org/ui/p/chromium/builders/ci/Linux%20Chromium%20OS%20ASan%20LSan%20Tests%20(1)/40578/test-results
> [ RUN ] TracingObserverProtoTest.AddChromeDumpToTraceIfEnabled_When_TraceLog_Disabled
> FATAL services_unittests[31665:31712]: [sequence_checker.h(136)] Check failed: checker.CalledOnValidSequence(&bound_at).
> #0 0x557c6c97d3db in backtrace /b/s/w/ir/cache/builder/src/third_party/llvm/compiler-rt/lib/asan/../sanitizer_common/sanitizer_common_interceptors.inc:4192:13
> #1 0x557c7761a0d9 in base::debug::CollectStackTrace(void**, unsigned long) ./../../base/debug/stack_trace_posix.cc:867:39
> #2 0x557c773a7583 in StackTrace ./../../base/debug/stack_trace.cc:200:12
> #3 0x557c773a7583 in base::debug::StackTrace::StackTrace() ./../../base/debug/stack_trace.cc:197:28
>
>
> Original change's description:
> > [fuchsia] Publish memory usage details to Inspect.
> >
> > Set up a "memory" node published by each web_instance, that will gather
> > details from the instance's processes regarding their memory usage, on-
> > demand.
> >
> > By default this reports the type and Private Memory Footprint of each
> > process. Additional detail can be included, from named allocator-dumps
> > specified in an |allocator-dump-names| argument in the web_instance's
> > config-data.
> >
> > Bug: 1238889
> > Change-Id: Id517c52650653e1a1dd681a721d845d6bd301545
> > Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3094003
> > Commit-Queue: Wez <wez@chromium.org>
> > Auto-Submit: Wez <wez@chromium.org>
> > Reviewed-by: Chris Hamilton <chrisha@chromium.org>
> > Reviewed-by: David Dorwin <ddorwin@chromium.org>
> > Cr-Commit-Position: refs/heads/main@{#914113}
>
> Bug: 1238889
> Change-Id: I4dbc3d2bbc752709c105c4919f7df61ae55fb3cb
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3110459
> Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
> Owners-Override: Maxim Kolosovskiy <kolos@chromium.org>
> Auto-Submit: Maxim Kolosovskiy <kolos@chromium.org>
> Commit-Queue: Maxim Kolosovskiy <kolos@chromium.org>
> Cr-Commit-Position: refs/heads/main@{#914261}
Bug: 1238889
Change-Id: I4052da8a2849379e24cea6bdc883249bbe579679
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3113669
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Auto-Submit: Wez <wez@chromium.org>
Commit-Queue: Maxim Kolosovskiy <kolos@chromium.org>
Reviewed-by: Maxim Kolosovskiy <kolos@chromium.org>
Owners-Override: Maxim Kolosovskiy <kolos@chromium.org>
Cr-Commit-Position: refs/heads/main@{#914285}
diff --git a/fuchsia/engine/BUILD.gn b/fuchsia/engine/BUILD.gn
index 73af0c3e..96900f8 100644
--- a/fuchsia/engine/BUILD.gn
+++ b/fuchsia/engine/BUILD.gn
@@ -240,6 +240,8 @@
"browser/web_engine_content_browser_client.h",
"browser/web_engine_devtools_controller.cc",
"browser/web_engine_devtools_controller.h",
+ "browser/web_engine_memory_inspector.cc",
+ "browser/web_engine_memory_inspector.h",
"browser/web_engine_net_log_observer.cc",
"browser/web_engine_net_log_observer.h",
"browser/web_engine_permission_delegate.cc",
diff --git a/fuchsia/engine/browser/DEPS b/fuchsia/engine/browser/DEPS
index 8ae2af5..549a52e 100644
--- a/fuchsia/engine/browser/DEPS
+++ b/fuchsia/engine/browser/DEPS
@@ -23,6 +23,7 @@
"+services/media_session/public/mojom",
"+services/metrics/public/cpp/ukm_source_id.h",
"+services/network/public/mojom",
+ "+services/resource_coordinator/public",
"+third_party/blink/public",
"+third_party/openscreen/src",
"+third_party/skia/include/core",
diff --git a/fuchsia/engine/browser/web_engine_browser_main_parts.cc b/fuchsia/engine/browser/web_engine_browser_main_parts.cc
index d04245ef..3b36107 100644
--- a/fuchsia/engine/browser/web_engine_browser_main_parts.cc
+++ b/fuchsia/engine/browser/web_engine_browser_main_parts.cc
@@ -37,6 +37,7 @@
#include "fuchsia/engine/browser/media_resource_provider_service.h"
#include "fuchsia/engine/browser/web_engine_browser_context.h"
#include "fuchsia/engine/browser/web_engine_devtools_controller.h"
+#include "fuchsia/engine/browser/web_engine_memory_inspector.h"
#include "fuchsia/engine/common/cast_streaming.h"
#include "fuchsia/engine/switches.h"
#include "gpu/command_buffer/service/gpu_switches.h"
@@ -127,6 +128,10 @@
base::ComponentContextForProcess());
cr_fuchsia::PublishVersionInfoToInspect(component_inspector_.get());
+ // Add a node providing memory details for this whole web instance.
+ memory_inspector_ =
+ std::make_unique<WebEngineMemoryInspector>(component_inspector_->root());
+
const auto* command_line = base::CommandLine::ForCurrentProcess();
// If Vulkan is not enabled then disable hardware acceleration. Otherwise gpu
diff --git a/fuchsia/engine/browser/web_engine_browser_main_parts.h b/fuchsia/engine/browser/web_engine_browser_main_parts.h
index 8f56326..5e2ff5c 100644
--- a/fuchsia/engine/browser/web_engine_browser_main_parts.h
+++ b/fuchsia/engine/browser/web_engine_browser_main_parts.h
@@ -38,6 +38,7 @@
}
class MediaResourceProviderService;
+class WebEngineMemoryInspector;
class WEB_ENGINE_EXPORT WebEngineBrowserMainParts
: public content::BrowserMainParts {
@@ -87,6 +88,7 @@
// Used to publish diagnostics including the active Contexts and FrameHosts.
std::unique_ptr<sys::ComponentInspector> component_inspector_;
+ std::unique_ptr<WebEngineMemoryInspector> memory_inspector_;
// Browsing contexts for the connected clients.
fidl::BindingSet<fuchsia::web::Context, std::unique_ptr<ContextImpl>>
diff --git a/fuchsia/engine/browser/web_engine_memory_inspector.cc b/fuchsia/engine/browser/web_engine_memory_inspector.cc
new file mode 100644
index 0000000..847e6ddb
--- /dev/null
+++ b/fuchsia/engine/browser/web_engine_memory_inspector.cc
@@ -0,0 +1,146 @@
+// Copyright 2021 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 "fuchsia/engine/browser/web_engine_memory_inspector.h"
+
+#include <lib/fpromise/promise.h>
+#include <lib/inspect/cpp/inspector.h>
+#include <sstream>
+
+#include "base/trace_event/memory_dump_request_args.h"
+#include "fuchsia/base/config_reader.h"
+#include "services/resource_coordinator/public/cpp/memory_instrumentation/memory_instrumentation.h"
+
+namespace {
+
+std::vector<std::string> GetAllocatorDumpNamesFromConfig() {
+ const absl::optional<base::Value>& config = cr_fuchsia::LoadPackageConfig();
+ if (!config)
+ return {};
+
+ const base::Value* names_list = config->FindListPath("allocator-dump-names");
+ if (!names_list)
+ return {};
+
+ std::vector<std::string> names;
+ names.reserve(names_list->GetList().size());
+ for (auto& name : names_list->GetList()) {
+ names.push_back(name.GetString());
+ }
+ return names;
+}
+
+// List of allocator dump names to include.
+const std::vector<std::string>& AllocatorDumpNames() {
+ static base::NoDestructor<std::vector<std::string>> allocator_dump_names(
+ GetAllocatorDumpNamesFromConfig());
+ return *allocator_dump_names;
+}
+
+} // namespace
+
+WebEngineMemoryInspector::WebEngineMemoryInspector(inspect::Node& parent_node) {
+ // Loading the allocator dump names from the config involves blocking I/O so
+ // trigger it to be done during construction, before the prohibition on
+ // blocking the main thread is applied.
+ AllocatorDumpNames();
+
+ node_ = parent_node.CreateLazyNode("memory", [this]() {
+ return fpromise::make_promise(fit::bind_member(
+ this, &WebEngineMemoryInspector::ResolveMemoryDumpPromise));
+ });
+}
+
+WebEngineMemoryInspector::~WebEngineMemoryInspector() = default;
+
+fpromise::result<inspect::Inspector>
+WebEngineMemoryInspector::ResolveMemoryDumpPromise(fpromise::context& context) {
+ // If there is a |dump_results_| then resolve the promise with it.
+ if (dump_results_) {
+ auto memory_dump = std::move(dump_results_);
+ return fpromise::ok(*memory_dump);
+ }
+
+ // If MemoryInstrumentation is not initialized then resolve an error.
+ auto* memory_instrumentation =
+ memory_instrumentation::MemoryInstrumentation::GetInstance();
+ if (!memory_instrumentation)
+ return fpromise::error();
+
+ // Request memory usage summaries for all processes, including details for
+ // any configured allocator dumps.
+ auto* coordinator = memory_instrumentation->GetCoordinator();
+ DCHECK(coordinator);
+
+ coordinator->RequestGlobalMemoryDump(
+ base::trace_event::MemoryDumpType::SUMMARY_ONLY,
+ base::trace_event::MemoryDumpLevelOfDetail::BACKGROUND,
+ base::trace_event::MemoryDumpDeterminism::NONE, AllocatorDumpNames(),
+ base::BindOnce(&WebEngineMemoryInspector::OnMemoryDumpComplete,
+ weak_this_.GetWeakPtr(), context.suspend_task()));
+
+ return fpromise::pending();
+}
+
+void WebEngineMemoryInspector::OnMemoryDumpComplete(
+ fpromise::suspended_task task,
+ bool success,
+ memory_instrumentation::mojom::GlobalMemoryDumpPtr raw_dump) {
+ DCHECK(!dump_results_);
+
+ dump_results_ = std::make_unique<inspect::Inspector>();
+
+ // If capture failed then there is no data to report.
+ if (!success || !raw_dump) {
+ task.resume_task();
+ return;
+ }
+
+ for (const auto& process_dump : raw_dump->process_dumps) {
+ auto node = dump_results_->GetRoot().CreateChild(
+ base::NumberToString(process_dump->pid));
+
+ // Include details of each process' role in the web instance.
+ std::ostringstream type;
+ type << process_dump->process_type;
+ node.CreateString("type", type.str(), dump_results_.get());
+
+ const auto service_name = process_dump->service_name;
+ if (service_name) {
+ node.CreateString("service", *service_name, dump_results_.get());
+ }
+
+ // Include the summary of the process' memory usage.
+ const auto& os_dump = process_dump->os_dump;
+ node.CreateUint("resident_kb", os_dump->resident_set_kb,
+ dump_results_.get());
+ node.CreateUint("private_kb", os_dump->private_footprint_kb,
+ dump_results_.get());
+ node.CreateUint("shared_kb", os_dump->shared_footprint_kb,
+ dump_results_.get());
+
+ // If provided, include detail from individual allocators.
+ auto detail_node = node.CreateChild("allocator_dump");
+ if (!process_dump->chrome_allocator_dumps.empty()) {
+ for (auto& it : process_dump->chrome_allocator_dumps) {
+ // Create a node using the allocator dump name.
+ auto allocator_node = detail_node.CreateChild(it.first);
+
+ // Publish the allocator-provided fields into the node.
+ for (auto& field : it.second->numeric_entries) {
+ allocator_node.CreateUint(field.first, field.second,
+ dump_results_.get());
+ }
+
+ dump_results_->emplace(std::move(allocator_node));
+ }
+
+ dump_results_->emplace(std::move(detail_node));
+ }
+
+ dump_results_->emplace(std::move(node));
+ }
+
+ task.resume_task();
+}
diff --git a/fuchsia/engine/browser/web_engine_memory_inspector.h b/fuchsia/engine/browser/web_engine_memory_inspector.h
new file mode 100644
index 0000000..d14523a
--- /dev/null
+++ b/fuchsia/engine/browser/web_engine_memory_inspector.h
@@ -0,0 +1,53 @@
+// Copyright 2021 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.
+
+#ifndef FUCHSIA_ENGINE_BROWSER_WEB_ENGINE_MEMORY_INSPECTOR_H_
+#define FUCHSIA_ENGINE_BROWSER_WEB_ENGINE_MEMORY_INSPECTOR_H_
+
+#include <lib/inspect/cpp/vmo/types.h>
+#include <memory>
+
+#include "base/memory/weak_ptr.h"
+#include "services/resource_coordinator/public/mojom/memory_instrumentation/memory_instrumentation.mojom.h"
+
+namespace fpromise {
+class context;
+}
+
+// Integrates with MemoryInstrumentation to publish memory details to Inspect
+// on-demand.
+class WebEngineMemoryInspector {
+ public:
+ // Creates a lazily-populated node called "memory" under |parent_node|.
+ explicit WebEngineMemoryInspector(inspect::Node& parent_node);
+ ~WebEngineMemoryInspector();
+
+ private:
+ // Returns the result of attempting to resolve the memory dump promise:
+ // - If the dump is available then the contents are serialized and returned.
+ // - Otherwise the |context| is suspended, a fresh dump requested, and a
+ // pending result returned. The |context| will be resumed only when the
+ // dump request completes.
+ // This is used as a continuation function for promises generated by the
+ // LazyNode's callback.
+ fpromise::result<inspect::Inspector> ResolveMemoryDumpPromise(
+ fpromise::context& context);
+
+ // Handles completion of a memory dump request.
+ void OnMemoryDumpComplete(
+ fpromise::suspended_task task,
+ bool success,
+ memory_instrumentation::mojom::GlobalMemoryDumpPtr raw_dump);
+
+ // Node holding memory usage information.
+ inspect::LazyNode node_;
+
+ // Holds an Inspector populated with data from the memory dump received by
+ // OnMemoryDumpComplete(), until the promise continuation is called.
+ std::unique_ptr<inspect::Inspector> dump_results_;
+
+ const base::WeakPtrFactory<WebEngineMemoryInspector> weak_this_{this};
+};
+
+#endif // FUCHSIA_ENGINE_BROWSER_WEB_ENGINE_MEMORY_INSPECTOR_H_
