Bug 211711 - WinCairo scores 1.00 on MotionMark benchmark since r261113
Summary: WinCairo scores 1.00 on MotionMark benchmark since r261113
Status: NEW
Alias: None
Product: WebKit
Classification: Unclassified
Component: Platform (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL: https://browserbench.org/MotionMark1.1/
Keywords:
Depends on:
Blocks:
 
Reported: 2020-05-10 21:29 PDT by Fujii Hironori
Modified: 2020-05-13 13:57 PDT (History)
1 user (show)

See Also:


Attachments
Patch not to use WM_TIMER (623 bytes, patch)
2020-05-10 21:31 PDT, Fujii Hironori
no flags Details | Formatted Diff | Diff
MotionMark of WinCairo WK1 (r261577) (283.77 KB, image/png)
2020-05-12 16:32 PDT, Fujii Hironori
no flags Details
MotionMark of WinCairo WK1 (r261577) (No updateRendering in WM_PAINT) (283.29 KB, image/png)
2020-05-12 19:09 PDT, Fujii Hironori
no flags Details
html to console.log the interval of requestAnimationFrame (483 bytes, text/html)
2020-05-12 22:24 PDT, Fujii Hironori
no flags Details
experimental patch to conditionally use a timer or callOnMainThread depending on the two-frame-previous tick (1.90 KB, patch)
2020-05-13 13:57 PDT, Fujii Hironori
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Fujii Hironori 2020-05-10 21:29:49 PDT
Since r261113 (Bug 204713), WinCairo scores 1.00 (the worst score) on MotionMark benchmark.

Bug 204713 – Throttling requestAnimationFrame should be controlled by RenderingUpdateScheduler
Comment 1 Fujii Hironori 2020-05-10 21:31:45 PDT
Created attachment 398997 [details]
Patch not to use WM_TIMER

WM_TIMER seems the bottleneck.
Comment 2 Said Abou-Hallawa 2020-05-11 11:23:17 PDT
Is the cause of the MotionMark regression r261113 or r261264 and r261333? I think r261113 was affecting the iOS Low power case only. But after r261264, scheduling the RenderingUpdate on Windows goes through DisplayRefreshMonitor which is different from before.
Comment 3 Fujii Hironori 2020-05-11 16:41:15 PDT
I have a WinCairo bulit binary of r261262. It already has the same issue.
But, I'm not sure which revision is the real culprit.
Comment 4 Fujii Hironori 2020-05-12 16:32:25 PDT
Created attachment 399200 [details]
MotionMark of WinCairo WK1 (r261577)

r261577 fixed another rAF issue of WinCairo WK1, I tried MotionMark with WinCairo WK1.
WinCairo WK1 works well for some tests, but scores 1.00 only for Focus and Images tests.
Those tests are run in Accelerated Compositing mode.
Comment 6 Fujii Hironori 2020-05-12 19:09:48 PDT
Created attachment 399227 [details]
MotionMark of WinCairo WK1 (r261577) (No updateRendering in WM_PAINT)

Why does WinCairo WK1 non-AC perform well for MotionMark even though it's also using a timer?
The reason seems that it calls Page::updateRendering twice in a single timer fire.
Once in WebView::paint, another in WebView::flushPendingGraphicsLayerChangesSoon.

I tried MotionMark with WinCairo WK1 again by applying a patch not to call Page::updateRendering in WebView::paint.
MotionMark score falled down to 1.39. (I don't know why it isn't 1.00)

diff --git a/Source/WebKitLegacy/win/WebView.cpp b/Source/WebKitLegacy/win/WebView.cpp
index fe7cb6a1d03..ec44027e06c 100644
--- a/Source/WebKitLegacy/win/WebView.cpp
+++ b/Source/WebKitLegacy/win/WebView.cpp
@@ -1285,7 +1285,7 @@ void WebView::paint(HDC dc, LPARAM options)
 {
     LOCAL_GDI_COUNTER(0, __FUNCTION__);
 
-    m_page->updateRendering();
+    //m_page->updateRendering();
 
     if (paintCompositedContentToHDC(dc)) {
         ::ValidateRect(m_viewWindow, nullptr);
Comment 7 Fujii Hironori 2020-05-12 22:24:28 PDT
Created attachment 399240 [details]
html to console.log the interval of requestAnimationFrame

This html shows the duplicated frame of WinCairo WK1 non-AC.
Comment 8 Fujii Hironori 2020-05-13 13:51:48 PDT
Chromium seems to be using own message pump.
Comment 9 Fujii Hironori 2020-05-13 13:57:16 PDT
Created attachment 399295 [details]
experimental patch to conditionally use a timer or callOnMainThread depending on the two-frame-previous tick

FPS can't be stable with this patch.