1210445 - 24% win8 e10s tp5o scroll regression on Inbound (v.44) September 29 from revision aa61d48eb6ae

Status: RESOLVED FIXED

(Core :: Panning and Zooming, defect)

Description

[Joel Maher ( :jmaher ) (UTC -8)]

as indicated in https://bugzilla.mozilla.org/show_bug.cgi?id=1208636#c11 we have a regression in tp5o scroll.  24%+ is pretty bad, so this is something we should understand and reduce/fix/document.

A comparison view of this shows regressions on most pages:
https://treeherder.allizom.org/perf.html#/comparesubtest?originalProject=mozilla-inbound&originalRevision=d51440cc7a2f&newProject=mozilla-inbound&newRevision=aa61d48eb6ae&originalSignature=0c8a722afb50907756257ad9df5781d6b03f9eb0&newSignature=0c8a722afb50907756257ad9df5781d6b03f9eb0

this is the change:
https://hg.mozilla.org/integration/mozilla-inbound/rev/aa61d48eb6ae

and I had collected various samples via retriggers:
https://treeherder.mozilla.org/#/jobs?repo=mozilla-inbound&fromchange=8b3a4fdebdfc&tochange=5f2acd473d59&filter-searchStr=talos

this appears to be e10s only.

Comment 1

[Joel Maher ( :jmaher ) (UTC -8)]

:mchang, is there any further information you need here? I konw we were looking to understand tp5o scroll more, if :blassey is afk, maybe :mconley can help out.

Comment 2

[Mike Conley (:mconley) (:⚙️)]

Cc'ing BenWa who also might have some ideas.

Comment 3

[Benoit Girard (:BenWa)]

First step would be to see if we end up with a larger display port with tp5o with this patch. If we're painting more then I'd expect a worse tp5o score. If that's it then a 24% regression would mean that we're painting quite a bit more.

Comment 4

[Kartikaya Gupta (email:[e-mail])]

The intent of the patch was to increase the size of the displayport on high-memory devices, so yes, I would expect that's what is happening.

Comment 5

[Mason Chang [Inactive] [:mchang]]

Yes, I just wanted to make sure that tp5o's "frame interval" was the amount of time spent. The patch increased the displayport multiplier on high memory systems to reduce checkerboarding. If tp5o just measures time spent painting, then we expect an increase and the regression is ok.

Comment 6

[Joel Maher ( :jmaher ) (UTC -8)]

:avih, can you help figure out :mchang's question about the "frame interval" and if tp5o scroll just measures the time painting?  I believe the answer is yes and this is something we need to accept as is.

Comment 7

[Benoit Girard (:BenWa)]

Actually this is tp5o_SCROLL so I take it back. If we scroll by x pixel, we should still only paint x pixel at the edge of the displayport. I don't think the score is including the time to load the page.

Comment 8

[Mason Chang [Inactive] [:mchang]]

(In reply to Benoit Girard (:BenWa) from comment #7)
> Actually this is tp5o_SCROLL so I take it back. If we scroll by x pixel, we
> should still only paint x pixel at the edge of the displayport. I don't
> think the score is including the time to load the page.

But the patch is based on velocity. If the scroll is fast, we extend the displayport more, so if we scroll by x pixels, but it is considered a skate scroll, the displayport grows so we'd paint more.

Comment 9

[Avi Halachmi (:avih)]

(In reply to Benoit Girard (:BenWa) from comment #7)
> I don't think the score is including the time to load the page.

Correct.

From here: https://wiki.mozilla.org/Buildbot/Talos/Tests#tp5o_scroll
> For each page, the test waits 500ms after the page load event fires, then
> iterates 100 scroll steps of 10px each (or until the bottom of the page is
> reached - whichever comes first), then reports the average frame interval. 

The scroll iterations are with rAF in ASAP mode, and the average interval is between the rAF callbacks.

Comment 10

[Benoit Girard (:BenWa)]

(In reply to Mason Chang [:mchang] from comment #8)
> (In reply to Benoit Girard (:BenWa) from comment #7)
> > Actually this is tp5o_SCROLL so I take it back. If we scroll by x pixel, we
> > should still only paint x pixel at the edge of the displayport. I don't
> > think the score is including the time to load the page.
> 
> But the patch is based on velocity. If the scroll is fast, we extend the
> displayport more, so if we scroll by x pixels, but it is considered a skate
> scroll, the displayport grows so we'd paint more.

Ahh ok, so the tp5o_scroll will measure the main thread scrolling but not how responsive the scroll is to the user. Then yes it's not as bad as it seems.

Comment 11

[Joel Maher ( :jmaher ) (UTC -8)]

this is backed out now

Comment 12

[Joel Maher ( :jmaher ) (UTC -8)]

are there future work items here?  I would like to close this bug out if we are not working on it specifically.

Comment 13

[Benoit Girard (:BenWa)]

Given that it is backed out I think it's fine to close this out. When it re-land I imagine we would catch it and file another similar bug.