New issue
Advanced search Search tips

Issue 804479 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Jan 2018
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 3
Type: Task

Blocking:
issue 805098



Sign in to add a comment

Open source our AFDO profiles for Android

Project Member Reported by g...@chromium.org, Jan 22 2018

Issue description

We've currently got a sort of awkward setup where we use CrOS profiles directly for //clank, but release a version where we redact any symbols that aren't obviously present in clank's libchrome.so. This "works", but pulling from one of two profiles is suboptimal (especially given that most perf testing is done on Chromium, not Chrome), and the scripts I made do the redaction are pretty terrible. :)

If it turns out that CrOS' internal symbol names (that only exist in the chrome binary) are fine to open source, great. Otherwise, they volunteered to try to collect profiles using a non-internal build of Chromium just for us.

Making these profiles universally accessible will also presumably make applying them to Chrome on Linux simple.
 
Project Member

Comment 1 by bugdroid1@chromium.org, Jan 23 2018

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

commit 1f751e41506c1b1c875ca589681872f725b6d7ab
Author: George Burgess IV <gbiv@chromium.org>
Date: Tue Jan 23 22:10:00 2018

Use the actual Chrome OS AFDO profiles on Android

(This is also the third attempt roll to a newer AFDO profile, which
we're currently using internally.)

While the gs bucket that holds these profiles is restricted to Googlers
only, our Chrome OS friends jump through hoops specifically to make
these AFDO profiles world-readable. So, all of the crazy redaction and
such that we're currently doing is entirely unnecessary. Woo!

As noted in the script, due to the permissions setup, actually accessing
these profiles is a bit awkward. But it's workable.

also ran update_afdo_profile.py on a few profiles from a non-Google
machine, and it succeeded on the ones I tried. Also ran
build/install-build-deps-android.sh.

Bug:  804479 
Test: `gclient runhooks` and built for Android with/without clank. I
Change-Id: I20d973b62251ebe249e80fad6051bd3a6805aa12
Reviewed-on: https://chromium-review.googlesource.com/879849
Commit-Queue: George Burgess <gbiv@chromium.org>
Reviewed-by: agrieve <agrieve@chromium.org>
Reviewed-by: Tommy Nyquist <nyquist@chromium.org>
Cr-Commit-Position: refs/heads/master@{#531368}
[modify] https://crrev.com/1f751e41506c1b1c875ca589681872f725b6d7ab/.gitignore
[modify] https://crrev.com/1f751e41506c1b1c875ca589681872f725b6d7ab/DEPS
[modify] https://crrev.com/1f751e41506c1b1c875ca589681872f725b6d7ab/build/config/android/config.gni
[modify] https://crrev.com/1f751e41506c1b1c875ca589681872f725b6d7ab/build/install-build-deps-android.sh
[delete] https://crrev.com/b51cc5cca26ada795185e72f692a9b39d54b16a7/chrome/android/profiles/cipd.yaml
[add] https://crrev.com/1f751e41506c1b1c875ca589681872f725b6d7ab/chrome/android/profiles/newest.txt
[add] https://crrev.com/1f751e41506c1b1c875ca589681872f725b6d7ab/chrome/android/profiles/update_afdo_profile.py

Comment 2 by g...@chromium.org, Jan 23 2018

Labels: -Restrict-View-Google
-restrict-view, since we're doing it.

Well, that was less work than I thought it was going to be. :)

Leaving this open until the internal-only bits are gone and the hack in 1f751e41506c1b1c875ca589681872f725b6d7ab is removed.

Comment 3 by g...@chromium.org, Jan 23 2018

Blocking: 805098
Project Member

Comment 4 by bugdroid1@chromium.org, Jan 24 2018

The following revision refers to this bug:
  https://chrome-internal.googlesource.com/clank/internal/apps/+/d7e2a88c055d6cbbb2451a56b22d507c056f0c7c

commit d7e2a88c055d6cbbb2451a56b22d507c056f0c7c
Author: George Burgess IV <gbiv@chromium.org>
Date: Wed Jan 24 03:40:40 2018

Project Member

Comment 5 by bugdroid1@chromium.org, Jan 24 2018

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

commit 2e080064e2839394e3f73ce1f3ce3d8e2590431e
Author: George Burgess IV <gbiv@chromium.org>
Date: Wed Jan 24 05:59:51 2018

Remove hack introduced by CL:879849

Internal profiling bits have been cleaned up.

Bug:  804479 
Test: Built internal Chrome with this applied.
Change-Id: I9611b34eb573f75e9dce0d1fd91ca3486aabc003
Reviewed-on: https://chromium-review.googlesource.com/882313
Reviewed-by: agrieve <agrieve@chromium.org>
Commit-Queue: George Burgess <gbiv@chromium.org>
Cr-Commit-Position: refs/heads/master@{#531450}
[modify] https://crrev.com/2e080064e2839394e3f73ce1f3ce3d8e2590431e/build/config/android/config.gni

Comment 6 by g...@chromium.org, Jan 24 2018

Status: Fixed (was: Assigned)
ᕕ( ᐛ )ᕗ
Project Member

Comment 7 by bugdroid1@chromium.org, Jan 25 2018

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

commit 4806902fb7692265bab0bb9f42e245e281a9f79d
Author: Raphael Kubo da Costa <raphael.kubo.da.costa@intel.com>
Date: Thu Jan 25 00:46:42 2018

AFDO profiles: Use urllib2 instead of urllib for HTTP communication.

urllib has issues with proxies similar to the ones reported in  bug 134165 
that are not present in urllib2.urlopen(). Namely, it does not perform HTTP
tunneling with proxy servers when fetching HTTPS URLs.

urllib2 does not have a urlretrieve() method though, so we need to do some
additional work ourselves to fetch the desired file (the code was borrowed
from depot_tools' gsutil.py which does soemthing similar).

See https://codereview.chromium.org/133933007 and
https://codereview.chromium.org/2554753005 for other instances of the same
problem.

Bug:  804479 
Test: `gclient runhooks' behind my corporate proxy
Change-Id: I272faefd380c2e6000df766e255aa932f7230ddc
Reviewed-on: https://chromium-review.googlesource.com/883362
Reviewed-by: agrieve <agrieve@chromium.org>
Reviewed-by: Tommy Nyquist <nyquist@chromium.org>
Commit-Queue: Tommy Nyquist <nyquist@chromium.org>
Cr-Commit-Position: refs/heads/master@{#531766}
[modify] https://crrev.com/4806902fb7692265bab0bb9f42e245e281a9f79d/chrome/android/profiles/update_afdo_profile.py

Comment 8 Deleted

Sign in to add a comment