Optimize chained string concatenation in SPDY code.

Optimize chained string concatenation in SPDY code.

C++ operator+ for std::string creates temporary strings that have
to be constructed and destructed inline. When chaining more than one +
those temporary objects makes the code larger and slower than necessary.

This shrinks the SPDY code by 1-2% (performance change is smaller).

Size data from Linux x64 content_shell:
Total change: -2531 bytes
         -2: (strings)
       -244: net::GetUrlFromHeaderBlock(std::map<std::string, std::string, std::less<std::string>, std::allocator<std::pair<std::string const, std::string> > > const&, net::SpdyMajorVersion, bool) type=t, (was 1285 bytes, now 1041 bytes)
       -395: net::SpdyStream::DecreaseRecvWindowSize(int) type=t, (was 969 bytes, now 574 bytes)
       -917: net::SpdyFramer::WriteHeaderBlockToZ(std::map<std::string, std::string, std::less<std::string>, std::allocator<std::pair<std::string const, std::string> > > const*, z_stream_s*) const type=t, (was 3913 bytes, now 2996 bytes)
         -9: net::SpdySession::OnWindowUpdate(unsigned int, unsigned int) type=t, (was 1331 bytes, now 1322 bytes)
       -143: net::(anonymous namespace)::SpdyHeaderBlockToListValue(std::map<std::string, std::string, std::less<std::string>, std::allocator<std::pair<std::string const, std::string> > > const&, net::NetLog::LogLevel) type=t, (was 520 bytes, now 377 bytes)
       -367: net::SpdySession::IncreaseSendWindowSize(int) type=t, (was 954 bytes, now 587 bytes)
       -454: net::SpdySession::DecreaseRecvWindowSize(int) type=t, (was 1177 bytes, now 723 bytes)

BUG=

[Bence, 2015-03-09 19:53:45]

bnc@chromium.org changed reviewers:
+ bnc@chromium.org, mmenke@chromium.org

[Bence, 2015-03-09 19:53:45]

bnc@chromium.org changed required reviewers:
+ mmenke@chromium.org

[Bence, 2015-03-09 19:53:46]

Matt, PTAL.

Daniel, sorry for not getting back to you earlier.  I am unsure what the policy
is on between binary size and readability, I do not think that your change makes
things much more difficult to read.

https://codereview.chromium.org/965773002/diff/1/net/spdy/spdy_http_utils.cc
File net/spdy/spdy_http_utils.cc (right):

https://codereview.chromium.org/965773002/diff/1/net/spdy/spdy_http_utils.cc#...
net/spdy/spdy_http_utils.cc:205: std::string url = scheme;
You don't seem to be conserving behavior here:  In the original code, |url| is
empty if any of scheme, host_port, and path is empty.

[mmenke, 2015-03-09 20:21:59]

https://codereview.chromium.org/965773002/diff/1/net/spdy/spdy_framer.cc
File net/spdy/spdy_framer.cc (right):

https://codereview.chromium.org/965773002/diff/1/net/spdy/spdy_framer.cc#newc...
net/spdy/spdy_framer.cc:1346: cookie.append(";");
Isn't this the same as:

if (i > 0)
  cookie.append("; ");
cookie.append(cookie_values[i].as_string());

https://codereview.chromium.org/965773002/diff/1/net/spdy/spdy_session.cc
File net/spdy/spdy_session.cc (right):

https://codereview.chromium.org/965773002/diff/1/net/spdy/spdy_session.cc#new...
net/spdy/spdy_session.cc:2539: .append(base::UintToString(delta_window_size)));
This could just be:

base::StringPrintf("Received... %i", delta_window_size);

(StringPrintf may be slower, but given that this is done on errors only, I don't
think performance matters...and it may well be faster, anyways, since it creates
2 fewer std::strings)

https://codereview.chromium.org/965773002/diff/1/net/spdy/spdy_stream.cc
File net/spdy/spdy_stream.cc (right):

https://codereview.chromium.org/965773002/diff/1/net/spdy/spdy_stream.cc#newc...
net/spdy/spdy_stream.cc:358: .append(base::IntToString(recv_window_size_)));
Worth considering std::StringPrintf here, too?

[Bence, 2015-03-09 21:00:08]

Daniel:  I propose https://crrev.com/996483002 instead of your change for
spdy_http_utils.cc.  Thanks for pointing out the inefficienty of compound inline
operator+().

[Daniel Bratell, 2015-03-10 10:03:55]

On 2015/03/09 21:00:08, Bence wrote:
> Daniel:  I propose https://crrev.com/996483002 instead of your change for
> spdy_http_utils.cc.  Thanks for pointing out the inefficienty of compound
inline
> operator+().

Be my guest. It's easier for you to do well tested changes so I don't mind.

append vs formatted strings in error management, it's a matter of taste I think.
I like formatted strings so I don't mind that change either. I'll update this.

[mmenke, 2015-03-30 15:22:46]

Removing myself as a reviewer, to clean up my pending review list.  Feel free to
add my back if you pick up this CL again (Removing everything but the
net/spdy/spdy_http_utils.cc changes?)

[mmenke, 2015-03-30 15:22:55]

mmenke@chromium.org changed reviewers:
- mmenke@chromium.org

[mmenke, 2015-03-30 15:22:55]

mmenke@chromium.org changed required reviewers:
- mmenke@chromium.org

[mmenke, 2015-03-30 15:23:22]

On 2015/03/30 15:22:46, mmenke wrote:
> Removing myself as a reviewer, to clean up my pending review list.  Feel free
to
> add my back if you pick up this CL again (Removing everything but the
> net/spdy/spdy_http_utils.cc changes?)

Erm...keeping everything but the spdy_http_utils changes, rather.

[Bence, 2015-03-30 15:28:11]

bnc@chromium.org changed reviewers:
- bnc@chromium.org

[Bence, 2015-03-30 15:28:48]

On 2015/03/30 15:22:46, mmenke wrote:
> Removing myself as a reviewer, to clean up my pending review list.  Feel free
to
> add my back if you pick up this CL again (Removing everything but the
> net/spdy/spdy_http_utils.cc changes?)

Same here: removing myself as a reviewer, feel free to add me back when/if you
update this CL.