Provide a simple ArrayBuilder api that does not drop array builders when they have a large number of elements in them #77069
Conversation
dotnet-issue-labeler
bot
added
Area-IDE
untriaged
| document, patternName, patternContainerOpt, declaredSymbolInfoKindsSet, t => results.Add(t), cancellationToken).ConfigureAwait(false); | ||
| // In parallel, search both the document requested, and any relevant 'related documents' we find for it. For the | ||
| // original document, search the entirety of it. For related documents, only search the spans of the | ||
| // partial-types/inheriting-types that we find for the types in this starting document. |
|
Awesome! I tried it out on my end and verified I can see results from partial types and base classes. Cyrus suggested to me that with this change, AIOS should now adjust sorting so that results that are literally from the current document are prioritized, so I'll make sure we take care of that on our end. Thanks for the insanely rapid turnaround after a fun and productive brainstorming chat this morning. <3 |
|
@genlu ptal |
...paces/SharedUtilitiesAndExtensions/Compiler/CSharp/Services/SyntaxFacts/CSharpSyntaxFacts.cs
Show resolved
Hide resolved
src/EditorFeatures/CSharpTest/NavigateTo/NavigateToSearcherTests.cs
Outdated
Show resolved
Hide resolved
src/Features/Core/Portable/NavigateTo/AbstractNavigateToSearchService.NormalSearch.cs
Outdated
Show resolved
Hide resolved
src/Features/Core/Portable/NavigateTo/AbstractNavigateToSearchService.NormalSearch.cs
Outdated
Show resolved
Hide resolved
src/EditorFeatures/CSharpTest/NavigateTo/NavigateToSearcherTests.cs
Outdated
Show resolved
Hide resolved
src/EditorFeatures/CSharpTest/NavigateTo/NavigateToSearcherTests.cs
Outdated
Show resolved
Hide resolved
src/Features/Core/Portable/NavigateTo/AbstractNavigateToSearchService.NormalSearch.cs
Outdated
Show resolved
Hide resolved
| instance = GetInstance(); | ||
| return new PooledDisposer<ArrayBuilder<T>>(instance); | ||
| } | ||
| => GetInstance(discardLargeInstances: true, out instance); |
There was a problem hiding this comment.
I like the array builder change, just wondering if it might make sense to have it in it's own PR so others interested in that change wouldn't need to look at the nav search stuff.
Also, feels like this might have conflicts with Tomas's PR: #76971
There was a problem hiding this comment.
extracted out to just this.
CyrusNajmabadi
changed the title
| TypeDeclarationSyntax or | ||
| EnumDeclarationSyntax; | ||
| } | ||
| => node is BaseNamespaceDeclarationSyntax or BaseTypeDeclarationSyntax; |
There was a problem hiding this comment.
Yes. Was touchign the 'AddMembers' codepaths to update them to use this, and it hits thsi method. so i fixed it at the same time.
There was a problem hiding this comment.
I'm a little confused by this change. Is EnumDeclarationSyntax not a top level node, or not one with members?
There was a problem hiding this comment.
EnumDecl and TypeDecl are the only subclasses of BaseTypeDeclarationSyntax. So we already have a single type that checks for both. we don't need two type checks.
| @@ -14,4 +14,19 @@ public static PooledDisposer<PooledStringBuilder> GetInstance(out StringBuilder | |||
| instance = pooledInstance; | |||
| return new PooledDisposer<PooledStringBuilder>(pooledInstance); | |||
| } | |||
|
|
|||
| void IPooled.Free(bool discardLargeInstances) | |||
There was a problem hiding this comment.
Trying to understand how I would use a large instance StringBuilder. I assume I would need to do what is done in ArrayBuilder, add an ObjectPool to this class and then add a GetInstance method that potentially pulls from that pool?
There was a problem hiding this comment.
PooledStringBuilder.GetInstance(out var sb)
You can now have this new behavior with:
PooledStringBuilder.GetInstance(discardLargeInstance: false, out var sb)
…hen they have a large number of elements in them Search related type sections in other documents when doing a document-filtered nav to search Use null for simplicity Apply suggestions from code review Keep around large array instances for speedometer Use a separate pool Make consistent Revert Expose similar api on string builder
CyrusNajmabadi
force-pushed
the
navToRelatedDocuments
branch
from
9a74ae1 to
42afc4c
Compare
Allows us to still use ArrayBuilder (and the api/ergonomics we like with it) while adjusting this piece of behavior in a few places where desired.