1009646 - 9% canvasmark regression on android 2.2 May 9th on Inbound (fx32)

Status: RESOLVED WONTFIX

(Core :: Graphics: Layers, defect)

Description

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

Good old Canvasmark, keeps finding regressions.  Here is a graph:
http://graphs.mozilla.org/graph.html#tests=[[281,132,25],[281,131,25]]&sel=1399390072952,1399994872953&displayrange=7&datatype=running

Here is a set of retriggers (to help filter out noise) and fill in missing data:
https://tbpl.mozilla.org/?tree=Mozilla-Inbound&fromchange=78948f024bdf&tochange=e9cfc11a722d&jobname=Android%202.2%20Tegra%20mozilla-inbound%20talos%20remote-tcanvasmark

It points at this push:
https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?changeset=ddd5e6e4c6e3

I suspect bug 1007897 is the cause, but I am not an expert at this stuff.

Comment 1

[Benoit Girard (:BenWa)]

The first link didn't work for me:
http://graphs.mozilla.org/graph.html#tests=[[281,132,25],[281,131,25],[289,11,29],[289,11,20]]&sel=none&displayrange=365&datatype=running

Comment 2

[Benoit Girard (:BenWa)]

This is interesting. I think the problem is something strange is happening with Skia Snapshot and not a problem with the original patch.

Investigating...

Comment 3

[Benoit Girard (:BenWa)]

There's something really fishy here. I can see in profiles without this patch that we spend 8% of our time in SourceSurfaceSkia::DrawTargetWillChange and with the patch the profiles look better.

I also get about the same score on desktop even through my tests are running faster.

Comment 4

[James Willcox (:snorp) ([e-mail]) (he/him)]

If the surface has living snapshots at the time of destruction we will copy the surface into it in each one of them. I bet something like that is happening here.

Comment 5

[Avi Halachmi (:avih)]

I'm not absolutely sure we're interpreting the graph correctly. In Canvasmark, higher values are better, but from May-9th onwards, I see only increase (improvements) in numbers - and none of them looks to me more than 1-2-3%.

I do see a meaningful regression though, on May-8th (4:50AM on whatever timezone which graph server uses) where the numbers go from ~254 to ~226 (~10% regression), but if the supposedly offending landed on May-9th, how can graphserver show the regression a full day earlier than that?

Comment 6

[Avi Halachmi (:avih)]

(In reply to Avi Halachmi (:avih) from comment #5)
> I'm not absolutely sure we're interpreting the graph correctly...

Apologies, I was looking at the graph linked from comment 0, but this graph accidentally links to tsvgx rather than to android canvasmark. This is the correct graph:

http://graphs.mozilla.org/graph.html#tests=[[289,64,20]]&sel=1399227045048.7837,1400017239406.1504,2500,6800&displayrange=30&datatype=running

And it does show a decrease in values (regression) on May-9th like comment 0 suggests. Please ignore comment 5.

Comment 7

[James Willcox (:snorp) ([e-mail]) (he/him)]

(In reply to James Willcox (:snorp) ([e-mail]) from comment #4)
> If the surface has living snapshots at the time of destruction we will copy
> the surface into it in each one of them. I bet something like that is
> happening here.

This no longer appears to be the case, so ignore this :)

Comment 8

[Benoit Girard (:BenWa)]

So CanvasMark on android is total disaster. We're super far from being canvas bound. Our bottlenecks are JS bailouts bug 1009829, sync canvas swaps, long uploads etc...

We decided that rather then try to find this regression we it would be better to trade forward facing improvemnts to CanvasMark. Instead I propose a fix to bug 1010558 which should improve CanvasMark.

Comment 9

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

Thanks BenWa!

ironically we added canvasmark for mobile ;)

I like the idea of fixing other areas that are the real bottlenecks/problems, especially if this isn't the root cause.

I want to hear from :blassey as to what he(mobile) would like to do here.

Comment 10

[Brad Lassey [:blassey] (use needinfo?)]

Canvasmark is a canvas and javascript benchmark. The fact that some of the big bottlenecks are caused by JS isn't surprising and is fairly useful. Bug 1010747 is a really good catch that fell out of this analysis. Thanks Benoit.

Comment 11

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

it sounds like we are fine closing this as a wontfix?  Should we track this regression as bug 1010747?

Comment 12

[Benoit Girard (:BenWa)]

Ok. I'm marking as WONTFIX. AFAIK there isn't a problem with the original patch. I rather spend time fixing the big problems of CanvasMark. Once that's solved if the skia snapshot problem is still an issue then it will become a bigger relative problems and will be easier to find.