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

Use proper String unsafeUninitializedCapacity initializer. #263

Merged
merged 1 commit into from
Nov 28, 2020

Conversation

Lukasa
Copy link
Contributor

@Lukasa Lukasa commented Nov 27, 2020

Motivation:

A while ago we shimmed out the String unsafeUninitializedCapacity
initializer because while it was on the road to being implemented, it
hadn't actually been implemented. Now it has!

On Apple platforms, the initializer is guarded behind an availability
guard. We need to add that guard here. This means we need to keep hold
of the backport for a good long time, until we raise our minimum Apple
platform support to a new enough version. Happily, Linux on 5.3 should
just see a net win here.

Modifications:

  • Updated to enable us to use the proper initializer.

Result:

Fewer allocations when screwing around with HPACK. Fractionally faster performance in general (~0.2%).

@Lukasa Lukasa added the 🔨 semver/patch No public API change. label Nov 27, 2020
Copy link
Contributor

@glbrntt glbrntt left a comment

Choose a reason for hiding this comment

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

LGTM

Motivation:

A while ago we shimmed out the String unsafeUninitializedCapacity
initializer because while it was on the road to being implemented, it
hadn't actually been implemented. Now it has!

On Apple platforms, the initializer is guarded behind an availability
guard. We need to add that guard here. This means we need to keep hold
of the backport for a good long time, until we raise our minimum Apple
platform support to a new enough version. Happily, Linux on 5.3 should
just see a net win here.

Modifications:

- Updated to enable us to use the proper initializer.

Result:

Fewer allocations when screwing around with HPACK.
@Lukasa Lukasa force-pushed the cb-remove-string-override branch from 5aaf12f to 9f4bb29 Compare November 27, 2020 18:04
@Lukasa Lukasa merged commit c61955a into apple:main Nov 28, 2020
@Lukasa Lukasa deleted the cb-remove-string-override branch November 28, 2020 15:56
Lukasa added a commit that referenced this pull request Dec 3, 2020
Motivation:

In #263 we took advantage of the addition of the
unsafeUninitializedCapacity:initializingWith constructor on String,
added in the latest Swift SDKs and Swift versions.

Unfortunately, there were a few Xcode releases that shipped with Swift
5.3 but _without_ the macOS 11 SDK. These releases therefore had a 5.3
compiler, but no access to the macOS 11 SDK where the constructor is
defined.

This means this change regressed builds on those platforms.

Modifications:

Removed access to the fast-path initializer on Apple platforms in 5.3.
The initializer is present and will continue to work on non-Apple
platforms. Apple platforms will get access to that initializer from 5.4
onwards.

Result:

Fewer build issues for users using Xcode 12.0 and 12.1.
@Lukasa Lukasa mentioned this pull request Dec 3, 2020
Davidde94 pushed a commit that referenced this pull request Dec 3, 2020
Motivation:

In #263 we took advantage of the addition of the
unsafeUninitializedCapacity:initializingWith constructor on String,
added in the latest Swift SDKs and Swift versions.

Unfortunately, there were a few Xcode releases that shipped with Swift
5.3 but _without_ the macOS 11 SDK. These releases therefore had a 5.3
compiler, but no access to the macOS 11 SDK where the constructor is
defined.

This means this change regressed builds on those platforms.

Modifications:

Removed access to the fast-path initializer on Apple platforms in 5.3.
The initializer is present and will continue to work on non-Apple
platforms. Apple platforms will get access to that initializer from 5.4
onwards.

Result:

Fewer build issues for users using Xcode 12.0 and 12.1.
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.

2 participants