868184 - OdinMonkey: enable by default on beta/release

Status: VERIFIED FIXED

(Core :: JavaScript Engine, defect)

Description

[Luke Wagner [:luke]]

Attachment: patch

At this point, we expect the asm.js subset to only grow over time.  Bug fallout has also been minor and well in line with regular JS engine development.  Thus, there isn't a good reason to leave OdinMonkey turned off in Beta/Release and there are several reasons to turn it on by default (viz., http://www.unrealengine.com/html5).  Lastly, the patch removes the "experimental_" prefix of "javascript.options.experimental_asmjs" since the pref is now on by default.

Comment 1

[Luke Wagner [:luke]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/d4ac16cbea47

Comment 2

[Luke Wagner [:luke]]

Comment on attachment 744822 [details] [diff] [review]
patch

[Approval Request Comment]
Bug caused by (feature/regressing bug #): 840282
User impact if declined: asm.js optimizations won't be enabled on beta/release
Testing completed (on m-c, etc.): m-c
Risk to taking this patch (and alternatives if risky): low

Comment 3

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/d4ac16cbea47

Comment 4

[Lukas Blakk [:lsblakk] use ?needinfo]

Can you provide tips on a test plan QA can use & add to automated tests in order to keep on top of potential regressions from enabling this new feature?

Comment 5

[Luke Wagner [:luke]]

(In reply to [e-mail] from comment #4)
Automated tests are already in the tree (js/src/jit-tests/tests/asm.js).

Comment 6

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/957075eb2de1

Comment 7

[Alex Keybl [:akeybl]]

Noted in FF22 as "asm.js optimizations (<a href="https://blog.mozilla.org/luke/2013/03/21/asm-js-in-firefox-nightly/">OdinMonkey</a>) enabled for major performance improvements"

Please email me if you'd like to help in reworking that note.

Comment 8

[Simona Badau, Desktop QA]

Is there any need for manual testing on this (I see in comment 5 that automated test are already in the tree)?

I can confirm that the preference javascript.options.asmjs is enabled by default on Firefox 23 beta 2. But Firefox closes and I get the following message in the terminal "Aborted (core dumped)" when loading http://www.unrealengine.com/html5.

Does anyone have any idea why this is happening?

Comment 9

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

Can you file a separate bug on that, and include OS and other support info?

Comment 10

[Jeff Walden [:Waldo]]

Note that the asm.js syntactic rules have been changing over the last several weeks in response to various bug reports and such, so it's not necessarily the case that something that passes as asm.js now (or at first landing) passed (or will pass now) as asm.js.  I do believe some care has been taken to keep the Unreal demo working, but at some point it wouldn't surprise me if some asm.js change means that that demo has to be regenerated with a newer version of Emscripten to preserve its performance gains.

Anyway, just a warning that it's not *necessarily* the case that an observed performance regression on a page that uses asm.js is a definite bug.

Comment 11

[Luke Wagner [:luke]]

(In reply to Jeff Walden [:Waldo] (remove +bmo to email) from comment #10)
Indeed, care was taken; Unreal, BananaBread, and all the other demos should work on all release channels of Firefox atm.

Comment 12

[Simona Badau, Desktop QA]

(In reply to Vladimir Vukicevic [:vlad] [:vladv] from comment #9)
> Can you file a separate bug on that, and include OS and other support info?

Filed bug 889802.

Comment 13

[Mihai Morar, (:MihaiMorar)]

Verified that the pref is enabled by default on all OS's on FF 22 and FF 23b4. As long Simona has filed a new bug for that crash on Ubuntu, I'm marking this bug as Verified