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

Issue 758739 link

Starred by 1 user
Status: Assigned
Owner:
erikc...@chromium.org
Cc:
█ xunji...@chromium.org
█ ajwong@chromium.org
etienneb@chromium.org
ssid@chromium.org
keishi@chromium.org
█ dskiba@chromium.org
primiano@chromium.org
█ kraynov@chromium.org
█ hjd@chromium.org
█ brettw@chromium.org
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug

Blocked on:
issue 763173
issue 765863
issue 766396
issue 827545


Sign in to add a comment

Tracking bug: Removal of heap-profiling code.

Project Member Reported by erikc...@chromium.org, Aug 24 2017 
Heap profiling [--enable-heap-profiling=*] is a tool used by a relatively small number of developers to diagnose Chrome memory issues. [I've attempted to cc all known consumers].

Out of Process heap-profiling is a replacement that works around some of the constraints of the legacy heap profiler. It's intended to be used by both developers, and potentially a broader range of users [as a passive, background profiling tool].  

Once OOP HP is able to solve/diagnose the same problems [even if the workflow is slightly different.], we'd like to remove the existing heap-profiling code, just to prevent confusion.

Known missing features in OOP HP:
  0) Work out usability/performance details on Android. [We think everything should work...but we're mostly desktop devs so everything is slower on Android].
  1) Integration with PartitionAllocMDP, BlinkGCMDP
  2) Propagation of "type" information for each allocation.
  3) Support for pseudo stacks.
  4) Minor fixes to OOP HP json exporter.

Functionality difference:
  * There is a different command-line flag and about://flags option for OOP HP. [--memlog=browser, --memlog=all]
  * In OOP HP, the stack traces will be in a different memory-dump event than the MDP light/background/detailed dumps. 

Other than that, existing workflows should "just work".

Comment 1 by erikc...@chromium.org, Aug 25 2017

Owner: brettw@chromium.org
Status: Assigned (was: Untriaged)

Comment 2 by primiano@chromium.org, Aug 25 2017 

>  There is a different command-line flag and about://flags option for OOP HP. [--memlog=browser, --memlog=all]

Once the legacy hp is no more, is it conceivable to just reuse the current --enable-heap-profiling=mode cmdline flag? Mostly to reduce mental overhead to existing users?  

> * In OOP HP, the stack traces will be in a different memory-dump event than the MDP light/background/detailed dumps. 
Mostly a note-to-self as a reminder to reply to the internal thread that is somewhere in my inbox. I think we can leverage some trick here. If we pass the dump_guid around you can emit another dump event with the same guid and the UI will just merge the dictionaries (today we use that for the "browser process dumps mmaps on behalf of child process" use case) and will pull the data under the same (M) thing in the UI.

Comment 3 by brettw@chromium.org, Sep 7 2017

Blockedon: 763173

Comment 4 by erikc...@chromium.org, Sep 19 2017

Blockedon: 765863
Another difference between OOP HP and traditional HP: when renderers are killed...their allocations persist in the profiling process. We should probably iterate through the AllocationTracker and remove all atoms from BacktraceStorage.

Comment 5 by erikc...@chromium.org, Sep 19 2017

Blockedon: 766396

Comment 6 by erikc...@chromium.org, Oct 24 2017 

From ssid's comment on design doc: https://docs.google.com/document/d/1qBANWBo9_LXtwLa8xm3GBZ0oBwWbGPu0_keVKGunXaQ/edit

"""
 I think we need support for pseudo stacks, being able to profile on low-end devices, profiling from the field for low-end Android, also native profiling on 32-bit with build flag
"""

Comment 7 by erikc...@chromium.org, Jan 11 2018

Owner: erikc...@chromium.org
Project Member

Comment 8 by bugdroid1@chromium.org, Jan 11 2018 

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

commit 3303fd023e6c5db58708458479ce1045982d799e
Author: Erik Chen <erikchen@chromium.org>
Date: Thu Jan 11 20:29:05 2018

Add pseudo/mixed stack support to out of process heap profiling.

This makes OOP HP fully feature compatible with the legacy heap profiling, and
is a first step to removing legacy heap profiling.

The current implementation of pseudo/mixed stacks allows stack frames to contain
a pointer to a static cstring in the TEXT section of the program. The one
exception [which is potentially a bug] is for thread names.

To implement this functionality for OOP HP, I created a new packet type [string
mapping] which describes a mapping between a pointer and a string. This mapping
persists for the lifetime of the process. When a client encounters a new
cstring, it first sends a string mapping packet, and then sends a packet as
usual. The profiling service, when exporting a heap dump, looks up pointers in
its string map and converts back to string if necessary.

The new functionality is available as a command line flag and about://flags
setting as --memlog-stack-mode.

I expanded the test support in ProfilingTestDriver, and removed redundant memlog
e2e tests.

Bug: 758739
Change-Id: I554edb1edbc90dec8f9a77be4b6bbaf19b13fc08
Reviewed-on: https://chromium-review.googlesource.com/850862
Commit-Queue: Erik Chen <erikchen@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Reviewed-by: Brett Wilson <brettw@chromium.org>
Reviewed-by: Albert J. Wong <ajwong@chromium.org>
Cr-Commit-Position: refs/heads/master@{#528735}
[modify] https://crrev.com/3303fd023e6c5db58708458479ce1045982d799e/chrome/browser/about_flags.cc
[modify] https://crrev.com/3303fd023e6c5db58708458479ce1045982d799e/chrome/browser/flag_descriptions.cc
[modify] https://crrev.com/3303fd023e6c5db58708458479ce1045982d799e/chrome/browser/flag_descriptions.h
[modify] https://crrev.com/3303fd023e6c5db58708458479ce1045982d799e/chrome/browser/profiling_host/chrome_browser_main_extra_parts_profiling.cc
[modify] https://crrev.com/3303fd023e6c5db58708458479ce1045982d799e/chrome/browser/profiling_host/memlog_browsertest.cc
[modify] https://crrev.com/3303fd023e6c5db58708458479ce1045982d799e/chrome/browser/profiling_host/profiling_process_host.cc
[modify] https://crrev.com/3303fd023e6c5db58708458479ce1045982d799e/chrome/browser/profiling_host/profiling_process_host.h
[modify] https://crrev.com/3303fd023e6c5db58708458479ce1045982d799e/chrome/browser/profiling_host/profiling_test_driver.cc
[modify] https://crrev.com/3303fd023e6c5db58708458479ce1045982d799e/chrome/browser/profiling_host/profiling_test_driver.h
[modify] https://crrev.com/3303fd023e6c5db58708458479ce1045982d799e/chrome/browser/profiling_host/test_android_shim.cc
[modify] https://crrev.com/3303fd023e6c5db58708458479ce1045982d799e/chrome/common/chrome_switches.cc
[modify] https://crrev.com/3303fd023e6c5db58708458479ce1045982d799e/chrome/common/chrome_switches.h
[modify] https://crrev.com/3303fd023e6c5db58708458479ce1045982d799e/chrome/common/profiling/memlog_allocator_shim.cc
[modify] https://crrev.com/3303fd023e6c5db58708458479ce1045982d799e/chrome/common/profiling/memlog_allocator_shim.h
[modify] https://crrev.com/3303fd023e6c5db58708458479ce1045982d799e/chrome/common/profiling/memlog_stream.h
[modify] https://crrev.com/3303fd023e6c5db58708458479ce1045982d799e/chrome/common/profiling/profiling_client.cc
[modify] https://crrev.com/3303fd023e6c5db58708458479ce1045982d799e/chrome/common/profiling/profiling_client.h
[modify] https://crrev.com/3303fd023e6c5db58708458479ce1045982d799e/chrome/common/profiling/profiling_client.mojom
[modify] https://crrev.com/3303fd023e6c5db58708458479ce1045982d799e/chrome/common/profiling/profiling_service.mojom
[modify] https://crrev.com/3303fd023e6c5db58708458479ce1045982d799e/chrome/profiling/allocation_tracker.cc
[modify] https://crrev.com/3303fd023e6c5db58708458479ce1045982d799e/chrome/profiling/allocation_tracker.h
[modify] https://crrev.com/3303fd023e6c5db58708458479ce1045982d799e/chrome/profiling/json_exporter.cc
[modify] https://crrev.com/3303fd023e6c5db58708458479ce1045982d799e/chrome/profiling/json_exporter.h
[modify] https://crrev.com/3303fd023e6c5db58708458479ce1045982d799e/chrome/profiling/memlog_connection_manager.cc
[modify] https://crrev.com/3303fd023e6c5db58708458479ce1045982d799e/chrome/profiling/memlog_connection_manager.h
[modify] https://crrev.com/3303fd023e6c5db58708458479ce1045982d799e/chrome/profiling/memlog_receiver.h
[modify] https://crrev.com/3303fd023e6c5db58708458479ce1045982d799e/chrome/profiling/memlog_stream_fuzzer.cc
[modify] https://crrev.com/3303fd023e6c5db58708458479ce1045982d799e/chrome/profiling/memlog_stream_fuzzer.dict
[modify] https://crrev.com/3303fd023e6c5db58708458479ce1045982d799e/chrome/profiling/memlog_stream_parser.cc
[modify] https://crrev.com/3303fd023e6c5db58708458479ce1045982d799e/chrome/profiling/memlog_stream_parser.h
[modify] https://crrev.com/3303fd023e6c5db58708458479ce1045982d799e/chrome/profiling/memlog_stream_parser_unittest.cc
[modify] https://crrev.com/3303fd023e6c5db58708458479ce1045982d799e/chrome/profiling/profiling_service.cc
[modify] https://crrev.com/3303fd023e6c5db58708458479ce1045982d799e/chrome/profiling/profiling_service.h
[modify] https://crrev.com/3303fd023e6c5db58708458479ce1045982d799e/tools/metrics/histograms/enums.xml

Comment 9 by dskiba@chromium.org, Jan 11 2018 

Sometimes thread name is the only thing we have in pseudo mode, and knowing that allocations came from e.g. "Chrome_SyncThread" greatly helps. Even in native mode it's useful as it gives an overview of where memory is allocated. So I'm glad you added thread names.

Comment 10 by erikc...@chromium.org, Jan 11 2018 

the bug portion refers to the fact that raw pointers to thread name cstrings are not guaranteed to be valid for the lifetime of the application, which can cause use-after-free or other problems.

Comment 11 by dskiba@chromium.org, Jan 11 2018 

They are guaranteed to be valid, thread names are leaked and never deleted, even after thread dies.

Comment 12 by erikc...@chromium.org, Jan 11 2018 

Ah yeah. So after we remove legacy HP, we won't need to leak the name anymore, right?

https://cs.chromium.org/chromium/src/base/threading/thread_id_name_manager.cc?type=cs&sq=package:chromium&l=52
Project Member

Comment 13 by bugdroid1@chromium.org, Jan 11 2018 

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

commit de53e4df1b873c46b13d69d95bb326d8a28c8a0e
Author: erikchen <erikchen@chromium.org>
Date: Thu Jan 11 23:58:36 2018

Fix ordering of pseudo stacks.

They were being emitted in backwards order.

Bug: 758739
Change-Id: I0812fff7736acafdd62c4ae0132309107f581172
Reviewed-on: https://chromium-review.googlesource.com/862663
Commit-Queue: Erik Chen <erikchen@chromium.org>
Reviewed-by: Albert J. Wong <ajwong@chromium.org>
Cr-Commit-Position: refs/heads/master@{#528821}
[modify] https://crrev.com/de53e4df1b873c46b13d69d95bb326d8a28c8a0e/chrome/browser/profiling_host/profiling_test_driver.cc
[modify] https://crrev.com/de53e4df1b873c46b13d69d95bb326d8a28c8a0e/chrome/common/profiling/memlog_allocator_shim.cc

Comment 14 by primiano@chromium.org, Jan 12 2018 

Re #12, iirc thread names were leaked before for base::trackedobjects
Project Member

Comment 16 by bugdroid1@chromium.org, Jan 18 2018 

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

commit 94aca89a2e3e9536a1548c83ab0d7e90aa2e26f2
Author: erikchen <erikchen@chromium.org>
Date: Thu Jan 18 00:21:19 2018

Clean up flag descriptions for OOP HP.

Bug: 758739
Change-Id: Id9d03a1b3cb57beb7fd9c20d0ea20bced2a10ac5
Reviewed-on: https://chromium-review.googlesource.com/862231
Commit-Queue: Erik Chen <erikchen@chromium.org>
Reviewed-by: Brett Wilson <brettw@chromium.org>
Cr-Commit-Position: refs/heads/master@{#529972}
[modify] https://crrev.com/94aca89a2e3e9536a1548c83ab0d7e90aa2e26f2/chrome/browser/about_flags.cc
[modify] https://crrev.com/94aca89a2e3e9536a1548c83ab0d7e90aa2e26f2/chrome/browser/flag_descriptions.cc

Comment 17 by erikc...@chromium.org, Jan 19 2018 

I went through and checked behavior of OOPHP vs NHP on Android, and everything seems to be working fine. I tried:
OOPHP [pseudo] + trace using chrome://inspect/?tracing#devices 
OOPHP [pseudo] + trace using startup tracing
OOPHP [native] + trace using chrome://inspect/?tracing#devices 
OOPHP [native] + trace using startup tracing.

I also tried all of those steps with NHP and compared traces - the traces more or less look the same. There are a couple of observable differences - I will attempt to enumerate them all:

* NHP uses heap v1 format, OOP HP uses v2
* NHP auto-condenses large stacks, OOP HP has a flag to either: omit small allocations, or else emit all allocations with no trimming. 
* NHP always run for all processes. OOP HP can select the types of processes.
* NHP must be enabled at application start. OOP HP can be started at any point in time [WIP CL for this] - but changing the type of profiling still requires a restart [if anyone cares we can improve this, but seems like an unnecessary luxury for now].
* OOPHP starts later than NHP, even when enabled as a flag [as it takes some type for profiling process to start and send the signal]. In the browser process on android, this results in 3.7MB of tracked allocations for NHP vs 3.4MB of tracked allocations for OOPHP using 30s startup profiling and an empty profile.
* NHP has some limits on stack depth, # of tracked allocations, # of unique backtraces, etc. OOPHP has no limits.

* The first frame in each NHP [native] stack is the thread. We've chosen to omit that in OOP HP. We can change this/add a flag if necessary. [In Pseudo mode - the first frame is the thread for both NHP and OOPHP.]
Project Member

Comment 18 by bugdroid1@chromium.org, Jan 19 2018 

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

commit 8b70a41f454b84509a014ff03f9e3fe09587ed97
Author: erikchen <erikchen@chromium.org>
Date: Fri Jan 19 15:53:54 2018

Allow OOP HP to be enabled dynamically.

All the functionally already exists, the only thing that has to change is some
logic in a switch statement to treat the "none" mode the same as "manual" mode.

Bug: 758739
Change-Id: I23627f4fb24e29f28b525ab4c69958e19b895edd
Reviewed-on: https://chromium-review.googlesource.com/875071
Commit-Queue: Brett Wilson <brettw@chromium.org>
Reviewed-by: Brett Wilson <brettw@chromium.org>
Cr-Commit-Position: refs/heads/master@{#530520}
[modify] https://crrev.com/8b70a41f454b84509a014ff03f9e3fe09587ed97/chrome/browser/ui/webui/memory_internals_ui.cc

Comment 19 by dskiba@chromium.org, Jan 19 2018 

> * The first frame in each NHP [native] stack is the thread. We've chosen to omit that in OOP HP. We can change this/add a flag if necessary.

What is the reason for not including thread name in native mode?
Project Member

Comment 20 by bugdroid1@chromium.org, Jan 26 2018 

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

commit 011ad3fd2bd3337ca93e51c54eb6b45f71a91ded
Author: erikchen <erikchen@chromium.org>
Date: Fri Jan 26 17:54:55 2018

Add support for thread names to out of process heap profiling.

This CL adds a new memlog stack mode: native-with-thread-names. When enabled,
the thread name will be inserted as the first frame of each stack.

Other minor changes:
  * Fixed minor bug in the parsing logic for ProfilingTestDriver.
  * Fixed a JSON exporter issue [node and string ids should begin at 1, not 0.
chrome://tracing UI ignores nodes with id 0.]
  * Add TLS destructor for allocator shim.
  * base::android::AttachCurrentThread() now preserves thread name.

Change-Id: I8c9d82084d6439e663f94e563068c987d1cf3b23
Bug: 758739
Reviewed-on: https://chromium-review.googlesource.com/877085
Reviewed-by: Maria Khomenko <mariakhomenko@chromium.org>
Reviewed-by: Richard Coles <torne@chromium.org>
Reviewed-by: Dmitry Skiba <dskiba@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Reviewed-by: Albert J. Wong <ajwong@chromium.org>
Commit-Queue: Erik Chen <erikchen@chromium.org>
Cr-Commit-Position: refs/heads/master@{#531992}
[modify] https://crrev.com/011ad3fd2bd3337ca93e51c54eb6b45f71a91ded/base/android/jni_android.cc
[modify] https://crrev.com/011ad3fd2bd3337ca93e51c54eb6b45f71a91ded/base/threading/thread_id_name_manager.cc
[modify] https://crrev.com/011ad3fd2bd3337ca93e51c54eb6b45f71a91ded/base/threading/thread_id_name_manager.h
[modify] https://crrev.com/011ad3fd2bd3337ca93e51c54eb6b45f71a91ded/chrome/android/javatests/src/org/chromium/chrome/browser/profiling_host/ProfilingProcessHostAndroidTest.java
[modify] https://crrev.com/011ad3fd2bd3337ca93e51c54eb6b45f71a91ded/chrome/android/javatests/src/org/chromium/chrome/browser/profiling_host/TestAndroidShim.java
[modify] https://crrev.com/011ad3fd2bd3337ca93e51c54eb6b45f71a91ded/chrome/browser/about_flags.cc
[modify] https://crrev.com/011ad3fd2bd3337ca93e51c54eb6b45f71a91ded/chrome/browser/flag_descriptions.cc
[modify] https://crrev.com/011ad3fd2bd3337ca93e51c54eb6b45f71a91ded/chrome/browser/flag_descriptions.h
[modify] https://crrev.com/011ad3fd2bd3337ca93e51c54eb6b45f71a91ded/chrome/browser/profiling_host/memlog_browsertest.cc
[modify] https://crrev.com/011ad3fd2bd3337ca93e51c54eb6b45f71a91ded/chrome/browser/profiling_host/profiling_process_host.cc
[modify] https://crrev.com/011ad3fd2bd3337ca93e51c54eb6b45f71a91ded/chrome/browser/profiling_host/profiling_process_host.h
[modify] https://crrev.com/011ad3fd2bd3337ca93e51c54eb6b45f71a91ded/chrome/browser/profiling_host/profiling_test_driver.cc
[modify] https://crrev.com/011ad3fd2bd3337ca93e51c54eb6b45f71a91ded/chrome/browser/profiling_host/profiling_test_driver.h
[modify] https://crrev.com/011ad3fd2bd3337ca93e51c54eb6b45f71a91ded/chrome/browser/profiling_host/test_android_shim.cc
[modify] https://crrev.com/011ad3fd2bd3337ca93e51c54eb6b45f71a91ded/chrome/browser/profiling_host/test_android_shim.h
[modify] https://crrev.com/011ad3fd2bd3337ca93e51c54eb6b45f71a91ded/chrome/common/chrome_switches.cc
[modify] https://crrev.com/011ad3fd2bd3337ca93e51c54eb6b45f71a91ded/chrome/common/chrome_switches.h
[modify] https://crrev.com/011ad3fd2bd3337ca93e51c54eb6b45f71a91ded/chrome/common/profiling/memlog_allocator_shim.cc
[modify] https://crrev.com/011ad3fd2bd3337ca93e51c54eb6b45f71a91ded/chrome/common/profiling/memlog_allocator_shim.h
[modify] https://crrev.com/011ad3fd2bd3337ca93e51c54eb6b45f71a91ded/chrome/common/profiling/profiling_client.mojom
[modify] https://crrev.com/011ad3fd2bd3337ca93e51c54eb6b45f71a91ded/chrome/profiling/json_exporter.cc

Comment 21 by erikc...@chromium.org, Jan 26 2018 

> What is the reason for not including thread name in native mode?

* Overhead
* Thread names will become less relevant as we move toward TaskScheduler, which will be replacing most threads.
Project Member

Comment 22 by bugdroid1@chromium.org, Jan 29 2018 

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

commit 1d87f734be618048a4ded43eeb94a018c9689aef
Author: erikchen <erikchen@chromium.org>
Date: Mon Jan 29 22:31:42 2018

Add support for AllocationContext::type_name to pseudo OOPHP traces.

The information already exists in the AllocationContext, and the plumbing
already exists from supporting type_name for PartitionAlloc. This CL just hooks
up the two pieces. This CL also adds escaping for type_name.

Bug: 758739
Change-Id: Ie3c0e07730f7fba8dda6efa7be5c06b422cbf648
Reviewed-on: https://chromium-review.googlesource.com/889880
Commit-Queue: Erik Chen <erikchen@chromium.org>
Reviewed-by: Siddhartha S <ssid@chromium.org>
Cr-Commit-Position: refs/heads/master@{#532632}
[modify] https://crrev.com/1d87f734be618048a4ded43eeb94a018c9689aef/chrome/browser/profiling_host/profiling_test_driver.cc
[modify] https://crrev.com/1d87f734be618048a4ded43eeb94a018c9689aef/chrome/common/profiling/memlog_allocator_shim.cc
[modify] https://crrev.com/1d87f734be618048a4ded43eeb94a018c9689aef/chrome/profiling/allocation_tracker.cc
Project Member

Comment 23 by bugdroid1@chromium.org, Feb 1 2018 

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

commit 535ef8e3deba255635634dcb362033e589788684
Author: erikchen <erikchen@chromium.org>
Date: Thu Feb 01 18:04:54 2018

Use unique IDs for OOP HP heap dumps.

The heap dump v2 format requires that multiple heap dumps in the same trace
share IDs for nodes and strings. Since the JSON exporter is stateless, and isn't
aware of whether the dump will be part of a given trace, just give all nodes
and strings a unique ID.

Bug: 758739
Change-Id: I87002ce9a84c60ee7a41ac38d3702248a4aa6ae4
Reviewed-on: https://chromium-review.googlesource.com/895666
Reviewed-by: Dmitry Skiba <dskiba@chromium.org>
Commit-Queue: Erik Chen <erikchen@chromium.org>
Cr-Commit-Position: refs/heads/master@{#533731}
[modify] https://crrev.com/535ef8e3deba255635634dcb362033e589788684/chrome/profiling/json_exporter.cc
[modify] https://crrev.com/535ef8e3deba255635634dcb362033e589788684/chrome/profiling/json_exporter.h
[modify] https://crrev.com/535ef8e3deba255635634dcb362033e589788684/chrome/profiling/json_exporter_unittest.cc
[modify] https://crrev.com/535ef8e3deba255635634dcb362033e589788684/chrome/profiling/memlog_connection_manager.cc
[modify] https://crrev.com/535ef8e3deba255635634dcb362033e589788684/chrome/profiling/memlog_connection_manager.h
Project Member

Comment 24 by bugdroid1@chromium.org, Feb 13 2018 

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

commit 99b4a583f634ba382fd597cda1148cc4299dc2e0
Author: erikchen <erikchen@chromium.org>
Date: Tue Feb 13 06:08:12 2018

Clean up of memory instrumentation interface.

This CL cleans up both the public interface and private implementation of memory
instrumentation. It is in preparation for allowing memory instrumentation to
asynchronously add heap dumps to traces.

memory_instrumentation supports two interfaces: HeapProfilerHelper and
Coordinator [specifically RequestGlobalMemoryDump]. They are used by different
clients, with different privileges, for different purposes. The internal
implementation used a single code path to support both, which further results in
leakage of implementation details into the mojom interface, in the form of
LevelOfDetail::VM_REGIONS_ONLY_FOR_HEAP_PROFILER.

The previous implementation only allowed 1 global dump to be issued at a time.
This causes problems for my upcoming refactor, which makes
memory_instrumentation, during a global dump, request a heap dump from the
profiling service. The profiling service in turn requests heaps from
memory_instrumentation, resulting in a deadlock.

This CL updates the implementation of CoordinatorImpl to use a separate queue of
|in_progress_vm_region_requests_| to track heap dump requests. These are
executed as soon as they are received. This CL also removes
LevelOfDetail::VM_REGIONS_ONLY_FOR_HEAP_PROFILER, and updates
GetVmRegionsForHeapProfiler to explicitly specify PIDs to dump.

Bug: 758739
Change-Id: I1ee41e272ac4379f791abe469e0d8efda42636de
Reviewed-on: https://chromium-review.googlesource.com/900222
Commit-Queue: Erik Chen <erikchen@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Reviewed-by: Ilya Sherman <isherman@chromium.org>
Reviewed-by: Primiano Tucci <primiano@chromium.org>
Cr-Commit-Position: refs/heads/master@{#536263}
[modify] https://crrev.com/99b4a583f634ba382fd597cda1148cc4299dc2e0/chrome/browser/metrics/process_memory_metrics_emitter_unittest.cc
[modify] https://crrev.com/99b4a583f634ba382fd597cda1148cc4299dc2e0/chrome/browser/profiling_host/background_profiling_triggers_unittest.cc
[modify] https://crrev.com/99b4a583f634ba382fd597cda1148cc4299dc2e0/chrome/browser/profiling_host/profiling_test_driver.cc
[modify] https://crrev.com/99b4a583f634ba382fd597cda1148cc4299dc2e0/chrome/profiling/memlog_connection_manager.cc
[modify] https://crrev.com/99b4a583f634ba382fd597cda1148cc4299dc2e0/chrome/profiling/memlog_connection_manager.h
[modify] https://crrev.com/99b4a583f634ba382fd597cda1148cc4299dc2e0/chrome/profiling/profiling_service.cc
[modify] https://crrev.com/99b4a583f634ba382fd597cda1148cc4299dc2e0/chrome/profiling/profiling_service.h
[modify] https://crrev.com/99b4a583f634ba382fd597cda1148cc4299dc2e0/services/resource_coordinator/memory_instrumentation/coordinator_impl.cc
[modify] https://crrev.com/99b4a583f634ba382fd597cda1148cc4299dc2e0/services/resource_coordinator/memory_instrumentation/coordinator_impl.h
[modify] https://crrev.com/99b4a583f634ba382fd597cda1148cc4299dc2e0/services/resource_coordinator/memory_instrumentation/coordinator_impl_unittest.cc
[modify] https://crrev.com/99b4a583f634ba382fd597cda1148cc4299dc2e0/services/resource_coordinator/memory_instrumentation/queued_request.cc
[modify] https://crrev.com/99b4a583f634ba382fd597cda1148cc4299dc2e0/services/resource_coordinator/memory_instrumentation/queued_request.h
[modify] https://crrev.com/99b4a583f634ba382fd597cda1148cc4299dc2e0/services/resource_coordinator/memory_instrumentation/queued_request_dispatcher.cc
[modify] https://crrev.com/99b4a583f634ba382fd597cda1148cc4299dc2e0/services/resource_coordinator/memory_instrumentation/queued_request_dispatcher.h
[modify] https://crrev.com/99b4a583f634ba382fd597cda1148cc4299dc2e0/services/resource_coordinator/public/cpp/memory_instrumentation/memory_instrumentation_mojom_traits.cc
[modify] https://crrev.com/99b4a583f634ba382fd597cda1148cc4299dc2e0/services/resource_coordinator/public/mojom/memory_instrumentation/memory_instrumentation.mojom
Project Member

Comment 25 by bugdroid1@chromium.org, Feb 13 2018 

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

commit 1c849e92aef656ed495be99af74442c75243789c
Author: Marc Treib <treib@chromium.org>
Date: Tue Feb 13 14:30:42 2018

Revert "Clean up of memory instrumentation interface."

This reverts commit 99b4a583f634ba382fd597cda1148cc4299dc2e0.

Reason for revert: Breaks Memlog/MemlogBrowserTest.EndToEnd/13 on Windows; see  crbug.com/811711 

Original change's description:
> Clean up of memory instrumentation interface.
> 
> This CL cleans up both the public interface and private implementation of memory
> instrumentation. It is in preparation for allowing memory instrumentation to
> asynchronously add heap dumps to traces.
> 
> memory_instrumentation supports two interfaces: HeapProfilerHelper and
> Coordinator [specifically RequestGlobalMemoryDump]. They are used by different
> clients, with different privileges, for different purposes. The internal
> implementation used a single code path to support both, which further results in
> leakage of implementation details into the mojom interface, in the form of
> LevelOfDetail::VM_REGIONS_ONLY_FOR_HEAP_PROFILER.
> 
> The previous implementation only allowed 1 global dump to be issued at a time.
> This causes problems for my upcoming refactor, which makes
> memory_instrumentation, during a global dump, request a heap dump from the
> profiling service. The profiling service in turn requests heaps from
> memory_instrumentation, resulting in a deadlock.
> 
> This CL updates the implementation of CoordinatorImpl to use a separate queue of
> |in_progress_vm_region_requests_| to track heap dump requests. These are
> executed as soon as they are received. This CL also removes
> LevelOfDetail::VM_REGIONS_ONLY_FOR_HEAP_PROFILER, and updates
> GetVmRegionsForHeapProfiler to explicitly specify PIDs to dump.
> 
> Bug: 758739
> Change-Id: I1ee41e272ac4379f791abe469e0d8efda42636de
> Reviewed-on: https://chromium-review.googlesource.com/900222
> Commit-Queue: Erik Chen <erikchen@chromium.org>
> Reviewed-by: Daniel Cheng <dcheng@chromium.org>
> Reviewed-by: Ilya Sherman <isherman@chromium.org>
> Reviewed-by: Primiano Tucci <primiano@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#536263}

TBR=dcheng@chromium.org,isherman@chromium.org,primiano@chromium.org,erikchen@chromium.org,lalitm@chromium.org

Change-Id: I6ba2c5548b4fcd2950a3e0a1734a10140c6c152a
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 758739,  811711 
Reviewed-on: https://chromium-review.googlesource.com/916063
Reviewed-by: Marc Treib <treib@chromium.org>
Commit-Queue: Marc Treib <treib@chromium.org>
Cr-Commit-Position: refs/heads/master@{#536353}
[modify] https://crrev.com/1c849e92aef656ed495be99af74442c75243789c/chrome/browser/metrics/process_memory_metrics_emitter_unittest.cc
[modify] https://crrev.com/1c849e92aef656ed495be99af74442c75243789c/chrome/browser/profiling_host/background_profiling_triggers_unittest.cc
[modify] https://crrev.com/1c849e92aef656ed495be99af74442c75243789c/chrome/browser/profiling_host/profiling_test_driver.cc
[modify] https://crrev.com/1c849e92aef656ed495be99af74442c75243789c/chrome/profiling/memlog_connection_manager.cc
[modify] https://crrev.com/1c849e92aef656ed495be99af74442c75243789c/chrome/profiling/memlog_connection_manager.h
[modify] https://crrev.com/1c849e92aef656ed495be99af74442c75243789c/chrome/profiling/profiling_service.cc
[modify] https://crrev.com/1c849e92aef656ed495be99af74442c75243789c/chrome/profiling/profiling_service.h
[modify] https://crrev.com/1c849e92aef656ed495be99af74442c75243789c/services/resource_coordinator/memory_instrumentation/coordinator_impl.cc
[modify] https://crrev.com/1c849e92aef656ed495be99af74442c75243789c/services/resource_coordinator/memory_instrumentation/coordinator_impl.h
[modify] https://crrev.com/1c849e92aef656ed495be99af74442c75243789c/services/resource_coordinator/memory_instrumentation/coordinator_impl_unittest.cc
[modify] https://crrev.com/1c849e92aef656ed495be99af74442c75243789c/services/resource_coordinator/memory_instrumentation/queued_request.cc
[modify] https://crrev.com/1c849e92aef656ed495be99af74442c75243789c/services/resource_coordinator/memory_instrumentation/queued_request.h
[modify] https://crrev.com/1c849e92aef656ed495be99af74442c75243789c/services/resource_coordinator/memory_instrumentation/queued_request_dispatcher.cc
[modify] https://crrev.com/1c849e92aef656ed495be99af74442c75243789c/services/resource_coordinator/memory_instrumentation/queued_request_dispatcher.h
[modify] https://crrev.com/1c849e92aef656ed495be99af74442c75243789c/services/resource_coordinator/public/cpp/memory_instrumentation/memory_instrumentation_mojom_traits.cc
[modify] https://crrev.com/1c849e92aef656ed495be99af74442c75243789c/services/resource_coordinator/public/mojom/memory_instrumentation/memory_instrumentation.mojom
Project Member

Comment 26 by bugdroid1@chromium.org, Feb 14 2018 

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

commit 53cddfe6e7d62a6ec57dac0ef9944e42707405f0
Author: erikchen <erikchen@chromium.org>
Date: Wed Feb 14 23:31:29 2018

[Reland #1] Clean up of memory instrumentation interface.

The original CL caused two race conditions to trigger more frequently. These
have been fixed at
https://chromium-review.googlesource.com/c/chromium/src/+/917243.

> This CL cleans up both the public interface and private implementation of memory
> instrumentation. It is in preparation for allowing memory instrumentation to
> asynchronously add heap dumps to traces.
>
> memory_instrumentation supports two interfaces: HeapProfilerHelper and
> Coordinator [specifically RequestGlobalMemoryDump]. They are used by different
> clients, with different privileges, for different purposes. The internal
> implementation used a single code path to support both, which further results in
> leakage of implementation details into the mojom interface, in the form of
> LevelOfDetail::VM_REGIONS_ONLY_FOR_HEAP_PROFILER.
>
> The previous implementation only allowed 1 global dump to be issued at a time.
> This causes problems for my upcoming refactor, which makes
> memory_instrumentation, during a global dump, request a heap dump from the
> profiling service. The profiling service in turn requests heaps from
> memory_instrumentation, resulting in a deadlock.
>
> This CL updates the implementation of CoordinatorImpl to use a separate queue of
> |in_progress_vm_region_requests_| to track heap dump requests. These are
> executed as soon as they are received. This CL also removes
> LevelOfDetail::VM_REGIONS_ONLY_FOR_HEAP_PROFILER, and updates
> GetVmRegionsForHeapProfiler to explicitly specify PIDs to dump.
>
> Bug: 758739
> Change-Id: I4add68c2e3acc8eeb66d99731fce59063ac5e23f
> Reviewed-on: https://chromium-review.googlesource.com/900222
> Commit-Queue: Erik Chen <erikchen@chromium.org>
> Reviewed-by: Daniel Cheng <dcheng@chromium.org>
> Reviewed-by: Ilya Sherman <isherman@chromium.org>
> Reviewed-by: Primiano Tucci <primiano@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#536263}

Change-Id: I4add68c2e3acc8eeb66d99731fce59063ac5e23f
Bug: 758739
TBR: isherman@chromium.org, dcheng@chromium.org, primiano@chromium.org
Reviewed-on: https://chromium-review.googlesource.com/916501
Commit-Queue: Erik Chen <erikchen@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Cr-Commit-Position: refs/heads/master@{#536878}
[modify] https://crrev.com/53cddfe6e7d62a6ec57dac0ef9944e42707405f0/chrome/browser/metrics/process_memory_metrics_emitter_unittest.cc
[modify] https://crrev.com/53cddfe6e7d62a6ec57dac0ef9944e42707405f0/chrome/browser/profiling_host/background_profiling_triggers_unittest.cc
[modify] https://crrev.com/53cddfe6e7d62a6ec57dac0ef9944e42707405f0/chrome/browser/profiling_host/profiling_test_driver.cc
[modify] https://crrev.com/53cddfe6e7d62a6ec57dac0ef9944e42707405f0/chrome/profiling/memlog_connection_manager.cc
[modify] https://crrev.com/53cddfe6e7d62a6ec57dac0ef9944e42707405f0/chrome/profiling/memlog_connection_manager.h
[modify] https://crrev.com/53cddfe6e7d62a6ec57dac0ef9944e42707405f0/chrome/profiling/profiling_service.cc
[modify] https://crrev.com/53cddfe6e7d62a6ec57dac0ef9944e42707405f0/chrome/profiling/profiling_service.h
[modify] https://crrev.com/53cddfe6e7d62a6ec57dac0ef9944e42707405f0/services/resource_coordinator/memory_instrumentation/coordinator_impl.cc
[modify] https://crrev.com/53cddfe6e7d62a6ec57dac0ef9944e42707405f0/services/resource_coordinator/memory_instrumentation/coordinator_impl.h
[modify] https://crrev.com/53cddfe6e7d62a6ec57dac0ef9944e42707405f0/services/resource_coordinator/memory_instrumentation/coordinator_impl_unittest.cc
[modify] https://crrev.com/53cddfe6e7d62a6ec57dac0ef9944e42707405f0/services/resource_coordinator/memory_instrumentation/queued_request.cc
[modify] https://crrev.com/53cddfe6e7d62a6ec57dac0ef9944e42707405f0/services/resource_coordinator/memory_instrumentation/queued_request.h
[modify] https://crrev.com/53cddfe6e7d62a6ec57dac0ef9944e42707405f0/services/resource_coordinator/memory_instrumentation/queued_request_dispatcher.cc
[modify] https://crrev.com/53cddfe6e7d62a6ec57dac0ef9944e42707405f0/services/resource_coordinator/memory_instrumentation/queued_request_dispatcher.h
[modify] https://crrev.com/53cddfe6e7d62a6ec57dac0ef9944e42707405f0/services/resource_coordinator/public/cpp/memory_instrumentation/memory_instrumentation_mojom_traits.cc
[modify] https://crrev.com/53cddfe6e7d62a6ec57dac0ef9944e42707405f0/services/resource_coordinator/public/mojom/memory_instrumentation/memory_instrumentation.mojom
Project Member

Comment 27 by bugdroid1@chromium.org, Mar 27 2018 

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

commit 51e2344126b64eab5a719364943300d85ddf127c
Author: erikchen <erikchen@chromium.org>
Date: Tue Mar 27 03:06:47 2018

Remove "--enable-heap-profiling"

The flag is deprecated and replaced by --memlog and co. See
https://chromium.googlesource.com/chromium/src/+/lkcr/docs/memory/debugging_memory_issues.md#taking-a-heap-dump
for more details.

Bug: 758739
Change-Id: I8f7ac614298334a90103cd5b4c9667468d662845
Reviewed-on: https://chromium-review.googlesource.com/970852
Reviewed-by: Tommy Martino <tmartino@chromium.org>
Reviewed-by: Avi Drissman <avi@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Reviewed-by: Juan Antonio Navarro Pérez <perezju@chromium.org>
Reviewed-by: Primiano Tucci <primiano@chromium.org>
Reviewed-by: Siddhartha S <ssid@chromium.org>
Reviewed-by: Dmitry Skiba <dskiba@chromium.org>
Commit-Queue: Erik Chen <erikchen@chromium.org>
Cr-Commit-Position: refs/heads/master@{#545929}
[modify] https://crrev.com/51e2344126b64eab5a719364943300d85ddf127c/base/base_switches.cc
[modify] https://crrev.com/51e2344126b64eab5a719364943300d85ddf127c/base/base_switches.h
[modify] https://crrev.com/51e2344126b64eab5a719364943300d85ddf127c/base/trace_event/heap_profiler_allocation_context_tracker.h
[modify] https://crrev.com/51e2344126b64eab5a719364943300d85ddf127c/base/trace_event/memory_dump_manager.cc
[modify] https://crrev.com/51e2344126b64eab5a719364943300d85ddf127c/base/trace_event/memory_dump_manager.h
[modify] https://crrev.com/51e2344126b64eab5a719364943300d85ddf127c/base/trace_event/memory_dump_manager_unittest.cc
[modify] https://crrev.com/51e2344126b64eab5a719364943300d85ddf127c/chrome/app/generated_resources.grd
[modify] https://crrev.com/51e2344126b64eab5a719364943300d85ddf127c/chrome/browser/about_flags.cc
[modify] https://crrev.com/51e2344126b64eab5a719364943300d85ddf127c/chrome/browser/flag_descriptions.cc
[modify] https://crrev.com/51e2344126b64eab5a719364943300d85ddf127c/chrome/browser/flag_descriptions.h
[modify] https://crrev.com/51e2344126b64eab5a719364943300d85ddf127c/chrome/browser/profiling_host/profiling_process_host.cc
[modify] https://crrev.com/51e2344126b64eab5a719364943300d85ddf127c/chrome/browser/ui/startup/bad_flags_prompt.cc
[modify] https://crrev.com/51e2344126b64eab5a719364943300d85ddf127c/components/tracing/common/trace_startup.cc
[delete] https://crrev.com/3277b80e5f0fa6ccdcf4c8e85be8b54b2dc66f36/components/tracing/docs/heap_profiler_internals.md
[modify] https://crrev.com/51e2344126b64eab5a719364943300d85ddf127c/content/browser/browser_main_loop.cc
[modify] https://crrev.com/51e2344126b64eab5a719364943300d85ddf127c/content/browser/gpu/gpu_process_host.cc
[modify] https://crrev.com/51e2344126b64eab5a719364943300d85ddf127c/content/browser/renderer_host/render_process_host_impl.cc
[modify] https://crrev.com/51e2344126b64eab5a719364943300d85ddf127c/content/child/child_thread_impl.cc
[modify] https://crrev.com/51e2344126b64eab5a719364943300d85ddf127c/docs/README.md
[modify] https://crrev.com/51e2344126b64eab5a719364943300d85ddf127c/docs/memory-infra/README.md
[delete] https://crrev.com/3277b80e5f0fa6ccdcf4c8e85be8b54b2dc66f36/docs/memory-infra/heap_profiler_internals.md
[modify] https://crrev.com/51e2344126b64eab5a719364943300d85ddf127c/tools/metrics/histograms/enums.xml
[modify] https://crrev.com/51e2344126b64eab5a719364943300d85ddf127c/tools/perf/benchmarks/memory.py
[modify] https://crrev.com/51e2344126b64eab5a719364943300d85ddf127c/tools/perf/benchmarks/system_health.py
[modify] https://crrev.com/51e2344126b64eab5a719364943300d85ddf127c/tools/perf/contrib/heap_profiling/heap_profiling.py
[modify] https://crrev.com/51e2344126b64eab5a719364943300d85ddf127c/tools/perf/contrib/memory_extras/memory_extras.py
Project Member

Comment 28 by bugdroid1@chromium.org, Mar 27 2018 

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

commit 6cf9e9ef8b09b264c8af672e9f1798acb3cb2260
Author: Erik Chen <erikchen@chromium.org>
Date: Tue Mar 27 23:04:31 2018

Revert "Remove "--enable-heap-profiling""

This reverts commit 51e2344126b64eab5a719364943300d85ddf127c.

Reason for revert: Sid points out:
"""
We just figured that memlog doesn't work in WebView. WebView does not build chrome/ folders and we used to have the legacy heap profiling working on it. We might have to revert this change that removes the old flag while we figure out webview case.
Sorry about this late finding..
"""

Original change's description:
> Remove "--enable-heap-profiling"
> 
> The flag is deprecated and replaced by --memlog and co. See
> https://chromium.googlesource.com/chromium/src/+/lkcr/docs/memory/debugging_memory_issues.md#taking-a-heap-dump
> for more details.
> 
> Bug: 758739
> Change-Id: I8f7ac614298334a90103cd5b4c9667468d662845
> Reviewed-on: https://chromium-review.googlesource.com/970852
> Reviewed-by: Tommy Martino <tmartino@chromium.org>
> Reviewed-by: Avi Drissman <avi@chromium.org>
> Reviewed-by: Daniel Cheng <dcheng@chromium.org>
> Reviewed-by: Juan Antonio Navarro Pérez <perezju@chromium.org>
> Reviewed-by: Primiano Tucci <primiano@chromium.org>
> Reviewed-by: Siddhartha S <ssid@chromium.org>
> Reviewed-by: Dmitry Skiba <dskiba@chromium.org>
> Commit-Queue: Erik Chen <erikchen@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#545929}

TBR=avi@chromium.org,dcheng@chromium.org,primiano@chromium.org,erikchen@chromium.org,perezju@chromium.org,ssid@chromium.org,dskiba@chromium.org,tmartino@chromium.org

Change-Id: Iaac5d628ff248d70830b0c19403780df9ec0cdef
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 758739
Reviewed-on: https://chromium-review.googlesource.com/982693
Reviewed-by: Erik Chen <erikchen@chromium.org>
Commit-Queue: Erik Chen <erikchen@chromium.org>
Cr-Commit-Position: refs/heads/master@{#546308}
[modify] https://crrev.com/6cf9e9ef8b09b264c8af672e9f1798acb3cb2260/base/base_switches.cc
[modify] https://crrev.com/6cf9e9ef8b09b264c8af672e9f1798acb3cb2260/base/base_switches.h
[modify] https://crrev.com/6cf9e9ef8b09b264c8af672e9f1798acb3cb2260/base/trace_event/heap_profiler_allocation_context_tracker.h
[modify] https://crrev.com/6cf9e9ef8b09b264c8af672e9f1798acb3cb2260/base/trace_event/memory_dump_manager.cc
[modify] https://crrev.com/6cf9e9ef8b09b264c8af672e9f1798acb3cb2260/base/trace_event/memory_dump_manager.h
[modify] https://crrev.com/6cf9e9ef8b09b264c8af672e9f1798acb3cb2260/base/trace_event/memory_dump_manager_unittest.cc
[modify] https://crrev.com/6cf9e9ef8b09b264c8af672e9f1798acb3cb2260/chrome/app/generated_resources.grd
[modify] https://crrev.com/6cf9e9ef8b09b264c8af672e9f1798acb3cb2260/chrome/browser/about_flags.cc
[modify] https://crrev.com/6cf9e9ef8b09b264c8af672e9f1798acb3cb2260/chrome/browser/flag_descriptions.cc
[modify] https://crrev.com/6cf9e9ef8b09b264c8af672e9f1798acb3cb2260/chrome/browser/flag_descriptions.h
[modify] https://crrev.com/6cf9e9ef8b09b264c8af672e9f1798acb3cb2260/chrome/browser/profiling_host/profiling_process_host.cc
[modify] https://crrev.com/6cf9e9ef8b09b264c8af672e9f1798acb3cb2260/chrome/browser/ui/startup/bad_flags_prompt.cc
[modify] https://crrev.com/6cf9e9ef8b09b264c8af672e9f1798acb3cb2260/components/tracing/common/trace_startup.cc
[add] https://crrev.com/6cf9e9ef8b09b264c8af672e9f1798acb3cb2260/components/tracing/docs/heap_profiler_internals.md
[modify] https://crrev.com/6cf9e9ef8b09b264c8af672e9f1798acb3cb2260/content/browser/browser_main_loop.cc
[modify] https://crrev.com/6cf9e9ef8b09b264c8af672e9f1798acb3cb2260/content/browser/gpu/gpu_process_host.cc
[modify] https://crrev.com/6cf9e9ef8b09b264c8af672e9f1798acb3cb2260/content/browser/renderer_host/render_process_host_impl.cc
[modify] https://crrev.com/6cf9e9ef8b09b264c8af672e9f1798acb3cb2260/content/child/child_thread_impl.cc
[modify] https://crrev.com/6cf9e9ef8b09b264c8af672e9f1798acb3cb2260/docs/README.md
[modify] https://crrev.com/6cf9e9ef8b09b264c8af672e9f1798acb3cb2260/docs/memory-infra/README.md
[add] https://crrev.com/6cf9e9ef8b09b264c8af672e9f1798acb3cb2260/docs/memory-infra/heap_profiler_internals.md
[modify] https://crrev.com/6cf9e9ef8b09b264c8af672e9f1798acb3cb2260/tools/metrics/histograms/enums.xml
[modify] https://crrev.com/6cf9e9ef8b09b264c8af672e9f1798acb3cb2260/tools/perf/benchmarks/memory.py
[modify] https://crrev.com/6cf9e9ef8b09b264c8af672e9f1798acb3cb2260/tools/perf/benchmarks/system_health.py
[modify] https://crrev.com/6cf9e9ef8b09b264c8af672e9f1798acb3cb2260/tools/perf/contrib/heap_profiling/heap_profiling.py
[modify] https://crrev.com/6cf9e9ef8b09b264c8af672e9f1798acb3cb2260/tools/perf/contrib/memory_extras/memory_extras.py

Comment 29 by erikc...@chromium.org, Mar 30 2018

Blockedon: 827545
Project Member

Comment 30 by bugdroid1@chromium.org, Apr 21 2018 

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

commit cd19c16f12317271a7dfacf6273eff71b38aea03
Author: Erik Chen <erikchen@chromium.org>
Date: Sat Apr 21 00:27:24 2018

[Reland #1] Remove "--enable-heap-profiling"

The original CL was reverted because --memlog did not support Android Webview,
but --enable-heap-profiling did. Now that they both support Android Webview,
there is no reason to keep "--enable-heap-profiling" anymore.

> The flag is deprecated and replaced by --memlog and co. See
> https://chromium.googlesource.com/chromium/src/+/lkcr/docs/memory/debugging_memory_issues.md#taking-a-heap-dump
> for more details.
>
> Bug: 758739
> Change-Id: I8f7ac614298334a90103cd5b4c9667468d662845
> Reviewed-on: https://chromium-review.googlesource.com/970852
> Reviewed-by: Tommy Martino <tmartino@chromium.org>
> Reviewed-by: Avi Drissman <avi@chromium.org>
> Reviewed-by: Daniel Cheng <dcheng@chromium.org>
> Reviewed-by: Juan Antonio Navarro Pérez <perezju@chromium.org>
> Reviewed-by: Primiano Tucci <primiano@chromium.org>
> Reviewed-by: Siddhartha S <ssid@chromium.org>
> Reviewed-by: Dmitry Skiba <dskiba@chromium.org>
> Commit-Queue: Erik Chen <erikchen@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#545929}

Change-Id: I2f503b6f685aa6030ca27b60fa2e578aa9766b94
TBR: dskiba@chromium.org
Reviewed-on: https://chromium-review.googlesource.com/1022533
Reviewed-by: Erik Chen <erikchen@chromium.org>
Commit-Queue: Erik Chen <erikchen@chromium.org>
Cr-Commit-Position: refs/heads/master@{#552529}
[modify] https://crrev.com/cd19c16f12317271a7dfacf6273eff71b38aea03/base/base_switches.cc
[modify] https://crrev.com/cd19c16f12317271a7dfacf6273eff71b38aea03/base/base_switches.h
[modify] https://crrev.com/cd19c16f12317271a7dfacf6273eff71b38aea03/base/trace_event/heap_profiler_allocation_context_tracker.h
[modify] https://crrev.com/cd19c16f12317271a7dfacf6273eff71b38aea03/base/trace_event/memory_dump_manager.cc
[modify] https://crrev.com/cd19c16f12317271a7dfacf6273eff71b38aea03/base/trace_event/memory_dump_manager.h
[modify] https://crrev.com/cd19c16f12317271a7dfacf6273eff71b38aea03/base/trace_event/memory_dump_manager_unittest.cc
[modify] https://crrev.com/cd19c16f12317271a7dfacf6273eff71b38aea03/chrome/app/generated_resources.grd
[modify] https://crrev.com/cd19c16f12317271a7dfacf6273eff71b38aea03/chrome/browser/about_flags.cc
[modify] https://crrev.com/cd19c16f12317271a7dfacf6273eff71b38aea03/chrome/browser/flag_descriptions.cc
[modify] https://crrev.com/cd19c16f12317271a7dfacf6273eff71b38aea03/chrome/browser/flag_descriptions.h
[modify] https://crrev.com/cd19c16f12317271a7dfacf6273eff71b38aea03/chrome/browser/ui/startup/bad_flags_prompt.cc
[modify] https://crrev.com/cd19c16f12317271a7dfacf6273eff71b38aea03/components/services/heap_profiling/public/cpp/settings.cc
[modify] https://crrev.com/cd19c16f12317271a7dfacf6273eff71b38aea03/components/tracing/common/trace_startup.cc
[delete] https://crrev.com/f054d0e42c4d2aff375f0c865204a18981b7f163/components/tracing/docs/heap_profiler_internals.md
[modify] https://crrev.com/cd19c16f12317271a7dfacf6273eff71b38aea03/content/app/content_main_runner.cc
[modify] https://crrev.com/cd19c16f12317271a7dfacf6273eff71b38aea03/content/browser/browser_main_loop.cc
[modify] https://crrev.com/cd19c16f12317271a7dfacf6273eff71b38aea03/content/browser/gpu/gpu_process_host.cc
[modify] https://crrev.com/cd19c16f12317271a7dfacf6273eff71b38aea03/content/browser/renderer_host/render_process_host_impl.cc
[modify] https://crrev.com/cd19c16f12317271a7dfacf6273eff71b38aea03/content/child/child_thread_impl.cc
[modify] https://crrev.com/cd19c16f12317271a7dfacf6273eff71b38aea03/docs/README.md
[modify] https://crrev.com/cd19c16f12317271a7dfacf6273eff71b38aea03/docs/memory-infra/README.md
[delete] https://crrev.com/f054d0e42c4d2aff375f0c865204a18981b7f163/docs/memory-infra/heap_profiler_internals.md
[modify] https://crrev.com/cd19c16f12317271a7dfacf6273eff71b38aea03/tools/metrics/histograms/enums.xml
[modify] https://crrev.com/cd19c16f12317271a7dfacf6273eff71b38aea03/tools/perf/benchmarks/memory.py
[modify] https://crrev.com/cd19c16f12317271a7dfacf6273eff71b38aea03/tools/perf/benchmarks/system_health.py
[modify] https://crrev.com/cd19c16f12317271a7dfacf6273eff71b38aea03/tools/perf/contrib/heap_profiling/heap_profiling.py
[modify] https://crrev.com/cd19c16f12317271a7dfacf6273eff71b38aea03/tools/perf/contrib/memory_extras/memory_extras.py
►

Sign in to add a comment