With the landing of bug 922942 we have regressed our svg tests by 10-25% depending on the specific test and platform.
It is my understanding that there is a secondary bug that will fix the majority of this regression (bug 934183) which should land before the next uplift.
Mozilla-Inbound-Non-PGO - SVG, Opacity Row Major - WINNT 6.1 (ix) - 83.9% increase
Previous: avg 238.458 stddev 4.234 of 12 runs up to revision 56fc3fc6a42a
New : avg 438.500 stddev 3.949 of 12 runs since revision a60b5eb6b8ba
Change : +200.042 (83.9% / z=47.242)
Graph : http://mzl.la/173eIgo
Regression: Mozilla-Inbound-Non-PGO - SVG-ASAP - WINNT 6.1 (ix) - 155% increase
Previous: avg 362.428 stddev 3.110 of 12 runs up to revision 56fc3fc6a42a
New : avg 925.322 stddev 7.674 of 12 runs since revision a60b5eb6b8ba
Change : +562.894 (155% / z=181.018)
Graph : http://mzl.la/173eGoK
mattwoodrow: Triage is trying to gauge the impact this will have can you give your thoughts since you worked on 922942. Were we expecting a regression like this?
Yes, this is definitely expected.
The problem is that Thebes and Moz2D have very different semantics when it comes to storing path data. My patches left SVG making calls that are optimized for the Thebes API, but manually maps to the Moz2D API in the backend. This added quite a bit of extra overhead (though admittedly more than I expected).
Jonathan Watt is working on patches that convert SVG to use the Moz2D API directly, which should all of this regression. We're expecting these patches to land before the uplift.
(In reply to Matt Woodrow (:mattwoodrow) from comment #3)
> Jonathan Watt is working on patches that convert SVG to use the Moz2D API
> directly, which should all of this regression. We're expecting these patches
> to land before the uplift.
Nice! I'm going to tag this as 1.3 so that we can keep track of the progress, as we're actually looking to improve SVG performance for 1.3, rather than just get back to where we were.
there was a SVG improvement landed yesterday, I don't see action on bug 934183, but it appears to be one of these two commits:
roc, bug 931464, bug 933354:
jwatt, bug 934156, bug 926564, bug 866659:
Can we confirm that one of these bugs did a slight improvement to svg tests:
For the record we haven't removed all the regressions from earlier in the week, but we have made measurable improvements.
for reference, you can see the improvement across different platforms here:
on b2g-inbound and fx-team we seem to have fix the svg talos regression:
I am not sure why I don't see this on the Firefox branch.
FWIW a number of bugs have been filed on SVG performance regressions blamed on bug 923193. At least some of them are also regressed by this bug. It's all a bit confusing, although I've tried to reduce the confusion by backing out 923193.
it appears we are back to normal, shall we close this or leave it open.
Please ask if you need any help reproducing this locally or working with talos.
(In reply to Joel Maher (:jmaher) from comment #9)
> it appears we are back to normal, shall we close this or leave it open.
If the Talos charts show us being close to back to where we were before this regression, that would most likely indicate that since this regression there have been perf gains in other areas that have canceled it out. (Or put another way, if correct, this regression is canceling out perf gains.) The code that caused the regression has still been in the tree, and I would doubt that it magically stopped being a perf drag.
There's no real need to keep the patch that caused this regression in the tree at the moment. Its goal was to remove some Thebes backed gfxContexts so that we can start removing a bunch of legacy code, but until far more/all instances of Thebes backed gfxContexts are removed it's not actually all that useful a change by itself. As discussed with Matt on IRC, I backed out the only part of the offending patch that could have caused the regression:
Bug 934183 will re-remove the Thebes backed gfxContexts that the regressing patch removed, but it a way that shouldn't regress perf.
this change looks to have improved the svg tests! Any objections to closing this bug?
Yes, it can just be closed when the commit is merged to mozilla-central as normal.