[webkit-dev] Why does Frame.h use mutable members instead of OwnPtr?

[Sam Weinig]

On Fri, Jun 11, 2010 at 3:03 AM, Maciej Stachowiak <mjs at apple.com> wrote:

>
> On Jun 10, 2010, at 4:22 PM, Eric Seidel wrote:
>
> > Example.  Use of a mutable member for AnimationController:
> > https://trac.webkit.org/browser/trunk/WebCore/page/Frame.h#L346
> >
> > Causes us to pull in AnimationController.h:
> > https://trac.webkit.org/browser/trunk/WebCore/page/Frame.h#L31
> >
> > Which pulls in additional headers of its own.
>
> I think what requires including the additional headers is the fact that
> it's a data member of class type instead of a pointer; the mutable
> declaration is unrelated.
>
> >
> > Frame.h is included by lots and lots of cpp files, most of which never
> > need AnimationController.h.  Thus all of those intermediate files
> > (preprocessor output, assembler output, .o file, etc.) are larger than
> > necessary, causing longer builds.  Also any time someone edits an
> > AnimationController.h (or a dependent .h) we end up re-building every
> > file which includes Frame.h (hundreds of files).
>
> Making the helper classes of Frame into separately allocated objects could
> certainly help compile time. It used to be they were all held in a separate
> FramePrivateData class. I'm not sure when that changed. I don't know of any
> considerations besides performance and memory use that would favor the
> current approach. Testing would show whether it actually matters. It might
> also be worthwhile to find the checkin that removed the separate private
> data struct for Frame, to see if that was done for a particular reason.
>
> Regards,
> Maciej
>
>
We moved away from the private approach in
http://trac.webkit.org/changeset/40024 for a PLT speedup (I apparently did
not mention a percentage gain weinig--).  It lately has also allowed us to
inline access to some of those properties for another PLT speedup.

-Sam
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-dev/attachments/20100611/8dca80f9/attachment.html>