Skip to content

[5.7] Explicitly copy defsIDependUpon into an appropriately sized array when initializing SourceFileDependencyGraph.Node #1124

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

Merged
merged 1 commit into from
Jul 18, 2022

Conversation

owenv
Copy link
Contributor

@owenv owenv commented Jun 28, 2022

5.7 cherry pick of #1119

@owenv owenv requested review from nkcsgexi and artemcm June 28, 2022 18:39
@owenv
Copy link
Contributor Author

owenv commented Jun 28, 2022

@swift-ci please test

@owenv owenv marked this pull request as draft June 28, 2022 19:03
…n initializing SourceFileDependencyGraph.Node

Previously, the removeAll call below the initializer would trigger CoW. If the scratch buffer was larger than the count, this would result in an unnecesarily large allocation. In a very large file, the source-file-provides-implementation node may have multiple orders of magnitude more edges compared to any other node in the graph, which could lead to excessive memory usage and a large slowdown deallocating the array storage when tearing down the graph.

rdar://95648282
@owenv owenv force-pushed the 5_7-swiftdeps-perf branch from 82ef7b2 to ec8128d Compare June 28, 2022 23:10
@owenv
Copy link
Contributor Author

owenv commented Jun 28, 2022

This is now updated with the additional type annotation needed to compile in regular swift CI

@owenv owenv marked this pull request as ready for review June 28, 2022 23:11
@owenv
Copy link
Contributor Author

owenv commented Jun 28, 2022

@swift-ci please test

@artemcm
Copy link
Contributor

artemcm commented Jul 15, 2022

@swift-ci please test

@owenv owenv requested a review from a team July 18, 2022 17:08
@nkcsgexi nkcsgexi merged commit 8f7b30b into swiftlang:release/5.7 Jul 18, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants