Move more code from headers to implementation.

Move more code from headers to implementation.

BUG=none
TEST=compiles

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

[Elliot Glaysher, 2010-12-07 21:07:01]

Things have degraded in the intervening month.

[James Hawkins, 2010-12-07 21:09:11]

LGTM

[Sergey Ulanov, 2010-12-07 21:15:42]

http://codereview.chromium.org/5624002/diff/8001/remoting/protocol/rtp_utils.cc
File remoting/protocol/rtp_utils.cc (right):

http://codereview.chromium.org/5624002/diff/8001/remoting/protocol/rtp_utils....
remoting/protocol/rtp_utils.cc:52: 
empty lines.

http://codereview.chromium.org/5624002/diff/8001/remoting/protocol/rtp_utils.h
File remoting/protocol/rtp_utils.h (right):

http://codereview.chromium.org/5624002/diff/8001/remoting/protocol/rtp_utils....
remoting/protocol/rtp_utils.h:14: RtpHeader();
This was inlined intentionally. This file is included only in several files, and
this constructor is used only in two places. It shouldn't affect compilation
performance in any significant way. Can we please keep it as it is?

http://codereview.chromium.org/5624002/diff/8001/remoting/protocol/rtp_utils....
remoting/protocol/rtp_utils.h:30: RtcpReceiverReport();
same here.

[Elliot Glaysher, 2010-12-07 21:49:24]

http://codereview.chromium.org/5624002/diff/8001/remoting/protocol/rtp_utils.cc
File remoting/protocol/rtp_utils.cc (right):

http://codereview.chromium.org/5624002/diff/8001/remoting/protocol/rtp_utils....
remoting/protocol/rtp_utils.cc:52: 
On 2010/12/07 21:15:42, sergeyu wrote:
> empty lines.

Done.

http://codereview.chromium.org/5624002/diff/8001/remoting/protocol/rtp_utils.h
File remoting/protocol/rtp_utils.h (right):

http://codereview.chromium.org/5624002/diff/8001/remoting/protocol/rtp_utils....
remoting/protocol/rtp_utils.h:14: RtpHeader();
> Can we please keep it as it is?

This was found by an automated process which is going to be deployed on the
waterfall Real Soon Now. I don't believe that this will make a difference either
way at runtime.

[Sergey Ulanov, 2010-12-07 22:00:38]

http://codereview.chromium.org/5624002/diff/8001/remoting/protocol/rtp_utils.h
File remoting/protocol/rtp_utils.h (right):

http://codereview.chromium.org/5624002/diff/8001/remoting/protocol/rtp_utils....
remoting/protocol/rtp_utils.h:14: RtpHeader();
On 2010/12/07 21:49:25, Elliot Glaysher wrote:
> > Can we please keep it as it is?
> 
> This was found by an automated process which is going to be deployed on the
> waterfall Real Soon Now. I don't believe that this will make a difference
either
> way at runtime.

Yes, it doesn't make much difference in performance in this particular case, but
IMHO it is more readable with inlined constructor.
With the new automated process, will it be possible to inline constructors when
it is necessary for performance reasons?

[Elliot Glaysher, 2010-12-07 22:33:10]

On 2010/12/07 22:00:38, sergeyu wrote:
> With the new automated process, will it be possible to inline constructors
when
> it is necessary for performance reasons?

1) So far, every time someone has pushed back on me (or jamesr@ in the case of
WebKit) on deinlining methods, we've actually had performance progressions on
various applicable perftests. I'm very skeptical of the claim that inlining
routinely is beneficial, but...

2) There is white-listing mechanism for the cases where it actually is
beneficial. As of now, there's three exceptions: (a) an RAII object used in
thread local storage, (b) some part of the GPU process that I admit that I don't
entirely understand, (c) part of the media/ framework's callback system. I'm
going to be applying a pretty high bar of evidence to get on this white-list
though.

[Sergey Ulanov, 2010-12-07 22:44:46]

LGTM