Skip to content
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

Adopt NIOAsyncSequenceProducer in grpc-swift #1477

Merged
merged 2 commits into from
Sep 27, 2022
Merged

Conversation

gjcairo
Copy link
Collaborator

@gjcairo gjcairo commented Sep 5, 2022

Motivation

We want to make use of NIOAsyncSequenceProducer (or its throwing counterpart) instead of using PassthroughMessageSequence and PassthroughMessageSource.

Modifications

  • Replaced usages of PassthroughMessageSequence and PassthroughMessageSource with NIOThrowingAsyncSequenceProducer

Result

grpc-swift now uses NIOAsyncSequenceProducer

@gjcairo gjcairo force-pushed the async-seq branch 2 times, most recently from f73b406 to 22c2078 Compare September 6, 2022 15:29
@gjcairo
Copy link
Collaborator Author

gjcairo commented Sep 6, 2022

Performance tests on 5.6 and 5.7 are failing but they're related to the change of swift-nio to main, and not related to other changes made as part of this PR. I'll spend some time looking into what could be the cause, and will either fix it in a follow-up PR or create a Radar to track for later.

@gjcairo gjcairo marked this pull request as ready for review September 6, 2022 15:36
@gjcairo
Copy link
Collaborator Author

gjcairo commented Sep 8, 2022

Will send follow-up CR to fix the allocation test failures - turns out we've willingly increased the number of allocations in swift-nio, so the limits in the failing tests have to be changed.

@gjcairo gjcairo added the 🆕 semver/minor Adds new public API. label Sep 8, 2022
Copy link
Collaborator

@FranzBusch FranzBusch left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! Nice work! We just have to wait until a new release of NIO is cut to merge this.

@gjcairo gjcairo force-pushed the async-seq branch 2 times, most recently from 41047d5 to 534f80d Compare September 27, 2022 13:57
@gjcairo gjcairo marked this pull request as draft September 27, 2022 14:17
We want to make use of `NIOAsyncSequenceProducer` (or its throwing counterpart) instead of using `PassthroughMessageSequence` and `PassthroughMessageSource`.

* Replaced usages of `PassthroughMessageSequence` and `PassthroughMessageSource` with `NIOThrowingAsyncSequenceProducer`

grpc-swift now uses `NIOAsyncSequenceProducer`
@gjcairo gjcairo marked this pull request as ready for review September 27, 2022 14:51
@glbrntt glbrntt merged commit 07233c5 into grpc:main Sep 27, 2022
@glbrntt glbrntt added 🔨 semver/patch No public API change. and removed 🆕 semver/minor Adds new public API. labels Sep 29, 2022
WendellXY pushed a commit to sundayfun/grpc-swift that referenced this pull request Aug 24, 2023
We want to make use of `NIOAsyncSequenceProducer` (or its throwing counterpart) instead of using `PassthroughMessageSequence` and `PassthroughMessageSource`.

* Replaced usages of `PassthroughMessageSequence` and `PassthroughMessageSource` with `NIOThrowingAsyncSequenceProducer`

grpc-swift now uses `NIOAsyncSequenceProducer`
pinlin168 pushed a commit to sundayfun/grpc-swift that referenced this pull request Aug 24, 2023
We want to make use of `NIOAsyncSequenceProducer` (or its throwing counterpart) instead of using `PassthroughMessageSequence` and `PassthroughMessageSource`.

* Replaced usages of `PassthroughMessageSequence` and `PassthroughMessageSource` with `NIOThrowingAsyncSequenceProducer`

grpc-swift now uses `NIOAsyncSequenceProducer`
@gjcairo gjcairo deleted the async-seq branch March 15, 2024 16:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🔨 semver/patch No public API change.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants