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

Issue 768373 link

Starred by 0 users
Status: Fixed
Owner:
█ lalitm@chromium.org
Last visit > 30 days ago
Closed: Dec 2017
Cc:
ssid@chromium.org
primiano@chromium.org
█ hjd@chromium.org
█ picksi@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Feature
Hotlist-MemoryInfra

Blocking:
issue 703184
issue 775041
issue 777820
issue 728199



Sign in to add a comment

Port graph processing code from Catapult to Chrome process

Project Member Reported by lalitm@chromium.org, Sep 25 2017 
Current state of affairs:
Currently the code which computes effective sizes by performing computations on the graph of memory allocations is a part of the Catapult tracing tool.

Why this is a problem:
This has worked well to date but as we now wish to compute effective sizes to report to UMA/UKM. This means that we need to perform the graph processing in Chrome itself rather than relying on tracing to import and process traces.

How to fix this:
We will be porting and adapting the graph processing code in Catapult to Chrome's browser process as part of the memory instrumentation mojo service. This will then be consumed by UKM/UMA as well as the tracing system to obtain effective sizes.
Project Member

Comment 1 by bugdroid1@chromium.org, Oct 2 2017 

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

commit 3ff20aca1dc9ee47aabe8c7195ceb15222b53353
Author: Lalit Maganti <lalitm@chromium.org>
Date: Mon Oct 02 11:49:49 2017

Add graph processing to memory instrumentation service

This initial CL adds the code to generate the memory dump graph similar
to the existing importer code in Javascript. Currently only the nodes of
the graphs are created; edges and the processing code will appear in a
subsequent CL.

Bug:  768373 
Change-Id: I614e33d78c2e5431a3774310bfc4525f84127231
Reviewed-on: https://chromium-review.googlesource.com/663725
Commit-Queue: Lalit Maganti <lalitm@chromium.org>
Reviewed-by: Hector Dearman <hjd@chromium.org>
Reviewed-by: Siddhartha S <ssid@chromium.org>
Reviewed-by: Primiano Tucci <primiano@chromium.org>
Cr-Commit-Position: refs/heads/master@{#505583}
[modify] https://crrev.com/3ff20aca1dc9ee47aabe8c7195ceb15222b53353/services/resource_coordinator/BUILD.gn
[add] https://crrev.com/3ff20aca1dc9ee47aabe8c7195ceb15222b53353/services/resource_coordinator/memory_instrumentation/graph.cc
[add] https://crrev.com/3ff20aca1dc9ee47aabe8c7195ceb15222b53353/services/resource_coordinator/memory_instrumentation/graph.h
[add] https://crrev.com/3ff20aca1dc9ee47aabe8c7195ceb15222b53353/services/resource_coordinator/memory_instrumentation/graph_processor.cc
[add] https://crrev.com/3ff20aca1dc9ee47aabe8c7195ceb15222b53353/services/resource_coordinator/memory_instrumentation/graph_processor.h
[add] https://crrev.com/3ff20aca1dc9ee47aabe8c7195ceb15222b53353/services/resource_coordinator/memory_instrumentation/graph_processor_unittest.cc
[add] https://crrev.com/3ff20aca1dc9ee47aabe8c7195ceb15222b53353/services/resource_coordinator/memory_instrumentation/graph_unittest.cc
Project Member

Comment 2 by bugdroid1@chromium.org, Oct 10 2017 

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

commit a3e064b72027bdf9f8188fb6c75de2e807b89d4f
Author: Lalit Maganti <lalitm@chromium.org>
Date: Tue Oct 10 11:55:29 2017

memory-infra: Process graph edges in memory instrumentation graph processor

This CL adds the the code to add edges to the graph of memory dumps.

TBR=dcheng@chromium.org

Bug:  768373 
Change-Id: I5e6202fbfc71407075262ae9a610ea86e6bdd903
Reviewed-on: https://chromium-review.googlesource.com/673623
Commit-Queue: Lalit Maganti <lalitm@chromium.org>
Reviewed-by: Primiano Tucci <primiano@chromium.org>
Reviewed-by: Hector Dearman <hjd@chromium.org>
Cr-Commit-Position: refs/heads/master@{#507640}
[modify] https://crrev.com/a3e064b72027bdf9f8188fb6c75de2e807b89d4f/base/memory/discardable_shared_memory_unittest.cc
[modify] https://crrev.com/a3e064b72027bdf9f8188fb6c75de2e807b89d4f/base/trace_event/process_memory_dump.h
[modify] https://crrev.com/a3e064b72027bdf9f8188fb6c75de2e807b89d4f/base/trace_event/process_memory_dump_unittest.cc
[modify] https://crrev.com/a3e064b72027bdf9f8188fb6c75de2e807b89d4f/services/resource_coordinator/memory_instrumentation/graph.cc
[modify] https://crrev.com/a3e064b72027bdf9f8188fb6c75de2e807b89d4f/services/resource_coordinator/memory_instrumentation/graph.h
[modify] https://crrev.com/a3e064b72027bdf9f8188fb6c75de2e807b89d4f/services/resource_coordinator/memory_instrumentation/graph_processor.cc
[modify] https://crrev.com/a3e064b72027bdf9f8188fb6c75de2e807b89d4f/services/resource_coordinator/memory_instrumentation/graph_processor_unittest.cc
[modify] https://crrev.com/a3e064b72027bdf9f8188fb6c75de2e807b89d4f/services/resource_coordinator/memory_instrumentation/graph_unittest.cc
[modify] https://crrev.com/a3e064b72027bdf9f8188fb6c75de2e807b89d4f/services/resource_coordinator/public/cpp/memory_instrumentation/struct_traits_unittest.cc
[modify] https://crrev.com/a3e064b72027bdf9f8188fb6c75de2e807b89d4f/third_party/WebKit/Source/platform/instrumentation/tracing/web_process_memory_dump_test.cc
Project Member

Comment 3 by bugdroid1@chromium.org, Oct 10 2017 

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

commit 83a271156e2407650c756cfd65fc734e5bc405ea
Author: Lalit Maganti <lalitm@chromium.org>
Date: Tue Oct 10 16:37:38 2017

memory-infra: restructure graph processor as a class

Structuring this as a class allows tests to access the functions
performing each of the steps of the overall processing code.

While this is not important at this stage, as we increase the
amount of processing, we will need to test individual functions
to avoid tests becoming huge.

Bug:  768373 
Change-Id: I44e9ca24afd3371136bd32371da1a3c29e16c6e8
Reviewed-on: https://chromium-review.googlesource.com/708816
Commit-Queue: Lalit Maganti <lalitm@chromium.org>
Reviewed-by: Primiano Tucci <primiano@chromium.org>
Reviewed-by: Hector Dearman <hjd@chromium.org>
Cr-Commit-Position: refs/heads/master@{#507689}
[modify] https://crrev.com/83a271156e2407650c756cfd65fc734e5bc405ea/services/resource_coordinator/memory_instrumentation/graph_processor.cc
[modify] https://crrev.com/83a271156e2407650c756cfd65fc734e5bc405ea/services/resource_coordinator/memory_instrumentation/graph_processor.h
[modify] https://crrev.com/83a271156e2407650c756cfd65fc734e5bc405ea/services/resource_coordinator/memory_instrumentation/graph_processor_unittest.cc
Project Member

Comment 4 by bugdroid1@chromium.org, Oct 10 2017 

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

commit dc65082398720d85e8c6c9fe9079b8445ebd18a2
Author: Lalit Maganti <lalitm@chromium.org>
Date: Tue Oct 10 17:01:56 2017

memory-infra: split up serialization of process memory dumps

Split the code which adds PMDs to traces into two code paths. The
current code path will just add allocator dumps and the new code path
will just add heap dumps in FinishAsyncProcessDump.

This is to allow for moving addition of allocator dumps to traces from
each process into the memory instrumentation service.

Note for perf sheriffs
----------------------
If a memory regression shows across the board,
this CL might be responsible

Bug:  768373 
Change-Id: Ieca61fe13c84a1818bf28928d46e77f18c77287d
Reviewed-on: https://chromium-review.googlesource.com/707067
Reviewed-by: Hector Dearman <hjd@chromium.org>
Reviewed-by: Primiano Tucci <primiano@chromium.org>
Commit-Queue: Lalit Maganti <lalitm@chromium.org>
Cr-Commit-Position: refs/heads/master@{#507696}
[modify] https://crrev.com/dc65082398720d85e8c6c9fe9079b8445ebd18a2/base/trace_event/memory_allocator_dump_unittest.cc
[modify] https://crrev.com/dc65082398720d85e8c6c9fe9079b8445ebd18a2/base/trace_event/memory_dump_manager.cc
[modify] https://crrev.com/dc65082398720d85e8c6c9fe9079b8445ebd18a2/base/trace_event/process_memory_dump.cc
[modify] https://crrev.com/dc65082398720d85e8c6c9fe9079b8445ebd18a2/base/trace_event/process_memory_dump.h
[modify] https://crrev.com/dc65082398720d85e8c6c9fe9079b8445ebd18a2/base/trace_event/process_memory_dump_unittest.cc
[modify] https://crrev.com/dc65082398720d85e8c6c9fe9079b8445ebd18a2/chrome/test/base/memory_tracing_browsertest.cc
[modify] https://crrev.com/dc65082398720d85e8c6c9fe9079b8445ebd18a2/services/resource_coordinator/public/cpp/memory_instrumentation/tracing_observer.cc
[modify] https://crrev.com/dc65082398720d85e8c6c9fe9079b8445ebd18a2/third_party/WebKit/Source/platform/instrumentation/tracing/web_process_memory_dump_test.cc
Project Member

Comment 5 by bugdroid1@chromium.org, Oct 11 2017 

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

commit da27433e0a5f1bdaf61e0447bd7b4444b6c96f9d
Author: Lalit Maganti <lalitm@chromium.org>
Date: Wed Oct 11 15:35:27 2017

memory-infra: make graph nodes have a pointer to parent

This is necessary as we will need to look at the parent of a node to perform computations about whether the node is weak.

Bug:  768373 
Change-Id: I5454c1ce625c5c1300a3ef2ebf36c6d0cc82740c
Reviewed-on: https://chromium-review.googlesource.com/709754
Reviewed-by: Primiano Tucci <primiano@chromium.org>
Commit-Queue: Lalit Maganti <lalitm@chromium.org>
Cr-Commit-Position: refs/heads/master@{#507989}
[modify] https://crrev.com/da27433e0a5f1bdaf61e0447bd7b4444b6c96f9d/services/resource_coordinator/memory_instrumentation/graph.cc
[modify] https://crrev.com/da27433e0a5f1bdaf61e0447bd7b4444b6c96f9d/services/resource_coordinator/memory_instrumentation/graph.h
[modify] https://crrev.com/da27433e0a5f1bdaf61e0447bd7b4444b6c96f9d/services/resource_coordinator/memory_instrumentation/graph_processor_unittest.cc
[modify] https://crrev.com/da27433e0a5f1bdaf61e0447bd7b4444b6c96f9d/services/resource_coordinator/memory_instrumentation/graph_unittest.cc
Project Member

Comment 7 by bugdroid1@chromium.org, Oct 13 2017 

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

commit cb56bc09225d15fb6632ca8ad138a4bf8c635d3a
Author: Lalit Maganti <lalitm@chromium.org>
Date: Fri Oct 13 14:47:58 2017

memory-infra: mark implicit parents of only weak nodes as weak

If a node is a parent and all its children are weak and the parent itself
is implicit then mark it as weak.

Bug:  768373 
Change-Id: I9e61aa7231e68d15fcf3e0d72e8ed938545d0638
Reviewed-on: https://chromium-review.googlesource.com/702478
Commit-Queue: Lalit Maganti <lalitm@chromium.org>
Reviewed-by: Primiano Tucci <primiano@chromium.org>
Cr-Commit-Position: refs/heads/master@{#508699}
[modify] https://crrev.com/cb56bc09225d15fb6632ca8ad138a4bf8c635d3a/services/resource_coordinator/memory_instrumentation/graph.h
[modify] https://crrev.com/cb56bc09225d15fb6632ca8ad138a4bf8c635d3a/services/resource_coordinator/memory_instrumentation/graph_processor.cc
[modify] https://crrev.com/cb56bc09225d15fb6632ca8ad138a4bf8c635d3a/services/resource_coordinator/memory_instrumentation/graph_processor.h
[modify] https://crrev.com/cb56bc09225d15fb6632ca8ad138a4bf8c635d3a/services/resource_coordinator/memory_instrumentation/graph_processor_unittest.cc

Comment 10 by primiano@chromium.org, Oct 24 2017

Blocking: 777820

Comment 11 by lalitm@chromium.org, Oct 26 2017

Blocking: 775041
Project Member

Comment 15 by bugdroid1@chromium.org, Nov 23 2017 

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

commit 2545af303a021ab08851868f29b00005843290dd
Author: Lalit Maganti <lalitm@chromium.org>
Date: Thu Nov 23 16:05:46 2017

memory-infra: cleanup graph processing tests

The test code for the graph processor was becoming unweildy to write
because it was trying to set up graph from first principles. While good
for isolation of components, it also makes the tests significantly more
verbose (not to mention very brittle to changes).

Change the code to utilise existing functions.

Bug:  768373 
Change-Id: I6dd2f39a6590e46a0523f51c8ea44088fbec51da
Reviewed-on: https://chromium-review.googlesource.com/779262
Commit-Queue: Lalit Maganti <lalitm@chromium.org>
Reviewed-by: Primiano Tucci <primiano@chromium.org>
Cr-Commit-Position: refs/heads/master@{#518949}
[modify] https://crrev.com/2545af303a021ab08851868f29b00005843290dd/services/resource_coordinator/memory_instrumentation/graph_processor_unittest.cc

Comment 17 by lalitm@chromium.org, Nov 30 2017

Blocking: 703184
Components: Internals>Instrumentation>Memory

Comment 18 by lalitm@chromium.org, Nov 30 2017

Blocking: 728199
Project Member

Comment 19 by bugdroid1@chromium.org, Dec 5 2017 

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

commit b9c129fbe67c9caff6bce37e3792c99ffc605369
Author: Lalit Maganti <lalitm@chromium.org>
Date: Tue Dec 05 14:00:26 2017

memory-infra: fix post order iterator

The iterator should work on the scope of the entire graph as it relies
on shared visited set between processes to do its job.

Bug:  768373 
Change-Id: I2be92608c8d709464a32f8f35a10cd5686fc7af9
Reviewed-on: https://chromium-review.googlesource.com/788870
Reviewed-by: Primiano Tucci <primiano@chromium.org>
Commit-Queue: Lalit Maganti <lalitm@chromium.org>
Cr-Commit-Position: refs/heads/master@{#521684}
[modify] https://crrev.com/b9c129fbe67c9caff6bce37e3792c99ffc605369/services/resource_coordinator/memory_instrumentation/graph.cc
[modify] https://crrev.com/b9c129fbe67c9caff6bce37e3792c99ffc605369/services/resource_coordinator/memory_instrumentation/graph.h
[modify] https://crrev.com/b9c129fbe67c9caff6bce37e3792c99ffc605369/services/resource_coordinator/memory_instrumentation/graph_processor.cc
[modify] https://crrev.com/b9c129fbe67c9caff6bce37e3792c99ffc605369/services/resource_coordinator/memory_instrumentation/graph_unittest.cc

Comment 23 by lalitm@chromium.org, Dec 8 2017

Status: Fixed (was: Started)
With the above CL, I think we can mark this bug as complete! Work remains to incorporate metrics production and moving tracing over to this system, but this will be tracked in other bugs.
Project Member

Comment 24 by bugdroid1@chromium.org, Dec 19 2017 

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

commit 948a3fea9e7bc2dd864ac8bf7785705b528d44c1
Author: Lalit Maganti <lalitm@chromium.org>
Date: Tue Dec 19 19:32:39 2017

memory-infra: add code to serialize the process dump graph

Add the code to serialize the memory graph and also add a command line
flag to switch whether we want to utilise the graph when adding dump
info to tracing.

Bug:  768373 
Change-Id: I2667609b75d3cf339649913284a2fc1c12669853
Reviewed-on: https://chromium-review.googlesource.com/809129
Commit-Queue: Lalit Maganti <lalitm@chromium.org>
Reviewed-by: Hector Dearman <hjd@chromium.org>
Cr-Commit-Position: refs/heads/master@{#525111}
[modify] https://crrev.com/948a3fea9e7bc2dd864ac8bf7785705b528d44c1/services/resource_coordinator/BUILD.gn
[modify] https://crrev.com/948a3fea9e7bc2dd864ac8bf7785705b528d44c1/services/resource_coordinator/memory_instrumentation/coordinator_impl.cc
[modify] https://crrev.com/948a3fea9e7bc2dd864ac8bf7785705b528d44c1/services/resource_coordinator/memory_instrumentation/graph.cc
[modify] https://crrev.com/948a3fea9e7bc2dd864ac8bf7785705b528d44c1/services/resource_coordinator/memory_instrumentation/graph.h
[modify] https://crrev.com/948a3fea9e7bc2dd864ac8bf7785705b528d44c1/services/resource_coordinator/memory_instrumentation/queued_request_dispatcher.cc
[modify] https://crrev.com/948a3fea9e7bc2dd864ac8bf7785705b528d44c1/services/resource_coordinator/memory_instrumentation/queued_request_dispatcher.h
[add] https://crrev.com/948a3fea9e7bc2dd864ac8bf7785705b528d44c1/services/resource_coordinator/memory_instrumentation/switches.cc
[add] https://crrev.com/948a3fea9e7bc2dd864ac8bf7785705b528d44c1/services/resource_coordinator/memory_instrumentation/switches.h
[modify] https://crrev.com/948a3fea9e7bc2dd864ac8bf7785705b528d44c1/services/resource_coordinator/public/cpp/memory_instrumentation/tracing_observer.h
►

Sign in to add a comment