Reduce the Spdy data frame sizes used. Attempt to make them fit within SSL...

Reduce the Spdy data frame sizes used.  Attempt to make them fit within SSL
Records, which should fit into MSS.

BUG=none
TEST=none


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

[kelindsay, 2010-12-17 21:36:02]

LGTM

[fenix, 2010-12-17 21:56:11]

I'm checking to see that it does what we want. I'm not yet sure that
breaking it into frames in  a while loop there does it.

I think you need to return bytes consumed and not have the while loop there
(and then change something around 2177

e.g. (changes on right)

2141 SendDataFrame(mci->stream_id, 2172 int64_t num_that_will_be_sent =
SendDataFrame(mci->stream_id, 2142 mci->file_data->body.data() +
mci->body_bytes_consumed, 2173 mci->file_data->body.data() +
mci->body_bytes_consumed, 2143 num_to_write, 0, should_compress); 2174
num_to_write, 0, should_compress); 2144 VLOG(2) << ACCEPTOR_CLIENT_IDENT <<
"SpdySM: GetOutput SendDataFrame[" 2175 VLOG(2) << ACCEPTOR_CLIENT_IDENT <<
"SpdySM: GetOutput SendDataFrame[" 2145 << mci->stream_id << "]: " <<
num_to_write; 2176 << mci->stream_id << "]: " << num_to_write; 2146
mci->body_bytes_consumed += num_to_write; 2177 mci->body_bytes_consumed +=
num_that_will_be_sent; 2147 mci->bytes_sent += num_to_write; 2178
mci->bytes_sent += num_that_will_be_sent;
-=R

On Fri, Dec 17, 2010 at 1:36 PM, <kelindsay@gmail.com> wrote:

> LGTM
>
>
> http://codereview.chromium.org/5966003/
>

[Mike Belshe, 2010-12-17 22:08:41]

That might be a simpler change.

Is the GetOutput() used across both the in-mem cases and the proxy case?  The
use of "MemCacheIter" made me think not, but in looking at the code, maybe it
is.  And the MemCacheIter is capable of buffering?



On 2010/12/17 21:56:11, fenix wrote:
> I'm checking to see that it does what we want. I'm not yet sure that
> breaking it into frames in  a while loop there does it.
> 
> I think you need to return bytes consumed and not have the while loop there
> (and then change something around 2177
> 
> e.g. (changes on right)
> 
> 2141 SendDataFrame(mci->stream_id, 2172 int64_t num_that_will_be_sent =
> SendDataFrame(mci->stream_id, 2142 mci->file_data->body.data() +
> mci->body_bytes_consumed, 2173 mci->file_data->body.data() +
> mci->body_bytes_consumed, 2143 num_to_write, 0, should_compress); 2174
> num_to_write, 0, should_compress); 2144 VLOG(2) << ACCEPTOR_CLIENT_IDENT <<
> "SpdySM: GetOutput SendDataFrame[" 2175 VLOG(2) << ACCEPTOR_CLIENT_IDENT <<
> "SpdySM: GetOutput SendDataFrame[" 2145 << mci->stream_id << "]: " <<
> num_to_write; 2176 << mci->stream_id << "]: " << num_to_write; 2146
> mci->body_bytes_consumed += num_to_write; 2177 mci->body_bytes_consumed +=
> num_that_will_be_sent; 2147 mci->bytes_sent += num_to_write; 2178
> mci->bytes_sent += num_that_will_be_sent;
> -=R
> 
> On Fri, Dec 17, 2010 at 1:36 PM, <mailto:kelindsay@gmail.com> wrote:
> 
> > LGTM
> >
> >
> > http://codereview.chromium.org/5966003/
> >

[Mike Belshe, 2011-01-15 04:02:58]

I took a long look at this, based on your feedback, Roberto.

Your comments were incorrect; in this case we're working through the proxy
delivery where we read from the socket, and push into the output queue.  The
reason for chopping here is not for prioritization (we don't want to buffer this
data anyway), but to make sure that it can be delivered out of SSL on the other
end as quickly as possible, and large packet trains make that difficult.

Further, the code snippet you pointed at already does chunking (see the lines
just prior to the ones you cited - they grab a chunk at a time of bytes as the
max send size).

So I believe the fix is correct, and I'm moving forward with it.

On 2010/12/17 22:08:41, Mike Belshe wrote:
> That might be a simpler change.
> 
> Is the GetOutput() used across both the in-mem cases and the proxy case?  The
> use of "MemCacheIter" made me think not, but in looking at the code, maybe it
> is.  And the MemCacheIter is capable of buffering?
> 
> 
> 
> On 2010/12/17 21:56:11, fenix wrote:
> > I'm checking to see that it does what we want. I'm not yet sure that
> > breaking it into frames in  a while loop there does it.
> > 
> > I think you need to return bytes consumed and not have the while loop there
> > (and then change something around 2177
> > 
> > e.g. (changes on right)
> > 
> > 2141 SendDataFrame(mci->stream_id, 2172 int64_t num_that_will_be_sent =
> > SendDataFrame(mci->stream_id, 2142 mci->file_data->body.data() +
> > mci->body_bytes_consumed, 2173 mci->file_data->body.data() +
> > mci->body_bytes_consumed, 2143 num_to_write, 0, should_compress); 2174
> > num_to_write, 0, should_compress); 2144 VLOG(2) << ACCEPTOR_CLIENT_IDENT <<
> > "SpdySM: GetOutput SendDataFrame[" 2175 VLOG(2) << ACCEPTOR_CLIENT_IDENT <<
> > "SpdySM: GetOutput SendDataFrame[" 2145 << mci->stream_id << "]: " <<
> > num_to_write; 2176 << mci->stream_id << "]: " << num_to_write; 2146
> > mci->body_bytes_consumed += num_to_write; 2177 mci->body_bytes_consumed +=
> > num_that_will_be_sent; 2147 mci->bytes_sent += num_to_write; 2178
> > mci->bytes_sent += num_that_will_be_sent;
> > -=R
> > 
> > On Fri, Dec 17, 2010 at 1:36 PM, <mailto:kelindsay@gmail.com> wrote:
> > 
> > > LGTM
> > >
> > >
> > > http://codereview.chromium.org/5966003/
> > >