564309 - mathml performance regression since 3.6 on windows XP

Status: RESOLVED FIXED

(Core :: MathML, defect)

Description

[Bruce Miller]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.2.4) Gecko/20100503 Firefox/3.6.4 (.NET CLR 3.5.30729)
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.2.4) Gecko/20100503 Firefox/3.6.4 (.NET CLR 3.5.30729)

Complex pages with much mathml are displayed significantly more
slowly, 10's of seconds in some cases, than they were
under firefox 3.5 versions, or under linux or macos.


Reproducible: Always

Steps to Reproduce:
1. visit the above page
2. wait
3. wait
Actual Results:  
wait

Expected Results:  
not wait

Comment 1

[Karl Tomlinson (:karlt)]

Might be due to changes in bug 479276?

Comment 2

[Boris Zbarsky [:bzbarsky]]

Hmm.  On Mac this is basically instantaneous for me.  Is this windows-specific?

Comment 3

[Bruce Miller]

On Linux as well (actually 3.6.x are quite snappy on linux);
that's why I was rather surprised when I got the report.

And seems to also be a problem with 3.6.3 on Vista,
so it hasn't yet been "fixed" on that end...

Comment 4

[Boris Zbarsky [:bzbarsky]]

Interesting.  3.6 on Mac is definitely fast on the linked-to page...  which is too bad, since that's where the decent profilers are.  ;)

Comment 5

[Matthias Versen [:Matti]]

confirming with SM trunk on vista. I get a 10s+ freeze on my core2 Duo 2Ghz

fast: 2009-02-23-03-mozilla-central/
slow: 2009-02-25-03-mozilla-central/

bug 479276 got fixed ar 2009-02-24

Comment 6

[Jonathan Kew [:jfkthame]]

Yes, this is (almost certainly) due to bug 479276, which was known to carry a risk of perf impact for MathML on Windows. For most pages (with a moderate amount of MathML) it's not a serious issue, but this example shows that for pages with a large amount of MathML it can become unacceptable.

(See also the related bug 445087 comment 68.)

I think we can fix this by some additional caching; I'll try to work up a patch and see how it behaves (both perf and memory impact).

Comment 7

[Jonathan Kew [:jfkthame]]

Attachment: patch, v1 - cache the secondary font used for mathml metrics

This fixes the mathml perf regression by caching the non-antialiased copy of the font used to get glyph metrics for mathml layout, instead of recreating it on the fly each time metrics are needed.

Comment 8

[Bruce Miller]

Nice, quick diagnosis, and the patch looks elegant (from a long distance!).
I'll look forward to testing... will this show up in a nightly
tonight, or soon?

Comment 9

[Karl Tomlinson (:karlt)]

Bruce, the nightly produced the morning after this bug gets marked FIXED will contain the patch.  Unfortunately committing patches is not a simple task and so it may be several days before the patch gets committed.

Comment 10

[Jonathan Kew [:jfkthame]]

Attachment: patch for 1.9.2 branch

This is an equivalent patch for the 1.9.2 branch (FF 3.6.x). The original patch doesn't apply to the branch because this code has been substantially restructured on trunk.

Tryserver build of 3.6 with this patch is available at:
https://build.mozilla.org/tryserver-builds/[e-mail]-try-67b74d7be154/

Comment 11

[Bruce Miller]

The try version seems to fix the problem,
without seeming to introduce any others.

Thanks!! looking forward to it's migrating into a release!

Comment 12

[Jonathan Kew [:jfkthame]]

Pushed to trunk:
http://hg.mozilla.org/mozilla-central/rev/5da242d9aa82

Comment 13

[Jonathan Kew [:jfkthame]]

Comment on attachment 444405 [details] [diff] [review]
patch for 1.9.2 branch

Requesting approval to land on 1.9.2 branch as this is a serious perf regression in FF3.6/Windows for MathML-heavy pages.

Comment 14

[LegNeato]

We aren't going to take this in 1.9.2.6 but will be a candidate for 1.9.2.7.

Comment 15

[LegNeato]

Comment on attachment 444405 [details] [diff] [review]
patch for 1.9.2 branch

(because we don't have a .7 flag yet parking this in .6?)

Comment 16

[LegNeato]

Comment on attachment 444405 [details] [diff] [review]
patch for 1.9.2 branch

We won't be taking this in 1.9.2.6...we alreeady have too many fixes and a tight schedule. Feel free to nominate it for .7 though

Comment 17

[Daniel Veditz [:dveditz]]

Comment on attachment 444405 [details] [diff] [review]
patch for 1.9.2 branch

Approved for 1.9.2.9, a=dveditz for release-drivers

Comment 18

[Boris Zbarsky [:bzbarsky]]

For the record, https://hg.mozilla.org/releases/mozilla-1.9.2/rev/898dc1c555ea