[AST] Perf: Improve getDesugaredType() efficiency #13691
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Make getDesugaredType() as fast as possible for now. With the old way: 1) Switching over the sugared types turned into a frequently mispredicted branch because the sugar in the type system is random as far as the processor is concerned. 2) Storing the underlying/singlely-desugared type at different offsets in memory adds more code bloat and misprediction. Short of a major redesign to avoid pointer chasing, this is probably as fast as the method will get.
|
@swift-ci please test |
|
For reference, if we cache all |
|
What sort of performance impact does this have in practice? |
|
The underlying type of a TypeAliasDecl should never change after being computed. We should add an assertion somewhere for that. |
|
Hi @slavapestov – If we add an assert, then |
|
Hi Joe – On my "Kaby Lake" MacBook Pro, I see about a 3.57% improvement when building |
|
Hi @slavapestov – Just FYI: https://bugs.swift.org/browse/SR-6694 |
|
@davezarzycki Nice! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Make getDesugaredType() as fast as possible for now. With the old way:
Short of a major redesign to avoid pointer chasing, this is probably as fast as the method will get.
@DougGregor / @CodaFi / @slavapestov – With this change,
NameAliasTypenow caches some – but not all – results to avoid pointer chasing (just like the syntax sugared types do so they don't need to ask the AST ever time). My question is this, is the caching I setup ingetSinglyDesugaredTypeSlow()correct? Could it cache more? It certainly cannot cache all results, otherwise a couple tests break.