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

Deprecate NIOAtomics in favor of Atomics #2204

Merged
merged 12 commits into from
Jul 1, 2022
Merged

Conversation

stevapple
Copy link
Contributor

Deprecate NIOAtomics in favor of Atomics, and replace all usages of NIOAtomic.

Motivation:

Since we have an official Atomics library, we can move from our own Atomics implementation to it.
Resolves #1948.

Modifications:

  • NIOAtomic and UnsafeEmbeddedAtomic wrapper types are now deprecated;
  • Swift NIO implementations and tests are moving from NIOAtomic to ManagedAtomic.

Result:

We no longer need to maintain our own atomic types.
NIO will gain a dependency on swift-atomics package.

@swift-server-bot
Copy link

Can one of the admins verify this patch?

11 similar comments
@swift-server-bot
Copy link

Can one of the admins verify this patch?

@swift-server-bot
Copy link

Can one of the admins verify this patch?

@swift-server-bot
Copy link

Can one of the admins verify this patch?

@swift-server-bot
Copy link

Can one of the admins verify this patch?

@swift-server-bot
Copy link

Can one of the admins verify this patch?

@swift-server-bot
Copy link

Can one of the admins verify this patch?

@swift-server-bot
Copy link

Can one of the admins verify this patch?

@swift-server-bot
Copy link

Can one of the admins verify this patch?

@swift-server-bot
Copy link

Can one of the admins verify this patch?

@swift-server-bot
Copy link

Can one of the admins verify this patch?

@swift-server-bot
Copy link

Can one of the admins verify this patch?

@stevapple
Copy link
Contributor Author

Note that ordering is set to .relaxed everywhere to preserve NIOAtomic's behavior. We can fine-tune the behavior later.

@Lukasa Lukasa added the 🔼 needs-minor-version-bump For PRs that when merged cause a bump of the minor version, ie. 1.x.0 -> 1.(x+1).0 label Jun 25, 2022
Copy link
Contributor

@Lukasa Lukasa left a comment

Choose a reason for hiding this comment

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

Fantastic, I'm really excited to see this come along. I have a few exceedingly tiny nits but otherwise I think this is a perfect patch and ready to go in. cc @weissi in case he's curious, and @lorentey because I know this is a thing he's wanted for a long time!

Package.swift Outdated Show resolved Hide resolved
Sources/NIOPosix/PendingDatagramWritesManager.swift Outdated Show resolved Hide resolved
Sources/NIOPosix/PendingWritesManager.swift Outdated Show resolved Hide resolved
@Lukasa
Copy link
Contributor

Lukasa commented Jun 25, 2022

@swift-nio-bot add to allowlist

@stevapple
Copy link
Contributor Author

Sendable appears to be a problem for Swift 5.5, where @preconcurrency is not available.

I’ll ping on apple/swift-atomics#45 for this.

@dnadoba
Copy link
Member

dnadoba commented Jun 25, 2022

EventCounterHandler has adopted Sendable in #2199 which is not yet part of any release. We could therefore just remove Sendable in Swift <= 5.5 as an alternative without actually breaking API.

@stevapple
Copy link
Contributor Author

cc @Lukasa @dnadoba

Copy link
Member

@dnadoba dnadoba left a comment

Choose a reason for hiding this comment

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

Thanks a lot @stevapple! Looks good from my side but we definitely need to wait for Cory's review.

Sources/NIOConcurrencyHelpers/NIOAtomic.swift Outdated Show resolved Hide resolved
Sources/NIOConcurrencyHelpers/atomics.swift Outdated Show resolved Hide resolved
Sources/NIOConcurrencyHelpers/atomics.swift Outdated Show resolved Hide resolved
@Lukasa
Copy link
Contributor

Lukasa commented Jun 28, 2022

@swift-nio-bot test this please

@Lukasa Lukasa enabled auto-merge (squash) July 1, 2022 09:16
@Lukasa Lukasa merged commit a501353 into apple:main Jul 1, 2022
@stevapple stevapple deleted the swift-atomics branch July 3, 2022 11:17
dnadoba added a commit to dnadoba/swift-nio-ssh that referenced this pull request Jul 8, 2022
`NIOAtomics` was deprecated in apple/swift-nio#2204 in favor of `swift-atomics` https://github.com/apple/swift-atomics
In addition, the `@preconcrrency` imports of `NIOCore` are not required and do not produce warnings/errors even with the currently latest released version of swift-nio (2.40.0), changes from `main` are not required.
dnadoba added a commit to dnadoba/swift-nio-ssh that referenced this pull request Jul 8, 2022
`NIOAtomics` was deprecated in apple/swift-nio#2204 in favor of `swift-atomics` https://github.com/apple/swift-atomics
In addition, the `@preconcrrency` imports of `NIOCore` are not required and do not produce warnings/errors even with the currently latest released version of swift-nio (2.40.0), changes from `main` are not required.
dnadoba added a commit to dnadoba/swift-nio-http2 that referenced this pull request Jul 8, 2022
glbrntt pushed a commit to apple/swift-nio-ssh that referenced this pull request Jul 8, 2022
`NIOAtomics` was deprecated in apple/swift-nio#2204 in favor of `swift-atomics` https://github.com/apple/swift-atomics
In addition, the `@preconcrrency` imports of `NIOCore` are not required and do not produce warnings/errors even with the currently latest released version of swift-nio (2.40.0), changes from `main` are not required.
dnadoba added a commit to dnadoba/async-http-client that referenced this pull request Jul 11, 2022
glbrntt pushed a commit to apple/swift-nio-http2 that referenced this pull request Jul 11, 2022
glbrntt added a commit to glbrntt/grpc-swift that referenced this pull request Jul 11, 2022
Motivation:

NIO deprecated its atomics in apple/swift-nio#2204.

Modifications:

- Use swift-atomics in the `QPSBenchmark` sub-package
- Use a lock in the one test which used `NIOAtomic`

Result:

Not using deprecated code.
glbrntt added a commit to glbrntt/grpc-swift that referenced this pull request Jul 11, 2022
Motivation:

NIO deprecated its atomics in apple/swift-nio#2204.

Modifications:

- Use swift-atomics in the `QPSBenchmark` sub-package
- Use a lock in the one test which used `NIOAtomic`

Result:

Not using deprecated code.
glbrntt added a commit to grpc/grpc-swift that referenced this pull request Jul 12, 2022
Motivation:

NIO deprecated its atomics in apple/swift-nio#2204.

Modifications:

- Use swift-atomics in the `QPSBenchmark` sub-package
- Use a lock in the one test which used `NIOAtomic`

Result:

Not using deprecated code.
dnadoba added a commit to swift-server/async-http-client that referenced this pull request Jul 13, 2022
Joannis pushed a commit to Joannis/swift-nio-ssh that referenced this pull request Nov 9, 2022
`NIOAtomics` was deprecated in apple/swift-nio#2204 in favor of `swift-atomics` https://github.com/apple/swift-atomics
In addition, the `@preconcrrency` imports of `NIOCore` are not required and do not produce warnings/errors even with the currently latest released version of swift-nio (2.40.0), changes from `main` are not required.
IceRocky added a commit to IceRocky/grpc-swift that referenced this pull request May 28, 2024
Motivation:

NIO deprecated its atomics in apple/swift-nio#2204.

Modifications:

- Use swift-atomics in the `QPSBenchmark` sub-package
- Use a lock in the one test which used `NIOAtomic`

Result:

Not using deprecated code.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🔼 needs-minor-version-bump For PRs that when merged cause a bump of the minor version, ie. 1.x.0 -> 1.(x+1).0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Replace NIOAtomics with swift-atomics
4 participants