stylo: 6.21 - 6.54% remote-tsvg / remote-twitter (android-6-0-armv8-api16, android-7-1-armv8-api16) regression on push db56323cd08f4883e4824199b441a3141be655e5 (Thu Nov 23 2017)

NEW
Assigned to

Status

()

defect
P2
normal
2 years ago
7 months ago

People

(Reporter: igoldan, Assigned: m_kato)

Tracking

(Depends on 1 bug, {perf, regression})

Trunk
Unspecified
Android
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox-esr52 unaffected, firefox57 unaffected, firefox58 unaffected, firefox59 disabled, firefox60+ wontfix, firefox61 wontfix, firefox62 wontfix, firefox63 wontfix, firefox64 fix-optional)

Details

(Whiteboard: [qf:p3][geckoview:p3])

We have detected an autophone (Android) regression from push:

https://hg.mozilla.org/integration/autoland/pushloghtml?changeset=db56323cd08f4883e4824199b441a3141be655e5

As author of one of the patches included in that push, we need your help to address this regression.

Regressions:

  7%  remote-tsvg summary android-6-0-armv8-api16 opt      84.13 -> 89.63
  6%  remote-twitter summary android-7-1-armv8-api16 opt   774.07 -> 823.84
  6%  remote-tsvg summary android-7-1-armv8-api16 opt      102.84 -> 109.23


You can find links to graphs and comparison views for each of the above tests at: https://treeherder.mozilla.org/perf.html#/alerts?id=10710

On the page above you can see an alert for each affected platform as well as a link to a graph showing the history of scores for this test. There is also a link to a treeherder page showing the jobs in a pushlog format.

To learn more about the regressing test(s), please see: https://wiki.mozilla.org/EngineeringProductivity/Autophone
:m_kato Looks like enabling Stylo on Android brought some perf regressions. Were these expected?
Flags: needinfo?(m_kato)
Has Regression Range: --- → yes
Component: Untriaged → CSS Parsing and Computation
Product: Firefox → Core
(Assignee)

Comment 2

2 years ago
(In reply to Ionuț Goldan [:igoldan], Performance Sheriffing from comment #1)
> :m_kato Looks like enabling Stylo on Android brought some perf regressions.
> Were these expected?

Yes and no.  But we need figure out why remote-tsvg is slow.  And, why does remote-twitter by android-6-0-armv8-api16 doesn't hit this?
Assignee: nobody → m_kato
Flags: needinfo?(m_kato)
P2 because we should address this regression before we ship stylo-android.
Priority: -- → P2
Summary: 6.21 - 6.54% remote-tsvg / remote-twitter (android-6-0-armv8-api16, android-7-1-armv8-api16) regression on push db56323cd08f4883e4824199b441a3141be655e5 (Thu Nov 23 2017) → stylo: 6.21 - 6.54% remote-tsvg / remote-twitter (android-6-0-armv8-api16, android-7-1-armv8-api16) regression on push db56323cd08f4883e4824199b441a3141be655e5 (Thu Nov 23 2017)
(Assignee)

Comment 4

2 years ago
Gecko profiler doesn't support android...
(Assignee)

Updated

2 years ago
Depends on: 586838
(Assignee)

Comment 5

2 years ago
twitter and gearflowers.svg have a long style attribute.
Tracking for 59 since we're aiming to enable this feature on nightly 59.
Are we still planning on shipping Stylo on Android in 59?
Flags: needinfo?(m_kato)
(In reply to Andrew Overholt [:overholt] from comment #7)
> Are we still planning on shipping Stylo on Android in 59?

Yes. Makoto thinks the NEON optimization patch in bug 586838 will fix this Android perf regression.
(Assignee)

Updated

a year ago
Flags: needinfo?(m_kato)
(Assignee)

Updated

a year ago
Depends on: 1430706
[Tracking Requested - why for this release]:
Apparently stylo on android is slipping to 60
AIUI we're not blocking stylo-android on this for 60, so I'll move this to fix-optional.
Since bug 586838 landed, has this gotten better?
Flags: needinfo?(m_kato)
(In reply to James Willcox (:snorp) (jwillcox@mozilla.com) from comment #11)
> Since bug 586838 landed, has this gotten better?

Not really.
(Assignee)

Comment 13

a year ago
(In reply to James Willcox (:snorp) (jwillcox@mozilla.com) from comment #11)
> Since bug 586838 landed, has this gotten better?

not enough.  I guess that this is into cssparser and rust.
Flags: needinfo?(m_kato)

Updated

11 months ago
Whiteboard: [qf] → [qf][geckoview]
Looking at this during QF triage, Panos is this actionable? Maybe we can get a profile?
Flags: needinfo?(past)
(In reply to Makoto Kato [:m_kato] from comment #13)
> (In reply to James Willcox (:snorp) (jwillcox@mozilla.com) from comment #11)
> > Since bug 586838 landed, has this gotten better?
> 
> not enough.  I guess that this is into cssparser and rust.

Do you have a profile that shows this? If not, could you get one and share it here? Emilio or Xidorn could take a look if it is cssparser related.
Flags: needinfo?(past) → needinfo?(m_kato)
(Assignee)

Comment 16

11 months ago
https://perfht.ml/2Ks8P1I

Since we don't use rust 1.27+ as build environment, walking stack on rust code isn't correct.  So profiler cannot get better data.
Flags: needinfo?(m_kato)
Whiteboard: [qf][geckoview] → [qf][geckoview:p3]

Updated

11 months ago
Whiteboard: [qf][geckoview:p3] → [qf:p3][geckoview:p3]
Looks like this is still not actionable.
Letting this fall out of regression triage since it's had several wontfixes in a row.
You need to log in before you can comment on or make changes to this bug.