Summary: | Remove branch from inside RenderObject::view now that renderer() is more expensive | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Elliott Sprehn <esprehn> | ||||||
Component: | New Bugs | Assignee: | Elliott Sprehn <esprehn> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Normal | CC: | eric, webkit.review.bot | ||||||
Priority: | P2 | ||||||||
Version: | 528+ (Nightly build) | ||||||||
Hardware: | Unspecified | ||||||||
OS: | Unspecified | ||||||||
Bug Depends on: | |||||||||
Bug Blocks: | 100057 | ||||||||
Attachments: |
|
Description
Elliott Sprehn
2012-11-05 16:43:07 PST
Created attachment 172438 [details]
Patch
Comment on attachment 172438 [details]
Patch
LGTM. Do we have numbers for this perf change?
(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. 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). Comment on attachment 172438 [details]
Patch
Are there places which call m_renderer = 0 instead of setRenderer that we need to worry about?
(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. Comment on attachment 172438 [details] Patch Attachment 172438 [details] did not pass mac-ews (mac): Output: http://queues.webkit.org/results/14721887 (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 Looks like you need to clean up the exported symbols list too: "__ZNK7WebCore8Document10renderViewEv", referenced from: -exported_symbol[s_list] command line option Created attachment 172632 [details]
Patch
Fix exports
Comment on attachment 172632 [details] Patch Clearing flags on attachment: 172632 Committed r133671: <http://trac.webkit.org/changeset/133671> All reviewed patches have been landed. Closing bug. |