OMTP should be enabled by default!

Let's attach blockers to this bug, if we don't want it on nightly by default until they're fixed.

Comment on attachment 8913297 [details] Bug 1403935: Enable OMTP by default on windows only. https://reviewboard.mozilla.org/r/184670/#review189864 \o/

Pushed by bschouten@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/843b90c48664 Enable OMTP by default on windows only. r=dvander

A feature as big as this deserves a blog post with motivation, basic design, basic implementation detail and perf improvements.

(In reply to Mayank Bansal from comment #5) > A feature as big as this deserves a blog post with motivation, basic design, > basic implementation detail and perf improvements. Yup, it's coming.

I'm on the latest inbound with this patch and "layers.omtp.enabled" is now true. However in about:support it says: OMTP opt-in by default: OMTP is an opt-in feature available by user: Enabled by pref On Fx58 it's not an opt-in feature anymore. Shouldn't the wording be different?

I filed bug 1404532 for comment 7.

Hey, You might want to take care about this crash bug 1395394 reported one month ago. I can reproduce quite easily, now that OMTP is enabled by default. Thanks.

Backout by archaeopteryx@coole-files.de: https://hg.mozilla.org/mozilla-central/rev/a5f92049b409 Backed out changeset 843b90c48664 for frequently asserting in mochitest gfx/tests/mochitest/test_font_whitelist.html on Windows non-stylo builds. r=backout a=backout

Backed out for frequently asserting in mochitest gfx/tests/mochitest/test_font_whitelist.html on Windows non-stylo builds: https://hg.mozilla.org/mozilla-central/rev/a5f92049b409adbb465586f6217416aa9b7b3157 Push with failures: https://treeherder.mozilla.org/#/jobs?repo=mozilla-inbound&revision=843b90c486643459c1dd90d0f074c7d8f1c97286&filter-searchStr=047198b05bb06a42bb5266b8a689ba00eb43cf58 Failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=134213138&repo=mozilla-inbound 20:24:28 INFO - 1974 INFO None1975 INFO TEST-START | gfx/tests/mochitest/test_font_whitelist.html 20:24:28 INFO - GECKO(6584) | ++DOMWINDOW == 10 (0000024E0890C800) [pid = 7080] [serial = 12] [outer = 0000024E08B62000] 20:24:29 INFO - GECKO(6584) | MEMORY STAT | vsize 1475MB | vsizeMaxContiguous 131762077MB | residentFast 99MB | heapAllocated 18MB 20:24:29 INFO - GECKO(6584) | Assertion failure: IsAcquired() && mOwningThread == PR_GetCurrentThread(), at z:/build/build/src/xpcom/threads/BlockingResourceBase.cpp:401 20:24:29 INFO - GECKO(6584) | #01: mozilla::AssertIsMainThreadOrServoFontMetricsLocked() [layout/style/ServoBindings.cpp:2459] 20:24:29 INFO - GECKO(6584) | #02: mozilla::WeakPtr<mozilla::gfx::UnscaledFont>::operator mozilla::gfx::UnscaledFont *() [mfbt/WeakPtr.h:276] 20:24:29 INFO - GECKO(6584) | #03: mozilla::SupportsWeakPtr<mozilla::gfx::UnscaledFont>::~SupportsWeakPtr<mozilla::gfx::UnscaledFont>() [mfbt/WeakPtr.h:207] 20:24:29 INFO - GECKO(6584) | #04: mozilla::gfx::UnscaledFont::~UnscaledFont() [gfx/2d/ScaledFontBase.cpp:34] 20:24:29 INFO - GECKO(6584) | #05: mozilla::gfx::UnscaledFontDWrite::`scalar deleting destructor'(unsigned int) 20:24:29 INFO - GECKO(6584) | #06: mozilla::detail::RefCounted<mozilla::gfx::UnscaledFont,0>::Release() [mfbt/RefCounted.h:140] 20:24:29 INFO - GECKO(6584) | #07: mozilla::gfx::ScaledFont::~ScaledFont() [gfx/2d/ScaledFontBase.cpp:41] 20:24:29 INFO - GECKO(6584) | #08: mozilla::gfx::ScaledFontDWrite::`scalar deleting destructor'(unsigned int) 20:24:29 INFO - GECKO(6584) | #09: mozilla::detail::RefCounted<mozilla::gfx::ScaledFont,0>::Release() [mfbt/RefCounted.h:140] 20:24:29 INFO - GECKO(6584) | #10: mozilla::gfx::FillGlyphsCommand::~FillGlyphsCommand() 20:24:29 INFO - GECKO(6584) | #11: mozilla::gfx::FillGlyphsCommand::`scalar deleting destructor'(unsigned int) 20:24:29 INFO - GECKO(6584) | #12: mozilla::gfx::DrawTargetCaptureImpl::~DrawTargetCaptureImpl() [gfx/2d/DrawTargetCapture.cpp:22] 20:24:29 INFO - GECKO(6584) | #13: mozilla::gfx::DrawTargetCaptureImpl::`scalar deleting destructor'(unsigned int) 20:24:29 INFO - GECKO(6584) | #14: mozilla::detail::RefCounted<mozilla::gfx::DrawTarget,0>::Release() [mfbt/RefCounted.h:140] 20:24:29 INFO - GECKO(6584) | #15: mozilla::detail::RunnableFunction<<lambda_a08043e6d837f3855247633b89253f78> >::`scalar deleting destructor'(unsigned int) 20:24:29 INFO - GECKO(6584) | #16: mozilla::Runnable::Release() [xpcom/threads/nsThreadUtils.cpp:47] 20:24:29 INFO - GECKO(6584) | #17: nsThread::ProcessNextEvent(bool,bool *) [xpcom/threads/nsThread.cpp:1047] 20:24:29 INFO - GECKO(6584) | #18: NS_ProcessNextEvent(nsIThread *,bool) [xpcom/threads/nsThreadUtils.cpp:524] 20:24:29 INFO - GECKO(6584) | #19: mozilla::ipc::MessagePumpForNonMainThreads::Run(base::MessagePump::Delegate *) [ipc/glue/MessagePump.cpp:368] 20:24:29 INFO - GECKO(6584) | #20: MessageLoop::RunHandler() [ipc/chromium/src/base/message_loop.cc:320] 20:24:29 INFO - GECKO(6584) | #21: MessageLoop::Run() [ipc/chromium/src/base/message_loop.cc:300] 20:24:29 INFO - GECKO(6584) | #22: nsThread::ThreadFunc(void *) [xpcom/threads/nsThread.cpp:429] 20:24:29 INFO - GECKO(6584) | #23: PR_NativeRunThread [nsprpub/pr/src/threads/combined/pruthr.c:406] 20:24:29 INFO - GECKO(6584) | #24: pr_root [nsprpub/pr/src/md/windows/w95thred.c:96] 20:24:29 INFO - GECKO(6584) | #25: ucrtbase.dll + 0x20369 20:24:29 INFO - GECKO(6584) | #26: KERNEL32.DLL + 0x12774 20:24:29 INFO - GECKO(6584) | #27: ntdll.dll + 0x70d61

The Nightly builds that had this enabled were about 1.5x crashier than normal, due to bug 1395394, bug 1404587, and bug 1404627. If it hadn't already been backed out I would have suggested doing so. Please fix them before re-enabling OMTP! :)

(In reply to Nicholas Nethercote [:njn] from comment #13) > The Nightly builds that had this enabled were about 1.5x crashier than > normal, due to bug 1395394, bug 1404587, and bug 1404627. If it hadn't > already been backed out I would have suggested doing so. Please fix them > before re-enabling OMTP! :) (What we believe to be) fixes for all those bugs are up/landed! So working on it :-).

> (What we believe to be) fixes for all those bugs are up/landed! So working > on it :-). I see the action! Excellent.

Pushed by bschouten@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/56675d0ea641 Enable OMTP by default on windows only. r=dvander

When this first landed, these improvements were noticed: == Change summary for alert #9751 (as of September 29 2017 17:49 UTC) == Improvements: 34% tsvgr_opacity summary windows7-32 opt e10s 437.97 -> 289.73 21% tsvgr_opacity summary windows10-64 opt e10s 290.61 -> 230.71 10% tsvgx summary windows7-32 opt e10s 522.98 -> 470.49 9% tsvgx summary windows10-64 opt e10s 265.09 -> 240.39 For up to date results, see: https://treeherder.mozilla.org/perf.html#/alerts?id=9751

The backout canceled these results: == Change summary for alert #9756 (as of September 30 2017 23:03 UTC) == Regressions: 64% tsvgr_opacity summary windows7-32 opt e10s 265.71 -> 434.57 32% tsvgr_opacity summary windows10-64 opt e10s 219.76 -> 289.70 14% tsvgx summary windows7-32 opt e10s 458.25 -> 521.64 12% tsvgx summary windows10-64 opt e10s 235.52 -> 262.66 For up to date results, see: https://treeherder.mozilla.org/perf.html#/alerts?id=9756

The repush brought those improvements back. The PGO results properly confirm the perf boost! == Change summary for alert #9770 (as of October 02 2017 02:21 UTC) == Improvements: 39% tsvgr_opacity summary windows7-32 opt e10s 435.66 -> 265.49 26% tsvgr_opacity summary windows10-64 opt e10s 290.31 -> 215.46 19% tsvgr_opacity summary windows10-64 pgo e10s 256.81 -> 206.84 12% tsvgx summary windows7-32 opt e10s 523.38 -> 461.34 11% tsvgx summary windows10-64 opt e10s 265.15 -> 235.37 For up to date results, see: https://treeherder.mozilla.org/perf.html#/alerts?id=9770

Can you provide some docs for the OMTP feature? What does it stand for?

OMTP = Off Main Thread Painting, an architectural graphics change that should improve browser responsiveness by freeing up the main thread. AFAIK, OMTP is not expected to improve graphics performance. Here is the OMTP design doc: https://docs.google.com/document/d/19sZYWo_Fe1x8hA2FJPExijt5rZNA_I4EFzgnEShNqpY/edit

(In reply to Ionuț Goldan [:igoldan], Performance Sheriffing from comment #21) > Can you provide some docs for the OMTP feature? What does it stand for? David is supposed to write a blog post.. I can still do it as well.. David?

We've got a draft blog post - I'll finish it up and send it around.

The blog post, for those waiting: https://mozillagfx.wordpress.com/2017/12/05/off-main-thread-painting/