The default bug view has changed. See this FAQ.

ARMv6 builds should specify JM usage, even when running on ARMv7 boards

RESOLVED FIXED in Firefox 16

Status

()

Firefox for Android
General
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: akeybl, Assigned: dvander)

Tracking

unspecified
Firefox 18
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox16+ fixed, firefox17 fixed)

Details

Attachments

(1 attachment)

(Reporter)

Description

5 years ago
ARMv6 builds should specify JM usage, even when running on ARMv7 boards.

Updated

5 years ago
Blocks: 793994
(Reporter)

Comment 1

5 years ago
dvander - do you have an estimate of when this can land? This blocks a final call on whether or not to release the ARMv6 build as part of FF16.
(Assignee)

Comment 2

5 years ago
Ideally this would be done by shipping different browser prefs with ARMv6 builds, by setting javascript.options.typeinference to false. I don't know how to do that, so if no one else does, another option would be to use a build-time flag in js/src.

Marty, are there any compiler #defines we can use to differentiate ARMv6 builds from ARMv7?
in the arm assembler from methodjit/jsc, there are a few of these:
#if WTF_ARM_ARCH_VERSION >= 7
but I'm pretty sure that not all armv7-specific code is correctly wrapped, but if we actually build and run on armv6 targets,  guess it is good enough?
(Assignee)

Comment 4

5 years ago
Created attachment 664664 [details] [diff] [review]
fix

Thanks, Marty, this should do it then.
Attachment #664664 - Flags: review?(mrosenberg)
Comment on attachment 664664 [details] [diff] [review]
fix

Review of attachment 664664 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good to me, Do we have any way of testing this? I've been burned in the past by WTF macros not actually being defined in a manner that makes sense.
Attachment #664664 - Flags: review?(mrosenberg) → review+
(In reply to Marty Rosenberg [:mjrosenb] from comment #5)
> Comment on attachment 664664 [details] [diff] [review]
> fix
> 
> Review of attachment 664664 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Looks good to me, Do we have any way of testing this? I've been burned in
> the past by WTF macros not actually being defined in a manner that makes
> sense.

Armv6 builds are built on our try, though we don't have any production armv6 devices running those tests atm. We do however have Armv7-devices running the unittests of these armv6 builds -- so if its testable, even by force-failing for just try, thats how.
(In reply to Marty Rosenberg [:mjrosenb] from comment #3)
> in the arm assembler from methodjit/jsc, there are a few of these:
> #if WTF_ARM_ARCH_VERSION >= 7
> but I'm pretty sure that not all armv7-specific code is correctly wrapped,
> but if we actually build and run on armv6 targets,  guess it is good enough?

Actually its not -- the reason we're even worrying about this BUILD side, is because we're currently testing on armv7 devices, our armv6 builds.
(Assignee)

Comment 8

5 years ago
Try builds with this patch are here: ftp://ftp.mozilla.org/pub/mobile/try-builds/danderson@mozilla.com-0d4ce027c42c/try-android-armv6/

I don't have an Android phone so I can't test this apk. Basically all we should need to verify that the patch works is that running SunSpider is significantly slower.
(In reply to David Anderson [:dvander] from comment #8)
> Try builds with this patch are here:
> ftp://ftp.mozilla.org/pub/mobile/try-builds/danderson@mozilla.com-
> 0d4ce027c42c/try-android-armv6/
> 
> I don't have an Android phone so I can't test this apk. Basically all we
> should need to verify that the patch works is that running SunSpider is
> significantly slower.

Testing on my S3, First --> install ftp://ftp.mozilla.org/pub/mobile/try-builds/danderson@mozilla.com-0d4ce027c42c/try-android --> reboot phone --> launch Nightly, run Sunspider --> E-mail myself url of results.

Then Uninstall Nightly, reinstall from ftp://ftp.mozilla.org/pub/mobile/try-builds/danderson@mozilla.com-0d4ce027c42c/try-android-armv6 --> reboot phone --> launch nightly, run sunspider --> e-mail myself url of results.

Load first URL on desktop, paste into text box second url, result:



TEST                   COMPARISON            FROM                 TO             DETAILS

=============================================================================

** TOTAL **:           1.76x as fast     3370.6ms +/- 2.3%   1909.7ms +/- 2.7%     significant

=============================================================================

  3d:                  1.35x as fast      543.8ms +/- 5.6%    403.2ms +/- 6.6%     significant
    cube:              1.28x as fast      195.8ms +/- 11.7%    152.5ms +/- 10.8%     significant
    morph:             2.62x as fast      128.7ms +/- 5.9%     49.2ms +/- 11.8%     significant
    raytrace:          -                  219.3ms +/- 7.8%    201.5ms +/- 9.6% 

  access:              3.85x as fast      455.8ms +/- 6.0%    118.5ms +/- 11.9%     significant
    binary-trees:      2.55x as fast       51.5ms +/- 12.4%     20.2ms +/- 14.0%     significant
    fannkuch:          2.41x as fast      120.7ms +/- 3.3%     50.0ms +/- 5.2%     significant
    nbody:             6.42x as fast      178.4ms +/- 8.9%     27.8ms +/- 43.6%     significant
    nsieve:            5.13x as fast      105.2ms +/- 7.1%     20.5ms +/- 4.4%     significant

  bitops:              2.22x as fast      199.1ms +/- 8.0%     89.6ms +/- 6.3%     significant
    3bit-bits-in-byte: 2.17x as fast       12.6ms +/- 29.7%      5.8ms +/- 5.2%     significant
    bits-in-byte:      3.66x as fast       53.0ms +/- 8.1%     14.5ms +/- 3.5%     significant
    bitwise-and:       2.07x as fast       78.5ms +/- 21.9%     37.9ms +/- 11.0%     significant
    nsieve-bits:       1.75x as fast       55.0ms +/- 1.9%     31.4ms +/- 11.7%     significant

  controlflow:         -                   20.5ms +/- 16.9%     16.9ms +/- 15.2% 
    recursive:         -                   20.5ms +/- 16.9%     16.9ms +/- 15.2% 

  crypto:              ??                 251.1ms +/- 11.7%    268.1ms +/- 12.0%     not conclusive: might be *1.068x as slow*
    aes:               -                  160.3ms +/- 18.9%    134.1ms +/- 12.6% 
    md5:               *1.61x as slow*     53.4ms +/- 4.3%     86.1ms +/- 6.5%     significant
    sha1:              ??                  37.4ms +/- 33.4%     47.9ms +/- 36.3%     not conclusive: might be *1.28x as slow*

  date:                -                  340.8ms +/- 8.7%    324.1ms +/- 3.3% 
    format-tofte:      -                  173.8ms +/- 8.0%    165.9ms +/- 4.3% 
    format-xparb:      -                  167.0ms +/- 12.1%    158.2ms +/- 5.9% 

  math:                3.30x as fast      364.2ms +/- 5.5%    110.4ms +/- 12.5%     significant
    cordic:            4.23x as fast      105.7ms +/- 17.6%     25.0ms +/- 1.9%     significant
    partial-sums:      3.54x as fast      203.1ms +/- 7.7%     57.4ms +/- 24.9%     significant
    spectral-norm:     1.98x as fast       55.4ms +/- 6.3%     28.0ms +/- 15.5%     significant

  regexp:              -                   81.9ms +/- 19.0%     70.5ms +/- 23.6% 
    dna:               -                   81.9ms +/- 19.0%     70.5ms +/- 23.6% 

  string:              2.19x as fast     1113.4ms +/- 3.7%    508.4ms +/- 8.9%     significant
    base64:            1.76x as fast       73.2ms +/- 11.1%     41.6ms +/- 16.7%     significant
    fasta:             9.20x as fast      559.3ms +/- 4.8%     60.8ms +/- 27.6%     significant
    tagcloud:          1.22x as fast      195.7ms +/- 6.0%    160.1ms +/- 14.9%     significant
    unpack-code:       -                  192.5ms +/- 11.9%    183.2ms +/- 12.3% 
    validate-input:    1.48x as fast       92.7ms +/- 8.8%     62.7ms +/- 8.4%     significant
(Assignee)

Comment 10

5 years ago
Thank you for testing, Callek! It looks like this works so I'll check it in shortly.
Status: NEW → ASSIGNED
(Reporter)

Comment 11

5 years ago
(In reply to David Anderson [:dvander] from comment #10)
> Thank you for testing, Callek! It looks like this works so I'll check it in
> shortly.

Please also nominate for uplift to m-a and m-b as soon as possible. We're working on a very short timeline to react to broken tests here.
What is the status of this bug? Did the changes get uplifted?
(Assignee)

Comment 13

5 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/ff77ff9b2033
(Reporter)

Comment 14

5 years ago
Comment on attachment 664664 [details] [diff] [review]
fix

Conditional on this check-in looking good on central, approving for Aurora/Beta as well to ensure that this is exercised in our tests asap.
Attachment #664664 - Flags: approval-mozilla-beta+
Attachment #664664 - Flags: approval-mozilla-aurora+
https://hg.mozilla.org/mozilla-central/rev/ff77ff9b2033
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 18
(Assignee)

Comment 16

5 years ago
https://hg.mozilla.org/releases/mozilla-beta/rev/773303abaae9
(Reporter)

Updated

5 years ago
status-firefox16: --- → fixed

Updated

5 years ago
status-firefox17: --- → affected
This needs to land on Aurora before Monday Oct 8th in order to be in our first 17.0 beta.  Email sent as well.
Comment on attachment 664664 [details] [diff] [review]
fix

Review of attachment 664664 [details] [diff] [review]:
-----------------------------------------------------------------

I attempted to land into aurora just now, but get:

$ hg qpush
applying bug793740-DisableARMv6.patch
patching file js/src/jsinfer.cpp
Hunk #1 FAILED at 2040
1 out of 1 hunks FAILED -- saving rejects to file js/src/jsinfer.cpp.rej
patch failed, unable to continue (try -v)
patch failed, rejects left in working dir
errors during apply, please fix and refresh bug793740-DisableARMv6.patch

And I'm not about to attempt unwinding .rej's right now.

Updated

5 years ago
Blocks: 787893
(Assignee)

Comment 19

5 years ago
https://hg.mozilla.org/releases/mozilla-beta/rev/2d380f304bf0

Updated

5 years ago
status-firefox17: affected → fixed
You need to log in before you can comment on or make changes to this bug.