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

[Eric Seidel]

I'm all for PLT speedups (despite it running too fast on modern
hardware to be useful, it's all we got).  But I'm very against
build-time explosion. :(

I bet we don't need to inline all of these.  Would be nice to know which ones.

Inlines requiring additional headers (especially for Frame.h) increase
the .o size of most WebCore .cpp files and increase overall build
time.  I need to write some sort of scripts to help us check for
header includes we don't need.

-eric

On Fri, Jun 11, 2010 at 5:54 PM, Sam Weinig <sam.weinig at gmail.com> wrote:
>
>
> 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