1004437 - 11% SVGX regression on OSX for fx 31,32 from bug 997551

Status: RESOLVED FIXED

(Core :: Graphics: Layers, defect)

Description

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

when http://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=d9a6a378349a&tochange=7eba5b89f831 landed on inbound it caused a tsvgx regression

here is a graph to show the difference:
http://graphs.mozilla.org/graph.html#tests=[[281,63,21]]&sel=1398575014000,1398747814000&displayrange=7&datatype=running

I did some retriggers, there is no doubt:
https://tbpl.mozilla.org/?tree=Mozilla-Inbound&fromchange=61b91c85a7aa&tochange=f3a57f0d89b0&jobname=Rev4%20MacOSX%20Snow%20Leopard%2010.6%20mozilla-inbound%20talos%20svgr

Comment 1

[Matt Woodrow (:mattwoodrow)]

Is this 10.6 only (again)?

Do we have any idea why 10.6 continually shows regressions that don't affect 10.8?

Comment 2

[Avi Halachmi (:avih)]

(In reply to Matt Woodrow (:mattwoodrow) from comment #1)
> Do we have any idea why 10.6 continually shows regressions that don't affect
> 10.8?

Bug 990490 comment 1.

Comment 3

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

this is 10.6 and 10.8, I see it on aurora now:
http://graphs.mozilla.org/graph.html#tests=[[281,63,21],[281,52,24],[281,52,21],[281,63,24]]&sel=1398575014000,1398747814000&displayrange=7&datatype=running

Luckily we have a few weeks to figure it out!

Comment 4

[Matt Woodrow (:mattwoodrow)]

Ok, we should fix this then.

Detailed breakdown of results:

Pre regression:

O -  |i|pagename|runs|
18:59:02     INFO -  |0;gearflowers.svg;381;164;161;161;164;161;162;164;160;163;165;161;165;165;167;162;163;167;174;168;164;165;169;166;169
18:59:02     INFO -  |1;composite-scale.svg;84;46;44;47;47;55;45;46;45;54;47;45;47;50;45;45;47;56;47;49;46;53;46;43;45
18:59:08     INFO -  |2;composite-scale-opacity.svg;226;213;211;222;210;211;223;211;213;223;210;214;224;212;210;221;210;211;221;212;211;226;211;212;223
18:59:08     INFO -  |3;composite-scale-rotate.svg;147;141;140;140;142;141;141;142;143;141;141;141;141;141;142;141;142;140;142;141;142;140;143;142;140
18:59:08     INFO -  |4;composite-scale-rotate-opacity.svg;375;373;371;370;371;373;371;370;370;372;369;369;369;371;370;369;370;368;371;368;371;368;374;370;371
18:59:08     INFO -  |5;hixie-001.xml;525;493;489;490;493;495;492;483;492;490;485;487;488;494;487;484;489;484;493;493;490;487;507;490;500
18:59:08     INFO -  |6;hixie-002.xml;505;494;495;492;490;493;491;487;493;491;494;491;487;497;486;490;490;484;486;489;485;488;490;488;492
18:59:08     INFO -  |7;hixie-003.xml;323;193;186;186;181;186;188;186;189;187;186;189;187;185;189;187;189;187;188;186;192;189;189;187;187
18:59:08     INFO -  |8;hixie-004.xml;1112;1118;1116;1118;1123;1117;1119;1104;1120;1121;1119;1103;1098;1093;1095;1097;1095;1095;1095;1094;1093;1097;1108;1095;1102
18:59:08     INFO -  |9;hixie-005.xml;1022;1019;1015;1018;1018;1021;1018;1016;1014;1017;1017;1013;1019;1018;1017;1012;1018;1016;1017;1014;1015;1016;1015;1015;1016
18:59:08     INFO -  |10;hixie-006.xml;1925;1923;1915;1923;1917;1918;1920;1926;1916;1922;1918;1920;1924;1927;1917;1930;1923;1916;1917;1925;1917;1919;1919;1915;1917
18:59:08     INFO -  |11;hixie-007.xml;363;346;322;348;322;346;323;346;323;343;321;350;324;345;321;347;325;348;324;346;322;347;322;346;323

Post regression:

22:08:56     INFO -  |0;gearflowers.svg;221;165;163;164;162;161;164;164;161;164;162;160;164;164;161;168;164;161;163;163;162;163;164;160;164
22:08:56     INFO -  |1;composite-scale.svg;183;153;154;153;154;154;154;153;154;154;153;153;155;154;154;156;157;159;155;156;155;156;156;155;157
22:09:01     INFO -  |2;composite-scale-opacity.svg;216;212;211;219;207;210;219;212;213;220;212;211;224;210;212;220;210;214;222;209;211;219;211;210;221
22:09:01     INFO -  |3;composite-scale-rotate.svg;262;258;255;256;256;259;257;259;258;257;257;256;258;258;258;258;258;257;258;259;259;257;256;258;257
22:09:01     INFO -  |4;composite-scale-rotate-opacity.svg;376;370;368;370;368;366;369;370;368;371;369;368;371;370;372;368;371;368;371;369;370;368;377;367;368
22:09:01     INFO -  |5;hixie-001.xml;533;504;493;483;496;501;493;494;504;494;487;490;496;498;492;500;494;495;496;495;497;488;491;492;498
22:09:01     INFO -  |6;hixie-002.xml;501;498;495;493;495;499;503;494;493;502;499;493;494;494;493;494;488;501;489;504;496;492;493;493;493
22:09:01     INFO -  |7;hixie-003.xml;324;181;184;183;185;184;181;186;192;189;188;190;190;190;189;188;190;190;192;189;190;188;185;189;189
22:09:01     INFO -  |8;hixie-004.xml;1313;1324;1323;1326;1319;1317;1316;1312;1317;1317;1303;1303;1301;1306;1303;1302;1370;1299;1313;1309;1306;1305;1301;1304;1303
22:09:01     INFO -  |9;hixie-005.xml;1022;1014;1016;1016;1014;1019;1017;1022;1021;1020;1026;1027;1022;1019;1017;1023;1026;1021;1018;1021;1025;1020;1017;1022;1020
22:09:01     INFO -  |10;hixie-006.xml;1930;1924;1930;1927;1922;1924;1920;1920;1929;1922;1922;1934;1923;1931;1927;1925;1934;1943;1929;1923;1955;1919;1930;1922;1957
22:09:01     INFO -  |11;hixie-007.xml;368;350;325;349;325;352;327;348;325;351;325;350;324;351;324;349;323;348;326;349;324;346;324;349;327

Looks like composite-scale.svg, composite-scale-rotate.svg and hixie-004.xml all regressed significantly and the others didn't.

Comment 5

[Matt Woodrow (:mattwoodrow)]

This is because Quartz caches scaled copies of images opaquely within a CGImage.

My patch made it so we re-create the CGImage each draw, losing this benefit.

Since we didn't actually win on any benchmarks, maybe we should backout.

Comment 6

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

do you know if this helped out on other non talos benchmarks?

Comment 7

[Sylvestre Ledru [:Sylvestre]]

Untracking. Beta 6 should be released today and we won't block the release on this bug.

Comment 8

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

this regression is partially fixed by the 32 uplift to aurora:
http://graphs.mozilla.org/graph.html#tests=[[281,63,21],[281,52,21]]&sel=none&displayrange=90&datatype=running

Comment 9

[Milan Sreckovic [:milan] (needinfo for best results)]

Partially fixed enough to stop tracking this?

Comment 10

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

we are still around 7% regressed since May 1:
http://graphs.mozilla.org/graph.html#tests=[[281,52,21],[281,63,21]]&sel=1375644425473,1407180425473&displayrange=365&datatype=running

Better than 11% though- I will provide the data, you guys can make any decisions :)

Comment 11

[Lawrence Mandel [:lmandel] (use needinfo)]

(In reply to Matt Woodrow (:mattwoodrow) from comment #5)
> This is because Quartz caches scaled copies of images opaquely within a
> CGImage.
> 
> My patch made it so we re-create the CGImage each draw, losing this benefit.
> 
> Since we didn't actually win on any benchmarks, maybe we should backout.

Matt - Is there any other benefit of your change in bug 997551? Should we just backout? What is the practical implication of the 7% regression that remains? Note that I have assigned this bug to you to help determine next steps, which may be do nothing, backout, fix.

Comment 12

[Matt Woodrow (:mattwoodrow)]

I don't think we should bother backing this out. imagelib has been converted to moz2d now, so I suspect we've actually fixed the underlying issue.

It was only ever likely to show up on testcases where we repeatedly draw the same image over and over with a significant downscale. It's hard to imagine that mattering much outside of synthetic benchmarks.

Comment 13

[Lawrence Mandel [:lmandel] (use needinfo)]

From comment 12 it sounds like the current state is as 'fixed' as this bug is going to get. I'm resolving as fixed. Feel free to reopen if you disagree.