search

apple / swift-nio-http2

HTTP/2 support for SwiftNIO
https://swiftpackageindex.com/apple/swift-nio-http2/main/documentation/niohttp2
Apache License 2.0
465 stars 82 forks source link

Replace estimatedFrameSize with flowControlledSize #387

Closed rnro closed 1 year ago

rnro commented 1 year ago

Motivation:

estimatedFrameSize aims to estimate the number of bytes a frame will use on the wire. It includes the 9-octed frame header and is currently used for flow control estimates but according to spec it shouldn't include those 9 bytes https://www.rfc-editor.org/rfc/rfc9113.html#section-6.9.1

Modifications:

Replace estimatedFrameSize with flowControlledSize which ignores the 9-octed frame header and update tests.

Result:

Flow control calculations should be more accurate

Lukasa commented 1 year ago

Too late here, but we need to fix this up. flowControlledSize is a bad name for what this calculates. More broadly, I don't think we should calculate in this way at all: I don't think counting control frames against our window here makes any sense.

The only frames in HTTP/2 that are actually flow controlled are DATA frames. I think either the channels should only count DATA frames, or we should rename this variable to something else.

rnro commented 1 year ago

See https://github.com/apple/swift-nio-http2/pull/388