Record the last packet send time before we start sending the packet

Record the last packet send time before we start sending the packet

Problem:

Every time QuicConnection sends a new packet, it records the time that
the packet was sent.  It passes this information to the
QuicSentPacketManager, which ultimately stores the information in the
TransmissionInfo.sent_time for the packet.

Later, when the QuicSentPacketManager receives an ack for the packet,
it retrieves its TransmissionInfo and takes a sample of the current
RTT as follows:

rtt_sample = ack_receive_time - transmission_info.sent_time

Previously, QuicConnection was recording the packet "sent_time" as the
time that WritePacket completes.  The problem with this approach is
that the write itself may pause the thread or take a long time.  In
this case, transmission_info.sent_time will be artificially inflated.
When that inflated value is subtracted from the ack time, as above, it
will cause the current RTT sample to become artificially small.

An artificially small RTT will affect our current estimate of the min
RTT, which we currently cannot recover from.  The min_rtt will be
pinned to the aberrant value.

Solution:

Changed the code to record the sent_time as the time that the write
begins.  The drawback of this approach is that any extra send time
will be temporarily factored into our smoothed-RTT calculations.  The
advantage is that it will prevent artificially small RTTs from setting
the min_rtt forever.

Added a corresponding test, which exercises this scenario by
artificially advancing the clock during the write.

Change QUIC to record send timestamp prior to write. Protected by
FLAGS_quic_record_send_time_before_write.

Merge internal change: 80138676

R=rch@chromium.org

[Ryan Hamilton, 2014-11-20 01:06:42]

lgtm

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

https://codereview.chromium.org/740793002/diff/1/net/quic/quic_flags.cc#newco...
net/quic/quic_flags.cc:63: bool FLAGS_quic_record_send_time_before_write =
false;
You can confirm with ian, but I think it's probably fine to make this true.

[ramant (doing other things), 2014-11-20 01:31:14]

On 2014/11/20 01:06:42, Ryan Hamilton wrote:
> lgtm
> 
> https://codereview.chromium.org/740793002/diff/1/net/quic/quic_flags.cc
> File net/quic/quic_flags.cc (right):
> 
>
https://codereview.chromium.org/740793002/diff/1/net/quic/quic_flags.cc#newco...
> net/quic/quic_flags.cc:63: bool FLAGS_quic_record_send_time_before_write =
> false;
> You can confirm with ian, but I think it's probably fine to make this true.

Will confirm with Ian and submit in the next CL. Thanks much Ryan.

[Ian Swett, 2014-11-20 15:32:32]

On 2014/11/20 01:31:14, ramant wrote:
> On 2014/11/20 01:06:42, Ryan Hamilton wrote:
> > lgtm
> > 
> > https://codereview.chromium.org/740793002/diff/1/net/quic/quic_flags.cc
> > File net/quic/quic_flags.cc (right):
> > 
> >
>
https://codereview.chromium.org/740793002/diff/1/net/quic/quic_flags.cc#newco...
> > net/quic/quic_flags.cc:63: bool FLAGS_quic_record_send_time_before_write =
> > false;
> > You can confirm with ian, but I think it's probably fine to make this true.
> 
> Will confirm with Ian and submit in the next CL. Thanks much Ryan.

Yes, it's fine to make this true.  Thanks.

[ramant (doing other things), 2014-11-20 22:41:25]

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

https://codereview.chromium.org/740793002/diff/1/net/quic/quic_flags.cc#newco...
net/quic/quic_flags.cc:63: bool FLAGS_quic_record_send_time_before_write =
false;
On 2014/11/20 01:06:42, Ryan Hamilton wrote:
> You can confirm with ian, but I think it's probably fine to make this true.

Changed it in https://codereview.chromium.org/744273002
Done.