1430706 - stylo: geckoservo::glue::Servo_ParseStyleAttribute is slower than Gecko's old CSS parser on ARM

Status: NEW

(Core :: CSS Parsing and Computation, defect, P2)

Description

[Makoto Kato [:m_kato]]

I am still investigating bug 1420369.  tsvg has performance regression by stylo.

This is tsvg data.

stylo off
gearflowers.svg;221;193;180;158;193;191
composite-scale.svg;110;109;60;69;69;53
composite-scale-opacity.svg;59;89;76;81;67;72
composite-scale-rotate.svg;94;63;78;38;84;65
composite-scale-rotate-opacity.svg;77;85;73;46;88;34
hixie-001.xml;138;125;72;127;71;119
hixie-003.xml;72;61;59;56;69;64
hixie-005.xml;73;47;55;51;64;62
hixie-007.xml;178;137;133;171;138;110

stylo on
gearflowers.svg;280;215;199;161;260;164
composite-scale.svg;91;121;53;92;75;77
composite-scale-opacity.svg;83;103;70;86;62;86
composite-scale-rotate.svg;91;67;71;83;72;86
composite-scale-rotate-opacity.svg;70;70;83;59;85;46
hixie-001.xml;90;110;76;111;106;121
hixie-003.xml;103;72;80;61;37;65
hixie-005.xml;98;68;64;63;69;60
hixie-007.xml;170;152;133;94;138;110


gearflowers.svg is worst with stylo.  This SVG has long style attribute, and this parsing is slower than Gecko now.

Since Fennec/Android cannot use Gecko profiler, this profiling data is Desktop (Windows).  nsAttrValue::ParseStyleAttribute is 5x-10x slower than old style system.

Stylo on
https://perfht.ml/2DgqLvZ

Stylo off
https://perfht.ml/2Dlyysk

Comment 1

[Xidorn Quan [:xidorn] UTC+10]

Could you provide some steps to reproduce this issue?

(The profile isn't immediately clear to me where the problem is.)

Comment 2

[Xidorn Quan [:xidorn] UTC+10]

My local profiling build on Windows doesn't show much difference between stylo and gecko for loading gearflowers.svg.

Stylo: https://perfht.ml/2DDKcfC
Gecko: https://perfht.ml/2DAvlTp

Comment 3

[Xidorn Quan [:xidorn] UTC+10]

(The result of comment 2 is from my local non-pgo profiling build.)

Comment 4

[Emilio Cobos Álvarez [:emilio]]

Is this an ARM-specific regression? Asking because from the stylo-chrome measurements, in Windows we're much faster in tsvg than Gecko...

If so, it may be worth to check the generated code, maybe it's a rust-compiler bug. I believe their coverage for arm may not be as great as for x86.

Comment 5

[Xidorn Quan [:xidorn] UTC+10]

(In reply to Emilio Cobos Álvarez [:emilio] from comment #4)
> Is this an ARM-specific regression? Asking because from the stylo-chrome
> measurements, in Windows we're much faster in tsvg than Gecko...
> 
> If so, it may be worth to check the generated code, maybe it's a
> rust-compiler bug. I believe their coverage for arm may not be as great as
> for x86.

Makoto said he was profiling on Windows, so it sounds like not ARM-specific... but I cannot reproduce that on Windows...


(In reply to Makoto Kato [:m_kato] from comment #0)
> Since Fennec/Android cannot use Gecko profiler, this profiling data is
> Desktop (Windows).  nsAttrValue::ParseStyleAttribute is 5x-10x slower than
> old style system.

Also, it seems to me that it is possible to use Gecko profiler on Firefox for Android [1], or at least it was possible. Not sure about what's the current status of that. Maybe mstange knows?


[1] https://developer.mozilla.org/en-US/docs/Mozilla/Performance/Profiling_with_the_Built-in_Profiler#Profiling_Firefox_mobile

Comment 6

[Makoto Kato [:m_kato]]

Sorry for confusion.  5x-10x is most worst case using mobile.twitter.com (1st page uses SVG that has 254 length style attribute).  But avg of gearflowers.svg is 25% slower on Gecko's.

I am collecting more build data for this issue.  (This data is Nexus 5X with the latest build)

Stylo .. 51.5ms when style attribute length > 100 (480 style attributes)

>     RefPtr<URLExtraData> data = new URLExtraData(baseURI, docURI,
>                                                  principal);
>     decl = ServoDeclarationBlock::FromCssText(aString, data,
>                                               ownerDoc->GetCompatibilityMode(),
>                                               ownerDoc->CSSLoader());

Gecko .. 44.5 ms when style attribute length > 100  (480 style attributes)

>     css::Loader* cssLoader = ownerDoc->CSSLoader();
>     nsCSSParser cssParser(cssLoader);
>     decl = cssParser.ParseStyleAttribute(aString, docURI, baseURI,
>                                          principal);

Comment 7

[Makoto Kato [:m_kato]]

(In reply to Xidorn Quan [:xidorn] UTC+10 (PTO Jan 19 ~ 29) from comment #5)
> > Since Fennec/Android cannot use Gecko profiler, this profiling data is
> > Desktop (Windows).  nsAttrValue::ParseStyleAttribute is 5x-10x slower than
> > old style system.
> 
> Also, it seems to me that it is possible to use Gecko profiler on Firefox
> for Android [1], or at least it was possible. Not sure about what's the
> current status of that. Maybe mstange knows?

https://github.com/devtools-html/Gecko-Profiler-Addon/issues/35

Comment 8

[Xidorn Quan [:xidorn] UTC+10]

(In reply to Makoto Kato [:m_kato] from comment #7)
> (In reply to Xidorn Quan [:xidorn] UTC+10 (PTO Jan 19 ~ 29) from comment #5)
> > > Since Fennec/Android cannot use Gecko profiler, this profiling data is
> > > Desktop (Windows).  nsAttrValue::ParseStyleAttribute is 5x-10x slower than
> > > old style system.
> > 
> > Also, it seems to me that it is possible to use Gecko profiler on Firefox
> > for Android [1], or at least it was possible. Not sure about what's the
> > current status of that. Maybe mstange knows?
> 
> https://github.com/devtools-html/Gecko-Profiler-Addon/issues/35

Ah... okay.

Comment 9

[Makoto Kato [:m_kato]]

And I get performance detail on Android/x86, this data is same or improved by stylo.  So this seems to be arm only.  I will setup Linux/arm to investigate this.

Comment 10

[Makoto Kato [:m_kato]]

humm, when I disable svg's style parser, it is still slow than Gecko.

Comment 11

[Makoto Kato [:m_kato]]

Although It cannot get clean stack dependency (I guess that rust supports unwind EHABI, but we cannot get clean depth),  this is Linux/arm
https://perfht.ml/2FqlzTw

Comment 12

[Emilio Cobos Álvarez [:emilio]]

Clarifying title based on comment 9.

Comment 13

[Chris Peterson [:cpeterson]]

Changing priority from P1 to P2 because we've decided that this ARM performance issue does not block shipping Stylo-android.

Comment 14

[Jan Henning [:JanH]]

Still an issue it seems.