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

Issue 612779 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Last visit > 30 days ago
Closed: Jun 2016
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 2
Type: Feature

Blocking:
issue 612140
issue 616483



Sign in to add a comment

Provide means for determining why a memory dump failed

Project Member Reported by petrcermak@chromium.org, May 18 2016

Issue description

There is currently no way to determine why a memory dump failed (i.e. why it
returned success=false). This makes it difficult to investigate certain bugs,
e.g. https://bugs.chromium.org/p/chromium/issues/detail?id=612140:

  [ RUN      ] load:media:soundcloud
  (INFO) 2016-05-14 20:14:37,197 chrome_tracing_agent._CreateTraceConfigFile:243  Trace config file string: {"trace_config":{"excluded_categories": ["*"], "included_categories": ["disabled-by-default-memory-infra"], "record_mode": "record-as-much-as-possible"}}
  (INFO) 2016-05-14 20:14:37,205 tracing_backend.StartTracing:124  Start Tracing Request: {'params': {'transferMode': 'ReturnAsStream', 'options': 'record-as-much-as-possible', 'categories': 'disabled-by-default-memory-infra,-*'}, 'method': 'Tracing.start'}
  (INFO) 2016-05-14 20:14:37,210 cache_temperature.EnsurePageCacheTemperature:29  PageCacheTemperature: any
  (ERROR) 2016-05-14 20:14:46,014 system_health_stories._TakeMemoryMeasurement:319  Unable to get a memory dump for load:media:soundcloud. <=== NO IDEA WHY?!
  ...
  [  FAILED  ] load:media:soundcloud (8915 ms)

This essentially boils down to:

  1. Logging warning messages in Chrome in the following places (there might be
     more that I don't know about):

     https://code.google.com/p/chromium/codesearch#chromium/src/base/trace_event/memory_dump_manager.cc&l=441&rcl=1463541638
     https://code.google.com/p/chromium/codesearch#chromium/src/base/trace_event/memory_dump_manager.cc&l=606&rcl=1463541638
     https://code.google.com/p/chromium/codesearch#chromium/src/content/browser/tracing/tracing_controller_impl.cc&l=928&rcl=1463541638
     https://code.google.com/p/chromium/codesearch#chromium/src/base/trace_event/memory_dump_manager.cc&l=357&rcl=1463541638

  2. Extracting Chrome's error/warning messages in Telemetry.
 
Thanks to petrcermak:
1 has been addressed in https://codereview.chromium.org/2049143002/
2 is being addressed in https://codereview.chromium.org/2050743002/

If there isn't anything left here please close the bug.
Blocking: 616483
Extracting the error messages in Telemetry actually turns out to be a little more tricky (see https://bugs.chromium.org/p/chromium/issues/detail?id=616483#c21), but I think I've finally figured out how to solve this: https://codereview.chromium.org/2053313002/
Cc: primiano@chromium.org
Owner: petrcermak@chromium.org
Status: Started (was: Untriaged)
Project Member

Comment 5 by bugdroid1@chromium.org, Jun 10 2016

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

commit cbd90f2b8b145f79e09cb0848c6d34df214898aa
Author: catapult-deps-roller <catapult-deps-roller@chromium.org>
Date: Fri Jun 10 14:43:37 2016

Roll src/third_party/catapult/ 2c5cde737..e37097a1c (1 commit).

https://chromium.googlesource.com/external/github.com/catapult-project/catapult.git/+log/2c5cde737888..e37097a1cbf9

$ git log 2c5cde737..e37097a1c --date=short --no-merges --format='%ad %ae %s'

BUG= 616483 , 612779 

TBR=catapult-sheriff@chromium.org

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

[modify] https://crrev.com/cbd90f2b8b145f79e09cb0848c6d34df214898aa/DEPS

Whenever a memory dump fails, Chrome now dumps a VLOG(1) message: https://codereview.chromium.org/2049143002/ (use "--enable-logging -v" flags).

This helped us figure out the underlying cause of  issue 616483  in Telemetry's unit tests.

nednguyen: I think that we should extend the approach of enabling logging and printing the log to *all* browser unit tests in Telemetry. WDYT?
Cc: kbr@chromium.org eyaich@google.com
Petr: sgtm. The base of all browser unit tests in telemetry is BrowserTestCase: https://github.com/catapult-project/catapult/blob/master/telemetry/telemetry/testing/browser_test_case.py#L26

I think the way to do this is probably extend AppCrashException in https://github.com/catapult-project/catapult/blob/master/telemetry/telemetry/core/exceptions.py#L54 to include "log". Then we add GetLog() api to the app API (default return None) & return the actual log when the browser's log is enabled.

Note that I chose not to enable browser's log by default due to the worry that it would affect performance. For telemetry unittest though, it's probably fine.

+kbr@, eyaich@ who may also fine this relevant.
Project Member

Comment 8 by bugdroid1@chromium.org, Jun 15 2016

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

commit cbd90f2b8b145f79e09cb0848c6d34df214898aa
Author: catapult-deps-roller <catapult-deps-roller@chromium.org>
Date: Fri Jun 10 14:43:37 2016

Roll src/third_party/catapult/ 2c5cde737..e37097a1c (1 commit).

https://chromium.googlesource.com/external/github.com/catapult-project/catapult.git/+log/2c5cde737888..e37097a1cbf9

$ git log 2c5cde737..e37097a1c --date=short --no-merges --format='%ad %ae %s'

BUG= 616483 , 612779 

TBR=catapult-sheriff@chromium.org

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

[modify] https://crrev.com/cbd90f2b8b145f79e09cb0848c6d34df214898aa/DEPS

#7: I don't think that AppCrashException is the right thing to extend. From what I can tell, it's only thrown in cases such as browser crashes, which is not always the case. For example, the flakiness in issue 646483 was an assertion error.

I think that the simplest way to achieve the desired outcome (browser stdout and log printed upon all browser test failures) would be to add a metaclass for BrowserTestCase and wrap all test* methods.
#9: I think you're right. Though I am not sure whether it's easy to wrap all test* method.
Status: Fixed (was: Started)
Done: https://codereview.chromium.org/2072713003/ (should roll into Chromium in a matter of minutes/hours).
Project Member

Comment 12 by bugdroid1@chromium.org, Jun 16 2016

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

commit a001c048c23ec80e2105afbc6f3d7da01a69eb5b
Author: catapult-deps-roller <catapult-deps-roller@chromium.org>
Date: Thu Jun 16 16:06:40 2016

Roll src/third_party/catapult/ d1858050f..752a1c413 (1 commit).

https://chromium.googlesource.com/external/github.com/catapult-project/catapult.git/+log/d1858050f3e8..752a1c413f80

$ git log d1858050f..752a1c413 --date=short --no-merges --format='%ad %ae %s'

BUG= 612779 

TBR=catapult-sheriff@chromium.org

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

[modify] https://crrev.com/a001c048c23ec80e2105afbc6f3d7da01a69eb5b/DEPS

Components: Internals>Instrumentation>Memory
Components: -Internals>Tracing

Sign in to add a comment