-
Notifications
You must be signed in to change notification settings - Fork 84
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Lift requirement to create streams in order #214
Comments
Lukasa
added a commit
to Lukasa/swift-nio-http2
that referenced
this issue
Jul 28, 2020
Motivation: As part of the work in apple#214 we need a way for the HTTP2StreamMultiplexer to hold multiple kinds of channels. It would be best if doing so didn't require substantial changes to HTTP2StreamMultiplexer, which is difficult because right now it is tightly coupled to HTTP2StreamChannel. There are a few ways we could do this. The most obvious option to most Swift programmers would be to define a protocol that covers the surface area we want to use, and then hold that. This works fairly well, but the downside of it is that it will force all calls from HTTP2StreamMultiplexer to become dynamically dispatched. These indirect calls are optimisation boundaries and so provide some performance costs. Another option would be to create a subclassing relationship. This is ok, but it also forces us into the realm of dynamic dispatch on some of these calls, and also provides general difficulty if we ever want to provide a backing channel that isn't easily made into a subclass. The remaining option is to provide an abstract object that uses an `enum` to store the kinds of channel we know about, and jump through it. This works really well for small numbers of channel types (1 and 2 channels being the best outcomes), as it introduces very few extra branches which are highly predictable, as well as allowing direct function calls. This is great: one predictable branch that cannot possibly miss cache, followed by a direct function call. Joy of joys! However, it would be best if we didn't tie ourselves into this implementation strategy. After all, down the road we could well end up with more kinds of channels, and at a certain point the cost of the indirect function call becomes _lower_ than the cost of jumping through the enumeration. For that reason, we should define an abstraction. Enter: `MultiplexerAbstractChannel`. This concrete type defines the interface that `HTTP2StreamMultiplexer` will use to interact with stream channels. As it's internal, the abstraction is totally porous: Swift is capable of optimising it away to nothing. This type is explicitly a reference type, maintaining the semantic expected by `HTTP2StreamMultiplexer`. This provides a nice encapsulation over our abstraction. `HTTP2StreamMultiplexer` never needs to know about how we abstract over multiple channel types: the entire abstraction lives in a single, simple file, and can easily be rewritten without difficulty. Modifications: - Define `MultiplexerAbstractChannel` and the interface expected by `HTTP2StreamMultiplexer`. - Define a `BaseChannel` `enum` that will be used to dispatch to multiple `Channel` types. - Rewrite `HTTP2StreamMultiplexer` to use this new abstraction. Result: A customisation point for new channels will exist.
glbrntt
added a commit
to glbrntt/swift-nio-http2
that referenced
this issue
Jul 28, 2020
Motivation: As part of the work for apple#214 to enforce stream creation order we need to remove the requirement that the data sent over the stream be aware of its stream ID ahead of time. This requires that the channel transporting the data provide the stream ID. As such we need a way to convert between data with stream IDs and data without stream IDs. Modifications: - Add 'HTTP2FrameConvertible' protocol whose conforming types can make an 'HTTP2Frame'. - Add 'HTTP2FramePayloadConvertible' protocol whose conforming types can make an 'HTTP2Frame.FramePayload'. - Conform 'HTTP2Frame' and 'HTTP2Frame.FramePayload' to the above protocols. Result: We have a protocol which an HTTP2 stream channel may be generic over.
Lukasa
pushed a commit
that referenced
this issue
Jul 28, 2020
Motivation: As part of the work for #214 to enforce stream creation order we need to remove the requirement that the data sent over the stream be aware of its stream ID ahead of time. This requires that the channel transporting the data provide the stream ID. As such we need a way to convert between data with stream IDs and data without stream IDs. Modifications: - Add 'HTTP2FrameConvertible' protocol whose conforming types can make an 'HTTP2Frame'. - Add 'HTTP2FramePayloadConvertible' protocol whose conforming types can make an 'HTTP2Frame.FramePayload'. - Conform 'HTTP2Frame' and 'HTTP2Frame.FramePayload' to the above protocols. Result: We have a protocol which an HTTP2 stream channel may be generic over.
Lukasa
added a commit
that referenced
this issue
Jul 28, 2020
Motivation: As part of the work in #214 we need a way for the HTTP2StreamMultiplexer to hold multiple kinds of channels. It would be best if doing so didn't require substantial changes to HTTP2StreamMultiplexer, which is difficult because right now it is tightly coupled to HTTP2StreamChannel. There are a few ways we could do this. The most obvious option to most Swift programmers would be to define a protocol that covers the surface area we want to use, and then hold that. This works fairly well, but the downside of it is that it will force all calls from HTTP2StreamMultiplexer to become dynamically dispatched. These indirect calls are optimisation boundaries and so provide some performance costs. Another option would be to create a subclassing relationship. This is ok, but it also forces us into the realm of dynamic dispatch on some of these calls, and also provides general difficulty if we ever want to provide a backing channel that isn't easily made into a subclass. The remaining option is to provide an abstract object that uses an `enum` to store the kinds of channel we know about, and jump through it. This works really well for small numbers of channel types (1 and 2 channels being the best outcomes), as it introduces very few extra branches which are highly predictable, as well as allowing direct function calls. This is great: one predictable branch that cannot possibly miss cache, followed by a direct function call. Joy of joys! However, it would be best if we didn't tie ourselves into this implementation strategy. After all, down the road we could well end up with more kinds of channels, and at a certain point the cost of the indirect function call becomes _lower_ than the cost of jumping through the enumeration. For that reason, we should define an abstraction. Enter: `MultiplexerAbstractChannel`. This concrete type defines the interface that `HTTP2StreamMultiplexer` will use to interact with stream channels. As it's internal, the abstraction is totally porous: Swift is capable of optimising it away to nothing. This type is explicitly a reference type, maintaining the semantic expected by `HTTP2StreamMultiplexer`. This provides a nice encapsulation over our abstraction. `HTTP2StreamMultiplexer` never needs to know about how we abstract over multiple channel types: the entire abstraction lives in a single, simple file, and can easily be rewritten without difficulty. Modifications: - Define `MultiplexerAbstractChannel` and the interface expected by `HTTP2StreamMultiplexer`. - Define a `BaseChannel` `enum` that will be used to dispatch to multiple `Channel` types. - Rewrite `HTTP2StreamMultiplexer` to use this new abstraction. Result: A customisation point for new channels will exist.
Lukasa
added a commit
to Lukasa/swift-nio-http2
that referenced
this issue
Jul 28, 2020
Motivation: As part of the work in apple#214, HTTP2StreamID will need to be able to tolerate not knowing its stream ID. For now this is not a problem, but we should think about everywhere we use the stream ID and make sure we have a solid principle in mind. This lays the groundwork for future enhancements. Modifications: - Update HTTP2StreamChannel to hold an optional stream ID. - Update interface to HTTP2StreamMultiplexer to tolerate closing channels without stream IDs (we'll need it later). - Move the "get a new stream ID" functionality to a method on HTTP2StreamMultiplexer. - Validate we behave correctly with optional stream IDs in the channel. Result: We can tolerate stream IDs being not present.
glbrntt
added a commit
to glbrntt/swift-nio-http2
that referenced
this issue
Jul 28, 2020
Motivation: As part of apple#214 we need to make `HTTP2StreamChannel` generic over the type of message it expects to read and write. This allows us to define an `HTTP2StreamChannel` which uses `HTTP2Frame.FramePayload` as its currency type, rather than `HTTP2Frame`. Modifications: - Make `HTTP2StreamChannel` generic over `Message` which is constrained by conformance to `HTTP2FrameConvertible` and `HTTP2FramePayloadConvertible` - `HTTP2StreamChannel` now expects to read in `Message`s (as opposed to `HTTP2Frame`s) and have `Message`s written in to it. - Added typealiases for frame and payload based stream channels. - Added support for the payload based channel in `MultiplexerAbstractChannel` (although one can't be created yet). Result: We can create payload based stream channels.
glbrntt
added a commit
to glbrntt/swift-nio-http2
that referenced
this issue
Jul 28, 2020
Motivation: As part of apple#214 we need to make `HTTP2StreamChannel` generic over the type of message it expects to read and write. This allows us to define an `HTTP2StreamChannel` which uses `HTTP2Frame.FramePayload` as its currency type, rather than `HTTP2Frame`. Modifications: - Make `HTTP2StreamChannel` generic over `Message` which is constrained by conformance to `HTTP2FrameConvertible` and `HTTP2FramePayloadConvertible` - `HTTP2StreamChannel` now expects to read in `Message`s (as opposed to `HTTP2Frame`s) and have `Message`s written in to it. - Added typealiases for frame and payload based stream channels. - Added support for the payload based channel in `MultiplexerAbstractChannel` (although one can't be created yet). Result: We can create payload based stream channels.
glbrntt
pushed a commit
that referenced
this issue
Jul 28, 2020
Motivation: As part of the work in #214, HTTP2StreamID will need to be able to tolerate not knowing its stream ID. For now this is not a problem, but we should think about everywhere we use the stream ID and make sure we have a solid principle in mind. This lays the groundwork for future enhancements. Modifications: - Update HTTP2StreamChannel to hold an optional stream ID. - Update interface to HTTP2StreamMultiplexer to tolerate closing channels without stream IDs (we'll need it later). - Move the "get a new stream ID" functionality to a method on HTTP2StreamMultiplexer. - Validate we behave correctly with optional stream IDs in the channel. Result: We can tolerate stream IDs being not present.
glbrntt
added a commit
to glbrntt/swift-nio-http2
that referenced
this issue
Jul 28, 2020
Motivation: As part of apple#214 we need to make `HTTP2StreamChannel` generic over the type of message it expects to read and write. This allows us to define an `HTTP2StreamChannel` which uses `HTTP2Frame.FramePayload` as its currency type, rather than `HTTP2Frame`. Modifications: - Make `HTTP2StreamChannel` generic over `Message` which is constrained by conformance to `HTTP2FrameConvertible` and `HTTP2FramePayloadConvertible` - `HTTP2StreamChannel` now expects to read in `Message`s (as opposed to `HTTP2Frame`s) and have `Message`s written in to it. - Added typealiases for frame and payload based stream channels. - Added support for the payload based channel in `MultiplexerAbstractChannel` (although one can't be created yet). Result: We can create payload based stream channels.
Lukasa
added a commit
to Lukasa/swift-nio-http2
that referenced
this issue
Jul 28, 2020
Motivation: As part of the work in apple#214 we're going to need to update the HTTP2ToHTTP1 codecs. These need to be replaced for the new channel pipelines. The core of the logic will be identical in both cases, so let's start by factoring that logic out into some nice standalone objects that we can reuse. Modifications: - Pull out the base codecs into structures. - Rewrite the main codecs in terms of these new structures. Result: Easier extension points.
This was referenced Jul 28, 2020
glbrntt
pushed a commit
that referenced
this issue
Jul 29, 2020
Motivation: As part of the work in #214 we're going to need to update the HTTP2ToHTTP1 codecs. These need to be replaced for the new channel pipelines. The core of the logic will be identical in both cases, so let's start by factoring that logic out into some nice standalone objects that we can reuse. Modifications: - Pull out the base codecs into structures. - Rewrite the main codecs in terms of these new structures. Result: Easier extension points.
Lukasa
added a commit
that referenced
this issue
Jul 29, 2020
Motivation: As part of #214 we need to make `HTTP2StreamChannel` generic over the type of message it expects to read and write. This allows us to define an `HTTP2StreamChannel` which uses `HTTP2Frame.FramePayload` as its currency type, rather than `HTTP2Frame`. Modifications: - Make `HTTP2StreamChannel` generic over `Message` which is constrained by conformance to `HTTP2FrameConvertible` and `HTTP2FramePayloadConvertible` - `HTTP2StreamChannel` now expects to read in `Message`s (as opposed to `HTTP2Frame`s) and have `Message`s written in to it. - Added typealiases for frame and payload based stream channels. - Added support for the payload based channel in `MultiplexerAbstractChannel` (although one can't be created yet). Result: We can create payload based stream channels. Co-authored-by: Cory Benfield <[email protected]>
glbrntt
added a commit
to glbrntt/swift-nio-http2
that referenced
this issue
Jul 30, 2020
Motivation: As part of the work for apple#214 we need new codecs which transform `HTTP2Frame.FramePayload` to and from the appropriate request and response parts. Modifications: - Add `HTTP2FramePayloadToHTTP1ClientCodec` - Add `HTTP2FramePayloadToHTTP1ServerCodec` - Duplicate the HTTP2ToHTTP1CodecTests and update the relevant parts to use payloads instead of frames. - Add relevant test helpers. - Note: the HTTP2 to HTTP1 (frame based) codecs haven't been deprecated: doing so without warnings depends on apple#221. Result: - We can transform `HTTP2Frame.FramePayload` types to the relevant HTTP client and server request and response types.
glbrntt
added a commit
to glbrntt/swift-nio-http2
that referenced
this issue
Jul 30, 2020
Motivation: As part of the work for apple#214 we need new codecs which transform `HTTP2Frame.FramePayload` to and from the appropriate request and response parts. Modifications: - Add `HTTP2FramePayloadToHTTP1ClientCodec` - Add `HTTP2FramePayloadToHTTP1ServerCodec` - Duplicate the HTTP2ToHTTP1CodecTests and update the relevant parts to use payloads instead of frames. - Add relevant test helpers. - Note: the HTTP2 to HTTP1 (frame based) codecs haven't been deprecated: doing so without warnings depends on apple#221. Result: - We can transform `HTTP2Frame.FramePayload` types to the relevant HTTP client and server request and response types.
glbrntt
added a commit
to glbrntt/swift-nio-http2
that referenced
this issue
Jul 30, 2020
Motivation: As part of the work for apple#214 we need new codecs which transform `HTTP2Frame.FramePayload` to and from the appropriate request and response parts. Modifications: - Add `HTTP2FramePayloadToHTTP1ClientCodec` - Add `HTTP2FramePayloadToHTTP1ServerCodec` - Duplicate the HTTP2ToHTTP1CodecTests and update the relevant parts to use payloads instead of frames. - Add relevant test helpers. - Note: the HTTP2 to HTTP1 (frame based) codecs haven't been deprecated: doing so without warnings depends on apple#221. Result: - We can transform `HTTP2Frame.FramePayload` types to the relevant HTTP client and server request and response types.
glbrntt
added a commit
to glbrntt/swift-nio-http2
that referenced
this issue
Jul 30, 2020
Motivation: To test deprecated functionality without a bunch of warnings, tests must be marked as deprecated. This also needs to be bubbled up to anywhere calling that test, such as in the generated linux test manifests. Our script to generate the manifests currently doesn't support this and as part of apple#214 we'll need to deprecate a few things while keeping their tests. Modifications: - Pull in the latest version of the `generate_linux_tests.rb` script from SwiftNIO. - Re-run the script, since it includes some formatting changes too. Result: - When we deprecate functionality, we can deprecate the tests without the generate manifests emitting warnings.
glbrntt
added a commit
that referenced
this issue
Jul 30, 2020
Motivation: To test deprecated functionality without a bunch of warnings, tests must be marked as deprecated. This also needs to be bubbled up to anywhere calling that test, such as in the generated linux test manifests. Our script to generate the manifests currently doesn't support this and as part of #214 we'll need to deprecate a few things while keeping their tests. Modifications: - Pull in the latest version of the `generate_linux_tests.rb` script from SwiftNIO. - Re-run the script, since it includes some formatting changes too. Result: - When we deprecate functionality, we can deprecate the tests without the generate manifests emitting warnings.
glbrntt
added a commit
to glbrntt/swift-nio-http2
that referenced
this issue
Jul 31, 2020
Motivation: As part of the work for apple#214 we need new codecs which transform `HTTP2Frame.FramePayload` to and from the appropriate request and response parts. Modifications: - Add `HTTP2FramePayloadToHTTP1ClientCodec` - Add `HTTP2FramePayloadToHTTP1ServerCodec` - Duplicate the HTTP2ToHTTP1CodecTests and update the relevant parts to use payloads instead of frames. - Add relevant test helpers. - Note: the HTTP2 to HTTP1 (frame based) codecs haven't been deprecated: doing so without warnings depends on apple#221. Result: - We can transform `HTTP2Frame.FramePayload` types to the relevant HTTP client and server request and response types.
glbrntt
added a commit
that referenced
this issue
Jul 31, 2020
Motivation: As part of the work for #214 we need new codecs which transform `HTTP2Frame.FramePayload` to and from the appropriate request and response parts. Modifications: - Add `HTTP2FramePayloadToHTTP1ClientCodec` - Add `HTTP2FramePayloadToHTTP1ServerCodec` - Duplicate the HTTP2ToHTTP1CodecTests and update the relevant parts to use payloads instead of frames. - Add relevant test helpers. - Note: the HTTP2 to HTTP1 (frame based) codecs haven't been deprecated: doing so without warnings depends on #221. Result: - We can transform `HTTP2Frame.FramePayload` types to the relevant HTTP client and server request and response types.
glbrntt
added a commit
to glbrntt/swift-nio-http2
that referenced
this issue
Jul 31, 2020
Motivation: We have many test utilities for `HTTP2Frame` which predominantly make assertions about the payload. As part of the work done for apple#214 we'll need to make these assertions on the payload rather than the frame. Modifications: - Refactor `HTTP2Frame` test utilities to pull the payload assertions into separate assertions on `FramePayload` - `HTTP2Frame` assertions call through to the payload assertions Result: We can make assertions about payloads.
glbrntt
added a commit
to glbrntt/swift-nio-http2
that referenced
this issue
Jul 31, 2020
Motivation: We have many test utilities for `HTTP2Frame` which predominantly make assertions about the payload. As part of the work done for apple#214 we'll need to make these assertions on the payload rather than the frame. Modifications: - Refactor `HTTP2Frame` test utilities to pull the payload assertions into separate assertions on `FramePayload` - `HTTP2Frame` assertions call through to the payload assertions Result: We can make assertions about payloads.
Lukasa
pushed a commit
that referenced
this issue
Aug 3, 2020
Motivation: We have many test utilities for `HTTP2Frame` which predominantly make assertions about the payload. As part of the work done for #214 we'll need to make these assertions on the payload rather than the frame. Modifications: - Refactor `HTTP2Frame` test utilities to pull the payload assertions into separate assertions on `FramePayload` - `HTTP2Frame` assertions call through to the payload assertions Result: We can make assertions about payloads.
glbrntt
added a commit
to glbrntt/swift-nio-http2
that referenced
this issue
Aug 3, 2020
Motivation: As part of apple#214 we added a stream channel which deals with `HTTP2Frame.FramePayload` as well as the relevant entry points in `HTTP2StreamMultiplexer` to create these streams. We didn't add a corresponding set of tests. Modifications: - Duplicate `HTTP2StreamMultiplexerTests` - Refactor to make use of new stream creation function and new inbound stream initializer - Fix up a couple of bugs in `HTTP2StreamMultiplexer`: - activate pending streams on `channelActive` - deactivate pending streams on `channelInactive` - change stream writability for pending streams on `channelWritabilityChanged` - add implementation for `childChannelClosed(channelID:)` Result: - Tests for payload-based stream creation via `HTTP2StreamMultiplexer`
Lukasa
pushed a commit
that referenced
this issue
Aug 3, 2020
Motivation: As part of #214 we added a stream channel which deals with `HTTP2Frame.FramePayload` as well as the relevant entry points in `HTTP2StreamMultiplexer` to create these streams. We didn't add a corresponding set of tests. Modifications: - Duplicate `HTTP2StreamMultiplexerTests` - Refactor to make use of new stream creation function and new inbound stream initializer - Fix up a couple of bugs in `HTTP2StreamMultiplexer`: - activate pending streams on `channelActive` - deactivate pending streams on `channelInactive` - change stream writability for pending streams on `channelWritabilityChanged` - add implementation for `childChannelClosed(channelID:)` Result: - Tests for payload-based stream creation via `HTTP2StreamMultiplexer`
Just to understand the ETA: with #222 being merged, when could we see a new version of swift-grpc and close this issue grpc/grpc-swift#912? |
@sweetbot there are still outstanding tasks to resolve this issue before we can tag a release of SwiftNIO HTTP/2. gRPC will then need to pick up the new version and any new changes that come with it. My hope is that we can tag a gRPC release by the end of the week. |
glbrntt
added a commit
to glbrntt/swift-nio-http2
that referenced
this issue
Aug 4, 2020
Motivation: As part of apple#214 we need to introduce new pipeline helpers to configure the channel pipeline with the new handlers. This PR adds those helpers and deprecates the old paths. Modifications: - Add pipeline helpers with the new streamID-less initializer - Switch old pipeline helpers which provided codecs to use the new path and codecs - Duplicate SimpleClientServerTests and ConfiguringPipelineTests to add frame payload based counterparts - Deprecate older helpers and multiplexer init, delete tests which don't hit deprecated paths (note: these still exist in the payload based counterparts) - Update Bench1Conn10kRequests to be payload based Result: - More 'FramePayload' based Tests - Paths for creating an 'HTTP2Frame' based stream via 'HTTP2StreamMultiplexer' are now deprecated
glbrntt
added a commit
to glbrntt/swift-nio-http2
that referenced
this issue
Aug 4, 2020
Motivation: As part of apple#214 we need to introduce new pipeline helpers to configure the channel pipeline with the new handlers. This PR adds those helpers and deprecates the old paths. Modifications: - Add pipeline helpers with the new streamID-less initializer - Switch old pipeline helpers which provided codecs to use the new path and codecs - Duplicate SimpleClientServerTests and ConfiguringPipelineTests to add frame payload based counterparts - Deprecate older helpers and multiplexer init, delete tests which don't hit deprecated paths (note: these still exist in the payload based counterparts) - Update Bench1Conn10kRequests to be payload based Result: - More 'FramePayload' based Tests - Paths for creating an 'HTTP2Frame' based stream via 'HTTP2StreamMultiplexer' are now deprecated
glbrntt
added a commit
to glbrntt/swift-nio-http2
that referenced
this issue
Aug 4, 2020
Motivation: As part of apple#214 we need to introduce new pipeline helpers to configure the channel pipeline with the new handlers. This PR adds those helpers and deprecates the old paths. Modifications: - Add pipeline helpers with the new streamID-less initializer - Switch old pipeline helpers which provided codecs to use the new path and codecs - Duplicate SimpleClientServerTests and ConfiguringPipelineTests to add frame payload based counterparts - Deprecate older helpers and multiplexer init, delete tests which don't hit deprecated paths (note: these still exist in the payload based counterparts) - Update Bench1Conn10kRequests to be payload based Result: - More 'FramePayload' based Tests - Paths for creating an 'HTTP2Frame' based stream via 'HTTP2StreamMultiplexer' are now deprecated
Lukasa
pushed a commit
that referenced
this issue
Aug 4, 2020
Motivation: As part of #214 we need to introduce new pipeline helpers to configure the channel pipeline with the new handlers. This PR adds those helpers and deprecates the old paths. Modifications: - Add pipeline helpers with the new streamID-less initializer - Switch old pipeline helpers which provided codecs to use the new path and codecs - Duplicate SimpleClientServerTests and ConfiguringPipelineTests to add frame payload based counterparts - Deprecate older helpers and multiplexer init, delete tests which don't hit deprecated paths (note: these still exist in the payload based counterparts) - Update Bench1Conn10kRequests to be payload based Result: - More 'FramePayload' based Tests - Paths for creating an 'HTTP2Frame' based stream via 'HTTP2StreamMultiplexer' are now deprecated
glbrntt
added a commit
to glbrntt/swift-nio-http2
that referenced
this issue
Aug 4, 2020
Motivation: We added HTTP2FramePayload-to-HTTP1 codecs as part of apple#214 but are yet to deprecate their HTTP2-to-HTTP1 counterparts. Modifications: - Deprecate `HTTP2ToHTTP1ClientCodec` - Deprecate `HTTP2ToHTTP1ServerCodec` - Deprecate tests containing them - (We already have tests for HTTP2FramePayload-to-HTTP1 codecs) Result: HTTP2-to-HTTP1 codecs are deprecated
Lukasa
pushed a commit
that referenced
this issue
Aug 4, 2020
Motivation: We added HTTP2FramePayload-to-HTTP1 codecs as part of #214 but are yet to deprecate their HTTP2-to-HTTP1 counterparts. Modifications: - Deprecate `HTTP2ToHTTP1ClientCodec` - Deprecate `HTTP2ToHTTP1ServerCodec` - Deprecate tests containing them - (We already have tests for HTTP2FramePayload-to-HTTP1 codecs) Result: HTTP2-to-HTTP1 codecs are deprecated
glbrntt
added a commit
to glbrntt/swift-nio-http2
that referenced
this issue
Aug 4, 2020
Motivation: We made a bunch of changes to resolve apple#214. We should add a test to ensure we actually fixed the issue. Modifications: - Add a test verifying that we can write on streams in a different order to the order in which we create them - Add a corresponding test for frame-based streams verifying that the first write on each stream must match the order in which the streams were created - Remove an unnecessary `throws` in the base HTTP2 to HTTP1 server codec Result: - No functionality change, just more tests.
glbrntt
added a commit
to glbrntt/swift-nio-http2
that referenced
this issue
Aug 4, 2020
Motivation: We made a bunch of changes to resolve apple#214. We should add a test to ensure we actually fixed the issue. Modifications: - Add a test verifying that we can write on streams in a different order to the order in which we create them - Add a corresponding test for frame-based streams verifying that the first write on each stream must match the order in which the streams were created - Remove an unnecessary `throws` in the base HTTP2 to HTTP1 server codec Result: - No functionality change, just more tests.
glbrntt
added a commit
to glbrntt/swift-nio-http2
that referenced
this issue
Aug 4, 2020
Motivation: We made a bunch of changes to resolve apple#214. We should add a test to ensure we actually fixed the issue. Modifications: - Add a test verifying that we can write on streams in a different order to the order in which we create them - Add a corresponding test for frame-based streams verifying that the first write on each stream must match the order in which the streams were created - Remove an unnecessary `throws` in the base HTTP2 to HTTP1 server codec Result: - No functionality change, just more tests.
glbrntt
added a commit
to glbrntt/swift-nio-http2
that referenced
this issue
Aug 5, 2020
Motivation: We made a bunch of changes to resolve apple#214. We should add a test to ensure we actually fixed the issue. Modifications: - Add a test verifying that we can write on streams in a different order to the order in which we create them - Add a corresponding test for frame-based streams verifying that the first write on each stream must match the order in which the streams were created - Remove an unnecessary `throws` in the base HTTP2 to HTTP1 server codec Result: - No functionality change, just more tests.
Lukasa
pushed a commit
that referenced
this issue
Aug 5, 2020
Motivation: We made a bunch of changes to resolve #214. We should add a test to ensure we actually fixed the issue. Modifications: - Add a test verifying that we can write on streams in a different order to the order in which we create them - Add a corresponding test for frame-based streams verifying that the first write on each stream must match the order in which the streams were created - Remove an unnecessary `throws` in the base HTTP2 to HTTP1 server codec Result: - No functionality change, just more tests.
glbrntt
added a commit
to glbrntt/swift-nio-http2
that referenced
this issue
Sep 22, 2020
Motivation: As part of the work done for apple#214 we made `HTTP2StreamChannel` generic over the type of data it reads and writes. Unfortunately this cost us a non-trivial amount of performance. Modifications: - Remove the genericism from 'HTTP2StreamChannel' by adding a mode for the stream data type the channel is operating on (frames or frame payloads), wrapping and unwrapping the data where necessary. - Simplify the implementation of 'MultiplexerAbstractChannel' - Remove 'HTTP2FrameConvertible' since it is no longer required Result: A perf win, hopefully.
glbrntt
added a commit
to glbrntt/swift-nio-http2
that referenced
this issue
Sep 22, 2020
Motivation: As part of the work done for apple#214 we made `HTTP2StreamChannel` generic over the type of data it reads and writes. Unfortunately this cost us a non-trivial amount of performance. Modifications: - Remove the genericism from 'HTTP2StreamChannel' by adding a mode for the stream data type the channel is operating on (frames or frame payloads), wrapping and unwrapping the data where necessary. - Simplify the implementation of 'MultiplexerAbstractChannel' - Remove 'HTTP2FrameConvertible' since it is no longer required Result: A perf win, hopefully.
Lukasa
pushed a commit
that referenced
this issue
Sep 22, 2020
Motivation: As part of the work done for #214 we made `HTTP2StreamChannel` generic over the type of data it reads and writes. Unfortunately this cost us a non-trivial amount of performance. Modifications: - Remove the genericism from 'HTTP2StreamChannel' by adding a mode for the stream data type the channel is operating on (frames or frame payloads), wrapping and unwrapping the data where necessary. - Simplify the implementation of 'MultiplexerAbstractChannel' - Remove 'HTTP2FrameConvertible' since it is no longer required Result: A perf win, hopefully.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
In swift-nio-http2 today we have an unspoken hard requirement, which is that if you call
createStreamChannel
multiple times you must send the first write on each of those channels in the order you created them. This is becausecreateStreamChannel
allocates a stream ID immediately for each of those channels, but that allocation isn't meaningful until we try to write to the network. If you write out of order, the later streams will use the higher stream ID allocated to them, and will implicitly retire the lower IDs used by the earlier channels. When those earlier channels try to send, the core state machine will reject them for violating stream ID ordering.After discussing with @glbrntt we think this is made up of several changes:
HTTP2FrameConvertible
. This is a protocol that encapsulates creatingHTTP2Frame
s from a given object and vice-versa. We will have two conforming types:HTTP2Frame
(trivial) andHTTP2Frame.Payload
(straightforward, but not trivial). (Add HTTP2FrameConvertible and HTTP2FramePayloadConvertible #216)HTTP2FramePayloadConvertible
. This is the inverse of the previous protocol: creates a payload from the given object, or vice versa. (Add HTTP2FrameConvertible and HTTP2FramePayloadConvertible #216)HTTP2StreamChannel
generic over an outbound payload type conforming toHTTP2FrameConvertible
. This will become the inbound and outbound message type. This requires updates to the various inbound and outbound reading and writing functions. This should not require any interface changes on the interface between the multiplexer and the stream. The multiplexer will need to be updated to hold the new type (HTTP2StreamChannel<HTTP2Frame>
). (Make HTTP2StreamChannel generic over a message type #218)struct
that provides the interfaceHTTP2StreamMultiplexer
expects fromHTTP2StreamChannel
, that holds the channel privately. This should be backed by anenum
, which we will use later to add a new case for the new channel type. RefactorHTTP2StreamMultiplexer
to use this type. (Define abstraction for multiplexer holding channel. #215)HTTP2StreamChannel
to tolerate its streamID being optional, and nil on initialisation. This requires a new function onHTTP2StreamMultiplexer
to get the next stream ID, which is currently done increateStreamChannel
. (Update HTTP2StreamChannel to tolerate nil stream ID. #217)createStreamChannel
function with a new initializer that no longer gets a stream ID. Plumb through this initializer to create new stream channels that useHTTP2Frame.Payload
instead ofHTTP2Frame
. Update theenum
created above for the new case. (Allow payload-based stream channels to be created. #221)HTTP2FramePayloadConvertible
, and rename them. Create typealiases for both the old version (keeping its old name) and the new one that usesHTTP2FramePayload
. Deprecate the old typealias. (Add HTTP2Frame.FramePayload codecs #222)HTTP2PipelineHelpers
function to use the new initializer. (Add new pipeline helpers and deprecate old paths #226, Deprecate the HTTP2 to HTTP1 codecs #227)The text was updated successfully, but these errors were encountered: