943351 - Performance regression in canvas strokeText

Status: VERIFIED FIXED

(Core :: Graphics, defect)

Description

[Erick Dransch [:edransch]]

There is strange canvas strokeText slowness in Nightly and Aurora.
Calling strokeText repeatedly causes the browser to spike to 100% CPU and triggers the unresponsive script prompt.
The issue occurs on both of the Linux PCs I've tested (I haven't tried any other OS). 


A small reproduction program can be found at http://jsfiddle.net/sT8RF/1/ 
Profiling data for the above page is at http://people.mozilla.org/~bgirard/cleopatra/#report=31e9133800d7d680923ae0ce8437b5161a2cddad 

mozregression narrows down the regression to:

Last good nightly: 2013-09-28
First bad nightly: 2013-09-29

Pushlog:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=2f4397db1830&tochange=8f805d3ef377

Comment 1

[Benoit Girard (:BenWa)]

Thanks for the regression. I'm eye balling this to bug 918613. Bas is still likely?

Comment 2

[Erick Dransch [:edransch]]

Since I'm not at the same computer I was at last night, I've profiled both the good and the bad version again:

I'm testing with http://jsfiddle.net/sT8RF/

Good Nightly (Sept 28, 2013):
I re-ran the javascript on the page 4 times here, each run took around 1 second to complete.
https://people.mozilla.org/~bgirard/cleopatra/#report=2d6dac03a1ded0a39e20baf7f272dd55a9101c08

Bad Nightly (Sept 29, 2013):
This is all from the first run (which never completed). The blips you see are me clicking "Continue" on the "Warning: Unresponsive Script" dialog.
https://people.mozilla.org/~bgirard/cleopatra/#report=3273854539c7356356baedad251d872b2bb85c1b

Comment 3

[Benoit Girard (:BenWa)]

This should block because the regression is very large. We went from
~300ms to 12 seconds for the test case.

Comment 4

[Bas Schouten (:bas.schouten)]

Attachment: Make sure to clear any existing paths before getting the glyph path off a context.

Comment 5

[Bas Schouten (:bas.schouten)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/a1523aeef7d8

Comment 6

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/a1523aeef7d8

Comment 7

[Benjamin Kerensa [:bkerensa]]

We are going to track this due to our concern for the impact on users and ask for an uplift to aurora.

Comment 8

[bhavana bajaj [:bajaj]]

(In reply to Bas Schouten (:bas.schouten) from comment #5)
> https://hg.mozilla.org/integration/mozilla-inbound/rev/a1523aeef7d8

Bas, is this ready to uplift on aurora ?

Comment 9

[Bas Schouten (:bas.schouten)]

Comment on attachment 8338590 [details] [diff] [review]
Make sure to clear any existing paths before getting the glyph path off a context.

[Approval Request Comment]
Bug caused by (feature/regressing bug #): 918613
User impact if declined: Incorrect rendering of stroked text in some cases.
Testing completed (on m-c, etc.): On nightly for 2 weeks.
Risk to taking this patch (and alternatives if risky): Very low.
String or IDL/UUID changes made by this patch: None

Comment 10

[u279076]

Erik, can you please confirm this is fixed for you now in Firefox 28 and 29?

Comment 11

[Erick Dransch [:edransch]]

(In reply to Anthony Hughes, QA Mentor (:ashughes) from comment #10)
> Erik, can you please confirm this is fixed for you now in Firefox 28 and 29?

Yes. It's fixed in both Aurora (version 28 - Dec 19 2013) and Nightly (version 29 - Dec 19, 2013)

Comment 12

[bhavana bajaj [:bajaj]]

NI on Bas to help with uplift on Firefox 27.

Comment 13

[Bas Schouten (:bas.schouten)]

Comment on attachment 8338590 [details] [diff] [review]
Make sure to clear any existing paths before getting the glyph path off a context.

[Approval Request Comment]
See Aurora approval request.

It seems I've failed to uplift this to aurora :(. Let's uplift it to beta then. It should be very safe and it's had extensive testing on nightly and Aurora.

Comment 14

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/71f37ad83d22

Comment 15

[Ioana (away)]

Verified as fixed on Firefox 27 beta 5 - Ubuntu 12.10 64bit.