From ceb5ef48c421bd98ef752c4060ca51010e8cebac Mon Sep 17 00:00:00 2001 From: Marten Seemann Date: Wed, 18 May 2016 18:19:18 +0700 Subject: [PATCH] improve WindowUpdate thresholds fixes #111 --- flow_controller.go | 12 ++++++------ flow_controller_test.go | 8 +++----- protocol/server_parameters.go | 13 +++---------- stream_test.go | 14 +++++++------- 4 files changed, 19 insertions(+), 28 deletions(-) diff --git a/flow_controller.go b/flow_controller.go index d759eff8..d7cd4c35 100644 --- a/flow_controller.go +++ b/flow_controller.go @@ -18,7 +18,6 @@ type flowController struct { bytesRead protocol.ByteCount highestReceived protocol.ByteCount - receiveWindowUpdateThreshold protocol.ByteCount receiveFlowControlWindow protocol.ByteCount receiveFlowControlWindowIncrement protocol.ByteCount @@ -27,16 +26,16 @@ type flowController struct { func newFlowController(streamID protocol.StreamID, connectionParametersManager *handshake.ConnectionParametersManager) *flowController { fc := flowController{ - streamID: streamID, - connectionParametersManager: connectionParametersManager, - receiveWindowUpdateThreshold: protocol.WindowUpdateThreshold, - receiveFlowControlWindowIncrement: protocol.ReceiveStreamFlowControlWindowIncrement, + streamID: streamID, + connectionParametersManager: connectionParametersManager, } if streamID == 0 { fc.receiveFlowControlWindow = connectionParametersManager.GetReceiveConnectionFlowControlWindow() + fc.receiveFlowControlWindowIncrement = fc.receiveFlowControlWindow } else { fc.receiveFlowControlWindow = connectionParametersManager.GetReceiveStreamFlowControlWindow() + fc.receiveFlowControlWindowIncrement = fc.receiveFlowControlWindow } return &fc @@ -144,7 +143,8 @@ func (c *flowController) MaybeTriggerWindowUpdate() (bool, protocol.ByteCount) { defer c.mutex.Unlock() diff := c.receiveFlowControlWindow - c.bytesRead - if diff < c.receiveWindowUpdateThreshold { + // Chromium implements the same threshold + if diff < (c.receiveFlowControlWindowIncrement / 2) { c.receiveFlowControlWindow += c.receiveFlowControlWindowIncrement return true, c.bytesRead + c.receiveFlowControlWindowIncrement } diff --git a/flow_controller_test.go b/flow_controller_test.go index b4d5ccfe..6b2c6917 100644 --- a/flow_controller_test.go +++ b/flow_controller_test.go @@ -155,13 +155,11 @@ var _ = Describe("Flow controller", func() { }) Context("receive flow control", func() { - var receiveFlowControlWindow protocol.ByteCount = 1337 - var receiveWindowUpdateThreshold protocol.ByteCount = 500 + var receiveFlowControlWindow protocol.ByteCount = 10000 var receiveFlowControlWindowIncrement protocol.ByteCount = 600 BeforeEach(func() { controller.receiveFlowControlWindow = receiveFlowControlWindow - controller.receiveWindowUpdateThreshold = receiveWindowUpdateThreshold controller.receiveFlowControlWindowIncrement = receiveFlowControlWindowIncrement }) @@ -172,7 +170,7 @@ var _ = Describe("Flow controller", func() { }) It("triggers a window update when necessary", func() { - readPosition := receiveFlowControlWindow - receiveWindowUpdateThreshold + 1 + readPosition := receiveFlowControlWindow - receiveFlowControlWindowIncrement/2 + 1 controller.bytesRead = readPosition updateNecessary, offset := controller.MaybeTriggerWindowUpdate() Expect(updateNecessary).To(BeTrue()) @@ -180,7 +178,7 @@ var _ = Describe("Flow controller", func() { }) It("triggers a window update when not necessary", func() { - readPosition := receiveFlowControlWindow - receiveWindowUpdateThreshold - 1 + readPosition := receiveFlowControlWindow - receiveFlowControlWindow/2 - 1 controller.bytesRead = readPosition updateNecessary, _ := controller.MaybeTriggerWindowUpdate() Expect(updateNecessary).To(BeFalse()) diff --git a/protocol/server_parameters.go b/protocol/server_parameters.go index 7378260e..b6a6ef15 100644 --- a/protocol/server_parameters.go +++ b/protocol/server_parameters.go @@ -25,16 +25,12 @@ const SmallPacketPayloadSizeThreshold = MaxPacketSize / 2 const SmallPacketSendDelay = 500 * time.Microsecond // ReceiveStreamFlowControlWindow is the stream-level flow control window for receiving data -// TODO: set a reasonable value here +// This is the value that Google servers are using const ReceiveStreamFlowControlWindow ByteCount = (1 << 20) // 1 MB -// ReceiveStreamFlowControlWindowIncrement is the amount that the stream-level flow control window is increased when sending a WindowUpdate -const ReceiveStreamFlowControlWindowIncrement = ReceiveStreamFlowControlWindow - // ReceiveConnectionFlowControlWindow is the stream-level flow control window for receiving data -// temporarily set this to a very high value, until proper connection-level flow control is implemented -// TODO: set a reasonable value here -const ReceiveConnectionFlowControlWindow ByteCount = (1 << 20) // 1 MB +// This is the value that Google servers are using +const ReceiveConnectionFlowControlWindow ByteCount = (1 << 20) * 1.5 // 1.5 MB // MaxStreamsPerConnection is the maximum value accepted for the number of streams per connection // TODO: set a reasonable value here @@ -44,9 +40,6 @@ const MaxStreamsPerConnection uint32 = 100 // TODO: set a reasonable value here const MaxIdleConnectionStateLifetime = 60 * time.Second -// WindowUpdateThreshold is the size of the receive flow control window for which we send out a WindowUpdate frame -const WindowUpdateThreshold = ReceiveStreamFlowControlWindow / 2 - // WindowUpdateNumRepitions is the number of times the same WindowUpdate frame will be sent to the client const WindowUpdateNumRepitions uint8 = 2 diff --git a/stream_test.go b/stream_test.go index c2826ccb..39ff6894 100644 --- a/stream_test.go +++ b/stream_test.go @@ -465,15 +465,15 @@ var _ = Describe("Stream", func() { }) Context("flow control window updating, for receiving", func() { - var receiveFlowControlWindow protocol.ByteCount = 1337 - var receiveWindowUpdateThreshold protocol.ByteCount = 1000 + var receiveFlowControlWindow protocol.ByteCount = 1000 + var receiveFlowControlWindowIncrement protocol.ByteCount = 1000 BeforeEach(func() { str.flowController.receiveFlowControlWindow = receiveFlowControlWindow - str.flowController.receiveWindowUpdateThreshold = receiveWindowUpdateThreshold + str.flowController.receiveFlowControlWindowIncrement = receiveFlowControlWindowIncrement }) It("updates the flow control window", func() { - len := int(receiveFlowControlWindow) - int(receiveWindowUpdateThreshold) + 1 + len := int(receiveFlowControlWindow)/2 + 1 frame := frames.StreamFrame{ Offset: 0, Data: bytes.Repeat([]byte{'f'}, len), @@ -490,8 +490,8 @@ var _ = Describe("Stream", func() { It("updates the connection level flow control window", func() { str.connectionFlowController.receiveFlowControlWindow = 100 - str.connectionFlowController.receiveWindowUpdateThreshold = 60 - len := 100 - 60 + 1 + str.connectionFlowController.receiveFlowControlWindowIncrement = 100 + len := 100/2 + 1 frame := frames.StreamFrame{ Offset: 0, Data: bytes.Repeat([]byte{'f'}, len), @@ -507,7 +507,7 @@ var _ = Describe("Stream", func() { }) It("does not update the flow control window when not enough data was received", func() { - len := int(receiveFlowControlWindow) - int(receiveWindowUpdateThreshold) - 1 + len := int(receiveFlowControlWindow)/2 - 1 frame := frames.StreamFrame{ Offset: 0, Data: bytes.Repeat([]byte{'f'}, len),