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

5.6: Fix SR-15785 by adding a cached reference to the last linked list node, avoiding n^2 append scaling #41192

Merged
merged 1 commit into from
Mar 23, 2022

Conversation

Catfish-Man
Copy link
Contributor

@Catfish-Man Catfish-Man commented Feb 3, 2022

Explanation: Adding children to a TaskGroup is currently n^2, which is causing severe performance issues for users of Swift Concurrency.

Scope: This change improves the performance of adding 50,000 children to a group from 10-15 seconds to less than 0.2 seconds. It costs one word of memory per child.

Radar/SR Issue: SR-15785

Risk: If I got the change wrong we could potentially see crashes or other misbehavior when using TaskGroups

Testing: Test suite passes, community provided benchmark shows expected improvement

Reviewer: @DougGregor (I think you're the Concurrency code owner? CODE_OWNERS doesn't list it)

Original PR: #41165

@Catfish-Man Catfish-Man requested a review from ktoso February 3, 2022 22:01
@Catfish-Man Catfish-Man self-assigned this Feb 3, 2022
@Catfish-Man Catfish-Man changed the base branch from main to release/5.6 February 3, 2022 22:02
@Catfish-Man Catfish-Man requested a review from a team as a code owner February 3, 2022 22:02
@Catfish-Man
Copy link
Contributor Author

@swift-ci please nominate

@Catfish-Man
Copy link
Contributor Author

@swift-ci please test

@Catfish-Man Catfish-Man closed this Mar 2, 2022
@Catfish-Man Catfish-Man reopened this Mar 22, 2022
@Catfish-Man
Copy link
Contributor Author

@swift-ci please test windows platform

@Catfish-Man
Copy link
Contributor Author

@swift-ci please test macOS platform

@DougGregor DougGregor merged commit 5420863 into swiftlang:release/5.6 Mar 23, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants