Update quic flags.

Update quic flags.

R=rch@chromium.org
BUG=

[Ryan Hamilton, 2016-08-10 16:22:22]

lgtm

https://codereview.chromium.org/2233943002/diff/1/net/quic/core/quic_flags.cc
File net/quic/core/quic_flags.cc (right):

https://codereview.chromium.org/2233943002/diff/1/net/quic/core/quic_flags.cc...
net/quic/core/quic_flags.cc:127: bool
FLAGS_quic_sequencer_buffer_retire_block_in_time = false;
Was this true because it fixes a crash bug?

[danzh1, 2016-08-10 18:05:06]

https://codereview.chromium.org/2233943002/diff/1/net/quic/core/quic_flags.cc
File net/quic/core/quic_flags.cc (right):

https://codereview.chromium.org/2233943002/diff/1/net/quic/core/quic_flags.cc...
net/quic/core/quic_flags.cc:127: bool
FLAGS_quic_sequencer_buffer_retire_block_in_time = false;
On 2016/08/10 16:22:22, Ryan Hamilton wrote:
> Was this true because it fixes a crash bug?

This fix just makes chunks of memory released more frequently as they should.
Otherwise, after all data in a memory block is read out, the memory block is not
released but stays there till sequencer buffer is destructed or it is reused by
following new data. This bug shouldn't cause any crush but allow memory
deallocation to happen in time.

As the current code doesn't cause any problem, this flag is by default false for
now.

[danzh1, 2016-08-10 20:54:07]

The CQ bit was checked by danzh@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-08-10 20:54:39]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Ryan Hamilton, 2016-08-10 21:06:14]

On 2016/08/10 18:05:06, danzh1 wrote:
> https://codereview.chromium.org/2233943002/diff/1/net/quic/core/quic_flags.cc
> File net/quic/core/quic_flags.cc (right):
> 
>
https://codereview.chromium.org/2233943002/diff/1/net/quic/core/quic_flags.cc...
> net/quic/core/quic_flags.cc:127: bool
> FLAGS_quic_sequencer_buffer_retire_block_in_time = false;
> On 2016/08/10 16:22:22, Ryan Hamilton wrote:
> > Was this true because it fixes a crash bug?
> 
> This fix just makes chunks of memory released more frequently as they should.
> Otherwise, after all data in a memory block is read out, the memory block is
not
> released but stays there till sequencer buffer is destructed or it is reused
by
> following new data. This bug shouldn't cause any crush but allow memory
> deallocation to happen in time.
> 
> As the current code doesn't cause any problem, this flag is by default false
for
> now.

I see. It's true in chrome currently and I'm curious how that happened. Was that
flag merged as part of the merge process or was it merged out of band?

[danzh1, 2016-08-11 14:33:34]

On 2016/08/10 21:06:14, Ryan Hamilton wrote:
> On 2016/08/10 18:05:06, danzh1 wrote:
> >
https://codereview.chromium.org/2233943002/diff/1/net/quic/core/quic_flags.cc
> > File net/quic/core/quic_flags.cc (right):
> > 
> >
>
https://codereview.chromium.org/2233943002/diff/1/net/quic/core/quic_flags.cc...
> > net/quic/core/quic_flags.cc:127: bool
> > FLAGS_quic_sequencer_buffer_retire_block_in_time = false;
> > On 2016/08/10 16:22:22, Ryan Hamilton wrote:
> > > Was this true because it fixes a crash bug?
> > 
> > This fix just makes chunks of memory released more frequently as they
should.
> > Otherwise, after all data in a memory block is read out, the memory block is
> not
> > released but stays there till sequencer buffer is destructed or it is reused
> by
> > following new data. This bug shouldn't cause any crush but allow memory
> > deallocation to happen in time.
> > 
> > As the current code doesn't cause any problem, this flag is by default false
> for
> > now.
> 
> I see. It's true in chrome currently and I'm curious how that happened. Was
that
> flag merged as part of the merge process or was it merged out of band?

It is merged manually last week.

[Ryan Hamilton, 2016-08-11 18:32:57]

On 2016/08/11 14:33:34, danzh1 wrote:
> On 2016/08/10 21:06:14, Ryan Hamilton wrote:
> > On 2016/08/10 18:05:06, danzh1 wrote:
> > >
> https://codereview.chromium.org/2233943002/diff/1/net/quic/core/quic_flags.cc
> > > File net/quic/core/quic_flags.cc (right):
> > > 
> > >
> >
>
https://codereview.chromium.org/2233943002/diff/1/net/quic/core/quic_flags.cc...
> > > net/quic/core/quic_flags.cc:127: bool
> > > FLAGS_quic_sequencer_buffer_retire_block_in_time = false;
> > > On 2016/08/10 16:22:22, Ryan Hamilton wrote:
> > > > Was this true because it fixes a crash bug?
> > > 
> > > This fix just makes chunks of memory released more frequently as they
> should.
> > > Otherwise, after all data in a memory block is read out, the memory block
is
> > not
> > > released but stays there till sequencer buffer is destructed or it is
reused
> > by
> > > following new data. This bug shouldn't cause any crush but allow memory
> > > deallocation to happen in time.
> > > 
> > > As the current code doesn't cause any problem, this flag is by default
false
> > for
> > > now.
> > 
> > I see. It's true in chrome currently and I'm curious how that happened. Was
> that
> > flag merged as part of the merge process or was it merged out of band?
> 
> It is merged manually last week.

I see. In that case, let's leave the flag true because I think it was part of
the chrome crash investigation.

[danzh1, 2016-08-11 19:03:44]

On 2016/08/11 18:32:57, Ryan Hamilton wrote:
> On 2016/08/11 14:33:34, danzh1 wrote:
> > On 2016/08/10 21:06:14, Ryan Hamilton wrote:
> > > On 2016/08/10 18:05:06, danzh1 wrote:
> > > >
> >
https://codereview.chromium.org/2233943002/diff/1/net/quic/core/quic_flags.cc
> > > > File net/quic/core/quic_flags.cc (right):
> > > > 
> > > >
> > >
> >
>
https://codereview.chromium.org/2233943002/diff/1/net/quic/core/quic_flags.cc...
> > > > net/quic/core/quic_flags.cc:127: bool
> > > > FLAGS_quic_sequencer_buffer_retire_block_in_time = false;
> > > > On 2016/08/10 16:22:22, Ryan Hamilton wrote:
> > > > > Was this true because it fixes a crash bug?
> > > > 
> > > > This fix just makes chunks of memory released more frequently as they
> > should.
> > > > Otherwise, after all data in a memory block is read out, the memory
block
> is
> > > not
> > > > released but stays there till sequencer buffer is destructed or it is
> reused
> > > by
> > > > following new data. This bug shouldn't cause any crush but allow memory
> > > > deallocation to happen in time.
> > > > 
> > > > As the current code doesn't cause any problem, this flag is by default
> false
> > > for
> > > > now.
> > > 
> > > I see. It's true in chrome currently and I'm curious how that happened.
Was
> > that
> > > flag merged as part of the merge process or was it merged out of band?
> > 
> > It is merged manually last week.
> 
> I see. In that case, let's leave the flag true because I think it was part of
> the chrome crash investigation.

Will flip it in Final CL.

flip

[danzh1, 2016-08-12 13:33:26]

The CQ bit was checked by danzh@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-08-12 13:33:37]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-08-12 14:16:17]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-08-12 14:16:18]

Dry run: Try jobs failed on following builders:
  linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)