Perf regression from "Omit the parent LayoutView's scroll offset from FrameRect" |
||||
Issue descriptionThis issue is split out from bug 814216 . A bisect there found: 📍 Found significant differences after each of 2 commits. https://pinpoint-dot-chromeperf.appspot.com/job/14cb244e440000 Renderer observer of frame submission by jonross@chromium.org https://chromium.googlesource.com/chromium/src/+/a2ff4f82109df55045dee9f54985a98054f86dc4 Omit the parent LayoutView's scroll offset from FrameRect by szager@chromium.org https://chromium.googlesource.com/chromium/src/+/cf91f7965c29e2bc93e81f79cef5d77481b00e6e Understanding performance regressions: http://g.co/ChromePerformanceRegressions This bug tracks the second regression caused by szager@'s patch.
,
Mar 15 2018
📍 Pinpoint job started. https://pinpoint-dot-chromeperf.appspot.com/job/1021fe36440000
,
Mar 15 2018
📍 Pinpoint job started. https://pinpoint-dot-chromeperf.appspot.com/job/13c3bf8e440000
,
Mar 15 2018
📍 Found significant differences after each of 2 commits. https://pinpoint-dot-chromeperf.appspot.com/job/13c3bf8e440000 Renderer observer of frame submission by jonross@chromium.org https://chromium.googlesource.com/chromium/src/+/a2ff4f82109df55045dee9f54985a98054f86dc4 Omit the parent LayoutView's scroll offset from FrameRect by szager@chromium.org https://chromium.googlesource.com/chromium/src/+/cf91f7965c29e2bc93e81f79cef5d77481b00e6e Understanding performance regressions: http://g.co/ChromePerformanceRegressions
,
Mar 15 2018
Disregard - just confirming the Mac benchmark is the same issue.
,
Mar 16 2018
📍 Pinpoint job started. https://pinpoint-dot-chromeperf.appspot.com/job/1434d2a1440000
,
Mar 16 2018
If I understand these graphs correctly, it seems like my change caused the number of tasks per frame to go up, but it causes the CPU time per frame to go *down*. I would guess that the change in tasks per frame is caused by the new call to FrameRectsChanged here: https://chromium-review.googlesource.com/c/chromium/src/+/923427/7/third_party/WebKit/Source/core/paint/PaintLayerScrollableArea.cpp I subsequently removed that call in this CL: https://chromium-review.googlesource.com/c/chromium/src/+/923427/7/third_party/WebKit/Source/core/paint/PaintLayerScrollableArea.cpp ... so I *think* my CL is off the hook.
,
Mar 16 2018
Whoops, the second link in my previous comment was wrong; should be: https://chromium-review.googlesource.com/951641
,
Mar 19 2018
Do you mean the other way around? It looks to me like the tasks per frame went down but the CPU time per frame went up, e.g. @537429: https://chromeperf.appspot.com/report?sid=e15a8dab82e58ee841e7e8fe8ffdab7357ad4295e3acc80b57771476e43767b6&start_rev=532705&end_rev=539040 But...your patch is actually outside that regression range so it's not responsible. I can't reproduce the regression locally - going as far as flashing to the same Android build - and neither can the bisect bots (see #6. Where your patch was implicated was on the Mac graph and that's since recovered). Given there's no ref build, I suspect the Android WebView regression might just be a configuration change of some kind. In any case, it's not actionable so I'm going to WontFix this. |
||||
►
Sign in to add a comment |
||||
Comment 1 by bokan@chromium.org
, Mar 14 2018Labels: OS-Android
Status: Assigned (was: assin)