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

Pre-box some errors to reduce allocations #2765

Merged
merged 5 commits into from
Jul 5, 2024

Conversation

glbrntt
Copy link
Contributor

@glbrntt glbrntt commented Jul 4, 2024

Motivation:

Existential errors (any Error) are unconditionally boxed. In NIO we often create errors to fail promises. One place we do this is in the head channel handler when the pipeline is closed. This error is created regardless of whether there are actually any promises to fail. The result is an unnecessary allocation when every channel is closed. This is particuarly egregious when multiplexed protocols such as HTTP/2 are used.

As the majority of errors in NIO carry no information beyond their type/name we can declare these statically and pay the cost of each error just once rather than once per use.

Modifications:

  • Add internal static constants for ChannelError and EventLoopError and use them throughout NIOCore and NIOPosix where they would otherwise be boxed.

Result:

Fewer allocations

Motivation:

Existential errors (`any Error`) are unconditionally boxed. In NIO we
often create errors to fail promises. One place we do this is in the
head channel handler when the pipeline is closed. This error is created
regardless of whether there are actually any promises to fail. The
result is an unnecessary allocation when every channel is closed. This
is particuarly egregious when multiplexed protocols such as HTTP/2 are
used.

As the majority of errors in NIO carry no information beyond their
type/name we can declare these statically and pay the cost of each error
just once rather than once per use.

Modifications:

- Add internal static constants for `ChannelError` and `EventLoopError`
  and use them throughout NIOCore and NIOPosix where they would
  otherwise be boxed.

Result:

Fewer allocations
@@ -373,6 +373,16 @@ public enum ChannelError: Error {
case unremovableHandler
}

extension ChannelError {
// 'any Error' is unconditionally boxed, avoid allocating per use by statically boxing them.
static let _alreadyClosed: any Error = ChannelError.alreadyClosed
Copy link
Member

Choose a reason for hiding this comment

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

I am wondering if should file a compiler bug here and this is something that the compiler ought to be able to optimise. The error enum is in the same module and it has zero associated values. So it might as well store the instance in a global memory region

Copy link
Contributor Author

@glbrntt glbrntt Jul 4, 2024

Choose a reason for hiding this comment

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

I don't think it's about the underlying error type; holding a ChannelError doesn't allocate. It's holding an any Error which always allocates. IIUC any Error has always been effectively just a pointer.

@glbrntt glbrntt force-pushed the avoid-error-allocs branch from 7bd094c to 2c76206 Compare July 5, 2024 08:38
@glbrntt glbrntt marked this pull request as ready for review July 5, 2024 09:15
@glbrntt glbrntt requested a review from FranzBusch July 5, 2024 09:15
@Lukasa Lukasa added the 🔨 semver/patch No public API change. label Jul 5, 2024
@glbrntt glbrntt enabled auto-merge (squash) July 5, 2024 14:26
@glbrntt glbrntt merged commit 9b86559 into apple:main Jul 5, 2024
15 of 16 checks passed
@glbrntt glbrntt deleted the avoid-error-allocs branch July 8, 2024 14:25
cgrindel-self-hosted-renovate bot referenced this pull request in cgrindel/rules_swift_package_manager Aug 19, 2024
This PR contains the following updates:

| Package | Update | Change |
|---|---|---|
| [apple/swift-nio](https://togithub.com/apple/swift-nio) | minor |
`2.68.0` -> `2.70.0` |

---

### Release Notes

<details>
<summary>apple/swift-nio (apple/swift-nio)</summary>

###
[`v2.70.0`](https://togithub.com/apple/swift-nio/releases/tag/2.70.0):
SwiftNIO 2.70.0

[Compare
Source](https://togithub.com/apple/swift-nio/compare/2.69.0...2.70.0)

<!-- Release notes generated using configuration in .github/release.yml
at main -->

#### What's Changed

##### SemVer Minor

- `FileSystem.copyItem` can parallelise directory copy by
[@&#8203;UncleMattHope](https://togithub.com/UncleMattHope) in
[https://github.com/apple/swift-nio/pull/2806](https://togithub.com/apple/swift-nio/pull/2806)
- `ChannelOption`: Allow types to be accessed with leading dot syntax by
[@&#8203;ayush1794](https://togithub.com/ayush1794) in
[https://github.com/apple/swift-nio/pull/2816](https://togithub.com/apple/swift-nio/pull/2816)
- Make `EventLoopPromise` conform to Equatable by
[@&#8203;gjcairo](https://togithub.com/gjcairo) in
[https://github.com/apple/swift-nio/pull/2714](https://togithub.com/apple/swift-nio/pull/2714)
- Provide a default `CopyStrategy` overload for copyItem. by
[@&#8203;UncleMattHope](https://togithub.com/UncleMattHope) in
[https://github.com/apple/swift-nio/pull/2818](https://togithub.com/apple/swift-nio/pull/2818)

##### SemVer Patch

- Better align shutdown semantics of testing event loops by
[@&#8203;simonjbeaumont](https://togithub.com/simonjbeaumont) in
[https://github.com/apple/swift-nio/pull/2800](https://togithub.com/apple/swift-nio/pull/2800)
- Clone files on Darwin rather than copying them by
[@&#8203;rnro](https://togithub.com/rnro) in
[https://github.com/apple/swift-nio/pull/2823](https://togithub.com/apple/swift-nio/pull/2823)

##### Other Changes

- Fix compose file used in update-benchmark-thresholds script by
[@&#8203;simonjbeaumont](https://togithub.com/simonjbeaumont) in
[https://github.com/apple/swift-nio/pull/2808](https://togithub.com/apple/swift-nio/pull/2808)
- Remove advice to generate linux tests. by
[@&#8203;PeterAdams-A](https://togithub.com/PeterAdams-A) in
[https://github.com/apple/swift-nio/pull/2807](https://togithub.com/apple/swift-nio/pull/2807)
- Make `testInstantTCPConnectionResetThrowsError` more reliable by
[@&#8203;hamzahrmalik](https://togithub.com/hamzahrmalik) in
[https://github.com/apple/swift-nio/pull/2810](https://togithub.com/apple/swift-nio/pull/2810)
- \[CI] Add `shellcheck` and fix up warnings by
[@&#8203;FranzBusch](https://togithub.com/FranzBusch) in
[https://github.com/apple/swift-nio/pull/2809](https://togithub.com/apple/swift-nio/pull/2809)
- \[CI] Fix docs check by
[@&#8203;FranzBusch](https://togithub.com/FranzBusch) in
[https://github.com/apple/swift-nio/pull/2811](https://togithub.com/apple/swift-nio/pull/2811)
- \[CI] Add Swift 6 language mode workflow by
[@&#8203;FranzBusch](https://togithub.com/FranzBusch) in
[https://github.com/apple/swift-nio/pull/2812](https://togithub.com/apple/swift-nio/pull/2812)
- Fix test compilation on non-macOS Darwin platforms by
[@&#8203;simonjbeaumont](https://togithub.com/simonjbeaumont) in
[https://github.com/apple/swift-nio/pull/2817](https://togithub.com/apple/swift-nio/pull/2817)
- Add `.index-build` to `.gitignore` by
[@&#8203;MaxDesiatov](https://togithub.com/MaxDesiatov) in
[https://github.com/apple/swift-nio/pull/2819](https://togithub.com/apple/swift-nio/pull/2819)
- \[CI] Add action and workflow to check for semver label by
[@&#8203;FranzBusch](https://togithub.com/FranzBusch) in
[https://github.com/apple/swift-nio/pull/2814](https://togithub.com/apple/swift-nio/pull/2814)
- Update repository docs for swift-version support and recent CI check
changes by [@&#8203;UncleMattHope](https://togithub.com/UncleMattHope)
in
[https://github.com/apple/swift-nio/pull/2815](https://togithub.com/apple/swift-nio/pull/2815)
- Fix failing build for test by
[@&#8203;ayush1794](https://togithub.com/ayush1794) in
[https://github.com/apple/swift-nio/pull/2824](https://togithub.com/apple/swift-nio/pull/2824)
- Fix typo in comment in `WebSocketErrorCodes.swift` by
[@&#8203;valeriyvan](https://togithub.com/valeriyvan) in
[https://github.com/apple/swift-nio/pull/2604](https://togithub.com/apple/swift-nio/pull/2604)
- \[CI] Add a scheduled workflow for tests and benchmarks by
[@&#8203;FranzBusch](https://togithub.com/FranzBusch) in
[https://github.com/apple/swift-nio/pull/2822](https://togithub.com/apple/swift-nio/pull/2822)
- \[CI] Fix label check by
[@&#8203;FranzBusch](https://togithub.com/FranzBusch) in
[https://github.com/apple/swift-nio/pull/2827](https://togithub.com/apple/swift-nio/pull/2827)

#### New Contributors

- [@&#8203;UncleMattHope](https://togithub.com/UncleMattHope) made their
first contribution in
[https://github.com/apple/swift-nio/pull/2806](https://togithub.com/apple/swift-nio/pull/2806)
- [@&#8203;ayush1794](https://togithub.com/ayush1794) made their first
contribution in
[https://github.com/apple/swift-nio/pull/2816](https://togithub.com/apple/swift-nio/pull/2816)
- [@&#8203;valeriyvan](https://togithub.com/valeriyvan) made their first
contribution in
[https://github.com/apple/swift-nio/pull/2604](https://togithub.com/apple/swift-nio/pull/2604)

**Full Changelog**:
apple/swift-nio@2.69.0...2.70.0

###
[`v2.69.0`](https://togithub.com/apple/swift-nio/releases/tag/2.69.0):
SwiftNIO 2.69.0

[Compare
Source](https://togithub.com/apple/swift-nio/compare/2.68.0...2.69.0)

<!-- Release notes generated using configuration in .github/release.yml
at main -->

#### What's Changed

##### SemVer Minor

- Add manual control to `NIOLockedValueBox` by
[@&#8203;glbrntt](https://togithub.com/glbrntt) in
[https://github.com/apple/swift-nio/pull/2786](https://togithub.com/apple/swift-nio/pull/2786)
- ChannelHandler: provide static `(un)wrap(In|Out)bound(In|Out)` by
[@&#8203;weissi](https://togithub.com/weissi) in
[https://github.com/apple/swift-nio/pull/2791](https://togithub.com/apple/swift-nio/pull/2791)

##### SemVer Patch

- Pre-box some errors to reduce allocations by
[@&#8203;glbrntt](https://togithub.com/glbrntt) in
[https://github.com/apple/swift-nio/pull/2765](https://togithub.com/apple/swift-nio/pull/2765)
- Allow in-place mutation of `NIOLoopBoundBox.value` by
[@&#8203;dnadoba](https://togithub.com/dnadoba) in
[https://github.com/apple/swift-nio/pull/2771](https://togithub.com/apple/swift-nio/pull/2771)
- Avoid creating a yield ID counter per async writer by
[@&#8203;glbrntt](https://togithub.com/glbrntt) in
[https://github.com/apple/swift-nio/pull/2768](https://togithub.com/apple/swift-nio/pull/2768)
- Combine the two `NIOAsyncChannel` channel handlers by
[@&#8203;glbrntt](https://togithub.com/glbrntt) in
[https://github.com/apple/swift-nio/pull/2779](https://togithub.com/apple/swift-nio/pull/2779)
- Use the new Android overlay and Bionic module from Swift 6 by
[@&#8203;finagolfin](https://togithub.com/finagolfin) in
[https://github.com/apple/swift-nio/pull/2784](https://togithub.com/apple/swift-nio/pull/2784)
- Change `unsafeDownCast` to `as!` by
[@&#8203;FranzBusch](https://togithub.com/FranzBusch) in
[https://github.com/apple/swift-nio/pull/2802](https://togithub.com/apple/swift-nio/pull/2802)

##### Other Changes

- CI migration to GitHub Action by
[@&#8203;FranzBusch](https://togithub.com/FranzBusch) in
([https://github.com/apple/swift-nio/pull/2760](https://togithub.com/apple/swift-nio/pull/2760)
[https://github.com/apple/swift-nio/pull/2762](https://togithub.com/apple/swift-nio/pull/2762)
[https://github.com/apple/swift-nio/pull/2763](https://togithub.com/apple/swift-nio/pull/2763)
[https://github.com/apple/swift-nio/pull/2764](https://togithub.com/apple/swift-nio/pull/2764)
[https://github.com/apple/swift-nio/pull/2767](https://togithub.com/apple/swift-nio/pull/2767)
[https://github.com/apple/swift-nio/pull/2766](https://togithub.com/apple/swift-nio/pull/2766)
[https://github.com/apple/swift-nio/pull/2776](https://togithub.com/apple/swift-nio/pull/2776)
[https://github.com/apple/swift-nio/pull/2780](https://togithub.com/apple/swift-nio/pull/2780)
[https://github.com/apple/swift-nio/pull/2785](https://togithub.com/apple/swift-nio/pull/2785)
[https://github.com/apple/swift-nio/pull/2781](https://togithub.com/apple/swift-nio/pull/2781)
[https://github.com/apple/swift-nio/pull/2787](https://togithub.com/apple/swift-nio/pull/2787)
[https://github.com/apple/swift-nio/pull/2783](https://togithub.com/apple/swift-nio/pull/2783)
[https://github.com/apple/swift-nio/pull/2789](https://togithub.com/apple/swift-nio/pull/2789)
[https://github.com/apple/swift-nio/pull/2790](https://togithub.com/apple/swift-nio/pull/2790))
- Ignore format commit from git blame by
[@&#8203;FranzBusch](https://togithub.com/FranzBusch) in
[https://github.com/apple/swift-nio/pull/2796](https://togithub.com/apple/swift-nio/pull/2796)
[https://github.com/apple/swift-nio/pull/2797](https://togithub.com/apple/swift-nio/pull/2797)
[https://github.com/apple/swift-nio/pull/2801](https://togithub.com/apple/swift-nio/pull/2801)
[https://github.com/apple/swift-nio/pull/2803](https://togithub.com/apple/swift-nio/pull/2803)
- Adopt swift-format by
[@&#8203;FranzBusch](https://togithub.com/FranzBusch) in
[https://github.com/apple/swift-nio/pull/2794](https://togithub.com/apple/swift-nio/pull/2794)
- `HTTPPart` Documentation Clarification by
[@&#8203;dimitribouniol](https://togithub.com/dimitribouniol) in
[https://github.com/apple/swift-nio/pull/2775](https://togithub.com/apple/swift-nio/pull/2775)
- Add benchmark for creating `NIOAsyncChannel` by
[@&#8203;glbrntt](https://togithub.com/glbrntt) in
[https://github.com/apple/swift-nio/pull/2774](https://togithub.com/apple/swift-nio/pull/2774)
- Disable warnings as errors on Swift 6 and main by
[@&#8203;glbrntt](https://togithub.com/glbrntt) in
[https://github.com/apple/swift-nio/pull/2793](https://togithub.com/apple/swift-nio/pull/2793)

**Full Changelog**:
apple/swift-nio@2.68.0...2.69.0

</details>

---

### Configuration

📅 **Schedule**: Branch creation - At any time (no schedule defined),
Automerge - At any time (no schedule defined).

🚦 **Automerge**: Enabled.

♻ **Rebasing**: Whenever PR is behind base branch, or you tick the
rebase/retry checkbox.

👻 **Immortal**: This PR will be recreated if closed unmerged. Get
[config help](https://togithub.com/renovatebot/renovate/discussions) if
that's undesired.

---

- [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check
this box

---

This PR has been generated by [Renovate
Bot](https://togithub.com/renovatebot/renovate).

<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzOC4xOC4xIiwidXBkYXRlZEluVmVyIjoiMzguMzkuMCIsInRhcmdldEJyYW5jaCI6Im1haW4iLCJsYWJlbHMiOltdfQ==-->

Co-authored-by: cgrindel-self-hosted-renovate[bot] <139595543+cgrindel-self-hosted-renovate[bot]@users.noreply.github.com>
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