Bug 101277 – Remove branch from inside RenderObject::view now that renderer() is more expensive

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Elliott Sprehn]

Remove branch from inside RenderObject::view now that renderer() is more expensive

[Elliott Sprehn]

Created attachment 172438 [details]
Patch

[Eric Seidel (no email)]

Comment on attachment 172438 [details]
Patch

LGTM.  Do we have numbers for this perf change?

[Elliott Sprehn]

(In reply to comment #2)
> (From update of attachment 172438 [details])
> LGTM.  Do we have numbers for this perf change?

Nope, but I can run Parser/html5-full-render. I don't know what other thing to run, have any ideas? I'm also looking into how to run the page cycler.

[Eric Seidel (no email)]

It might show up in html5-full-render.

Regardless of the perf impact, I think this is a good change and worth the space trade-off (for code-clarity if nothing else).

[Eric Seidel (no email)]

Comment on attachment 172438 [details]
Patch

Are there places which call m_renderer = 0 instead of setRenderer that we need to worry about?

[Elliott Sprehn]

(In reply to comment #5)
> (From update of attachment 172438 [details])
> Are there places which call m_renderer = 0 instead of setRenderer that we need to worry about?

Nope, m_renderer is in Node.h and is private, and I made sure Node always goes through setRenderer(0) in the previous patch.

[Build Bot]

Comment on attachment 172438 [details]
Patch

Attachment 172438 [details] did not pass mac-ews (mac):
Output: http://queues.webkit.org/results/14721887

[Elliott Sprehn]

(In reply to comment #4)
> It might show up in html5-full-render.
> 
> Regardless of the perf impact, I think this is a good change and worth the space trade-off (for code-clarity if nothing else).

This looks like about a 1% improvement on Parser/html5-full-render.html

r133501 baseline:
4713.22	± 0.59%	1.01% Worse 	
4655.67	± 0.51%		
4666.32	± 0.47%

With this patch:
4608.43	± 0.68%	1.24% Better
4643.20	± 0.28%	0.50% Better
4632.21	± 0.45%	0.73% Better

[Eric Seidel (no email)]

Looks like you need to clean up the exported symbols list too:

  "__ZNK7WebCore8Document10renderViewEv", referenced from:
     -exported_symbol[s_list] command line option

[Elliott Sprehn]

Created attachment 172632 [details]
Patch

Fix exports

[WebKit Review Bot]

Comment on attachment 172632 [details]
Patch

Clearing flags on attachment: 172632

Committed r133671: <http://trac.webkit.org/changeset/133671>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.