New issue
Advanced search Search tips

Issue 821953 link

Starred by 1 user

Issue metadata

Status: WontFix
Owner: ----
Closed: Mar 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 3
Type: Bug

Blocking:
issue 770343



Sign in to add a comment

Perf regression from "Omit the parent LayoutView's scroll offset from FrameRect"

Project Member Reported by bokan@chromium.org, Mar 14 2018

Issue description

This 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.
 

Comment 1 by bokan@chromium.org, Mar 14 2018

Blocking: 770343
Labels: OS-Android
Status: Assigned (was: assin)
I'm going to take a look at this as I have an N6 and szager@ couldn't repro
Project Member

Comment 4 by 42576172...@developer.gserviceaccount.com, Mar 15 2018

Cc: chrishtr@chromium.org kenrb@chromium.org fsam...@chromium.org kylec...@chromium.org jonr...@chromium.org piman@chromium.org
Owner: szager@chromium.org
📍 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

Comment 5 by bokan@chromium.org, Mar 15 2018

Cc: -kenrb@chromium.org -fsam...@chromium.org -piman@chromium.org -kylec...@chromium.org -jonr...@chromium.org
Owner: ----
Disregard - just confirming the Mac benchmark is the same issue.

Comment 7 by szager@chromium.org, 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.

Comment 8 by szager@chromium.org, Mar 16 2018

Whoops, the second link in my previous comment was wrong; should be:

https://chromium-review.googlesource.com/951641

Comment 9 by bokan@chromium.org, Mar 19 2018

Status: WontFix (was: Assigned)
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