1139709 - nsMathMLContainerFrame should cache its intrinsic width

Status: RESOLVED FIXED

(Core :: MathML, defect)

Description

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Attachment: inch.zip

Someone sent me a test page full of MathML which renders incredibly slowly in Firefox. I'm attaching a version of it which takes 68s to render for me on trunk.

We spend the majority of the time in nsMathMLContainerFrame::GetMinISize/GetPrefISize --- which actually compute the same thing. We should cache the intrinsic width, which will cut that time in half by sharing the results between GetMinISize/GetPrefISize, and probably by more since we can also reuse intrinsic widths across reflows.

There is a risk that dynamic changes to MathML could break if intrinsic widths are not currently invalidated correctly. However, nsMathMLContainerFrame is currently very conservative and invalidates intrinsic widths on any AttributeChanged. Also, wherever MathML fails to invalidate intrinsic widths is already a bug since inline MathML contributes to the cached intrinsic widths of non-MathML containers.

Comment 1

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Attachment: Cache nsMathMLContainerFrame's intrinsic width

For me, this reduces the load time of the testcase in bug 1139709 from
68s to 34s.

Comment 2

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Attachment: Remove unused parameters from nsMathMLChar::GetMaxWidth

Comment 3

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Now this page spends > 50% of its load time in nsMathMLChar::GetMaxWidth. A global stretchy-char-max-width cache would probably make this a lot faster. However, it's slightly tricky to implement and probably isn't worth implementing right now.

Comment 4

[Mats Palmgren (inactive)]

Comment on attachment 8572968 [details] [diff] [review]
Cache nsMathMLContainerFrame's intrinsic width

r=mats

>layout/mathml/nsMathMLContainerFrame.cpp
> ...::UpdateIntrinsicWidth(nsRenderingContext *aRenderingContext)

Perhaps you could change GetMinISize, GetPrefISize to use the more
common code style "nsRenderingContext* " instead?

Would be nice, since it's the last instances in this file, AFAICT.

Comment 5

[Mats Palmgren (inactive)]

Comment on attachment 8572970 [details] [diff] [review]
Remove unused parameters from nsMathMLChar::GetMaxWidth

r=mats, same nsRenderingContext* comment here.

Comment 6

[Frédéric Wang (:fredw)]

(In reply to Robert O'Callahan (:roc) (Mozilla Corporation) from comment #3)
> Now this page spends > 50% of its load time in nsMathMLChar::GetMaxWidth. A
> global stretchy-char-max-width cache would probably make this a lot faster.
> However, it's slightly tricky to implement and probably isn't worth
> implementing right now.

In general, nsMathMLChar is not very efficient at the moment. See also bug 1009582.

Comment 7

[Robert O'Callahan (:roc) (email my personal email if necessary)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/d151e628b2e8
https://hg.mozilla.org/integration/mozilla-inbound/rev/22cf89e35a51
https://hg.mozilla.org/integration/mozilla-inbound/rev/3c86caa855ee

Comment 8

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/d151e628b2e8
https://hg.mozilla.org/mozilla-central/rev/22cf89e35a51
https://hg.mozilla.org/mozilla-central/rev/3c86caa855ee