Bug 632954 (android-pgo)

Enable PGO for Android ARMv7

RESOLVED FIXED in Firefox 68

Status

defect
P3
normal
RESOLVED FIXED
9 years ago
2 months ago

People

(Reporter: blassey, Assigned: mshal)

Tracking

(Depends on 1 bug, Blocks 1 bug, {mobile, perf})

Trunk
mozilla68
ARM
Android
Dependency tree / graph

Firefox Tracking Flags

(firefox-esr60 wontfix, firefox65 wontfix, firefox66 wontfix, firefox67 wontfix, firefox68 fixed)

Details

(Whiteboard: mobilestartupshrink [geckoview:p2])

Attachments

(14 attachments, 2 obsolete attachments)

5.89 KB, patch
Details | Diff | Splinter Review
2.94 KB, patch
Details | Diff | Splinter Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
No description provided.
Keywords: mobile, perf
Posted patch WIP (obsolete) — Splinter Review
this patch has some configure goop to make pgo work for android, plus some stuff to make using other toolchains work. Unfortunately, the toolchain chokes on the -fprofile-use phase of the build

with ndk r4 gcc4.4 and ndk r5 gcc4.4, I get these sorts of errors while compiling mozalloc.cpp:

{standard input}:902: Error: branch out of range
{standard input}:1178: Error: branch out of range
{standard input}: Error: cant' resolve '_GLOBAL_OFFSET_TABLE_'
{standard input}: Error: cant' resolve '_GLOBAL_OFFSET_TABLE_'

with the ndk r5 gcc4.4.3 I get slightly different errors:
{standard input}: Assembler messages:
{standard input}:902: Error: branch out of range
{standard input}:1178: Error: branch out of range
{standard input}:951: Error: can't resolve `.bss' {.bss section} - `.LPIC92' {.text section}
{standard input}:952: Error: can't resolve `.text.unlikely' {.text.unlikely section} - `.LPIC102' {.text section}
{standard input}:1226: Error: can't resolve `.bss' {.bss section} - `.LPIC122' {.text section}
{standard input}:1227: Error: can't resolve `.text.unlikely' {.text.unlikely section} - `.LPIC132' {.text section}

Comment 2

9 years ago
(In reply to comment #1)
> Created attachment 511476 [details] [diff] [review]
> WIP
> 
> this patch has some configure goop to make pgo work for android, plus some
> stuff to make using other toolchains work. Unfortunately, the toolchain chokes
> on the -fprofile-use phase of the build
> 
> with ndk r4 gcc4.4 and ndk r5 gcc4.4, I get these sorts of errors while
> compiling mozalloc.cpp:
> 
> {standard input}:902: Error: branch out of range
> {standard input}:1178: Error: branch out of range
> {standard input}: Error: cant' resolve '_GLOBAL_OFFSET_TABLE_'
> {standard input}: Error: cant' resolve '_GLOBAL_OFFSET_TABLE_'
> 
> with the ndk r5 gcc4.4.3 I get slightly different errors:
> {standard input}: Assembler messages:
> {standard input}:902: Error: branch out of range
> {standard input}:1178: Error: branch out of range
> {standard input}:951: Error: can't resolve `.bss' {.bss section} - `.LPIC92'
> {.text section}
> {standard input}:952: Error: can't resolve `.text.unlikely' {.text.unlikely
> section} - `.LPIC102' {.text section}
> {standard input}:1226: Error: can't resolve `.bss' {.bss section} - `.LPIC122'
> {.text section}
> {standard input}:1227: Error: can't resolve `.text.unlikely' {.text.unlikely
> section} - `.LPIC132' {.text section}

I recall seeing something similar on desktop. Try taking out -fno-reorder-functions from our build. Can you also paste the full commandline that fails?
For the record, I'm trying to build a NDK with gcc 4.6.1 which should allow PGO.
Whiteboard: mobilestartupshrink
Depends on: 675572
I've been using this patch with gcc 4.6 + gold, and building with:
$ make -f client.mk MOZ_PROFILE_GENERATE=1 MOZ_PROFILE_BASE=/sdcard/mozilla-pgo
$ make -C objdir package
install the apk on the device, start fennec, browse to sunspider
pull /sdcard/mozilla-pgo from the device, and copy the tree to / (if the objdir is in /tmp/objdir, the sdcard will contain /sdcard/mozilla-pgo/tmp/objdir/...)
$ make -f client.mk MOZ_PROFILE_USE=1

This however is not exactly in good shape to land as-is, because it also ends up moving the nss gcda files, and they wouldn't be purged by bug 659942.
Posted patch imported patch -nspr (obsolete) — Splinter Review
Attachment #550403 - Attachment is obsolete: true
(In reply to comment #4)
> Created attachment 550402 [details] [diff] [review] [diff] [details] [review]
> PGO support for Android
> 
> I've been using this patch with gcc 4.6 + gold, and building with:
> $ make -f client.mk MOZ_PROFILE_GENERATE=1
> MOZ_PROFILE_BASE=/sdcard/mozilla-pgo
> $ make -C objdir package
> install the apk on the device, start fennec, browse to sunspider
> pull /sdcard/mozilla-pgo from the device, and copy the tree to / (if the
> objdir is in /tmp/objdir, the sdcard will contain
> /sdcard/mozilla-pgo/tmp/objdir/...)

Forgot to add:
$ make -f client.mk maybe_clobber_profiledbuild
here

> $ make -f client.mk MOZ_PROFILE_USE=1
Attachment #511476 - Attachment is obsolete: true
A new build, corresponding to yesterday's nightly, and profiled running V8, Sunspider and PageLoad from the Zippity Test Harness add-on:
http://people.mozilla.org/~mhommey/pgo/fennec-8.0a1.en-US.android-arm.1dddaeb1366b.pgo.apk

No wonders, it's still slower on Sunspider than the nightly or the corresponding non PGO GCC 4.6 build ( http://people.mozilla.org/~mhommey/pgo/fennec-8.0a1.en-US.android-arm.1dddaeb1366b.gcc4.6.apk ), but is faster on V8 than the nightly, but that is also the case with GCC 4.6 without PGO...
Priority: -- → P1
Assignee: nobody → mh+mozilla
So, I just got an apparently proper PGO profile for android, and the result is not very good, but better than before
Most sunspider tests are faster (between 3 and 12% depending on the test), except a few that are significantly slower, making the whole result lower
On the not so bright side, the apk is 900K bigger.

Sunspider result for PGOed build (best of 3 runs):
http://www.webkit.org/perf/sunspider-0.9.1/sunspider-0.9.1/results.html?%7B%22v%22:%20%22sunspider-0.9.1%22,%20%223d-cube%22:%5B191,193,193,197,191,194,195,190,172,192%5D,%223d-morph%22:%5B79,78,78,77,78,78,77,80,77,78%5D,%223d-raytrace%22:%5B184,179,183,180,178,182,176,181,177,177%5D,%22access-binary-trees%22:%5B42,42,43,43,43,42,43,42,42,43%5D,%22access-fannkuch%22:%5B147,147,103,158,146,147,147,144,146,146%5D,%22access-nbody%22:%5B202,205,202,202,204,203,203,202,113,110%5D,%22access-nsieve%22:%5B71,73,72,71,73,72,74,72,73,72%5D,%22bitops-3bit-bits-in-byte%22:%5B6,6,6,6,6,5,6,6,6,6%5D,%22bitops-bits-in-byte%22:%5B54,54,45,53,53,53,53,54,53,55%5D,%22bitops-bitwise-and%22:%5B18,17,16,16,17,17,17,17,17,17%5D,%22bitops-nsieve-bits%22:%5B39,38,39,37,37,36,37,37,37,39%5D,%22controlflow-recursive%22:%5B25,24,24,24,24,23,23,24,23,25%5D,%22crypto-aes%22:%5B109,114,108,103,113,103,102,104,104,106%5D,%22crypto-md5%22:%5B55,51,55,51,52,52,51,52,53,53%5D,%22crypto-sha1%22:%5B33,33,33,33,33,33,33,35,33,35%5D,%22date-format-tofte%22:%5B154,154,151,150,148,151,152,150,149,150%5D,%22date-format-xparb%22:%5B140,140,139,138,141,141,140,140,138,140%5D,%22math-cordic%22:%5B79,80,80,79,79,79,80,79,79,79%5D,%22math-partial-sums%22:%5B118,116,118,116,117,116,117,117,116,118%5D,%22math-spectral-norm%22:%5B77,78,78,85,77,78,77,81,76,77%5D,%22regexp-dna%22:%5B95,91,96,94,94,96,94,94,95,96%5D,%22string-base64%22:%5B35,34,37,34,33,36,34,34,36,35%5D,%22string-fasta%22:%5B100,100,102,101,100,101,101,102,102,103%5D,%22string-tagcloud%22:%5B151,152,156,152,152,155,154,155,154,158%5D,%22string-unpack-code%22:%5B158,153,152,154,153,153,153,153,157,151%5D,%22string-validate-input%22:%5B73,73,72,72,73,80,72,72,73,72%5D%7D

Sunspider result for non-PGOed build (best of 3 runs):
http://www.webkit.org/perf/sunspider-0.9.1/sunspider-0.9.1/results.html?%7B%22v%22:%20%22sunspider-0.9.1%22,%20%223d-cube%22:%5B196,196,196,195,195,195,193,193,193,196%5D,%223d-morph%22:%5B89,78,78,80,78,79,77,77,78,80%5D,%223d-raytrace%22:%5B177,180,180,180,189,180,179,179,184,178%5D,%22access-binary-trees%22:%5B45,46,46,46,45,47,45,45,45,45%5D,%22access-fannkuch%22:%5B148,145,146,147,101,145,142,143,147,147%5D,%22access-nbody%22:%5B78,79,77,78,80,78,76,76,89,78%5D,%22access-nsieve%22:%5B72,72,73,74,73,73,70,70,73,74%5D,%22bitops-3bit-bits-in-byte%22:%5B6,5,6,6,6,6,6,7,6,6%5D,%22bitops-bits-in-byte%22:%5B56,54,43,55,54,53,54,54,45,46%5D,%22bitops-bitwise-and%22:%5B16,17,17,17,17,17,17,16,17,17%5D,%22bitops-nsieve-bits%22:%5B40,39,48,39,37,39,152,37,38,39%5D,%22controlflow-recursive%22:%5B24,24,24,23,22,26,24,29,34,26%5D,%22crypto-aes%22:%5B110,110,108,111,109,110,104,105,117,109%5D,%22crypto-md5%22:%5B55,54,55,56,53,55,51,53,53,54%5D,%22crypto-sha1%22:%5B34,35,35,35,35,35,34,35,36,34%5D,%22date-format-tofte%22:%5B173,170,173,171,174,171,165,167,169,171%5D,%22date-format-xparb%22:%5B157,154,150,148,153,146,144,148,149,148%5D,%22math-cordic%22:%5B80,80,80,80,80,80,79,79,80,79%5D,%22math-partial-sums%22:%5B115,116,113,112,114,114,112,112,113,114%5D,%22math-spectral-norm%22:%5B78,79,77,76,77,77,76,76,78,77%5D,%22regexp-dna%22:%5B94,95,95,93,94,93,93,90,94,95%5D,%22string-base64%22:%5B43,35,39,39,38,38,39,35,39,37%5D,%22string-fasta%22:%5B104,106,104,104,105,106,104,101,105,106%5D,%22string-tagcloud%22:%5B166,165,164,165,164,166,160,160,165,168%5D,%22string-unpack-code%22:%5B160,160,160,160,161,162,156,158,162,162%5D,%22string-validate-input%22:%5B77,76,74,77,78,80,77,78,77,75%5D%7D

The significant differences where PGO is slower are:
access nbody:             *2.34x as slow*     78.9ms +/- 3.4%    184.6ms +/- 14.9%     significant
math partial-sums:      *1.030x as slow*   113.5ms +/- 0.9%    116.9ms +/- 0.5%     significant
math spectral-norm:     *1.017x as slow*    77.1ms +/- 0.9%     78.4ms +/- 2.4%     significant

Only the first is a *really* significant.

I haven't compared to the corresponding tinderbox build.

If people want to test the builds:
http://people.mozilla.org/~mhommey/pgo/fennec-9.0a1.en-US.android-arm.19a5f6177257.gcc4.6.apk for the non-PGOed build
http://people.mozilla.org/~mhommey/pgo/fennec-9.0a1.en-US.android-arm.19a5f6177257.pgo.apk for the PGOed build
(should be up in about 5 to 10 minutes)

Corresponding changeset is 19a5f6177257 (from build-system)
Note that the PGO build is actually only less than 200K bigger than the tinderbox build. The 900K are between gcc 4.6 and gcc 4.6+PGO.
Best of 3 runs with the tinderbox build:
http://www.webkit.org/perf/sunspider-0.9.1/sunspider-0.9.1/results.html?%7B%22v%22:%20%22sunspider-0.9.1%22,%20%223d-cube%22:%5B199,214,184,199,204,200,188,198,185,187%5D,%223d-morph%22:%5B81,81,79,80,79,79,79,79,80,80%5D,%223d-raytrace%22:%5B189,187,190,186,191,192,194,193,192,196%5D,%22access-binary-trees%22:%5B46,46,45,47,46,45,46,46,48,45%5D,%22access-fannkuch%22:%5B145,106,144,145,103,147,146,147,146,147%5D,%22access-nbody%22:%5B77,78,80,78,83,80,79,79,80,78%5D,%22access-nsieve%22:%5B74,72,73,74,76,76,75,75,77,73%5D,%22bitops-3bit-bits-in-byte%22:%5B7,6,6,6,6,6,6,5,6,6%5D,%22bitops-bits-in-byte%22:%5B53,55,44,46,54,53,44,54,53,55%5D,%22bitops-bitwise-and%22:%5B17,17,17,17,17,16,17,17,16,17%5D,%22bitops-nsieve-bits%22:%5B39,40,38,39,38,38,40,39,38,40%5D,%22controlflow-recursive%22:%5B24,27,24,25,25,24,26,26,26,26%5D,%22crypto-aes%22:%5B112,152,112,114,121,125,116,126,114,114%5D,%22crypto-md5%22:%5B54,56,54,55,54,53,55,56,53,56%5D,%22crypto-sha1%22:%5B36,36,35,36,35,35,35,36,35,35%5D,%22date-format-tofte%22:%5B179,181,178,177,176,178,182,179,179,176%5D,%22date-format-xparb%22:%5B162,162,160,159,160,160,159,166,161,162%5D,%22math-cordic%22:%5B80,79,80,80,80,85,87,87,87,79%5D,%22math-partial-sums%22:%5B116,117,126,116,118,115,118,118,118,117%5D,%22math-spectral-norm%22:%5B77,78,78,77,78,76,79,79,85,85%5D,%22regexp-dna%22:%5B97,95,94,94,95,93,95,94,94,94%5D,%22string-base64%22:%5B36,34,37,36,36,37,36,36,40,36%5D,%22string-fasta%22:%5B115,127,118,116,117,115,118,115,121,117%5D,%22string-tagcloud%22:%5B169,169,170,171,169,172,170,171,235,171%5D,%22string-unpack-code%22:%5B166,167,168,166,168,169,166,172,171,168%5D,%22string-validate-input%22:%5B74,74,75,74,76,75,76,75,73,74%5D%7D

Between 2 and 18% improvement depending on the test, except for access:nbody, which is 2.33x as slow.
(In reply to Mike Hommey [:glandium] from comment #13)
> Between 2 and 18% improvement depending on the test, except for
> access:nbody, which is 2.33x as slow.

for the PGO build, compared to the tinderbox build, that is.
As a side node, latest NSS broke ARM PGO builds because of the MPI assembly using too many registers for the profiling code to be happy.
OS: Android → MeeGo
Target Milestone: --- → mozilla9
Version: Trunk → Other Branch
OS: MeeGo → Android
Target Milestone: mozilla9 → ---
Version: Other Branch → Trunk
Depends on: 736066
I recommend that we postpone building NSS itself with PGO until we have the testing infrastructure issues with NSS resolved (which will happen in Q2). NSS has never been tested in a PGO configuration, on any platform,

AFAICT, almost none of NSS is exercised by the profile-gathering runs, so it might be better for performance to build it with normal link-time optimization instead of PGO. I don't know how the profiler works exactly w.r.t. DLLs, but I would hate to see all of NSS de-optimized as cold/dead code. Has anybody measured the performance difference?
P1's that have been inactive for 22 months are not P1's.
Priority: P1 → P3
We're building Fennec with gcc 4.9 now. I wonder if it might be worth looking at PGO again.
Summary: enable pgo for android → Enable PGO for Android
Nathan, what do you think? PGO could be a thing now?
Flags: needinfo?(nfroyd)
(In reply to James Willcox (:snorp) (jwillcox@mozilla.com) from comment #19)
> Nathan, what do you think? PGO could be a thing now?

Um, sure?  I'd want some measurements first, though; I'm fairly sure GCC doesn't really try tuning their PGO stuff for ARM Android...
Flags: needinfo?(nfroyd)

Updated

Last year
Product: Core → Firefox Build System
Alias: android-pgo
Whiteboard: mobilestartupshrink → mobilestartupshrink [geckoview:p2]
Duplicate of this bug: 748488
Depends on: 781179
See Also: → 781179, 748488, 777440
Assignee

Updated

5 months ago
Depends on: 1525510
Assignee

Updated

4 months ago
Depends on: 1529587
Assignee

Updated

4 months ago
Depends on: 1529597
Assignee

Updated

4 months ago
Depends on: 1529656
Assignee

Updated

4 months ago
Blocks: 1531133
Depends on: 1531632
Depends on: 1529894
Assignee

Updated

3 months ago
Depends on: 1298921
Assignee

Updated

3 months ago
Assignee: mh+mozilla → mshal
Assignee

Comment 22

3 months ago

Without this flag, Android PGO profile-use builds may fail with
"Function control flow change detected" errors.

Assignee

Comment 23

3 months ago

When Android shuts down the ndk process, it doesn't call the registered
atexit() handlers, which is normally where the profile data gets written
to file. Since the PGO test suite closes the browser when it is
finished, the nativeRun routine can manually call out to
__llvm_profile_dump() before returning.

This method has a downside that only the profile data from the calling
library gets written out, rather than for the whole process. Since we
are most interested in optimizing libxul, a new hook is added in
Bootstrap to make sure we get the profile data for the right library.

Depends on D22816

Assignee

Comment 24

3 months ago

The Fennec process needs a few extra environment variables for PGO,
notably LLVM_PROFILE_FILE and MOZ_JAR_LOG_FILE to give the locations for
the profile run outputs. FennecInstance needs to pass the "env"
parameter on down so it can be used by DeviceRunner.

Depends on D22817

Assignee

Comment 26

3 months ago

test-linux.sh defaults to true for NEED_XVFB, while build-linux.sh
defaults to false. If we are using test-linux.sh from mozharness (rather
than mozharness-test), we need to explicitly set NEED_XVFB to false in
order to not use xvfb.

Depends on D22819

Assignee

Comment 27

3 months ago

In order to call test-linux.sh with the job-script parameter, it needs
to have executable permissions.

Depends on D22820

Assignee

Comment 28

3 months ago

The mozharness.py transform passes in "options" parameters through the
MOZHARNESS_OPTIONS environment variable. This will allow the Android PGO
run task to pass in the mozharness script name to test-linux.sh

Depends on D22821

Assignee

Comment 29

3 months ago

The regex should match '-' characters as well as \w to properly trim the
error message if the device string contains a dash.

Depends on D22822

Assignee

Comment 30

3 months ago

This is the first stage of the Android PGO task pipeline to generate an
instrumented build.

Depends on D22823

Assignee

Comment 31

3 months ago

This introduces a mozharness script, android_emulator_pgo.py, to run the
profileserver suite with the PGO-instrumented Android build, and collect
the profile data and jarlog.

The mozharness script contains some redundancy with
build/pgo/profileserver.py, but the additional requirements for Android
to use adb and existing mozharness classes to control the emulator made
it difficult to share the desktop profileserver implementation.

Since this runs similar to a test case, but uses a source checkout
rather than a test tarball, a new MozbaseSourceMixin was created to use
mozbase_source_requirements.txt so ensure that we use the correct paths
for the virtualenv.

Depends on D22824

Assignee

Comment 32

3 months ago

Depends on D22825

Assignee

Comment 34

3 months ago

Per https://bugzilla.mozilla.org/show_bug.cgi?id=1298921#c14 we won't be able to use marionette.quit() to close the browser. I am testing out the alternate solutions that whimboo suggested.

No longer depends on: 1298921
Assignee

Updated

3 months ago
Depends on: 1535132
Assignee

Updated

3 months ago
Blocks: 1535364
Attachment #9049705 - Attachment description: Bug 632954 - Enable tests on Android PGO; r?tomprince → Bug 632954 - Enable tests on Android PGO; r?tomprince,jmaher
Assignee

Comment 35

3 months ago

I am hitting some intermittent failures in the run task, sometimes failing on the Marionette() initialization, and sometimes on the driver.navigate call. I had to bump the Marionette startup_timeout which fixed a few other failures, since the startup delay ran between 360-552s, and I had the timeout set to 480:

https://treeherder.mozilla.org/#/jobs?repo=try&revision=63929c03d8599f2ea2c67f7994b1a4290e9c5674&selectedJob=234553737

gbrown, any idea if the other two cases are something that's easy to fix? I tried to add a loop on the driver.navigate call if it hit an ADBTimeoutError (https://treeherder.mozilla.org/#/jobs?repo=try&revision=f492621762b86cfe0c7406f566b18cbf0c617fc1&selectedJob=234436371) but that just resulted in: "InvalidSessionIdException: Please start a session"

Flags: needinfo?(gbrown)

The Android 4.3 emulator intermittently becomes unresponsive. We have spent a lot of time investigating and never fully understood why; instead, we abandon the task and retry when we detect conditions that look like the emulator is unresponsive. Basically, whenever a mozharness test encounters ADBTimeoutError, it makes sure to return TBPL_RETRY from the mozharness script, and taskcluster retries the task. You can see that here:

https://treeherder.mozilla.org/#/jobs?repo=mozilla-central&revision=891a30b8eee20648df8b62217304fb0a0636349d&searchStr=android%2C4.3&selectedJob=234514008

All of the blue jobs have a log with content like

https://taskcluster-artifacts.net/a89H6Hd0QIGg3lL3sL6xCA/0/public/logs/live_backing.log

[task 2019-03-18T11:59:17.023Z] 11:59:17 CRITICAL - ADBTimeoutError: args: adb-5554 wait-for-device shell rm /sdcard/tests/logs/mochitest.log; echo adb_returncode=$?, exitcode: None, stdout:
...
[task 2019-03-18T12:04:17.094Z] 12:04:17 CRITICAL - # TBPL RETRY #
[task 2019-03-18T12:04:17.094Z] 12:04:17 WARNING - setting return code to 4

I think you should do the same: in android_emulator_pgo.py, catch ADBTimeoutError and when you see it, exit the script with TBPL_RETRY.

Flags: needinfo?(gbrown)

There is some taskcluster plumbing that translates the TBPL_RETRY into a task retry, like:

https://searchfox.org/mozilla-central/rev/7abb9117c8500ed20833746c9f8e800fce3a4688/taskcluster/taskgraph/transforms/tests.py#1042

I think that will just work for you once your script returns TBPL_RETRY, but if it doesn't, let me know.

Assignee

Comment 38

3 months ago

Adding TBPL_RETRY works great, thanks! The latest patch contains this fix, as well as a the new image type from bug 1535132. We can land this once that bug lands.

Comment 39

3 months ago
Pushed by mshal@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/3dfc0e4f8e7c
Add -Wno-error=backend-plugin to support Android PGO; r=glandium
https://hg.mozilla.org/integration/autoland/rev/0615c775a0cf
Add an explicit __llvm_profile_dump() call for Android; r=snorp
https://hg.mozilla.org/integration/autoland/rev/142ae187478d
Allow passing in env to Fennec Marionette; r=gbrown
https://hg.mozilla.org/integration/autoland/rev/503bcac73583
Add Android PGO mozconfigs; r=nalexander
https://hg.mozilla.org/integration/autoland/rev/53d3443e55d9
Explicitly set NEED_XVFB to false if need-xvfb isn't set; r=tomprince
https://hg.mozilla.org/integration/autoland/rev/6f5fc0d644dd
Add execute bit to test-linux.sh; r=tomprince
https://hg.mozilla.org/integration/autoland/rev/097f141a499d
Add support for MOZHARNESS_OPTIONS to test-linux.sh; r=tomprince
https://hg.mozilla.org/integration/autoland/rev/26031d362333
Ignore dashes in adb error messages; r=bc
https://hg.mozilla.org/integration/autoland/rev/b96dd954a456
Add Android PGO-instrumented build task; r=tomprince
https://hg.mozilla.org/integration/autoland/rev/c151ebf303ca
Add Android profile generation task; r=tomprince,gbrown
https://hg.mozilla.org/integration/autoland/rev/de8beacc5eb4
Add final Android PGO task; r=tomprince
https://hg.mozilla.org/integration/autoland/rev/429c96e4de32
Enable tests on Android PGO; r=jmaher

Comment 41

3 months ago
Pushed by nerli@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/9954a61f0b3a
Add -Wno-error=backend-plugin to support Android PGO; r=glandium
https://hg.mozilla.org/integration/autoland/rev/5fce433c8670
Add an explicit __llvm_profile_dump() call for Android; r=snorp
https://hg.mozilla.org/integration/autoland/rev/c5e1e425dd5c
Allow passing in env to Fennec Marionette; r=gbrown
https://hg.mozilla.org/integration/autoland/rev/8acddb36a316
Add Android PGO mozconfigs; r=nalexander
https://hg.mozilla.org/integration/autoland/rev/7df96a607be8
Explicitly set NEED_XVFB to false if need-xvfb isn't set; r=tomprince
https://hg.mozilla.org/integration/autoland/rev/a257e15d267f
Add execute bit to test-linux.sh; r=tomprince
https://hg.mozilla.org/integration/autoland/rev/0450380b2693
Add support for MOZHARNESS_OPTIONS to test-linux.sh; r=tomprince
https://hg.mozilla.org/integration/autoland/rev/578d02d41139
Ignore dashes in adb error messages; r=bc
https://hg.mozilla.org/integration/autoland/rev/831b503b1e5c
Add Android PGO-instrumented build task; r=tomprince
https://hg.mozilla.org/integration/autoland/rev/407f264cc3ee
Add Android profile generation task; r=tomprince,gbrown
https://hg.mozilla.org/integration/autoland/rev/d78d224fbb3f
Add final Android PGO task; r=tomprince
https://hg.mozilla.org/integration/autoland/rev/d8203a0665d2
Enable tests on Android PGO; r=jmaher CLOSED TREE
No longer blocks: 1531133
See Also: → 1542707

I believe PGO wasn't enabled for Android AARCH64, right? If so, are there plans for that?

(In reply to Ionuț Goldan [:igoldan], Performance Sheriffing from comment #44)

I believe PGO wasn't enabled for Android AARCH64, right? If so, are there plans for that?

That is correct. Android PGO has only been enabled for ARMv7 (32-bit) at this time. AArch64/ARM64 support should be ready in 68 or 69. More than half of Fennec users have ARM64 devices, so it's a high priority. I filed bug 1543212 for ARM64.

Blocks: 1543212
Summary: Enable PGO for Android → Enable PGO for Android ARMv7

I filed bug 1543016, because the addition of the "-pgo" suffix to the platform name brought some confusion in interpreting the Perf dashboards related to Android 32bit.
Could you explain the need for this platform renaming? Is this a temporary change? For example, are you planning to remove the android-hw-p2-8-0-arm7-api-16-pgo/opt definition from taskcluster/ci/test/test-platforms.yml in favor of the old android-hw-g5-7-0-arm7-api-16/opt?

(In reply to Ionuț Goldan [:igoldan], Performance Sheriffing from comment #46)

Could you explain the need for this platform renaming? Is this a temporary change? For example, are you planning to remove the android-hw-p2-8-0-arm7-api-16-pgo/opt definition from taskcluster/ci/test/test-platforms.yml in favor of the old android-hw-g5-7-0-arm7-api-16/opt?

^ mshal or chmanchester?

Flags: needinfo?(mshal)
Flags: needinfo?(cmanchester)
Assignee

Comment 48

2 months ago

I think jmaher covered this in bug 1543016, in particular in comment https://bugzilla.mozilla.org/show_bug.cgi?id=1543016#c3. Historically for Windows/Linux, PGO has been a separate platform from opt. When enabling Android PGO for the first time in this bug, I followed that example and created a new platform type. What probably adds to the confusion is that there is a separate effort to reduce the number of test jobs, in particular the redundant coverage that PGO tests and opt tests provide. So when we turned on Android PGO builds and tests, jmaher suggested that at the same time we turn off Android opt tests, otherwise we'd run into capacity limitations for the physical device tests. This ends up looking like the platform name changed unnecessarily, but in reality we added a new one and turned off an old one.

I'm not saying this was necessarily the right thing to do in this case, but that was how we got here. I'm also not sure if there's a way to unify the two platforms (ie: have the tests named "opt" for consistency, but have them run against the PGO build, while also keeping the opt build enabled).

Flags: needinfo?(mshal)
Flags: needinfo?(cmanchester)
You need to log in before you can comment on or make changes to this bug.