Bug 43462 – Complex MathML test takes over 20s to render

• Status: RESOLVED
• Resolution: FIXED
• Component: MathML

[Jon]

Running this W3 MathML2 test on my i7 iMac results in a beachball on ToT WebKit, eventually resolving after 15 - 20 seconds. I'll try again later and get some samples.

[François Sausset]

The MathML implementation in WebKit is just at its early stage, so no optimization work was really done.
We still have a lot of work on layout issues and I think we should next focus on optimizations.

If you could isolate the bottleneck in this example and build a test case, it would be very useful!

[Alex Milowski]

I think this has actually gotten worse.  I can't get the page to render after a couple of minutes on a fast machine.

[Alex Milowski]

Nesting rows (actual or implied) more than 9 levels causes serious performance problems.  This has to do with the layout algorithm from RenderMathMLRow.  The multiple calls to layout() cause a cascade of layouts that multiply as the levels increase.

[Alex Milowski]

Created attachment 70931 [details]
Example 1 - Slowest

[Alex Milowski]

Created attachment 70932 [details]
Example 2 - A bit faster

[Alex Milowski]

Created attachment 70933 [details]
Example 3 - Faster - still has a delay

[Alex Milowski]

Created attachment 70965 [details]
Fix for long layout time

This patch changes the row layout algorithm so it doesn't recurse into rows multiple times.

[Alex Milowski]

This bug is a high priority as the row layout algorithm is central to MathML and it is currently very broken from a performance perspective.

[Alex Milowski]

Created attachment 70983 [details]
Updated patch with latest test results

[Alex Milowski]

Created attachment 71125 [details]
Updated patch with latest optimizations for layout

[Kenneth Rohde Christiansen]

Comment on attachment 71125 [details]
Updated patch with latest optimizations for layout

View in context: https://bugs.webkit.org/attachment.cgi?id=71125&action=review

Generally looks good. It actually seems to simplify the code. I notice that in simple fractions, the denominator almost touches the line. I do not know if this is a regression, but maybe this be improved?

> WebCore/mathml/RenderMathMLRow.cpp:79
> +            // Check to see if the non-operator block has a greater height

Please add a dot at the end of comments, that is out webkit style

[WebKit Commit Bot]

Comment on attachment 71125 [details]
Updated patch with latest optimizations for layout

Rejecting patch 71125 from commit-queue.

Failed to run "['./WebKitTools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '--bot-id=abarth-cq-sl', 'apply-attachment', '--force-clean', '--non-interactive', '--quiet', 71125]" exit_code: 2
Cleaning working directory
Updating working directory
Logging in as [e-mail]...
Fetching: https://bugs.webkit.org/attachment.cgi?id=71125&action=edit
Fetching: https://bugs.webkit.org/show_bug.cgi?id=43462&ctype=xml
Processing 1 patch from 1 bug.
Processing patch 71125 from bug 43462.
Failed to run "[u'/Users/abarth/git/webkit-queue/WebKitTools/Scripts/svn-apply', u'--reviewer', u'Kenneth Rohde Christiansen', u'--force']" exit_code: 1

Full output: http://queues.webkit.org/results/4524014

[Alex Milowski]

(In reply to comment #11)
> (From update of attachment 71125 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=71125&action=review
> 
> Generally looks good. It actually seems to simplify the code. I notice that in simple fractions, the denominator almost touches the line. I do not know if this is a regression, but maybe this be improved?

As the layout algorithm for row is now less agressive, the operators won't keep adding to the row height.  In some cases, we'll need to add extra padding.  I'll look at this a file a separate bug if we need to do that for fractions.

[Alex Milowski]

(In reply to comment #12)
> (From update of attachment 71125 [details])
> Rejecting patch 71125 from commit-queue.
> 
> Failed to run "['./WebKitTools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '--bot-id=abarth-cq-sl', 'apply-attachment', '--force-clean', '--non-interactive', '--quiet', 71125]" exit_code: 2
> Cleaning working directory
> Updating working directory
> Logging in as [e-mail]...
> Fetching: https://bugs.webkit.org/attachment.cgi?id=71125&action=edit
> Fetching: https://bugs.webkit.org/show_bug.cgi?id=43462&ctype=xml
> Processing 1 patch from 1 bug.
> Processing patch 71125 from bug 43462.
> Failed to run "[u'/Users/abarth/git/webkit-queue/WebKitTools/Scripts/svn-apply', u'--reviewer', u'Kenneth Rohde Christiansen', u'--force']" exit_code: 1
> 
> Full output: http://queues.webkit.org/results/4524014

The output is just the above.  What does this mean?  Where's the error?

[Alex Milowski]

The patch fails due to a change in the baselinePosition() method signature.  I still don't understand where the error output from the svn-apply went to as that would have told me that in the bug comment.

[Alex Milowski]

Created attachment 71334 [details]
Updated patch that conforms with the changes to baselinePosition

[WebKit Commit Bot]

Comment on attachment 71334 [details]
Updated patch that conforms with the changes to baselinePosition

Clearing flags on attachment: 71334

Committed r70221: <http://trac.webkit.org/changeset/70221>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.