Bug 73733

Summary:

Inline RenderObject::view()

Product:

WebKit

Reporter:

Julien Chaffraix <jchaffraix>

Component:

Layout and Rendering

Assignee:

Julien Chaffraix <jchaffraix>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

darin, simon.fraser, webkit.review.bot

Priority:

Version:

528+ (Nightly build)

Hardware:

All

OS:

All

Attachments:

Description	Flags
Proposed inlining, moved the method to RenderView.h	none
Updated patch: updated WebCore.exp.in and fixed the comment.	none
Fix 3: re-add the client->view() to RenderSVGResourcesContainer as it looks fine for an SVGDocument not to have a RenderView and yet the render tree being rooted.	none

Julien Chaffraix

Reported 2011-12-02 19:53:35 PST

The function is super hot and is showing up in some benchmarks (~4-5 percent under pprof). For some reason, it is not inlined when it should be (it's just a function call + a cast in release). Let's just inline it!

Attachments
Proposed inlining, moved the method to RenderView.h (4.01 KB, patch) 2011-12-02 20:10 PST, Julien Chaffraix	no flags	Details Formatted Diff Diff
Updated patch: updated WebCore.exp.in and fixed the comment. (4.71 KB, patch) 2011-12-05 12:31 PST, Julien Chaffraix	no flags	Details Formatted Diff Diff
Fix 3: re-add the client->view() to RenderSVGResourcesContainer as it looks fine for an SVGDocument not to have a RenderView and yet the render tree being rooted. (4.44 KB, patch) 2011-12-06 13:24 PST, Julien Chaffraix	no flags	Details Formatted Diff Diff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.

Julien Chaffraix

Comment 1 2011-12-02 20:10:38 PST

Created attachment 117737 [details] Proposed inlining, moved the method to RenderView.h

Darin Adler

Comment 2 2011-12-03 19:36:03 PST

Comment on attachment 117737 [details] Proposed inlining, moved the method to RenderView.h View in context: https://bugs.webkit.org/attachment.cgi?id=117737&action=review Concept is great. The .exp file needs to be updated to avoid breaking the Mac build. > Source/WebCore/rendering/RenderObject.h:517 > + // Inlined into RenderView for performance and to avoid a cyclic dependency. This comment is confusing as written. I don’t know what it means to “inline something into RenderView”. Maybe if you said RenderView.h it’d be easier to understand.

Julien Chaffraix

Comment 3 2011-12-05 12:31:20 PST

Created attachment 117916 [details] Updated patch: updated WebCore.exp.in and fixed the comment.

Julien Chaffraix

Comment 4 2011-12-06 12:49:34 PST

Comment on attachment 117916 [details] Updated patch: updated WebCore.exp.in and fixed the comment. The patch seems to be causing some crashes mostly in svg in the cq. I will investigate what's going on. s

Julien Chaffraix

Comment 5 2011-12-06 13:24:17 PST

Created attachment 118101 [details] Fix 3: re-add the client->view() to RenderSVGResourcesContainer as it looks fine for an SVGDocument not to have a RenderView and yet the render tree being rooted.

WebKit Review Bot

Comment 6 2011-12-06 15:38:55 PST

Comment on attachment 118101 [details] Fix 3: re-add the client->view() to RenderSVGResourcesContainer as it looks fine for an SVGDocument not to have a RenderView and yet the render tree being rooted. Clearing flags on attachment: 118101 Committed r102185: <http://trac.webkit.org/changeset/102185>

WebKit Review Bot

Comment 7 2011-12-06 15:38:59 PST

All reviewed patches have been landed. Closing bug.

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