-
Notifications
You must be signed in to change notification settings - Fork 4.2k
Change SolutionState.ProjectStates from an ImmutableDictionary<ProjectId, ProjectState> to an ImmutableArray<ProjectState> #77971
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
Conversation
… considered *** Change SolutionState to keep/expose and ImmutableArray<ProjectState> instead of an IReadOnlyList<ProjectId> By exposing the project states, there is a bunch of code that can be changed to enumerate over that collection rather than over the values of an ImmutableDictionary<ProjectId, ProjectState>. That enumeration is actually *very* expensive. For example, during roslyn load, local etl traces indicates the following CPU usage enumerating this dictionary: Under ComputeDocumentIdsWithFilePath: 2.6 seconds Under GetFirstRelatedDocumentId: 1.1 seconds There is a bit of a tradeoff associated with this change. The IReadOnlyList<ProjectId> could be reused very often, whereas there is much less opportunity for sharing ImmutableArray<ProjectState> when forking the solution state. I expect there to be more memory allocated in the speedometer runs, so I'd like to get a feel on how much worse that is before elevating this PR out of draft status. Additionally, SolutionState had a CWT over a sorted version of this IReadOnlyList<ProjectId>. As mentioned earlier, the sharing of this data is quite a bit less common now that it's an ImmutableArray<ProjectState>. I locally measured the stopwatch time spent always sorting this data in ComputeChecksumsAsync and it was a total of 4 ms (averaged over two separate VS runs) when loading the roslyn sln. The ImmutableDictionary is still present in SolutionState, but it was renamed to indicate that it's primarily useful as a lookup tool, and all references treating it as an enumeration mechanism were changed to instead use the ImmutableArray<ProjectState>.
...tures/Core/Portable/ExternalAccess/UnitTesting/SolutionCrawler/UnitTestingWorkCoordinator.cs
Outdated
Show resolved
Hide resolved
…pProjectStatesInsteadOfProjectIds
src/Workspaces/Core/Portable/Workspace/Solution/SolutionState_Checksum.cs
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall I love the approach, but there's a lot of cases where we're calling Sort() where I imagine it doesn't matter. Even if whatever algorithm it uses is O(n) in the case of an already sorted array, I think there still removable. I'd vote remove those, and just add an extra assert that if the array coming into Branch() and friends is unsorted, then flag it.
src/Workspaces/Core/Portable/Workspace/Solution/SolutionState.cs
Outdated
Show resolved
Hide resolved
| solutionAttributes ??= SolutionAttributes; | ||
| projectIds ??= ProjectIds; | ||
| idToProjectStateMap ??= ProjectStates; | ||
| projectStates = projectStates == null ? ProjectStates : projectStates.Value.Sort(CompareProjectStates); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So this would mean any time we branch we'll still call this sort method, right? Can we avoid that by only having the cases that add/remove do this sort and the rest of the time can just pass it in?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is changing with the new requirement that callers have to pass this in sorted, but previously this would have only sorted had the caller passed in a new projectStates array.
| projectIds: newProjectIds, | ||
| idToProjectStateMap: newStateMap, | ||
| projectStates: newProjectStates, | ||
| projectCountByLanguage: AddLanguageCounts(ProjectCountByLanguage, languageCountDeltas), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wait, we keep a map of this? That'd be useful for your other PR about updating UI contexts where we're just enumerating all projects again...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not too worried about that enumeration, plus the comments in that code indicated that it's trying not to read CurrentSolution off the main thread.
src/Workspaces/Core/Portable/Workspace/Solution/SolutionState.cs
Outdated
Show resolved
Hide resolved
| var updatedIdToTrackerMapBuilder = originalProjectIdToTrackerMap.ToBuilder(); | ||
|
|
||
| foreach (var projectId in this.SolutionState.ProjectIds) | ||
| newProjectStatesBuilder.AddRange(this.SolutionState.ProjectStates); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this better than just calling the .ToBuilder() method on the original array? I would have imagined this might enumerate more, but maybe not.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ImmutableArray.Builder is a class that requires an allocation. ArrayBuilder.AddRange uses the pooled ImmutableArray.Builder to do the work for it.
| var newProjectStates = this.SolutionState.ProjectStates; | ||
| if (projectStateChanged) | ||
| { | ||
| newProjectStatesBuilder.Sort(SolutionState.CompareProjectStates); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we enumerated in order, didn't we keep the ordering? So how could this change?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's a lot going on in here with trackers and whatnot to create the new project state. If we're certain that their project id is the same, then this can be simplified. I'll go ahead and make that change and test it out.
Change a couple places to not do an unneeded sort
| /// <remarks>Requires the input array to be sorted by Id</remarks> | ||
| private static ProjectState? GetProjectState(ImmutableArray<ProjectState> sortedPojectStates, ProjectId projectId) | ||
| { | ||
| var index = sortedPojectStates.BinarySearch(projectId, static (projectState, projectId) => CompareProjectIds(projectState.Id, projectId)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
add a debug asser tthat sortedProjectStates is actually sorted?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Debug is already slow enough, and this is called a bunch. I'll pass as it's asserted now during the Branch.
| => CompareProjectIds(projectState1.Id, projectState2.Id); | ||
|
|
||
| private static int CompareProjectIds(ProjectId projectId1, ProjectId projectId2) | ||
| => projectId1.Id.CompareTo(projectId2.Id); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should we make ProjectState itself IComparable?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ProjectState:
yep, will go ahead and do that
ProjectId:
this class implements IComparable, but it doesn't implement the method publicly. So, it would require casts to IComparable to be able to use it. It seemed easier to just define the method, but I can do the casts or change the interface impl to be public if you prefer.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll just go ahead and cast it
|
|
||
| internal sealed partial class SolutionState | ||
| { | ||
| private static readonly ConditionalWeakTable<IReadOnlyList<ProjectId>, IReadOnlyList<ProjectId>> s_projectIdToSortedProjectsMap = new(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
woot.
| if (other is null) | ||
| return 1; | ||
|
|
||
| return ((IComparable<ProjectId>)this.Id).CompareTo(other.Id); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
make an internal CompareTo that we can call directly.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done!
| /// A list of all the project states contained by the solution. | ||
| /// Ordered by <see cref="ProjectState.Id"/>'s <see cref="ProjectId.Id"/> value. | ||
| /// </summary> | ||
| internal ImmutableArray<ProjectState> ProjectStates => this.SolutionState.ProjectStates; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please call this SortedProjectStates. It will help (imo) with readability everywhere and feeling safe around code that depends on that sortedness. it will also make it abundantly clear if we break this what the contract is supposed to be.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done!
| // Solution and SG version maps must correspond to the same set of projects. | ||
| Contract.ThrowIfFalse(this.SolutionState.ProjectStates | ||
| .Select(kvp => kvp.Key) | ||
| .Select(static projectState => projectState.Id) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i find your inconsistent use of stati disturbning.
(j/k. don't care).
| } | ||
|
|
||
| private (ImmutableDictionary<ProjectId, ProjectState>, ImmutableSegmentedDictionary<ProjectId, ICompilationTracker>) ComputeFrozenSnapshotMaps(CancellationToken cancellationToken) | ||
| private (ImmutableArray<ProjectState>, ImmutableSegmentedDictionary<ProjectId, ICompilationTracker>) ComputeFrozenSnapshotMaps(CancellationToken cancellationToken) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| private (ImmutableArray<ProjectState>, ImmutableSegmentedDictionary<ProjectId, ICompilationTracker>) ComputeFrozenSnapshotMaps(CancellationToken cancellationToken) | |
| private (ImmutableArray<ProjectState> sortedProjectStates, ImmutableSegmentedDictionary<ProjectId, ICompilationTracker>) ComputeFrozenSnapshotMaps(CancellationToken cancellationToken) |
nit
| { | ||
| var originalProjectIdToTrackerMap = _projectIdToTrackerMap; | ||
| var newIdToProjectStateMapBuilder = this.SolutionState.ProjectStates.ToBuilder(); | ||
| using var _ = ArrayBuilder<ProjectState>.GetInstance(out var newProjectStatesBuilder); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| using var _ = ArrayBuilder<ProjectState>.GetInstance(out var newProjectStatesBuilder); | |
| using var _ = ArrayBuilder<ProjectState>.GetInstance(out var newSortedProjectStatesBuilder); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
would it make sense to give this an initial size? given that you're going to addRange below?
also, can we do the addrange right after creating this? i think it would be clearer.
| newProjectStatesBuilder[i] = newTracker.ProjectState; | ||
| newIdToTrackerMapBuilder[projectId] = newTracker; | ||
| projectStateChanged = true; | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
going to trust that sorting stayes the same. i think so, but it might be worthwhile to doc. since we're not changing IDs, the order would stay the same.
| fallbackAnalyzerOptions, | ||
| projectCountByLanguage: ImmutableDictionary<string, int>.Empty, | ||
| idToProjectStateMap: ImmutableDictionary<ProjectId, ProjectState>.Empty, | ||
| projectStates: ImmutableArray<ProjectState>.Empty, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| projectStates: ImmutableArray<ProjectState>.Empty, | |
| projectStates: [], |
| /// </summary> | ||
| public bool ContainsProject([NotNullWhen(returnValue: true)] ProjectId? projectId) | ||
| => projectId != null && ProjectStates.ContainsKey(projectId); | ||
| => projectId != null && GetProjectState(projectId) != null; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| => projectId != null && GetProjectState(projectId) != null; | |
| => GetProjectState(projectId) != null; |
i think this works as i think GetProjectState is null safe. if i'm wrong. don't change this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't want to change this. The nullability constraints on GetProjectState indicate it shouldn't take in null, and with the new internal CompareTo method impl, that doesn't check for null.
| /// <remarks>Requires the input array to be sorted by Id</remarks> | ||
| private static ProjectState? GetProjectState(ImmutableArray<ProjectState> sortedPojectStates, ProjectId projectId) | ||
| { | ||
| var index = sortedPojectStates.BinarySearch(projectId, static (projectState, projectId) => ((IComparable<ProjectId>)projectState.Id).CompareTo(projectId)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
call new internla method.
| /// <summary> | ||
| /// Searches for the project state with the specified project id in the given project states. | ||
| /// </summary> | ||
| /// <remarks>Requires the input array to be sorted by Id</remarks> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
consider a debug assert of this.
| using var _1 = ArrayBuilder<ProjectId>.GetInstance(ProjectIds.Count + projectStates.Count, out var newProjectIdsBuilder); | ||
| using var _2 = PooledHashSet<ProjectId>.GetInstance(out var addedProjectIds); | ||
| var newStateMapBuilder = ProjectStates.ToBuilder(); | ||
| using var _3 = ArrayBuilder<ProjectState>.GetInstance(ProjectStates.Length + projectStates.Count, out var newProjectStatesBuilder); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| using var _3 = ArrayBuilder<ProjectState>.GetInstance(ProjectStates.Length + projectStates.Count, out var newProjectStatesBuilder); | |
| using var _3 = ArrayBuilder<ProjectState>.GetInstance(ProjectStates.Length + projectStates.Count, out var newSortedProjectStatesBuilder); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
or maybe not, since you sort later. consider calling this unsortedProjectStatedBuilder then :D
| foreach (var projectId in projectIds) | ||
| newStateMapBuilder.Remove(projectId); | ||
| var newStateMap = newStateMapBuilder.ToImmutable(); | ||
| var newProjectStates = ProjectStates.WhereAsArray(p => !projectIdsSet.Contains(p.Id)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
static lamba? pass arg?
|
|
||
| newDependencyGraph ??= _dependencyGraph; | ||
|
|
||
| var newSolutionState = this.Branch( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i think i may have missed it when reviewing, but add an Assert in Branch that if is passed newProjectStates that they are soted.
| public static ProjectDependencyGraph CreateDependencyGraph( | ||
| IReadOnlyList<ProjectId> projectIds, | ||
| ImmutableDictionary<ProjectId, ProjectState> projectStates) | ||
| ImmutableArray<ProjectState> sortedNewProjectStates) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
see. here you're naming things like i like it :)
* Addressing feedback * Addressing PR feedback * Addressing PR comments * Remove workaround for dotnet/msbuild#10306 The VS SDK was fixed, so the problem shouldn't occur anymore. * move to a workspace factory * No need for solution * Simplify handlers * Add helper * move more over * move more over * Add remove calls * Move handlers over * Move remote service over * async lock * async lock * async lock * More work * Fixup * Correct * Move to immutable dictionary * in progress * in progress * in progress * in progress * in progress * in progress * Simplify * Serialize * simplify * simplify * simplify * in progress * in progress * Reset code * Clelar * renames * Simplify * rename * No more null * Simplify * Exception throwing * Extensions: update feature status (#77923) * Add ignored directives feature status (#77965) * Extensions: misc checks on receiver parameter and extension members (#77937) * Add porject back in * CHange how locking works * Simplify * Docs * Add new message for getting registered method names * IN progres * in progress * Simplify * Simplify * Handle exceptions * Simplify * Docs * move method * Simplify and regions * Tweaks * Docs * Break into files * Simplify exception handling * Simplify * Simplify * Move * Docs * Simplify * Simplify * Share code * Split into local and remote calls * Add throw * Add throw * Simplify * Simplify * Simplify * Simplify * Simplify * Simplify * Simplify * Simplify * Simplify * Simplify * Simplify * Simplify * Avoid potential disposal exception during MEF shutdown (#77990) Previously, if VS was shutdown before Workspace.EnsureEventListeners was invoked, we would cause an exception during MEF disposal. This would happen because our workspace disposal would attempt to get a service that hadn't already been cached, and thus would ask MEF to compose the item during MEF disposal (which doesn't work). Instead, just cache the IWorkspaceEventListenerService for use during the dispose. * Remove unused elements form PublishData.json (#77994) Removing some elements that seem to be left over from how the publish tooling used to work * Reduce main thread switches in VisualStudioProjectFactory.CreateAndAddToWorkspaceAsync (#77920) * Reduce main thread switches in VisualStudioProjectFactory.CreateAndAddToWorkspaceAsync This method is called once for each project in the solution during solution open, and each call was essentially switching to the main thread twice (once explicitly, once via call to _visualStudioWorkspaceImpl.RefreshProjectExistsUIContextForLanguageAsync). For my testing of opening Roslyn.sln, this removed over 600 main thread switches. Instead: 1) Move the first main thread switch to just occur once in an initialization task. This task is joined when CreateAndAddToWorkspaceAsync is invoked to ensure that that initialization is completed. 2) Move the second main thread switch to instead move inside the callback from an ABWQ. The code being called needs the UI thread, but doesn't need to occur synchronously during the call to CreateAndAddToWorkspaceAsync. * Explicitly releasE * Cancellation is fine * Reorder * remove net * docs * docs * remove exception handling * Add public api * lint * Reduce main thread switches in VisualStudioProjectFactory.CreateAndAddToWorkspaceAsync (#77920) (#77998) * Reduce main thread switches in VisualStudioProjectFactory.CreateAndAddToWorkspaceAsync This method is called once for each project in the solution during solution open, and each call was essentially switching to the main thread twice (once explicitly, once via call to _visualStudioWorkspaceImpl.RefreshProjectExistsUIContextForLanguageAsync). For my testing of opening Roslyn.sln, this removed over 600 main thread switches. Instead: 1) Move the first main thread switch to just occur once in an initialization task. This task is joined when CreateAndAddToWorkspaceAsync is invoked to ensure that that initialization is completed. 2) Move the second main thread switch to instead move inside the callback from an ABWQ. The code being called needs the UI thread, but doesn't need to occur synchronously during the call to CreateAndAddToWorkspaceAsync. * make nested * Simplify * Simplify * Simplify * Simplify * Simplify * restore * Update dependencies from https://github.com/dotnet/source-build-reference-packages build 20250402.2 (#77982) Microsoft.SourceBuild.Intermediate.source-build-reference-packages From Version 10.0.618101 -> To Version 10.0.620202 Co-authored-by: dotnet-maestro[bot] <dotnet-maestro[bot]@users.noreply.github.com> * Allow null * Simplify * No VoidResult * Make everything non-capturing * 'nameof(T<>)' not supported yet. * Update references * Revert "Another attempt to move MiscellaneousFilesWorkspace to a bg thread (#77983) This reverts commit 2e8fcbe. * fix * Exception work * Update Roslyn.sln * More exception work * Add remote end * Complete the exception handling work * Add docs * docs * Add docs * Rename type * Rename type * update publishdata * Change SolutionState.ProjectStates from an ImmutableDictionary<ProjectId, ProjectState> to an ImmutableArray<ProjectState> (#77971) * *** WIP. Definitely needs supporting speedometer numbers before being considered *** Change SolutionState to keep/expose and ImmutableArray<ProjectState> instead of an IReadOnlyList<ProjectId> By exposing the project states, there is a bunch of code that can be changed to enumerate over that collection rather than over the values of an ImmutableDictionary<ProjectId, ProjectState>. That enumeration is actually *very* expensive. For example, during roslyn load, local etl traces indicates the following CPU usage enumerating this dictionary: Under ComputeDocumentIdsWithFilePath: 2.6 seconds Under GetFirstRelatedDocumentId: 1.1 seconds There is a bit of a tradeoff associated with this change. The IReadOnlyList<ProjectId> could be reused very often, whereas there is much less opportunity for sharing ImmutableArray<ProjectState> when forking the solution state. I expect there to be more memory allocated in the speedometer runs, so I'd like to get a feel on how much worse that is before elevating this PR out of draft status. Additionally, SolutionState had a CWT over a sorted version of this IReadOnlyList<ProjectId>. As mentioned earlier, the sharing of this data is quite a bit less common now that it's an ImmutableArray<ProjectState>. I locally measured the stopwatch time spent always sorting this data in ComputeChecksumsAsync and it was a total of 4 ms (averaged over two separate VS runs) when loading the roslyn sln. The ImmutableDictionary is still present in SolutionState, but it was renamed to indicate that it's primarily useful as a lookup tool, and all references treating it as an enumeration mechanism were changed to instead use the ImmutableArray<ProjectState>. * Update dependencies from https://github.com/dotnet/source-build-reference-packages build 20250404.2 (#78008) Microsoft.SourceBuild.Intermediate.source-build-reference-packages From Version 10.0.620202 -> To Version 10.0.620402 Co-authored-by: dotnet-maestro[bot] <dotnet-maestro[bot]@users.noreply.github.com> * Fix VSTypeScript lsp tests * localize servicehub service name * Simplify workspace initialization in the LSP server Right now we have the logic for which analyzers get added to a workspace in Program.cs, which calls a special method that specifically initializes the main host workspace. This refactors it so that could be used to initialize any workspace, and removes any tricky ordering problems that might happen by using cleaner MEF imports when we have them. * Update test * Update dependencies from https://github.com/dotnet/arcade build 20250404.5 Microsoft.SourceBuild.Intermediate.arcade , Microsoft.DotNet.Arcade.Sdk , Microsoft.DotNet.Helix.Sdk , Microsoft.DotNet.XliffTasks From Version 9.0.0-beta.25164.2 -> To Version 9.0.0-beta.25204.5 * Update maintenance-packages versions (#78005) * Update maintenance-packages versions * Update dependencies from https://github.com/dotnet/source-build-reference-packages build 20250404.2 Microsoft.SourceBuild.Intermediate.source-build-reference-packages From Version 10.0.620202 -> To Version 10.0.620402 * Address feedback --------- Co-authored-by: dotnet-maestro[bot] <dotnet-maestro[bot]@users.noreply.github.com> * Fix EA layering for Razor.ExternalAccess (#77927) Moves things to /Shared to ship in both MS.CA.EA.Razor and MS.CA.EA.Razor.Features At some point we can rename MS.CA.EA.Razor to MS.CA.EA.Razor.EditorFeatures but I didn't bother with that in this PR Shipping in both to not require dual insertion. We can fix this after a consuming change in Razor is inserted * Delete * Update PublishData.json after VS snap to rel/d17.14 * Handle nameof(indexer) in ref analysis (#78015) * Make a couple of features non-experimental * Ensure external access extensions package gets codebase * Feedback * Add extension message handler service teest * Add extension message handler service test * FIx * Add testS * In progress * Test work * Tests * Add test * More tests * Add tests * Add tests * Add tests * Add test * Add test * Add test * Add test * Add test * Update comment * Disable TransitiveVersioningPinning for RoslynAnalyzers. * Remove dependency on EditorFeatures from lsp tests * Remove dependency on EditorFeatures from codelens layer * Update dependencies from https://github.com/dotnet/source-build-reference-packages build 20250407.2 Microsoft.SourceBuild.Intermediate.source-build-reference-packages From Version 10.0.620402 -> To Version 10.0.620702 * Remove dependency on EditorFeatures from build tools * Update dependency versions * Extensions: ref safety analysis (#77967) * Update PublishData.json after VS span to main * In progress * Fix crash in backing field nullability cycle scenario (#77993) * More work * Pass along when an extension was unloaded * Add teset * Add tests * Docs * Simplify * Using alias * Fix * Add tests * Add docs * Update gladstone/roslyn api * Simplify channels in SemanticSearch * Semantic Search: Add support for async queries and FAR tool (#77906) * Simplify further * Simplify further * Localize exception messages * Docs * Docs * Docs * named args * named args * Docs * Move type * Docs * Fix name * Compile just for NET * Unseal LSP types (#78041) * Simplify analyzer api * Update RoslynAnalyzer package projects with dependencies This will fix issues with transitive pinning during source builds. * Ensure LSP uses actual signature help trigger characters * Improve detection of code whose updates may not have effect (#78009) * Do not return metadata names for document symbols * Fix nullability warnings * Review feedback * Remove EditorFeatures from OOP (#78069) * Add back EA.Razor for servicing branches publishdata is pulled from main always. this needs to be present even though the package no longer exists for servicing branches * Remove unused ISemanticSearchWorkspaceHost (#78083) * Split query execution into compile and execute calls (#78081) * Update resourceManagement.yml (#77948) * Expression trees: support optional arguments and named arguments in parameter order (#77972) * Update ignored directives public API (#77968) * Remove experiment * Add Content token to IgnoredDirectiveTriviaSyntax * Add Content token to ShebangDirectiveTriviaSyntax * Fixup public API * Fixup semantic search API list * Fix CLI flag in error message * Remove Update API * Update SemanticSearch API list --------- Co-authored-by: Matteo Prosperi <maprospe@microsoft.com> Co-authored-by: Jason Malinowski <jason.malinowski@microsoft.com> Co-authored-by: Cyrus Najmabadi <cyrus.najmabadi@gmail.com> Co-authored-by: Cyrus Najmabadi <cyrusn@microsoft.com> Co-authored-by: Julien Couvreur <julien.couvreur@gmail.com> Co-authored-by: Jan Jones <janjones@microsoft.com> Co-authored-by: Matteo Prosperi <41970398+matteo-prosperi@users.noreply.github.com> Co-authored-by: Todd Grunke <toddgrun@microsoft.com> Co-authored-by: Jared Parsons <jared@paranoidcoding.org> Co-authored-by: dotnet-maestro[bot] <42748379+dotnet-maestro[bot]@users.noreply.github.com> Co-authored-by: dotnet-maestro[bot] <dotnet-maestro[bot]@users.noreply.github.com> Co-authored-by: David Barbet <dabarbet@microsoft.com> Co-authored-by: Carlos Sánchez López <1175054+carlossanlop@users.noreply.github.com> Co-authored-by: Andrew Hall <andrha@microsoft.com> Co-authored-by: Charles Stoner <10732005+cston@users.noreply.github.com> Co-authored-by: Tomas Matousek <tomat@microsoft.com> Co-authored-by: Joey Robichaud <jorobich@microsoft.com> Co-authored-by: Rikki Gibson <rigibson@microsoft.com> Co-authored-by: Tomáš Matoušek <tmat@users.noreply.github.com>
A bunch of code currently enumerate over an ImmutableDictionary<ProjectId, ProjectState> which is surprisingly expensive. Instead, change this to an ImmutableArray<ProjectState> which is much faster to enumerate over.
For example, during roslyn load, local etl traces indicate the following CPU usage enumerating this dictionary:
Under ComputeDocumentIdsWithFilePath: 2.6 seconds
Under GetFirstRelatedDocumentId: 1.1 seconds
Also, SolutionState had a CWT over a sorted version of this IReadOnlyList<ProjectId>. This is no longer necessary as the ImmutableArray<ProjectState> is now sorted as that code needed.
Code that needed lookup from the ImmutableDictionary is now replaced by calls to ContainsProject/GetProjectState. There is no asymptotic performance change as the ImmutableArray<ProjectState> are sorted by ProjectId and do binary searches (O(lg n)) whereas before these did ImmutableDictionary lookups (also O(lg n))
Once tests pass on ci will create a test insertion for speedometer numbers.