Move implementation from header to source.

Move implementation from header to source.

This is an effort to speed up compile and link time, and also minimizing the
size of the intermediary .o files on disk. For example, just moving the
constructor/destructor from the classes in chrome/browser/pref_member.{cc,h}
netted a 368k drop in total .o file size. In aggregate, this shrinks
libbrowser.a by 10 megabytes, and a few odd megabytes on most other chrome .a files.

A lot of this was done before I started harvesting what the most included
symbols were across all of chrome's code. Most of them are in webkit, but
there's plenty in base/ that are used everywhere to keep me busy for several
patches to come.

BUG=none
TEST=none

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=52528

[tfarina, 2010-07-15 17:24:36]

Should be this documented somewhere? So new people starting working on chromium
will be aware of this.

[Elliot Glaysher, 2010-07-15 17:27:09]

On Thu, Jul 15, 2010 at 10:24 AM,  <tfarina@chromium.org> wrote:
> Should be this documented somewhere? So new people starting working on
> chromium
> will be aware of this.

It should and once I have some better data and experience, I plan on
doing a big postmortem style post to chromium-dev.

>
> http://codereview.chromium.org/3012001/show
>

[Peter Kasting, 2010-07-15 17:33:28]

On Thu, Jul 15, 2010 at 10:26 AM, Elliot Glaysher (Chromium) <
erg@chromium.org> wrote:

> On Thu, Jul 15, 2010 at 10:24 AM,  <tfarina@chromium.org> wrote:
> > Should be this documented somewhere? So new people starting working on
> > chromium
> > will be aware of this.
>
> It should and once I have some better data and experience, I plan on
> doing a big postmortem style post to chromium-dev.


When you do (or even before), please update the coding style page:
http://dev.chromium.org/developers/coding-style

PK

[Evan Martin, 2010-07-15 17:37:32]

In general, I'm surprised this makes a difference.

http://codereview.chromium.org/3012001/diff/2001/3002
File base/command_line.cc (right):

http://codereview.chromium.org/3012001/diff/2001/3002#newcode444
base/command_line.cc:444: CommandLine::CommandLine() {
Maybe add a comment near this saying "private".  It seems really strange
otherwise.  Perhaps put it near the other ctors?

Since this is never used, does it really cost us during builds?

http://codereview.chromium.org/3012001/diff/2001/3009
File base/task.h (right):

http://codereview.chromium.org/3012001/diff/2001/3009#newcode31
base/task.h:31: ~CancelableTask();
Why are these needed?
Should this be virtual?

http://codereview.chromium.org/3012001/diff/2001/3011
File base/tracked.h (right):

http://codereview.chromium.org/3012001/diff/2001/3011#newcode53
base/tracked.h:53: bool operator < (const Location& other) const;
Comparators being inlined might be important for performance.

[Elliot Glaysher, 2010-07-15 18:08:10]

In general, constructors and destructors are some of the worse things you can
try to inline because the synthesized constructors are much more complex than
people would expect. *Especially* constructors of derived classes. See Item 30
of Effective C++.

http://codereview.chromium.org/3012001/diff/2001/3002
File base/command_line.cc (right):

http://codereview.chromium.org/3012001/diff/2001/3002#newcode444
base/command_line.cc:444: CommandLine::CommandLine() {
On 2010/07/15 17:37:33, Evan Martin wrote:
> Maybe add a comment near this saying "private".  It seems really strange
> otherwise.  Perhaps put it near the other ctors?

Added comment.
 
> Since this is never used, does it really cost us during builds?

Yes. Because the code for the constructor is still emitted. Individually, the
cost of this constructor is probably small, but the theory behind this patch is
death by a hundred papercuts.

http://codereview.chromium.org/3012001/diff/2001/3009
File base/task.h (right):

http://codereview.chromium.org/3012001/diff/2001/3009#newcode31
base/task.h:31: ~CancelableTask();
On 2010/07/15 17:37:33, Evan Martin wrote:
> Why are these needed?

Because a copy of the synthesized constructor/destructor is still emitted in the
resulting object files. Task()/~Task()/CancelableTask()/~CancelableTask() were
some of the hotspots that showed up when I did my big script to find the most
commonly included weak symbols.

> Should this be virtual?

Done.

http://codereview.chromium.org/3012001/diff/2001/3011
File base/tracked.h (right):

http://codereview.chromium.org/3012001/diff/2001/3011#newcode53
base/tracked.h:53: bool operator < (const Location& other) const;
On 2010/07/15 17:37:33, Evan Martin wrote:
> Comparators being inlined might be important for performance.

I've undone this one. I saw strings and I thought string class comparisons (in
which case I might have been right). A relatively small number of pure
arithmetic operations shouldn't slow things down and this has a chance of
actually being inlined.

[Evan Martin, 2010-07-15 18:11:30]

LGTM rubber stamp