Bug 73733

Summary: Inline RenderObject::view()
Product: WebKit Reporter: Julien Chaffraix <jchaffraix>
Component: Layout and RenderingAssignee: Julien Chaffraix <jchaffraix>
Status: RESOLVED FIXED    
Severity: Normal CC: darin, simon.fraser, webkit.review.bot
Priority: P2    
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

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.