Bug 73733 - Inline RenderObject::view()
Summary: Inline RenderObject::view()
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Layout and Rendering (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Julien Chaffraix
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-12-02 19:53 PST by Julien Chaffraix
Modified: 2011-12-06 15:38 PST (History)
3 users (show)

See Also:

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 an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Julien Chaffraix 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!
Comment 1 Julien Chaffraix 2011-12-02 20:10:38 PST 
Created attachment 117737 [details]
Proposed inlining, moved the method to RenderView.h
Comment 2 Darin Adler 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.
Comment 3 Julien Chaffraix 2011-12-05 12:31:20 PST 
Created attachment 117916 [details]
Updated patch: updated WebCore.exp.in and fixed the comment.
Comment 4 Julien Chaffraix 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
Comment 5 Julien Chaffraix 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.
Comment 6 WebKit Review Bot 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>
Comment 7 WebKit Review Bot 2011-12-06 15:38:59 PST 
All reviewed patches have been landed.  Closing bug.