mathml performance regression since 3.6 on windows XP




7 years ago
7 years ago


(Reporter: Bruce Miller, Assigned: jfkthame)



Windows XP

Firefox Tracking Flags

(status1.9.2 .9-fixed)




(2 attachments)



7 years ago
User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv: 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: 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:  

Expected Results:  
not wait
Component: General → MathML
Product: Firefox → Core
QA Contact: general → mathml
Might be due to changes in bug 479276?
Blocks: 479276
Hmm.  On Mac this is basically instantaneous for me.  Is this windows-specific?

Comment 3

7 years ago
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...
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.  ;)
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
Ever confirmed: true

Comment 6

7 years ago
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

7 years ago
Created attachment 444297 [details] [diff] [review]
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.
Assignee: nobody → jfkthame
Attachment #444297 - Flags: review?(roc)
Attachment #444297 - Flags: review?(roc) → review+

Comment 8

7 years ago
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?
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

7 years ago
Created attachment 444405 [details] [diff] [review]
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:
Attachment #444405 - Flags: review?(roc)

Comment 11

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

Thanks!! looking forward to it's migrating into a release!
Attachment #444405 - Flags: review?(roc) → review+

Comment 12

7 years ago
Pushed to trunk:
Last Resolved: 7 years ago
Resolution: --- → FIXED

Comment 13

7 years ago
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.
Attachment #444405 - Flags: approval1.9.2.4?


7 years ago
Keywords: regression

Comment 14

7 years ago
We aren't going to take this in but will be a candidate for

Comment 15

7 years ago
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?)
Attachment #444405 - Flags: approval1.9.2.6?
Attachment #444405 - Flags: approval1.9.2.4?
Attachment #444405 - Flags: approval1.9.2.4-

Comment 16

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

We won't be taking this in alreeady have too many fixes and a tight schedule. Feel free to nominate it for .7 though
Attachment #444405 - Flags: approval1.9.2.6? → approval1.9.2.6-
Attachment #444405 - Flags: approval1.9.2.7?
Comment on attachment 444405 [details] [diff] [review]
patch for 1.9.2 branch

Approved for, a=dveditz for release-drivers
Attachment #444405 - Flags: approval1.9.2.9? → approval1.9.2.9+


7 years ago
status1.9.2: --- → .9-fixed
For the record,
You need to log in before you can comment on or make changes to this bug.