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

Issue 753514 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Jan 2018
Cc:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug

Blocking:
issue 751321



Sign in to add a comment

memlog: Hook into Slow reports so we can start getting data from field

Project Member Reported by ajwong@chromium.org, Aug 8 2017

Issue description

Tracing work needed to get into slow reports so we can enable for google usage as initial test set.
 
Project Member

Comment 1 by bugdroid1@chromium.org, Aug 24 2017

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

commit fc034266f0c538f1fb70baf6e3b8a7081e20ad20
Author: Etienne Bergeron <etienneb@chromium.org>
Date: Thu Aug 24 16:43:56 2017

Upload memory trace manually triggered to the crash service

This patch is adding a button for manually triggering a slow-report.
The browser process is requesting a heap profiling dump to the profiling
service and waiting for the callback.

The next step is to add uploading of the trace to the crash server
(slow-reports).


R=erikchen@chromium.org
TBR=dcheng@chromium.org

Bug:  753514 
Cq-Include-Trybots: master.tryserver.chromium.linux:closure_compilation
Change-Id: I5f8f3ff14aa0f71a8e79d5511b3fbaaa09c6e477
Reviewed-on: https://chromium-review.googlesource.com/628818
Commit-Queue: Erik Chen <erikchen@chromium.org>
Reviewed-by: Erik Chen <erikchen@chromium.org>
Reviewed-by: Brett Wilson <brettw@chromium.org>
Cr-Commit-Position: refs/heads/master@{#497084}
[modify] https://crrev.com/fc034266f0c538f1fb70baf6e3b8a7081e20ad20/chrome/browser/profiling_host/profiling_process_host.cc
[modify] https://crrev.com/fc034266f0c538f1fb70baf6e3b8a7081e20ad20/chrome/browser/profiling_host/profiling_process_host.h
[modify] https://crrev.com/fc034266f0c538f1fb70baf6e3b8a7081e20ad20/chrome/browser/resources/memory_internals.js
[modify] https://crrev.com/fc034266f0c538f1fb70baf6e3b8a7081e20ad20/chrome/browser/ui/webui/memory_internals_ui.cc
[modify] https://crrev.com/fc034266f0c538f1fb70baf6e3b8a7081e20ad20/chrome/common/profiling/memlog.mojom
[modify] https://crrev.com/fc034266f0c538f1fb70baf6e3b8a7081e20ad20/chrome/profiling/memlog_connection_manager.cc
[modify] https://crrev.com/fc034266f0c538f1fb70baf6e3b8a7081e20ad20/chrome/profiling/memlog_connection_manager.h
[modify] https://crrev.com/fc034266f0c538f1fb70baf6e3b8a7081e20ad20/chrome/profiling/memlog_impl.cc
[modify] https://crrev.com/fc034266f0c538f1fb70baf6e3b8a7081e20ad20/chrome/profiling/memlog_impl.h

Project Member

Comment 2 by bugdroid1@chromium.org, Aug 30 2017

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

commit 8c47d79c9efb640be7dd524c736be59f90bfe2a7
Author: Etienne Bergeron <etienneb@chromium.org>
Date: Wed Aug 30 17:42:30 2017

Add uploading of manually triggered trace to crash (slow-report)

This CL is adding the call to the trace uploader.
The uploader returns a crash-id which can be used to retrieve the crash/trace.

Should we bring back the crash-id to the user/UI?
Should we add metadata to bring more information on the trigger?

R=erikchen@chromium.org, brettw@chromium.org

Bug:  753514 
Change-Id: Ib16998a3957ba7f5e8c0cd1cf2fe3eb72be3d6e6
Reviewed-on: https://chromium-review.googlesource.com/642111
Reviewed-by: Erik Chen <erikchen@chromium.org>
Reviewed-by: Brett Wilson <brettw@chromium.org>
Commit-Queue: Etienne Bergeron <etienneb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#498528}
[modify] https://crrev.com/8c47d79c9efb640be7dd524c736be59f90bfe2a7/chrome/browser/profiling_host/profiling_process_host.cc

Project Member

Comment 3 by bugdroid1@chromium.org, Sep 6 2017

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

commit e8cd0bfad15d6d64a55ced69d3e03ccac163b953
Author: Etienne Bergeron <etienneb@chromium.org>
Date: Wed Sep 06 00:16:19 2017

Add periodic checks for triggering a memory dump

R=brettw@chromium.org, erikchen@chromium.org

Bug:  753514 
Change-Id: I2ff72d92a78c4b3c2d541309b2e2a749eb839ce8
Reviewed-on: https://chromium-review.googlesource.com/644117
Commit-Queue: Etienne Bergeron <etienneb@chromium.org>
Reviewed-by: Erik Chen <erikchen@chromium.org>
Reviewed-by: Brett Wilson <brettw@chromium.org>
Cr-Commit-Position: refs/heads/master@{#499813}
[modify] https://crrev.com/e8cd0bfad15d6d64a55ced69d3e03ccac163b953/chrome/browser/profiling_host/BUILD.gn
[add] https://crrev.com/e8cd0bfad15d6d64a55ced69d3e03ccac163b953/chrome/browser/profiling_host/background_profiling_triggers.cc
[add] https://crrev.com/e8cd0bfad15d6d64a55ced69d3e03ccac163b953/chrome/browser/profiling_host/background_profiling_triggers.h
[add] https://crrev.com/e8cd0bfad15d6d64a55ced69d3e03ccac163b953/chrome/browser/profiling_host/background_profiling_triggers_unittest.cc
[modify] https://crrev.com/e8cd0bfad15d6d64a55ced69d3e03ccac163b953/chrome/browser/profiling_host/profiling_process_host.cc
[modify] https://crrev.com/e8cd0bfad15d6d64a55ced69d3e03ccac163b953/chrome/browser/profiling_host/profiling_process_host.h
[modify] https://crrev.com/e8cd0bfad15d6d64a55ced69d3e03ccac163b953/chrome/test/BUILD.gn

Project Member

Comment 4 by bugdroid1@chromium.org, Sep 7 2017

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

commit aa7726c0ac6fd261d1cd9932fb5e4e6e51c43676
Author: erikchen <erikchen@chromium.org>
Date: Thu Sep 07 00:23:38 2017

Add metadata to memlog uploaded trace

These fields are mandatory to get a valid slow-report (crash) with the
|special_protos.background_tracing_config| protobuf.

For now, we do not add any trigger rules. This could be added later when
we have more than one trigger rules.

R=erikchen@chromium.org, brettw@chromium.org

Bug:  753514 
Change-Id: I99974eab03c7a89bbcb4f4558f1528503d757e0f
Reviewed-on: https://chromium-review.googlesource.com/654139
Commit-Queue: Erik Chen <erikchen@chromium.org>
Reviewed-by: Brett Wilson <brettw@chromium.org>
Reviewed-by: Erik Chen <erikchen@chromium.org>
Cr-Commit-Position: refs/heads/master@{#500153}
[modify] https://crrev.com/aa7726c0ac6fd261d1cd9932fb5e4e6e51c43676/chrome/browser/profiling_host/profiling_process_host.cc

Project Member

Comment 5 by bugdroid1@chromium.org, Sep 8 2017

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

commit 81e64c831b9e91aa9317ed4500829223d4eeb67b
Author: Etienne Bergeron <etienneb@chromium.org>
Date: Fri Sep 08 21:18:05 2017

Fix missing trace header events for catapult

Slow report we are getting from memlog are not processed correctly by
catapult metrics extractor. The browser is not recognized correctly
which leads to incorrect metric names. (unknown_browser)

i.e.:
  memory:unknown_browser:all_processes:dump_count:detailed

R=erikchen@chromium.org, brettw@chromium.org

Bug:  753514 
Change-Id: Iced6d20bd5aa195a2abeadbc99e7676436a80f1a
Reviewed-on: https://chromium-review.googlesource.com/657301
Commit-Queue: Etienne Bergeron <etienneb@chromium.org>
Reviewed-by: Erik Chen <erikchen@chromium.org>
Reviewed-by: Brett Wilson <brettw@chromium.org>
Cr-Commit-Position: refs/heads/master@{#500674}
[modify] https://crrev.com/81e64c831b9e91aa9317ed4500829223d4eeb67b/chrome/profiling/json_exporter.cc
[modify] https://crrev.com/81e64c831b9e91aa9317ed4500829223d4eeb67b/chrome/profiling/json_exporter_unittest.cc

Project Member

Comment 6 by bugdroid1@chromium.org, Sep 8 2017

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

commit 8ec3364dfc85c58fc9129bdd8c99ae11c5983947
Author: Etienne Bergeron <etienneb@chromium.org>
Date: Fri Sep 08 23:26:27 2017

Add metadata to specify which trigger caused a trace upload

This CL is adding metadata to the trace so we can determine which trigger
caused a slow-report to be uploaded.

R=brettw@chromium.org, erikchen@chromium.org

Bug:  753514 
Change-Id: Idff248014d5265910b450f059b64ee73604005e6
Reviewed-on: https://chromium-review.googlesource.com/657921
Reviewed-by: Erik Chen <erikchen@chromium.org>
Reviewed-by: Siddhartha S <ssid@chromium.org>
Reviewed-by: Brett Wilson <brettw@chromium.org>
Commit-Queue: Erik Chen <erikchen@chromium.org>
Cr-Commit-Position: refs/heads/master@{#500737}
[modify] https://crrev.com/8ec3364dfc85c58fc9129bdd8c99ae11c5983947/chrome/browser/profiling_host/background_profiling_triggers.cc
[modify] https://crrev.com/8ec3364dfc85c58fc9129bdd8c99ae11c5983947/chrome/browser/profiling_host/profiling_process_host.cc
[modify] https://crrev.com/8ec3364dfc85c58fc9129bdd8c99ae11c5983947/chrome/browser/profiling_host/profiling_process_host.h
[modify] https://crrev.com/8ec3364dfc85c58fc9129bdd8c99ae11c5983947/chrome/browser/ui/webui/memory_internals_ui.cc

Project Member

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

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

commit 0a7c55b418dcb1e2001cf87fc167d8ee7f78a767
Author: Etienne Bergeron <etienneb@chromium.org>
Date: Wed Sep 13 00:06:19 2017

Fix incorrect computation of size exported to JSON traces.

The outputted size is not the size of each allocation, but the sum of
every allocation for a given stackframe.

R=erikchen@chromium.org

Bug:  753514 
Change-Id: I1840f89d06a63e5f30af66da080ef7ea91d1715a
Reviewed-on: https://chromium-review.googlesource.com/664250
Reviewed-by: Erik Chen <erikchen@chromium.org>
Reviewed-by: Brett Wilson <brettw@chromium.org>
Commit-Queue: Erik Chen <erikchen@chromium.org>
Cr-Commit-Position: refs/heads/master@{#501465}
[modify] https://crrev.com/0a7c55b418dcb1e2001cf87fc167d8ee7f78a767/chrome/profiling/json_exporter.cc
[modify] https://crrev.com/0a7c55b418dcb1e2001cf87fc167d8ee7f78a767/chrome/profiling/json_exporter_unittest.cc

Project Member

Comment 8 by bugdroid1@chromium.org, Sep 19 2017

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

commit dd771615bd85f169e21c618d4243eb967d162b0b
Author: Etienne Bergeron <etienneb@chromium.org>
Date: Tue Sep 19 20:42:01 2017

Adjust background triggers threshold from metrics from the field

This CL is lowering down the thresholds to trigger a detailed memory dump.
Android is now using a different set of metrics.
These metrics are based on recent memory usage for canary users.

Bug:  753514 
Change-Id: I3a56d150d29f027fcc954e4fa1228670a6db7b5e
Reviewed-on: https://chromium-review.googlesource.com/671768
Reviewed-by: Erik Chen <erikchen@chromium.org>
Commit-Queue: Etienne Bergeron <etienneb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#502941}
[modify] https://crrev.com/dd771615bd85f169e21c618d4243eb967d162b0b/chrome/browser/profiling_host/background_profiling_triggers.cc

Status: Fixed (was: Assigned)

Sign in to add a comment