New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 676682 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Last visit > 30 days ago
Closed: Mar 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 2
Type: Bug

Blocking:
issue 698266



Sign in to add a comment

MemoryInfra: Mojo services name should be used to categorize memory in heap profiler

Project Member Reported by ssid@chromium.org, Dec 22 2016

Issue description

Background context:go/memory-infra: memory profiling in chrome://tracing

The heap profiler in memory-infra attributes memory allocations to components based on the folder where the tasks are posted from. This is not always accurate since posted from location does not directly relate to the task sometimes. One such case is the mojo services where the tasks are posted always from https://chromium.googlesource.com/chromium/src/+/master/mojo/public/cpp/system/watcher.cc#105 and all services that allocate memory while handling tasks from ipc messages are attributed to "mojo" component. To attribute it right, we need to have the mojo service's name as the component name.

Currently we have a macro in heap profiler to annotate the memory allcoated in a scope to a component name:
TRACE_HEAP_PROFILER_API_SCOPED_TASK_EXECUTION.
All the memory allocated in this scope will be attributed to the component in the UI automatically.
A fix for the example above would be adding this macro to Watcher::OnHandleReady:
TRACE_HEAP_PROFILER_API_SCOPED_TASK_EXECUTION event(service_name_str);

I am not sure how many other types of message handlers are there in mojo and how do we get the service name.

 
Owner: jcivelli@chromium.org
Status: Assigned (was: Untriaged)
Components: Internals>Instrumentation>Memory
Project Member

Comment 3 by bugdroid1@chromium.org, Jan 26 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/2207af1a2e5f829ea98198d87802872781df60b8

commit 2207af1a2e5f829ea98198d87802872781df60b8
Author: jcivelli <jcivelli@chromium.org>
Date: Thu Jan 26 20:46:00 2017

Tag some of Mojo heap allocations for the heap profiler.

First step in tagging heap allocations for Mojo as to provide context
for the heap profiler.
Allocations done by mojo::Watcher are now tagged with the master
interface name if available, otherwise with the location where the
Watcher was allocated.
Non Mojo IPCs are tagged as "IPC Channel" at this point.

BUG= 676682 
TBR=sergeyu

Review-Url: https://codereview.chromium.org/2623263005
Cr-Commit-Position: refs/heads/master@{#446435}

[modify] https://crrev.com/2207af1a2e5f829ea98198d87802872781df60b8/chrome/browser/media/cast_remoting_sender.cc
[modify] https://crrev.com/2207af1a2e5f829ea98198d87802872781df60b8/content/browser/loader/mojo_async_resource_handler.cc
[modify] https://crrev.com/2207af1a2e5f829ea98198d87802872781df60b8/content/child/url_response_body_consumer.cc
[modify] https://crrev.com/2207af1a2e5f829ea98198d87802872781df60b8/content/child/web_data_consumer_handle_impl.cc
[modify] https://crrev.com/2207af1a2e5f829ea98198d87802872781df60b8/ios/web/webui/mojo_facade.h
[modify] https://crrev.com/2207af1a2e5f829ea98198d87802872781df60b8/ios/web/webui/mojo_facade.mm
[modify] https://crrev.com/2207af1a2e5f829ea98198d87802872781df60b8/ipc/ipc_mojo_bootstrap.cc
[modify] https://crrev.com/2207af1a2e5f829ea98198d87802872781df60b8/ipc/ipc_sync_channel.cc
[modify] https://crrev.com/2207af1a2e5f829ea98198d87802872781df60b8/media/mojo/common/mojo_decoder_buffer_converter.cc
[modify] https://crrev.com/2207af1a2e5f829ea98198d87802872781df60b8/media/remoting/demuxer_stream_adapter.cc
[modify] https://crrev.com/2207af1a2e5f829ea98198d87802872781df60b8/mojo/android/system/watcher_impl.cc
[modify] https://crrev.com/2207af1a2e5f829ea98198d87802872781df60b8/mojo/common/data_pipe_drainer.cc
[modify] https://crrev.com/2207af1a2e5f829ea98198d87802872781df60b8/mojo/edk/js/drain_data.cc
[modify] https://crrev.com/2207af1a2e5f829ea98198d87802872781df60b8/mojo/edk/js/waiting_callback.cc
[modify] https://crrev.com/2207af1a2e5f829ea98198d87802872781df60b8/mojo/edk/system/data_pipe_unittest.cc
[modify] https://crrev.com/2207af1a2e5f829ea98198d87802872781df60b8/mojo/edk/system/multiprocess_message_pipe_unittest.cc
[modify] https://crrev.com/2207af1a2e5f829ea98198d87802872781df60b8/mojo/public/cpp/bindings/connector.h
[modify] https://crrev.com/2207af1a2e5f829ea98198d87802872781df60b8/mojo/public/cpp/bindings/lib/connector.cc
[modify] https://crrev.com/2207af1a2e5f829ea98198d87802872781df60b8/mojo/public/cpp/bindings/lib/multiplex_router.cc
[modify] https://crrev.com/2207af1a2e5f829ea98198d87802872781df60b8/mojo/public/cpp/bindings/lib/multiplex_router.h
[modify] https://crrev.com/2207af1a2e5f829ea98198d87802872781df60b8/mojo/public/cpp/system/tests/watcher_unittest.cc
[modify] https://crrev.com/2207af1a2e5f829ea98198d87802872781df60b8/mojo/public/cpp/system/watcher.cc
[modify] https://crrev.com/2207af1a2e5f829ea98198d87802872781df60b8/mojo/public/cpp/system/watcher.h

Project Member

Comment 4 by bugdroid1@chromium.org, Feb 6 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/dbaa71fb0ffc6eb3b122e98c69f1e63ff4f77639

commit dbaa71fb0ffc6eb3b122e98c69f1e63ff4f77639
Author: ssid <ssid@chromium.org>
Date: Mon Feb 06 20:38:11 2017

Fix null heap profiler tag for allocations

If the profiler tag is not set then the Connector should not pass null
tag to the Watcher.

BUG= 676682 
R=jcivelli@chromium.org,rockot@chromium.org

Review-Url: https://codereview.chromium.org/2677663004
Cr-Commit-Position: refs/heads/master@{#448378}

[modify] https://crrev.com/dbaa71fb0ffc6eb3b122e98c69f1e63ff4f77639/mojo/public/cpp/bindings/lib/connector.cc

Blocking: 698266
Status: Fixed (was: Assigned)
I think we can close this.

Sign in to add a comment