New issue
Advanced search Search tips

Issue 872900 link

Starred by 1 user

Issue metadata

Status: Closed
Owner:
Closed: Sep 20
Cc:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug



Sign in to add a comment

Add disk performance to slow-reports trace

Project Member Reported by etienneb@chromium.org, Aug 9

Issue description

We suspect disk usage to be the cause of many janks observed on slow-reports.

  crbug/868419
  crbug/868372
  crbug/867972

We want to add the disk usage counters to the trace to validate our hypothesis.

The plan is to use |NtQuerySystemInformation| to retrieve |SystemPerformanceInformation| and append them to the trace under the category |system_stats|.

see: https://docs.microsoft.com/en-us/windows/desktop/api/winternl/nf-winternl-ntquerysysteminformation
 
Project Member

Comment 1 by bugdroid1@chromium.org, Aug 10

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

commit 657073af9e82fa2400eb4f384207b555f66d83ea
Author: Etienne Bergeron <etienneb@chromium.org>
Date: Fri Aug 10 20:39:48 2018

Add system stats to navigation category in slow-reports

This CL is activating the "system_stats" category for the navigation
slow-reports.

The idea is to collect more system wide information to determine the
cause of janks. We believe many janks are related to system wide
properties not visible without system wide metrics.

R=fdoray,oysteine

Bug:  872900 
Change-Id: I2151616b07b79997b20243181eb31c5270c4ee68
Reviewed-on: https://chromium-review.googlesource.com/1171109
Reviewed-by: oysteine <oysteine@chromium.org>
Commit-Queue: Etienne Bergeron <etienneb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#582319}
[modify] https://crrev.com/657073af9e82fa2400eb4f384207b555f66d83ea/content/browser/tracing/background_tracing_manager_impl.cc

Description: Show this description
Project Member

Comment 3 by bugdroid1@chromium.org, Aug 15

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

commit 0b9d2b69455485af0fee122074fd68affb0434f7
Author: Etienne Bergeron <etienneb@chromium.org>
Date: Wed Aug 15 20:34:10 2018

Add windows performance counters

This CL is adding the system performance counters on Windows.

A way to compute the system IO activity is to sum IO counters for
every processes. On windows, the GetIoCounters(...) call returns
counters by process. Using this technique has two drawbacks
  1) an expensive processes enumeration is required
  2) processes may disappear during the sample slice, which
     is leading to inaccurate metric.

We are proposing to call a ntdll API which returns system wide
counters. The operating system is keeping system wide counters
for IO accesses through ZwReadFile and ZwWriteFile.

see:
https://docs.microsoft.com/en-us/windows/desktop/api/winternl/nf-winternl-ntquerysysteminformation

  "Returns an opaque SYSTEM_PERFORMANCE_INFORMATION structure that can be used to generate an
   unpredictable seed for a random number generator. "

Layout:
  https://cs.chromium.org/chromium/src/third_party/perl/c/i686-w64-mingw32/include/winternl.h?type=cs&l=609
  https://processhacker.sourceforge.io/doc/struct___s_y_s_t_e_m___p_e_r_f_o_r_m_a_n_c_e___i_n_f_o_r_m_a_t_i_o_n.html


To see these metrics, you can activate "system_stats" category and grab a trace.
A sample is collected every two seconds and will be displayed in the tracing UI.

Next step is to modify catapult to convert these tracing tracks as rate instead than raw counters.

see:
  catapult/tracing/tracing/ui/extras/system_stats/system_stats_instance_track.html
Change-Id: I45b6b04192c78dd5a0dc95eca0c2d357e74ca285


R=fdoray, gab

Bug:  872900 
Change-Id: I45b6b04192c78dd5a0dc95eca0c2d357e74ca285
Reviewed-on: https://chromium-review.googlesource.com/1169871
Reviewed-by: Bruce Dawson <brucedawson@chromium.org>
Reviewed-by: François Doray <fdoray@chromium.org>
Reviewed-by: Gabriel Charette <gab@chromium.org>
Commit-Queue: Etienne Bergeron <etienneb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#583374}
[modify] https://crrev.com/0b9d2b69455485af0fee122074fd68affb0434f7/base/process/process_metrics.cc
[modify] https://crrev.com/0b9d2b69455485af0fee122074fd68affb0434f7/base/process/process_metrics.h
[modify] https://crrev.com/0b9d2b69455485af0fee122074fd68affb0434f7/base/process/process_metrics_win.cc

Project Member

Comment 4 by bugdroid1@chromium.org, Aug 16

The following revision refers to this bug:
  https://chromium.googlesource.com/catapult/+/ccd130b9a2bdd7dc519ada5f7a04ea7097012444

commit ccd130b9a2bdd7dc519ada5f7a04ea7097012444
Author: Etienne Bergeron <etienneb@chromium.org>
Date: Thu Aug 16 15:57:00 2018

Add windows performance counters processing.

This CL is adding the processing of the system metrics provided by
windows API.

screenshot: https://imgur.com/a/5XSUy14

see: https://chromium-review.googlesource.com/c/chromium/src/+/1169871

These metrics are collected by chrome://tracing under the category
'system_stats'.

R=benjhayden@chromium.org

Bug:  chromium:872900 
Change-Id: I3449bff6800c34f307550755ab749b15c2b5bc56
Reviewed-on: https://chromium-review.googlesource.com/1171107
Commit-Queue: Etienne Bergeron <etienneb@chromium.org>
Reviewed-by: Ben Hayden <benjhayden@chromium.org>

[modify] https://crrev.com/ccd130b9a2bdd7dc519ada5f7a04ea7097012444/tracing/tracing/ui/extras/system_stats/system_stats_instance_track.html
[modify] https://crrev.com/ccd130b9a2bdd7dc519ada5f7a04ea7097012444/tracing/tracing/ui/extras/system_stats/system_stats_instance_track_test.html
[modify] https://crrev.com/ccd130b9a2bdd7dc519ada5f7a04ea7097012444/tracing/tracing/extras/system_stats/system_stats_snapshot.html

Project Member

Comment 5 by bugdroid1@chromium.org, Aug 16

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

commit c0cd880d09098de5f6569b6cd4ce98b13ba7048d
Author: catapult-chromium-autoroll <catapult-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com>
Date: Thu Aug 16 18:21:41 2018

Roll src/third_party/catapult 922ba81b497b..ed63b1319414 (4 commits)

https://chromium.googlesource.com/catapult.git/+log/922ba81b497b..ed63b1319414


git log 922ba81b497b..ed63b1319414 --date=short --no-merges --format='%ad %ae %s'
2018-08-16 nednguyen@google.com Add retry for ts_proxy commands upon raised exception
2018-08-16 perezju@chromium.org [Telemetry] Add story expectations for Pixel 2 bots
2018-08-16 etienneb@chromium.org Add windows performance counters processing.
2018-08-16 nednguyen@google.com Make ts_proxy error handleable by Telemetry story_runner


Created with:
  gclient setdep -r src/third_party/catapult@ed63b1319414

The AutoRoll server is located here: https://catapult-roll.skia.org

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+/master/autoroll/README.md

If the roll is causing failures, please contact the current sheriff, who should
be CC'd on the roll, and stop the roller if necessary.

CQ_INCLUDE_TRYBOTS=luci.chromium.try:android_optional_gpu_tests_rel;luci.chromium.try:linux_optional_gpu_tests_rel;luci.chromium.try:mac_optional_gpu_tests_rel;luci.chromium.try:win_optional_gpu_tests_rel

BUG= chromium:873616 , chromium:874391 , chromium:872900 , chromium:873616 
TBR=sullivan@chromium.org

Change-Id: Ia5d6e592153afef2fee1316ec25f3d09c44b0b90
Reviewed-on: https://chromium-review.googlesource.com/1178241
Reviewed-by: catapult-chromium-autoroll <catapult-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com>
Commit-Queue: catapult-chromium-autoroll <catapult-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com>
Cr-Commit-Position: refs/heads/master@{#583741}
[modify] https://crrev.com/c0cd880d09098de5f6569b6cd4ce98b13ba7048d/DEPS

Project Member

Comment 6 by bugdroid1@chromium.org, Aug 27

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

commit b546594642a24addde1bb9022257ff96f82ef0f7
Author: Etienne Bergeron <etienneb@chromium.org>
Date: Mon Aug 27 15:23:33 2018

Add average disk usage for browser process to performance monitor

This CL is adding the average disk usage (bytes transfered by seconds)
to base::process_metrics. This metric is sampled by the performance
monitor every 2 minutes and an UMA metric
(e.g. PerformanceMonitor.AverageDisk.BrowserProcess) is produced.

The current implementation is only working for windows. Other platforms
need to implement |GetCumulativeDiskUsage|.

The purpose of this metric is to allow adding slow-report triggers based
on disk usage.

R=fdoray, gab, oysteine

Bug:  872900 
Change-Id: I168a066cf5ebb3b0e879f8bbc5cc73740de1e938
Reviewed-on: https://chromium-review.googlesource.com/1174904
Commit-Queue: Etienne Bergeron <etienneb@chromium.org>
Reviewed-by: François Doray <fdoray@chromium.org>
Reviewed-by: Robert Kaplow (slow) <rkaplow@chromium.org>
Reviewed-by: kylechar <kylechar@chromium.org>
Reviewed-by: oysteine <oysteine@chromium.org>
Cr-Commit-Position: refs/heads/master@{#586269}
[modify] https://crrev.com/b546594642a24addde1bb9022257ff96f82ef0f7/base/process/process_metrics.cc
[modify] https://crrev.com/b546594642a24addde1bb9022257ff96f82ef0f7/base/process/process_metrics.h
[modify] https://crrev.com/b546594642a24addde1bb9022257ff96f82ef0f7/base/process/process_metrics_unittest.cc
[modify] https://crrev.com/b546594642a24addde1bb9022257ff96f82ef0f7/base/process/process_metrics_win.cc
[modify] https://crrev.com/b546594642a24addde1bb9022257ff96f82ef0f7/chrome/browser/performance_monitor/process_metrics_history.cc
[modify] https://crrev.com/b546594642a24addde1bb9022257ff96f82ef0f7/chrome/browser/performance_monitor/process_metrics_history.h
[modify] https://crrev.com/b546594642a24addde1bb9022257ff96f82ef0f7/tools/metrics/histograms/histograms.xml

Status: Closed (was: Assigned)

Sign in to add a comment