New issue
Advanced search Search tips

Issue 843561 link

Starred by 5 users

Issue metadata

Status: Fixed
Owner:
Closed: Jul 25
Cc:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 2
Type: Bug



Sign in to add a comment

Add telemetry benchmarks for use with orderfile generation

Project Member Reported by mattcary@chromium.org, May 16 2018

Issue description

The orderfile reordering project aims to reduce the chrome native library footprint on android by more sophisticated instrumentation when creating the orderfile [1].

Current orderfile generation uses a custom benchmark based on catapult/devil [2]. For both generating the orderfile as well as for benchmarking its performance, we would like to add extended benchmarks.

One class of extensions adds user interaction to benchmarks, such as swiping the screen, to create benchmarks that cover more code that is (almost) certain to be used in practical situations. 

The other modifies the benchmarks to play nicely with the instrumented code. Mostly this means giving an instrumented version of chrome extra time to dump code coverage.

[1] 
https://docs.google.com/document/d/1dAt_puAX4SyqXDOdglxe6ku9GwnyBiOMjHAsDNwN0HY/edit#

[2]
tools/cygprofile/profile_android_startup.py

 
Cc: pasko@chromium.org lizeb@chromium.org
After offline discussion with Juan, we've agreed with the telemetry team to

(1) Use the system_health.memory_mobile benchmarks, which already includes interaction. So additional benchmarks do not need to be added.

(2) Use the devtools memory dump request as a signal for the instrumentation to dump profile. For the time being we will continue to use a time delay on the startup profiling.
Project Member

Comment 3 by bugdroid1@chromium.org, Jun 11 2018

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

commit b8daed94b89de7ec0fa503397241f0dd83ce3695
Author: Matthew Cary <mattcary@chromium.org>
Date: Mon Jun 11 10:58:08 2018

Orderfile: move instrumentation from tools to base.

As part of  crbug.com/843561 , some control over instrumentation is being exposed
to telemetry. To facilitate that, this CL moves orderfile instrumentation from
tools/ to base/android. This will make instrumentation control (such as Dump())
accessible from content/ and devtools tracing, allowing telemetry benchmarks
to trigger instrumented profile dumps when memory traces are requested.

All of this is part of the project to improve android orderfile
generation, which reduces the memory footprint of chrome by ~10MB (details
in [1]).

[1] https://docs.google.com/document/d/1dAt_puAX4SyqXDOdglxe6ku9GwnyBiOMjHAsDNwN0HY/edit#


Bug:  843561 
Change-Id: I9973857fac36953935906cb908b3ffd6efa8185e
Reviewed-on: https://chromium-review.googlesource.com/1078427
Reviewed-by: Tobias Sargeant <tobiasjs@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Reviewed-by: David Turner <digit@chromium.org>
Reviewed-by: Antoine Labour <piman@chromium.org>
Reviewed-by: Dirk Pranke <dpranke@chromium.org>
Reviewed-by: Bernhard Bauer <bauerb@chromium.org>
Reviewed-by: agrieve <agrieve@chromium.org>
Reviewed-by: Benoit L <lizeb@chromium.org>
Commit-Queue: Matthew Cary <mattcary@chromium.org>
Cr-Commit-Position: refs/heads/master@{#565958}
[modify] https://crrev.com/b8daed94b89de7ec0fa503397241f0dd83ce3695/android_webview/BUILD.gn
[modify] https://crrev.com/b8daed94b89de7ec0fa503397241f0dd83ce3695/base/BUILD.gn
[modify] https://crrev.com/b8daed94b89de7ec0fa503397241f0dd83ce3695/base/android/library_loader/library_loader_hooks.cc
[modify] https://crrev.com/b8daed94b89de7ec0fa503397241f0dd83ce3695/base/android/library_loader/library_prefetcher.cc
[add] https://crrev.com/b8daed94b89de7ec0fa503397241f0dd83ce3695/base/android/orderfile/BUILD.gn
[rename] https://crrev.com/b8daed94b89de7ec0fa503397241f0dd83ce3695/base/android/orderfile/orderfile_instrumentation.cc
[rename] https://crrev.com/b8daed94b89de7ec0fa503397241f0dd83ce3695/base/android/orderfile/orderfile_instrumentation.h
[rename] https://crrev.com/b8daed94b89de7ec0fa503397241f0dd83ce3695/base/android/orderfile/orderfile_instrumentation_perftest.cc
[modify] https://crrev.com/b8daed94b89de7ec0fa503397241f0dd83ce3695/build/config/BUILD.gn
[modify] https://crrev.com/b8daed94b89de7ec0fa503397241f0dd83ce3695/build/config/BUILDCONFIG.gn
[modify] https://crrev.com/b8daed94b89de7ec0fa503397241f0dd83ce3695/build/config/android/BUILD.gn
[modify] https://crrev.com/b8daed94b89de7ec0fa503397241f0dd83ce3695/chrome/android/BUILD.gn
[modify] https://crrev.com/b8daed94b89de7ec0fa503397241f0dd83ce3695/content/browser/gpu/browser_gpu_channel_host_factory.cc
[modify] https://crrev.com/b8daed94b89de7ec0fa503397241f0dd83ce3695/tools/android/md5sum/BUILD.gn
[delete] https://crrev.com/afd059ad87eb7e45a6bf154f0d91d32d13e72222/tools/cygprofile/BUILD.gn
[delete] https://crrev.com/afd059ad87eb7e45a6bf154f0d91d32d13e72222/tools/cygprofile/delayed_dumper.cc
[modify] https://crrev.com/b8daed94b89de7ec0fa503397241f0dd83ce3695/tools/cygprofile/orderfile_generator_backend.py
[modify] https://crrev.com/b8daed94b89de7ec0fa503397241f0dd83ce3695/tools/cygprofile/profile_android_startup.py

Project Member

Comment 4 by bugdroid1@chromium.org, Jun 21 2018

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

commit f25fc5924e8c1ad91493607d18d19af218aa7580
Author: Matthew Cary <mattcary@chromium.org>
Date: Thu Jun 21 10:35:27 2018

Cygprofile: Telemetry benchmarking support.

The Clank orderfile is being improved as part of an effort to improve
Clank memory usage [1]. One step in this effort is to give telemetry
benchmarks more control over instrumentation profiling.

In this CL I extend the devtools RequestMemoryDump command to also
tell instrumentation support to dump the current profile. This only
happens if chrome is compiled with instrumentation turned on
(use_order_profiling=true). The instrumentation dump is done at the
same time as the memory dump is handled on the client side.

[1] https://docs.google.com/document/d/1dAt_puAX4SyqXDOdglxe6ku9GwnyBiOMjHAsDNwN0HY/edit#

Bug:  843561 
Change-Id: I6d08e1cd24dc6f16ca12f8fa519ab9725d3ed914
Reviewed-on: https://chromium-review.googlesource.com/1087959
Commit-Queue: Matthew Cary <mattcary@chromium.org>
Reviewed-by: agrieve <agrieve@chromium.org>
Reviewed-by: Primiano Tucci <primiano@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Reviewed-by: Benoit L <lizeb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#569210}
[modify] https://crrev.com/f25fc5924e8c1ad91493607d18d19af218aa7580/base/BUILD.gn
[modify] https://crrev.com/f25fc5924e8c1ad91493607d18d19af218aa7580/base/android/orderfile/orderfile_instrumentation.cc
[modify] https://crrev.com/f25fc5924e8c1ad91493607d18d19af218aa7580/base/android/orderfile/orderfile_instrumentation.h
[modify] https://crrev.com/f25fc5924e8c1ad91493607d18d19af218aa7580/build/config/android/abi.gni

Project Member

Comment 5 by bugdroid1@chromium.org, Jun 25 2018

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

commit f26c826ab06f26c9d951617c56fa2cd03e5582fe
Author: Matthew Cary <mattcary@chromium.org>
Date: Mon Jun 25 14:18:32 2018

Orderfile: do not instrument MemoryDumpProvider.

When compiled with devtools_instrumentation_dumping=true, orderfile
profile dumps are triggered by MemoryDumpProvider::OnMemoryDump. We mark
the overriden function used to control the orderfile as
not-to-be-instrumented in order to keep the profile as clean as
possible.

Bug:  843561 
Change-Id: I710029753d776fac182b16fcce8aa6c7f5738f83
Reviewed-on: https://chromium-review.googlesource.com/1109970
Reviewed-by: Benoit L <lizeb@chromium.org>
Reviewed-by: Egor Pasko <pasko@chromium.org>
Reviewed-by: agrieve <agrieve@chromium.org>
Commit-Queue: agrieve <agrieve@chromium.org>
Cr-Commit-Position: refs/heads/master@{#570031}
[modify] https://crrev.com/f26c826ab06f26c9d951617c56fa2cd03e5582fe/base/android/orderfile/orderfile_instrumentation.cc

Project Member

Comment 6 by bugdroid1@chromium.org, Jul 3

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

commit dc55839a93b56bf3c15f52b0fcf3ebdd5d579a07
Author: Matthew Cary <mattcary@chromium.org>
Date: Tue Jul 03 12:43:04 2018

Add tools/perf/contrib/orderfile/.

This will be used for orderfile generation benchmarks. More precisely,
they are thin wrappers around the system health benchmarks that make it
more convenient to run orderfile generation.

Bug:  843561 
Change-Id: I5d3a2288d4f4a2f74004ef90f3cec0e4b44c7198
Reviewed-on: https://chromium-review.googlesource.com/1124161
Reviewed-by: Ned Nguyen <nednguyen@google.com>
Commit-Queue: Matthew Cary <mattcary@chromium.org>
Cr-Commit-Position: refs/heads/master@{#572181}
[add] https://crrev.com/dc55839a93b56bf3c15f52b0fcf3ebdd5d579a07/tools/perf/contrib/orderfile/OWNERS

Project Member

Comment 7 by bugdroid1@chromium.org, Jul 3

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

commit 3c6537d81069e7d3b8208e45ed3222243f8f7853
Author: Matthew Cary <mattcary@chromium.org>
Date: Tue Jul 03 15:15:15 2018

Orderfile: training and testing benchmarks.

These benchmarks will be used to train the new production android orderfile.

Bug:  843561 
Change-Id: Iea097d784773c98ec3076f297aac16558c2a8104
Reviewed-on: https://chromium-review.googlesource.com/1124561
Reviewed-by: Benoit L <lizeb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#572219}
[add] https://crrev.com/3c6537d81069e7d3b8208e45ed3222243f8f7853/tools/perf/contrib/orderfile/__init__.py
[add] https://crrev.com/3c6537d81069e7d3b8208e45ed3222243f8f7853/tools/perf/contrib/orderfile/orderfile.py
[add] https://crrev.com/3c6537d81069e7d3b8208e45ed3222243f8f7853/tools/perf/contrib/orderfile/orderfile_unittest.py

Project Member

Comment 8 by bugdroid1@chromium.org, Jul 3

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

commit 100610c77335578bb37d59ec601f0005c432e026
Author: Ned Nguyen <nednguyen@google.com>
Date: Tue Jul 03 17:20:41 2018

Revert "Orderfile: training and testing benchmarks."

This reverts commit 3c6537d81069e7d3b8208e45ed3222243f8f7853.

Reason for revert: block other CL from landing with PRESUBMIT error:

https://logs.chromium.org/v/?s=chromium%2Fbuildbucket%2Fcr-buildbucket.appspot.com%2F8942003279127111280%2F%2B%2Fsteps%2Fpresubmit%2F0%2Fstdout

** Presubmit ERRORS **
Pylint (261 files using ['--disable=cyclic-import'] on 8 cores) (8.11s) failed
************* Module contrib.orderfile.orderfile
E:109,20: story_class is not callable (not-callable)
W:113,19: Access to a protected member _IterAllSystemHealthStoryClasses of a client class (protected-access)
R:144, 0: Too many ancestors (8/7) (too-many-ancestors)
R:155, 0: Too many ancestors (8/7) (too-many-ancestors)
R:189, 0: Too many ancestors (8/7) (too-many-ancestors)
R:194, 0: Too many ancestors (8/7) (too-many-ancestors)
R:200, 0: Too many ancestors (8/7) (too-many-ancestors)
R:206, 0: Too many ancestors (8/7) (too-many-ancestors)
R:212, 0: Too many ancestors (8/7) (too-many-ancestors)
R:218, 0: Too many ancestors (8/7) (too-many-ancestors)
Presubmit checks took 15.4s to calculate.

Original change's description:
> Orderfile: training and testing benchmarks.
> 
> These benchmarks will be used to train the new production android orderfile.
> 
> Bug:  843561 
> Change-Id: Iea097d784773c98ec3076f297aac16558c2a8104
> Reviewed-on: https://chromium-review.googlesource.com/1124561
> Reviewed-by: Benoit L <lizeb@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#572219}

TBR=pasko@chromium.org,lizeb@chromium.org,mattcary@chromium.org

Change-Id: I0f22ff64f7532aaa913bb4dbd844be6cfdb7b5ef
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug:  843561 
Reviewed-on: https://chromium-review.googlesource.com/1124702
Reviewed-by: Ned Nguyen <nednguyen@google.com>
Commit-Queue: Ned Nguyen <nednguyen@google.com>
Cr-Commit-Position: refs/heads/master@{#572266}
[delete] https://crrev.com/cb297d9bbd8ec3dc6d0789045973e6d95a0c8761/tools/perf/contrib/orderfile/__init__.py
[delete] https://crrev.com/cb297d9bbd8ec3dc6d0789045973e6d95a0c8761/tools/perf/contrib/orderfile/orderfile.py
[delete] https://crrev.com/cb297d9bbd8ec3dc6d0789045973e6d95a0c8761/tools/perf/contrib/orderfile/orderfile_unittest.py

Project Member

Comment 9 by bugdroid1@chromium.org, Jul 18

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

commit 453ff145bd24bd648b74d047fc2bfa967f8a5f67
Author: Matthew Cary <mattcary@chromium.org>
Date: Wed Jul 18 12:19:35 2018

Orderfile: Add device flag to orderfile generator.

Previously, the profiling tools selected the first connected
devices. This gives inconsistent and surprising results if there is more
than one device connected. This change asserts that either exactly one
device is connected, or the device is explicitly specified via a new
--device flag.

Bug:  843561 
Change-Id: I51e310b83028876936bc8e042d96c9e16044360b
Reviewed-on: https://chromium-review.googlesource.com/1124843
Reviewed-by: Benoit L <lizeb@chromium.org>
Commit-Queue: Matthew Cary <mattcary@chromium.org>
Cr-Commit-Position: refs/heads/master@{#576012}
[modify] https://crrev.com/453ff145bd24bd648b74d047fc2bfa967f8a5f67/tools/cygprofile/orderfile_generator_backend.py
[modify] https://crrev.com/453ff145bd24bd648b74d047fc2bfa967f8a5f67/tools/cygprofile/profile_android_startup.py

Project Member

Comment 10 by bugdroid1@chromium.org, Jul 18

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

commit 717b551f44c51fcd96f09d264f9da5f6770c153b
Author: Matthew Cary <mattcary@chromium.org>
Date: Wed Jul 18 14:26:23 2018

Orderfile: training and testing benchmarks.

These benchmarks will be used to train the new production android orderfile.

This is a reland of crrev.com/c/1124561 which was reverted due to
presubmit errors which have been fixed.

Bug:  843561 
Change-Id: Id200a5957d20d19dc7fe83555c04aa9cecf51f27
Reviewed-on: https://chromium-review.googlesource.com/1125842
Reviewed-by: Benoit L <lizeb@chromium.org>
Commit-Queue: Matthew Cary <mattcary@chromium.org>
Cr-Commit-Position: refs/heads/master@{#576044}
[add] https://crrev.com/717b551f44c51fcd96f09d264f9da5f6770c153b/tools/perf/contrib/orderfile/__init__.py
[add] https://crrev.com/717b551f44c51fcd96f09d264f9da5f6770c153b/tools/perf/contrib/orderfile/orderfile.py
[add] https://crrev.com/717b551f44c51fcd96f09d264f9da5f6770c153b/tools/perf/contrib/orderfile/orderfile_unittest.py

Status: Fixed (was: Started)

Sign in to add a comment