| Summary: | Complex MathML test takes over 20s to render | ||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Jon <jon> | ||||||||||||||||
| Component: | MathML | Assignee: | Alex Milowski <alex> | ||||||||||||||||
| Status: | RESOLVED FIXED | ||||||||||||||||||
| Severity: | Normal | CC: | alex, commit-queue, hyatt, kenneth, sausset | ||||||||||||||||
| Priority: | P2 | ||||||||||||||||||
| Version: | 528+ (Nightly build) | ||||||||||||||||||
| Hardware: | Mac (Intel) | ||||||||||||||||||
| OS: | OS X 10.6 | ||||||||||||||||||
| URL: | http://www.w3.org/Math/testsuite/mml2-testsuite/TortureTests/Complexity/complex3.xml | ||||||||||||||||||
| Attachments: |
|
||||||||||||||||||
|
Description
Jon
2010-08-03 20:12:25 PDT
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! I think this has actually gotten worse. I can't get the page to render after a couple of minutes on a fast machine. 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. Created attachment 70931 [details]
Example 1 - Slowest
Created attachment 70932 [details]
Example 2 - A bit faster
Created attachment 70933 [details]
Example 3 - Faster - still has a delay
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.
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. Created attachment 70983 [details]
Updated patch with latest test results
Created attachment 71125 [details]
Updated patch with latest optimizations for layout
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 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 commit-queue@webkit.org... 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 (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. (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 commit-queue@webkit.org... > 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? 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. Created attachment 71334 [details]
Updated patch that conforms with the changes to baselinePosition
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> All reviewed patches have been landed. Closing bug. |