Created attachment 744822 [details] [diff] [review]
Comment on attachment 744822 [details] [diff] [review]
[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
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?
(In reply to firstname.lastname@example.org from comment #4)
Automated tests are already in the tree (js/src/jit-tests/tests/asm.js).
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.
Is there any need for manual testing on this (I see in comment 5 that automated test are already in the tree)?
Does anyone have any idea why this is happening?
Can you file a separate bug on that, and include OS and other support info?
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.
(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.
(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.
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