Skip to content

GSB: Optimize PotentialArchetype::getDependentType() #35773

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 5 commits into from
Feb 6, 2021

Conversation

slavapestov
Copy link
Contributor

@slavapestov slavapestov commented Feb 5, 2021

We were spending a lot of time in DependentMemberType::get(), via PotentialArchetype::getDependentType(), which is a recursive walk from the root to the leaf. We can reduce this cost with two changes:

  • DerivedSameTypeComponent doesn't need to store a PotentialArchetype * at all, only a Type.
  • PotentialArchetype's representation can be changed to just store the canonical dependent type directly, instead of a union of GenericParamKey with AssociatedTypeDecl *.

We still have to rebuild DependentMemberType to "re-sugar" them with the right generic parameter name. While this is important when the type is ultimately meant for user consumption, there are many places where we do it unnecessarily. Fixing those is the next step here.

On this incredibly silly benchmark, an asserts build is now twice as fast as without this change, from ~8 seconds to 4:

public protocol P1 {
  associatedtype A
  associatedtype B
  associatedtype C
  associatedtype D
}

public protocol P2 {
  associatedtype A : P1
  associatedtype B : P1
  associatedtype C : P1
  associatedtype D : P1
}

public protocol P3 {
  associatedtype A : P2
  associatedtype B : P2
  associatedtype C : P2
  associatedtype D : P2
}

public protocol P4 {
  associatedtype A : P3
  associatedtype B : P3
  associatedtype C : P3
  associatedtype D : P3
}

public protocol P5 {
  associatedtype A : P4
  associatedtype B : P4
  associatedtype C : P4
  associatedtype D : P4
}

public protocol P6 {
  associatedtype A : P5
  associatedtype B : P5
  associatedtype C : P5
  associatedtype D : P5
}

public protocol P7 {
  associatedtype A : P6
  associatedtype B : P6
  associatedtype C : P6
  associatedtype D : P6
}

public protocol P8 {
  associatedtype A : P7
  associatedtype B : P7
  associatedtype C : P7
  associatedtype D : P7
}

public protocol P9 {
  associatedtype A : P8
  associatedtype B : P8
  associatedtype C : P8
  associatedtype D : P8
}

public protocol P10 {
  associatedtype A : P9
  associatedtype B : P9
  associatedtype C : P9
  associatedtype D : P9
}

@slavapestov slavapestov changed the title Gsb optimizations GSB: Optimize PotentialArchetype::getDependentType() Feb 5, 2021
@slavapestov slavapestov requested a review from CodaFi February 5, 2021 21:50
@slavapestov slavapestov marked this pull request as ready for review February 5, 2021 21:50
Copy link
Member

@DougGregor DougGregor left a comment

Choose a reason for hiding this comment

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

Very nice!

@slavapestov
Copy link
Contributor Author

@swift-ci Please test

@slavapestov
Copy link
Contributor Author

@swift-ci Please test source compatibility

Instead of recomputing it on every call to getDependentType(),
we can just store it in there instead of the GenericParamKey
or AssociatedTypeDecl.

We still need to 're-sugar' user-visible dependent types to
get the right generic parameter name; a new getSugaredDependentType()
utility method is called in the right places for that.
@slavapestov
Copy link
Contributor Author

@swift-ci Please test

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