Bug 101409 – Make Document::renderer faster by using the cached ptr for RenderView

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

[Elliott Sprehn]

Make Document::renderer faster by using the cached ptr for RenderView

[Elliott Sprehn]

Created attachment 172677 [details]
Patch

[Elliott Sprehn]

In pondering this patch I wonder if it's better to just change callers to use document()->renderView() and make Document::renderer private? 

That avoids all this casting stuff and the out of line method, but it means changing many more files. As an advantage it would also stop people from doing the silliness that's in several places where they think document()->renderer() != document()->renderer()->view(), the only downside is that it means including RenderView.h whenever you want to access the renderer on the document.

Any opinion on this Eric?

[Eric Seidel (no email)]

So this is just reverting your previous change?

[Elliott Sprehn]

(In reply to comment #3)
> So this is just reverting your previous change?

Only partly, my previous change made renderView() return a pointer stored in Document skipping the RareData union stuff in Node::renderer().

so without this patch:

Document::renderer() => Still goes through Node::renderer() and checks the hasRareData() and stuff.
Document::renderView() => Direct ptr access without the conditional

With this patch:

Document::renderer() => Returns m_renderer and skips the union hasRareData logic.
Document::renderView() => Also kips the logic, but needs the cast and to be in RenderView.h

My other alternative is:

Document::renderer() is private and just calls the "slow" Node::renderer()
Change all callers of document()->renderer() to do document()->renderView()

That last option means changing tons of files, but is more explicit at least.

[Eric Seidel (no email)]

Comment on attachment 172677 [details]
Patch

OK.

[WebKit Review Bot]

Comment on attachment 172677 [details]
Patch

Clearing flags on attachment: 172677

Committed r133711: <http://trac.webkit.org/changeset/133711>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.