Closed Bug 1232742 Opened 5 years ago Closed 5 years ago

Enable OSX's CEMPEngine by default

Categories

(Core :: Canvas: WebGL, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla46
Tracking Status
firefox46 --- fixed

People

(Reporter: BenWa, Assigned: BenWa)

References

Details

Attachments

(1 file)

+++ This bug was initially created as a clone of Bug #1223540 +++
I experimented a bit with CEMPengine and the performance is generally very good.

We discussed this during the last WebGL session in Orlando. The consensus was that the approach is interesting and that we should consider shipping it if it's not much work. Our plan is to turn on the feature and if it proves problematic to ship we will disable it.

On the Unity benchmark[1] I get a 15% speed improvement (86856 vs. 100044).

The outcome of this will likely inform how we want to pursue remoting GL to another thread and/or process for performance reason.

[1] http://beta.unity3d.com/jonas/WebGLBenchmark/
Summary: Add a pref for CGL's multithreaded mode (CEMPEngine) → Enable OSX's CEMPEngine by default
Attached patch patchSplinter Review
Assignee: nobody → bgirard
Status: NEW → ASSIGNED
Attachment #8698535 - Flags: review?(jmuizelaar)
This feature might be a good candidate for fuzzing. In the past jruderman did a 'drive-by' fuzzing of OSX's accelerated canvas feature when we were thinking of enabling it. Based on these results we decided not to ship the feature. Fuzzing here could be beneficial. General rendering fuzzing and WebGL fuzzing on the mac platform with this feature turned on should trigger the new code path.
Flags: needinfo?(abillings)
Comment on attachment 8698535 [details] [diff] [review]
patch

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

Wait for 46
Attachment #8698535 - Flags: review?(jmuizelaar) → review+
I'll mention the fuzzing aspect to the team. Thanks for pinging me on it, Benoit.
Flags: needinfo?(abillings)
Already randomizing the pref in DOMFuzz:
https://github.com/MozillaSecurity/funfuzz/commit/9fef5bea3be144c526e1910b76a287c05b8f68c9

Anything new I need to do to fuzz this code well?
Cool, I figured that might of been the case but I'm not familiar with how preferences are picked up.

As long as the fuzzing includes WebGL / web content on Mac then we should be covered. I'm assuming here that no news is good news.

Thanks!
I'll enable this in early Jan 2016. If anything goes wrong I'll be more responsive to back it out then. NI for landing reminder.
Flags: needinfo?(bgirard)
I'm leaving out the 'gfxPrefs.h' hunk. If the pref isn't read for some reason it can default to false.
Flags: needinfo?(bgirard)
Apparently you landed this patch with the wrong number (1223540). Am I right?
Flags: needinfo?(bgirard)
That's correct. It should of been this bug # :(
Flags: needinfo?(bgirard)
https://hg.mozilla.org/integration/mozilla-inbound/rev/0a8b0fc58afea139dbe815a3ba74d19d5cef017a (landed yesterday)
https://hg.mozilla.org/mozilla-central/rev/0a8b0fc58afe (landed today)
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla46
You need to log in before you can comment on or make changes to this bug.