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

Avoid unnecessary allocation by way of tuple conversion. #994

Merged
merged 1 commit into from May 3, 2019

Conversation

Projects
None yet
2 participants
@Lukasa
Copy link
Contributor

commented May 3, 2019

Motivation:

Due to https://bugs.swift.org/browse/SR-10614, assigning an array containing a tuple type
to a variable that expects an array containing a different-but-compatible tuple type will cause
an allocation and copy of that array storage.

In some cases this is necessary, but we were doing it in the HTTPHeaders constructors, which meant
that the swift-nio-http2 code was hitting a hilariously over the top number of allocations.

Modifications:

  • Change the internal storage type of HTTPHeaders to match the public constructor.

Result:

Fewer allocations when constructing HTTPHeaders

@Lukasa Lukasa added this to the 2.1.0 milestone May 3, 2019

@Lukasa Lukasa requested a review from weissi May 3, 2019

@Lukasa Lukasa force-pushed the Lukasa:cb-change-httpheaders-internal-type branch from db7aa1e to 59a6b38 May 3, 2019

@weissi

weissi approved these changes May 3, 2019

Copy link
Member

left a comment

yay, awesome

@Lukasa

This comment has been minimized.

Copy link
Contributor Author

commented May 3, 2019

Shoot, forgot to deal with the changed repr. Will follow up.

@Lukasa Lukasa force-pushed the Lukasa:cb-change-httpheaders-internal-type branch from 59a6b38 to cc68bbb May 3, 2019

Avoid unnecessary allocation by way of tuple conversion.
Motivation:

Due to https://bugs.swift.org/browse/SR-10614, assigning an array containing a tuple type
to a variable that expects an array containing a different-but-compatible tuple type will cause
an allocation and copy of that array storage.

In some cases this is necessary, but we were doing it in the HTTPHeaders constructors, which meant
that the swift-nio-http2 code was hitting a hilariously over the top number of allocations.

Modifications:

- Change the internal storage type of HTTPHeaders to match the public constructor.

Result:

Fewer allocations when constructing HTTPHeaders

@Lukasa Lukasa force-pushed the Lukasa:cb-change-httpheaders-internal-type branch from cc68bbb to f9ca8ef May 3, 2019

@Lukasa Lukasa merged commit 768c3ab into apple:master May 3, 2019

2 checks passed

pull request validation (5.0) Build finished.
Details
pull request validation (5.1) Build finished.
Details

@Lukasa Lukasa deleted the Lukasa:cb-change-httpheaders-internal-type branch May 3, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.