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

Issue 772345 link

Starred by 3 users

Issue metadata

Status: Fixed
Owner:
Closed: Oct 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug

Blocking:
issue 639831
issue 769284



Sign in to add a comment

profile_android_startup should stop using legacy wpr_server

Project Member Reported by nedngu...@google.com, Oct 6 2017

Issue description

We are removing the legacy wpr in Telemetry (bit.ly/wpr-go-migration). Yet profile_android_startup.py still rely on the legacy wpr server: https://cs.chromium.org/chromium/src/tools/cygprofile/profile_android_startup.py?rcl=78a168924315e3f9c988ba992a7ea6c57376a415&l=38

lizeb@, pasko@: what does profile_android_startup.py? Is this supposed to go away once we overhaul android startup benchmarks?
 

Comment 1 by pasko@chromium.org, Oct 9 2017

Blocking: 639831

Comment 2 by pasko@chromium.org, Oct 9 2017

Cc: lizeb@chromium.org mattcary@chromium.org xunji...@chromium.org
Owner: pasko@chromium.org
Status: Started (was: Assigned)
Gave it a look today, the first attempt at using WPR go outside of Telemetry.

As usual, doing it via the ugly hack like "from telemetry.internal.util import [something]", hopefully we will make it better some day.

Currently --ignore-certificate-errors-spki-list=PhrPvGIaAMmd29hj8BCZOq096yj7uMpRNHpn5PDxI6I= still produces a cert error for me.

I vaguely remember needing to provide --user-data-dir, will try that tomorrow ..
Components: Speed>Benchmarks
pasko@ if our effort of overhauling Android startup benchmark can cover the use cases of profile_android_startup script, I think we can keep the legacy wpr_server around a bit & just remove profile_android_startup.py completely once the new Android startup benchmark is ready.

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

The script is just a way to run the instrumented build and collect special logs/data from it. The parts that install the APK and collect the data are too tool-specific and probably should not pollute Telemetry.

The overhaul should help refactoring the script to make sure only the public Telemetry API is used to run the benchmark. Still makes what I am doing not strictly necessary, but I think this small step should rather reduce cruft, so looking at it right now.

Comment 5 by pasko@chromium.org, Oct 10 2017

The short term fix is up for review: https://chromium-review.googlesource.com/#/c/chromium/src/+/709055
Project Member

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

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

commit 6394bed4cbbdb83009c81e82f675c28dea2e9d08
Author: Egor Pasko <pasko@chromium.org>
Date: Tue Oct 10 13:10:47 2017

Migrate profile_android_startup.py to use wprgo

Reusing the archive from
tools/perf/page_sets/data/memory_top_10_mobile_000.wprgo.sha1. Copying
the file containing the hash to tools/cygprofile.

Also remove the old dance around installing certificate authority on the
device - it did not work and is unused right now.

Bug:  772345 
Change-Id: I3ce2eeba28cb4b40616f048d64d01562bf253770
Reviewed-on: https://chromium-review.googlesource.com/709055
Commit-Queue: Egor Pasko <pasko@chromium.org>
Reviewed-by: Benoit L <lizeb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#507652}
[modify] https://crrev.com/6394bed4cbbdb83009c81e82f675c28dea2e9d08/.gitignore
[add] https://crrev.com/6394bed4cbbdb83009c81e82f675c28dea2e9d08/tools/cygprofile/memory_top_10_mobile_000.wprgo.sha1
[modify] https://crrev.com/6394bed4cbbdb83009c81e82f675c28dea2e9d08/tools/cygprofile/profile_android_startup.py
[delete] https://crrev.com/c31e3896b8fe7f6e0a73c70633b37f7ab06f0236/tools/cygprofile/top_10_mobile_002.wpr.sha1

Comment 7 by pasko@chromium.org, Oct 10 2017

nednguyen: hopefully this commit sticks. I am going to mark this as fixed once we have a few orderfile bot cycles succeeding. Feel free to comment on the change if something is likely to cause trouble for you, will try to address those.

Comment 8 by pasko@chromium.org, Oct 11 2017

Status: Fixed (was: Started)
orderfile bot is green after this change

Sign in to add a comment