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

Find dependent types more efficiently by storing inheritance info directly in our indices. #11313

Merged
merged 73 commits into from
May 17, 2016

Conversation

CyrusNajmabadi
Copy link
Member

@CyrusNajmabadi CyrusNajmabadi commented May 15, 2016

Fixes #11255

This change introduces a new way of finding "dependent-types" in Roslyn. Dependent-type finding is used extensively in features like FAR and FindImplementations to dicover the inheritance hierarchy for classes/interfaces. For example, FindImplementation uses this functionality to find all the subclasses of an abstract class.

Previously, the way we would find these dependent types was to literally walk every viable type in the system and check if it had the inheritance relationship we were looking for. for example, for FindImplementation we would look at each and every type in a dependent project to see if it inherited from the type we were searching for. This was clearly quite expensive and scaled linearly with the number of types in the system.

The new system optimizes this operation greatly by augmenting our syntax-tree indices with the textual information about what a type says it inherits from. For example, if we have class X : A.B.C<D>, then in our index we store information stating "class type X derives from some type called C". Later on, when we're looking for derived types of 'C', we can just ignore all types that don't have this index information stored with them.

Now, because inheritance is transitive, this does mean we end up having to continue our search until we find no more new types. i.e. in the above example, after finding 'X', we'd then have to go look for derived types of X, and so on, and so on. However, this set is still massively smaller than the entire set of types in the system.

As a practical example, i searched for implementations of ILanguageService in roslyn. There are roughly 250 implementations of this type scattered over many projects. The initial pass of ILanguageService found 60 direct inheriting, and successive passes found the total set of derived types. Because we only needed to examine a small handful of types, this operation dropped from several seconds, to subsecond on my computer.

Notes:

The bulk of this work happened in a rewrite of DependentTypeFinder. I recommend you simply look at the 'after' for this code as the diff is virtually useless.

A lot of the complexity in DependentTypeFinder is around ensuring that we process a relevant project only once. i.e. we do not start with ILanguageService and then find all immediately derived tyeps across all projects, and then repeating that procedure over and over again. That would require us to realize compilations and whatnot across all projects, only to throw them away and do that work again. This would exacerbate our caches and hit the GC system hard. Instead, we determine the right order to walk projects, and we examine each project only once. When we examine a project we find all relevant dependent types, and we use that information when we walk to the next. For example, when searching for implementations of ILanguageService, we find all derived interfaces of it as well as all implementations in the Workspace project. We then take those types and use them to search for potential implementations in the next project, and so on and so forth. Because of this, we only ever have one project's compilation alive at a time. This greatly diminishes the stress on the system, and prevents us from continually sweeping over many projects over and over again.

Also, I did a lot of renaming to make it clear what hte different was between methods that transitively search for results, versus methods that only search one level deep. I hope that makes things clearer.

Finally, This work doesn't really help when we're searching for metadata types. In that case we still walk all metadata types seeing if they fit the relation. We could consider building similar indices for metadata so we don't need to realize and walk all types in that case.

…ync by creating far less source symbols when searching for matches.
…FromInterfacesAsync and FindDerivedClassesAsync
@@ -757,49 +759,56 @@ public bool TryGetDeclaredSymbolInfo(SyntaxNode node, out DeclaredSymbolInfo dec
GetFullyQualifiedContainerName(node.Parent),
DeclaredSymbolInfoKind.Constructor,
ctorDecl.Identifier.Span,
ImmutableArray<string>.Empty,
Copy link
Member

@jasonmalinowski jasonmalinowski May 16, 2016

Choose a reason for hiding this comment

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

named arguments (for this and rest of file)

s_derivedClassesCache,
cancellationToken);
Func<HashSet<INamedTypeSymbol>, INamedTypeSymbol, bool> metadataTypeMatches =
(s, t) => TypeDerivesFrom(s, t, transitive);
Copy link
Member

Choose a reason for hiding this comment

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

Nit: variable names longer than s and t? Normally wouldn't care, but can't figure out from context what they are.

@jasonmalinowski
Copy link
Member

So overall, some nitpicks but 👍. I admit seeing the forest in this review is difficult. There's lots of small methods that seems good, but the concept count is quite high as a result and might make it harder to follow.

@CyrusNajmabadi
Copy link
Member Author

test vsi please

@CyrusNajmabadi CyrusNajmabadi merged commit a247cea into dotnet:master May 17, 2016
@CyrusNajmabadi CyrusNajmabadi deleted the findRefsOneProject branch May 17, 2016 01:30
@CyrusNajmabadi CyrusNajmabadi restored the findRefsOneProject branch May 30, 2016 21:04
@CyrusNajmabadi CyrusNajmabadi deleted the findRefsOneProject branch January 25, 2020 00:36
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.

4 participants