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

[AST] Cache the override generic signatures #26190

Merged
merged 4 commits into from
Jul 22, 2019
Merged

[AST] Cache the override generic signatures #26190

merged 4 commits into from
Jul 22, 2019

Conversation

theblixguy
Copy link
Collaborator

Follow up to #24484.

There was a small performance regression due to creating many more GSBs when computing the override generic signature of methods. This PR adds a cache to the AST to prevent getOverrideGenericSignature from re-computing the generic signature for methods each time.

@theblixguy
Copy link
Collaborator Author

cc @slavapestov

This is a very simple implementation, but let me know your thoughts!

@theblixguy
Copy link
Collaborator Author

@slavapestov I have addressed your comments. Does it look good to you now? Thanks!

include/swift/AST/ASTContext.h Outdated Show resolved Hide resolved
include/swift/AST/ASTContext.h Outdated Show resolved Hide resolved
lib/AST/ASTContext.cpp Outdated Show resolved Hide resolved
lib/AST/ASTContext.cpp Show resolved Hide resolved
@theblixguy
Copy link
Collaborator Author

Thanks @slavapestov, could you take another look?

@slavapestov
Copy link
Contributor

@swift-ci Please smoke test

@slavapestov
Copy link
Contributor

@swift-ci Please test compiler performance

@theblixguy
Copy link
Collaborator Author

theblixguy commented Jul 19, 2019

Hmm it seems like the caching didn’t make a huge difference: https://ci.swift.org/job/swift-PR-compiler-performance-macOS/439/artifact/comment.md/*view*/

-0.29% GSBs in debug, -0.46% in release. I’m not sure if we can improve further here?

@slavapestov
Copy link
Contributor

One possible improvement would be to fast-path a few cases. If the base class signature and the derived class signature have the same generic parameters and requirements, meaning the canonical signatures are equal, you don't need to create a GSB at all -- just return the base method's signature.

@slavapestov
Copy link
Contributor

Also, using getOverrideGenericSignature() in synthesizeDesignatedInitOverride() would probably help too.

@slavapestov slavapestov merged commit 87199d5 into swiftlang:master Jul 22, 2019
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