Bug 57077

Summary:

SVG <use> element performance improvement

Product:

WebKit

Reporter:

Leo Yang <leo.yang>

Component:

SVG

Assignee:

Nobody <webkit-unassigned>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

abarth, commit-queue, eric, krit, webkit.review.bot, zimmermann

Priority:

Version:

528+ (Nightly build)

Hardware:

All

OS:

All

URL:

http://upload.wikimedia.org/wikipedia/commons/4/4e/Sierpinski_carpet_6.svg

Bug Depends on:

Bug Blocks:

54546

Attachments:

Description	Flags
Patch	none
Patch - manual test case	zimmermann: review-
Patch - manual test case revised	none

Leo Yang

Reported 2011-03-24 21:35:44 PDT

SVG <use> element expands nesting <use> and <symbol> elements in an inefficient way. After it expands an <use> or a <symbol> element it will restart expanding from the shadow tree root. This leads about 160 millions of calls to expandUseElementInShadowTree or expandSymbolElementInShadowTree for a single shadow tree which is illustrated by the *URL* field of this bug. But the effective calls, which will really expand <use> or <symbol> elements, are about 5200; others are passing-by calls which are recursively down to the children. Patch is coming soon...

Attachments
Patch (8.55 KB, patch) 2011-03-24 23:45 PDT, Leo Yang	no flags	Details Formatted Diff Diff
Patch - manual test case (5.78 KB, patch) 2011-03-25 01:40 PDT, Leo Yang	zimmermann: review-	Details Formatted Diff Diff
Patch - manual test case revised (6.46 KB, patch) 2011-03-25 03:27 PDT, Leo Yang	no flags	Details Formatted Diff Diff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.

Leo Yang

Comment 1 2011-03-24 23:45:35 PDT

Created attachment 86896 [details] Patch

Nikolas Zimmermann

Comment 2 2011-03-25 00:55:40 PDT

Comment on attachment 86896 [details] Patch This looks wonderful! Great work Leo, r=me. Are you saying we can render Sierpinski_carpet_6 now??

Dirk Schulze

Comment 3 2011-03-25 01:09:33 PDT

(In reply to comment #2) > (From update of attachment 86896 [details]) > This looks wonderful! Great work Leo, r=me. Are you saying we can render Sierpinski_carpet_6 now?? Please add the test as a manual perf test to the svg directory. IIRC aroben created a new dir. You can do it as a followup

Leo Yang

Comment 4 2011-03-25 01:15:39 PDT

(In reply to comment #2) > (From update of attachment 86896 [details]) > This looks wonderful! Great work Leo, r=me. Are you saying we can render Sierpinski_carpet_6 now?? We can render it now. But seems render it not same as FireFox, I will look at the rendering issue.

Leo Yang

Comment 5 2011-03-25 01:16:29 PDT

(In reply to comment #3) > (In reply to comment #2) > > (From update of attachment 86896 [details] [details]) > > This looks wonderful! Great work Leo, r=me. Are you saying we can render Sierpinski_carpet_6 now?? > > Please add the test as a manual perf test to the svg directory. IIRC aroben created a new dir. You can do it as a followup ok, will upload it soon.

Leo Yang

Comment 6 2011-03-25 01:40:40 PDT

Created attachment 86909 [details] Patch - manual test case

Nikolas Zimmermann

Comment 7 2011-03-25 01:45:25 PDT

Comment on attachment 86909 [details] Patch - manual test case r=me. Keen to test the file locally :-)

Nikolas Zimmermann

Comment 8 2011-03-25 01:51:33 PDT

Comment on attachment 86909 [details] Patch - manual test case Revoking r+ after discussion with Dirk. The directory is wrong. Source/WebCore/manual-tests/svg-performance doesn't exist. I think a new directory in PerformanceTests might be better. Maybe we should ask Adam before...

WebKit Commit Bot

Comment 9 2011-03-25 02:20:18 PDT

Comment on attachment 86896 [details] Patch Clearing flags on attachment: 86896 Committed r81950: <http://trac.webkit.org/changeset/81950>

WebKit Commit Bot

Comment 10 2011-03-25 02:20:24 PDT

All reviewed patches have been landed. Closing bug.

Leo Yang

Comment 11 2011-03-25 03:27:43 PDT

Created attachment 86914 [details] Patch - manual test case revised

Dirk Schulze

Comment 12 2011-03-25 03:47:30 PDT

Comment on attachment 86914 [details] Patch - manual test case revised Ah sorry, you forgot to modify the Changelog in PerformanceTest/

Dirk Schulze

Comment 13 2011-03-25 03:49:07 PDT

Comment on attachment 86914 [details] Patch - manual test case revised My fault. Thought you modified the wrong ChangeLog. Sorry. :-(

Leo Yang

Comment 14 2011-03-25 05:51:43 PDT

Reopen so cq can land the review test case

WebKit Commit Bot

Comment 15 2011-03-25 07:17:23 PDT

Comment on attachment 86914 [details] Patch - manual test case revised Clearing flags on attachment: 86914 Committed r81960: <http://trac.webkit.org/changeset/81960>

WebKit Commit Bot

Comment 16 2011-03-25 07:17:29 PDT

All reviewed patches have been landed. Closing bug.

WebKit Review Bot

Comment 17 2011-03-25 09:47:55 PDT

http://trac.webkit.org/changeset/81960 might have broken Windows 7 Release (Tests) The following tests are not passing: http/tests/websocket/tests/frame-length-skip.html

Note You need to log in before you can comment on or make changes to this bug.