Open Bug 1430706 Opened 6 years ago Updated 1 year ago

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

Categories

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

defect

Tracking

()

Tracking Status
firefox-esr52 --- unaffected
firefox57 --- wontfix
firefox58 --- wontfix
firefox59 --- wontfix
firefox60 --- affected

People

(Reporter: m_kato, Unassigned)

References

(Blocks 1 open bug)

Details

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
See Also: → 1354566
See Also: → 1344131
Could you provide some steps to reproduce this issue?

(The profile isn't immediately clear to me where the problem is.)
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
(The result of comment 2 is from my local non-pgo profiling build.)
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.
Flags: needinfo?(m_kato)
(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
Flags: needinfo?(mstange)
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);
(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
(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.
Flags: needinfo?(mstange)
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.
Flags: needinfo?(m_kato)
Summary: stylo: geckoservo::glue::Servo_ParseStyleAttribute is 5x - 10 x slower than Gecko's old CSS parser → stylo: geckoservo::glue::Servo_ParseStyleAttribute is slower than Gecko's old CSS parser
humm, when I disable svg's style parser, it is still slow than Gecko.
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
Clarifying title based on comment 9.
Summary: stylo: geckoservo::glue::Servo_ParseStyleAttribute is slower than Gecko's old CSS parser → stylo: geckoservo::glue::Servo_ParseStyleAttribute is slower than Gecko's old CSS parser on ARM
See Also: → 1212398
Changing priority from P1 to P2 because we've decided that this ARM performance issue does not block shipping Stylo-android.
Priority: P1 → P2

Still an issue it seems.

Severity: normal → S3
You need to log in before you can comment on or make changes to this bug.