Don't use a pool in SemanticTokensHelper.ComputeTokens #79622
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
This can be just allocated directly into an array instead of fiddling with a pool. Simpler and more efficient. This shows up as 2.5% of allocations in a scenario in a scenario in RazorEditingTest.CompletionInCohosting for the Roslyn OOP. Most of that is due to the list resizing.
ToddGrun
changed the title
ToddGrun
changed the title
CyrusNajmabadi
approved these changes
Jul 26, 2025
src/LanguageServer/Protocol/Handler/SemanticTokens/SemanticTokensHelpers.cs
Show resolved
Hide resolved
3 tasks
This was referenced Jul 30, 2025
333fred
added a commit
that referenced
this pull request
Aug 7, 2025
* Remove NetAnalyzers version override * Put back dotnet8 feed * Fix analyzer violations * Address more analyzer violations * Revert breaking fixes * Address more analyzer violations * Fix Xaml MEF import * Generate semantic search ref assemblies package * Change the delay for how often we synchronize solutions Now that we're doing all the WorkspaceChanged notifications on background threads, that means the notifications won't be quite as batched as before; this was causing an increase in synchronizations that was creating some wasted work. * Update dependencies from https://github.com/dotnet/dotnet build 276797 Updated Dependencies: System.CommandLine (Version 2.0.0-beta7.25374.102 -> 2.0.0-rc.1.25375.106) * PR feedback * Update dependencies from https://github.com/dotnet/dotnet build 276828 Updated Dependencies: System.CommandLine (Version 2.0.0-rc.1.25375.106 -> 2.0.0-rc.1.25375.119) * Use Append overload for repeating chars * Fix formatting of CDATA sections in quick info * Fix * Fix * Handle concurrent requests to update workspace contents and workspace SG info * Fallout * Add test * Apply suggestions from code review * Docs * Docs * Produce a better return type when trying to return an anonymous type in an async method * Fix test * Don't use a pool in SemanticTokensHelper.ComputeTokens (#79622) * Don't use a pool in SemanticTokensHelper.ComputeTokens This can be just allocated directly into an array instead of fiddling with a pool. Simpler and more efficient. This shows up as 2.5% of allocations in a scenario in a scenario in RazorEditingTest.CompletionInCohosting for the Roslyn OOP. Most of that is due to the list resizing. * Refine Caching example (#79593) Add comment to improve readability. * Add reference assemblies dir parameter (#79614) * Update Microsoft.ILVerification (#79555) * Reduce allocations in CSharpVirtualCharService.TryConvertStringToVirtualChars (#79623) This method is accounting for 1.4% of allocations in the OOP process in a completion scenario in the RazorEditing.CompletionInCohosting speedometer test I am looking at. Instead of doing all the work and hitting the pools, we can just handle the normal case of there not being any surrogates or escape characters and just create a VirtualCharSequence wrapper around the existing string. Locally, I see about 75% of strings that come through hit this optimization. Going to do a test insertion with this change to verify speedometer numbers before elevating out of draft status. * modified tagged text instead to resolve LSP issues * Remove unnecessary methods from interface * Move logic into helper * Better handle quick info for code with suppressions in it * Simplify * Revert * Simplify * fix diff * restore optionality * Remove unnecessary feed (#79635) * Reduce some UI thread blocking It looks like in various refactorings we accidentally had a ConfigureAwait(false) appear in this method which could cause us to jump to a background thread when we don't need to do that. I'm not sure why we didn't just use CA(true) only here, since that seems to do what we want without any extra trickery. * Pass in referenceAssembliesDir to ISemanticSearchSolutionService * reverted tests back to main * File-based program directive diagnostics in editor (#79421) * Sign contents of nupkg, but not the nupkg itself * Pull publishdata from the branch being published * Update dependencies from https://github.com/dotnet/dotnet build 276981 (#79647) [main] Source code updates from dotnet/dotnet * Incorporated PR feedback - Restored removed BuildHost methods. - Separated MSBuild instance finding from loading - Removed loop in favor of a single relaunch * Show nullability of a local variable when hovering on the decl of it * Add tests * Add more var cases * Add work items * Docs * Share code * nrt * Improve QuickInfo NRT info in the presence of nullability altering checks * Simplify * Simplify * Simplify * Simplify * Add test cases * Update src/Workspaces/Core/Portable/Workspace/Solution/SourceGeneratorExecutionVersion.cs Co-authored-by: Joey Robichaud <[email protected]> * Add tests * Add tests * Restore name * Make nupkg signing exclusion conditional * Fix logging when item counts change in LoadedProject (#79640) * Use checksum * Add lsp tests * remove testing branch name * reverted tests back to main * Add comment for nuget package behavior * Trap on-the-fly-doc exceptions in quick info * Cleanup * Update debugger.contracts to 18.0.0-beta.25379.1 (#79661) * Catch more * Catch more * Tweak map spans API to match document service, and call it * Enable View -> Stack Trace Explorer by default Tool windows can always be opened before their packages are loaded. The act of running the command creates the package. * Strongly type the mapping API, and allow Razor to check generator type etc. * argh don't wanna talk about it * Delete unused quick info code * SelectAsArray * Recognize known `ToString` methods inside interpolation * Fix extension-method add-import completion in syntactically ambiguous scenarios * Docs * `comp` -> `compilation` * Use pooled builder * Update src/Analyzers/Core/Analyzers/SimplifyInterpolation/AbstractSimplifyInterpolationHelpers.cs Co-authored-by: Cyrus Najmabadi <[email protected]> * Update src/Analyzers/Core/Analyzers/SimplifyInterpolation/AbstractSimplifyInterpolationHelpers.cs Co-authored-by: Cyrus Najmabadi <[email protected]> * Check for `static` just in case * Paranoia :) * Sort * Fix spelling * remove unnecessary code * Allow PR val build insertion step to be re-run * Simplify * Spelling * Move tests * Remove * Add back * Too many type name collisions in Razor tests * Revert "Move methods back to a place where TypeScript can reference them (#79208) * Fix Call Hierarchy false positives by filtering non-reference contexts * test * added typeof filtering * fix call hierarchy false positives by filtering out nameof and typeof calls * used ReferenceLocation for nameof and typeof detection * added space as per CR * wip tests * Simplify determination of live vs. build diagnostics * Add Copilot.EA IVT * walk up functionality + nameof tests for call hierarchy * formatting issues * Extract helper method * Use span mapping in codelens * Use span mapping in naviation * Use span mapping in FAR * VB: EmitDelegateCreation - account for the fact that static methods can be marked virtual in metadata (#79697) Fixes #79696 * Update based on review feedback * Update src/Compilers/Core/Portable/Diagnostic/WellKnownDiagnosticTags.cs Co-authored-by: Rikki Gibson <[email protected]> * defer tail slicing for efficiency * Remove untriaged label automation (#79701) The `Status` field on our project board supercedes this label. Removing it since it's no longer used. * test member access read/write not affected by walkup * removed commented out test * Extensions: address instrumentation follow-ups (#79557) * Move extension to helper class. Extensions on Document seemed a bit broad * Remove a hack, and clarify another * Fix tests * Allow more lifecycle events in Razor cohosting * Fix formatting in use-auto-prop when attributes are present * Move 'use auto prop' into code style layer * In progress * Move fix all helpers to shared location so that CodeStyle can reference them * IN progress * in progress * Share more code * wip * Working * Simplify * Simplify * Remove hacks * Simplify * Add assert * move service into shared location * Revert "Pull publishdata from the branch being published" This reverts commit 0721c3b. * helper method + formatting issues fix * made helper method into local method * Add Copilot semantic search (#79673) * Add null check * Semantic search updates (#79742) * [main] Source code updates from dotnet/dotnet (#79744) [main] Source code updates from dotnet/dotnet - Remove properties that now live in Version.Details.props - Update Versions.props - Add Microsoft.Extensions.FileSystemGlobbing to V.D.xml * Refactor how TestWorkspaces interact with TestCompositions TestWorkspace only had constructors that took TestCompositions, but this reintroduces a constructor that takes an ExportProvider directly if the MEF composition requires too much setup -- specifically our LSP server tests that already have their own system for creating MEF compositions. A few things got cleaned up along the way: 1. The Composition property (which was oddly nullable) gets removed entirely; a few tests that were creating a second TestWorkspace to mirror a first one are able to just use the ExportProvider directly. 2. We had a hack where we'd inject a TestWorkspaceConfigurationService into the MEF container if the call to create the TestWorkspace passed along custom options. This just adds that type directly into the base configuration so we can reuse the TestCompositions better. Any tests trying to use that feature and creating custom MEF compositons are just expected to inject that type. * Allow our LanguageServer tests to use Protocol.Test.Utilities The problem here was that adding that dependency carries along many things we don't want to put into the MEF composition, but we now have a subfolder we can use for MEF discovery that contains just the product binaries we expect. * Run our LSP Miscellaneous Files tests against both implementations We have a set of tests for miscellaneous files tests, and this now runs that against both our base implementation and also the file-based application implementation in our language server. This required a few subtle changes: 1. Making AbstractLanguageServerProtocolTests deal with test workspaces that aren't our usual MEF TestCompositions. This is because these tests against the real server implementation are using the code that creates our real MEF container. Our code assumed we had some test mocks available, but in the real product those aren't there. We can gracefully skip those since we don't need them to properly test the end-to-end product. 2. Switching the tests away from assuming there is a misc files workspace but rather that a given documenet may or may not be a miscellaneous files document. This gets rid of a TODO we had referring to this, and although required a lot of changes it's a mechanical change. * Consolidate and unify the miscellaneous files tests more This moves one test into the abstract base, and also fixes the tests to avoid using TestLspServer.TestWorkspace when in the real server tests we will be using our other host workspace implementation. * Fix file-based programs getting stuck in the host workspace If a file is loaded that can be treated as a file-based program, and then a real project is loaded that contains that file, we should unload the file-based program since we're treating that as a miscellaneous file. But we had a bug here: the code in LspWorkspaceManager that loops through workspaces assumes that the miscellaneous files workspace would be found last, so if we find a document in any other workspace first, we're good to use it. This assumption was broken once we started putting file-based programs in the host workspace, since we might find both the file-based program and the 'real' project in the same workspace. The fix is straightforward: if when we're finding a file context we amd we end up with a miscellaneous file, check if we have any other files too, and if so, prefer those. This will then allow us to unload the miscellaneous file as normal. Fixes dotnet/vscode-csharp#8471 * fix formatting issues * Tests to ensure everything is hooked up * Apply suggestions from code review Co-authored-by: Joey Robichaud <[email protected]> * Allow Razor to control the lifetime of caches, rather than using statics * Don't know how that happened * use SelectAsArray * Where+Select * Use WhereAsArray * Add docs * rename * Formatting * Simplify * Remove lazy enumerables * Delete unused functin * Fix crash when generating explicit impl properties * Improve error reporting from RemoteInitializationService (#79761) * Do not report 'no-effect' warning when active top-level code is updated (#79746) * Allow Razor to call into codelens * Make internal * Update dependencies from https://github.com/dotnet/dotnet build 278022 (#79796) [main] Source code updates from dotnet/dotnet * Simplify code * remove comment * remove comment * Support getting diagnostic descriptors from OOP * Simplify * Formatting * Has predicate, so don't use count * Named parameters * Remove unused test impl of an interface * Cleanup * Update dependencies from https://github.com/dotnet/dotnet build 278224 (#79822) [main] Source code updates from dotnet/dotnet --------- Co-authored-by: Cyrus Najmabadi <[email protected]> Co-authored-by: Jan Jones <[email protected]> Co-authored-by: David Barbet <[email protected]> Co-authored-by: Tomas Matousek <[email protected]> Co-authored-by: [email protected] <[email protected]> Co-authored-by: Jason Malinowski <[email protected]> Co-authored-by: dotnet-maestro[bot] <dotnet-maestro[bot]@users.noreply.github.com> Co-authored-by: David Wengier <[email protected]> Co-authored-by: Stuart Lang <[email protected]> Co-authored-by: Cyrus Najmabadi <[email protected]> Co-authored-by: Gen Lu <[email protected]> Co-authored-by: Todd Grunke <[email protected]> Co-authored-by: RaymondY <[email protected]> Co-authored-by: Tomáš Matoušek <[email protected]> Co-authored-by: Gobind Singh <[email protected]> Co-authored-by: Rikki Gibson <[email protected]> Co-authored-by: Joey Robichaud <[email protected]> Co-authored-by: dotnet-maestro[bot] <42748379+dotnet-maestro[bot]@users.noreply.github.com> Co-authored-by: Joey Robichaud <[email protected]> Co-authored-by: David Kean <[email protected]> Co-authored-by: DoctorKrolic <[email protected]> Co-authored-by: DoctorKrolic <[email protected]> Co-authored-by: AlekseyTs <[email protected]> Co-authored-by: David Barbet <[email protected]> Co-authored-by: Rikki Gibson <[email protected]> Co-authored-by: Jared Parsons <[email protected]> Co-authored-by: Julien Couvreur <[email protected]>
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.
This can be just allocated directly into an array instead of fiddling with a pool. Simpler and more efficient.
This shows up as 2.5% of allocations in a scenario in a scenario in RazorEditingTest.CompletionInCohosting for the Roslyn OOP. Most of that is due to the list resizing.