Skip to content

Explicitly copy defsIDependUpon into an appropriately sized array when initializing SourceFileDependencyGraph.Node #1119

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
Jun 28, 2022

Conversation

owenv
Copy link
Contributor

@owenv owenv commented Jun 28, 2022

Previously, the removeAll call below the initializer would trigger CoW. If the scratch buffer was larger than the count, this would result in an unnecessarily large allocation. In a very large source 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.

I tested this using the swiftdeps from an artificially generated swiftdeps with 50k empty struct declarations. Before this change, it took 31s to load and destroy, after this change it only took 0.25s.

rdar://95648282

@owenv owenv requested review from CodaFi and artemcm June 28, 2022 01:20
@owenv
Copy link
Contributor Author

owenv commented Jun 28, 2022

@swift-ci please test

…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 swiftdeps-deserialize-perf branch from 27c525b to 1fd4250 Compare June 28, 2022 01:34
@owenv
Copy link
Contributor Author

owenv commented Jun 28, 2022

@swift-ci please test

Copy link
Contributor

@artemcm artemcm left a comment

Choose a reason for hiding this comment

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

Awesome catch!

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.

2 participants