Closed Bug 1009646 Opened 11 years ago Closed 11 years ago

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

Categories

(Core :: Graphics: Layers, defect)

Product:
Core
Component:
Graphics: Layers
Platform:
ARM
Android
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED WONTFIX

People

(Reporter: jmaher, Assigned: BenWa)

References

Details

(Keywords: perf, regression, Whiteboard: [talos_regression])

This is interesting. I think the problem is something strange is happening with Skia Snapshot and not a problem with the original patch. Investigating...
Assignee: nobody → bgirard
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.
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.
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?
(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.
(In reply to James Willcox (:snorp) (jwillcox@mozilla.com) 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 :)
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.
Depends on: 1010558
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.
Flags: needinfo?(blassey.bugs)
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.
Flags: needinfo?(blassey.bugs)
it sounds like we are fine closing this as a wontfix? Should we track this regression as bug 1010747?
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.
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → WONTFIX
You need to log in before you can comment on or make changes to this bug.