Skip to content

PooledObjects cleanup #78382

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 3 commits into from
May 8, 2025
Merged

PooledObjects cleanup #78382

merged 3 commits into from
May 8, 2025

Conversation

tmat
Copy link
Member

@tmat tmat commented Apr 29, 2025

Further work to consolidate and simplify extensions and source packages.

Moves most ArrayBuilder extension methods to ArrayBuilder.
Removes duplicate helpers.
Renames helpers for consistency.
Adds IPooled to PooledHashSet, PooledDictionary and PooledStringBuilder using the same pattern as ArrayBuilder.

Follow up: #78388

@tmat tmat requested review from a team as code owners April 29, 2025 23:21
@ghost ghost added Area-Compilers untriaged Issues and PRs which have not yet been triaged by a lead labels Apr 29, 2025
@tmat
Copy link
Member Author

tmat commented Apr 29, 2025

@CyrusNajmabadi @jaredpar @dotnet/roslyn-compiler @dotnet/roslyn-ide

@jaredpar
Copy link
Member

jaredpar commented May 7, 2025

@dotnet/roslyn-compiler PTAL

@tmat
Copy link
Member Author

tmat commented May 8, 2025

/azp run

Copy link

Azure Pipelines will not run the associated pipelines, because the pull request was updated after the run command was issued. Review the pull request again and issue a new run command.

@tmat tmat merged commit 1b379e0 into dotnet:main May 8, 2025
27 of 28 checks passed
@tmat tmat deleted the Extensions2 branch May 8, 2025 21:48
@dotnet-policy-service dotnet-policy-service bot added this to the Next milestone May 8, 2025
333fred added a commit that referenced this pull request May 29, 2025
* Add XML Solution (SLNX) support to MSBuildWorkspace

* Update `AbstractImplementAbstractClassCodeFixProvider.cs` source

Signed-off-by: Emmanuel Ferdman <[email protected]>

* IN progress

* IN progress

* in progress

* in progress

* Simplify

* Simplify

* Cleanup

* Rename

* Push through initial description

* Rename

* Removed

* Add back disposal

* Linked source

* Rename

* Update docs

* Remove

* Add comment

* Add comment

* Fix

* Delay loading of CodeRefactoringProvider's till absolutely needed

* CR feedback and tests.

* Fix comment

* CR feedback

* Fix crash in 'introduce variable' on top-level statements

* CR feedback

* Seal certain types in LSP layer

* Seal certain types in LSP layer

* Publish PR validation to internal feeds

* PooledObjects cleanup (#78382)

* moved condition above top-level statements

* REvert

* Seal

* Gracefully handle span mapping failing

* Fix information logs getting logged as debug in VSCode

* Don't unnecessarily create a document when only the document state is needed. (#78463)

Cyrus noticed this during a chat where I was considering a larger change to this code. Might as well make the simple/safe change while I consider the larger change too.

Other small changes:
1) Early exit if no changed documents
2) Unrelated change adding Checksum overload as I noticed a high frequency caller during solution load (ProjectSystemProjectOptionsProcessor.ReparseCommandLineIfChanged_NoLock) had an ImmutableArray and it was getting boxed and enumerator alloced.

* Ensure loghub collects the now verbose level logs

* Adjust some more logging messages

* Cancel running requests when the connection terminates

* Shorten log category name

* Extensions: handle extensions in VB SymbolDisplay (#78512)

* Switch to non-scouting queue to unblock CI (#78521)

* Fix test helper message to avoid NRE (#78532)

* Collect data about which code fixes end up making changes that conflict with other code fixes

* docs

* Revert

* Source package fixes (#78534)

* Extract helper to make it easier for other features to request copilot analysis

* lint

* lint

* Extensions: only count extensions for determining identifier (#78523)

* Cache extension method import info per project ID

* Remove telemetry

* Use pattern matching

* Lint

* Add VB ref assembly to semantic search (#78537)

* Add support for FieldRva to EnC delta (#78033)

* [main] Source code updates from dotnet/dotnet (#78527)

* [VMR] Codeflow c53bdd8-c53bdd8

[[ commit created by automation ]]

* Update dependencies from https://github.com/dotnet/dotnet build 267468

* [VMR] Codeflow b422f78-b422f78

[[ commit created by automation ]]

* Update dependencies from https://github.com/dotnet/dotnet build 267725

* Update dependencies from https://github.com/dotnet/dotnet build 267776

---------

Co-authored-by: dotnet-maestro[bot] <dotnet-maestro[bot]@users.noreply.github.com>

* Introduce -productBuild and -sourceBuild switches (#78519)

* Introduce -productBuild and -sourceBuild switches

* Update eng/build.sh

Co-authored-by: Viktor Hofer <[email protected]>

* Update build.sh

* Add new options for SB and PB switches

* Remove leftover SB arg

---------

Co-authored-by: Viktor Hofer <[email protected]>

* Document more steps in our C# release process (#78539)

* Extensions: analyzer actions (#78319)

* Update XAML EA to use DocumentUri instead of System.Uri

* Update field value for NavBar Integration Tests (#78553)

* fix

* fix

* Update GetFirstRelatedDocumentId to not return documents with the same path within the same project (#78475)

GetFirstRelatedDocumentId had a mismatch in behavior for files within the same project that have the same path (shouldn't really happen, would cause compile errors). Previously, the code would return files from the same proejct when searching the cache, but not when doing the linear walk. Talked with Jason, and he indicated that it was better to have the code not return files from the same project, as callers wouldn't expect that.

* Implement PDG.WithProjectsRemoved (#78428)

This allows batch removal of projects from the ProjectDependencyGraph. This should eventually give us a small perf benefit, but only at the point where our projects can be disposed (or processed) in batches, which is not something we currently do.

* Selectively persist the commandline to temporary storage (#78441)

Only write command lines to temporary storage when they may later be needed.

Persisting project command line arguments is surprisingly costly, as evidenced by the profile image below. As this storage is only needed when there is an effective ruleset path, we can limit when we persist to storage to be limited to that scenario.

See PR for detailed performance information.

* File based programs IDE support (#78488)

Co-authored-by: Cyrus Najmabadi <[email protected]>
Co-authored-by: Jason Malinowski <[email protected]>

* Update dependencies from https://github.com/dotnet/arcade build 20250513.2

Microsoft.DotNet.Arcade.Sdk , Microsoft.DotNet.Helix.Sdk , Microsoft.DotNet.XliffTasks
 From Version 9.0.0-beta.25255.5 -> To Version 9.0.0-beta.25263.2

* Extensions: pattern-based constructs (#78480)

* Unset CompilerType from Csc (#78483)

* Unset CompilerType from Csc

* Remove CompilerType logic

* Add Microsoft.CodeAnalysis.Extensions package (#78388)

* Fix MoveType to create debuggable documents (#78554)

* Move SpecializedCollections to Microsoft.CodeAnalysis.Collections namespace (#78466)

* Add a reference to the tracking bug for a workaround

* Fix embedded language classification inside multi-line string

* Remove 'Opt' suffixes

* Unify resource strings for special case checks

* NRT

* mIn progress

* Simplify

* Simplify

* Remove localized strings from Collections source package (#78576)

* Revert "Update XAML EA to use DocumentUri instead of System.Uri (#78555)"

This reverts commit 2611c9a, reversing
changes made to f4e6964.

* Implement `Enum.IsDefined` check refactoring

* Rename LSP messages for VisualStudio.Extensibility integration (#78598)

Co-authored-by: Matteo Prosperi <[email protected]>

* More improvements to source packages (#78587)

* Editor configs for source packages

* Add nullable enable

* Include linked files in source packages

* Suppress RS1024

* Remove duplicate nullability directives

* Remove unnecessary extensions

* Move FatalError and FailFast to Contracts package

* Add disclaimer

* Ensure we pass unique binlog paths to each BuildHost

Otherwise if we launch multiple processes, they might step atop
each other and cause locking issues.

* Clean up our SpellingExclusions

Somehow almost every line started with a UTF-8 BOM. This fixes it.

* Track used assemblies of data section string literals (#78552)

* Track used assemblies of data section string literals

* Move reporting to module

* Fix a failing test

* Improve test

* Add a comment

* Support local functions in breadcrumbs

* hotfix to fix restore and stop including bin/obj artifacts in directory with loose files (#78615)

* Fix angle brackets in generics in hover

* more directly walk the tree for local functions and add tests

* Give .NET Framework build task in sdk different identity (#78584)

closes #78565

* [VMR] Codeflow 015a854-015a854

[[ commit created by automation ]]

* Update dependencies from https://github.com/dotnet/dotnet build 268651
No dependency updates to commit

* [VMR] Codeflow 604dfc7-170498a

[[ commit created by automation ]]

* Update dependencies from https://github.com/dotnet/dotnet build 268722
No dependency updates to commit

* review feedback

* Fix option reading

* Simplify equivalence keys

* Make refactoring resilient against absence of `InvalidEnumArgumentException` type

* Fix formatting

* Update Workspace.MSBuild to reference Microsoft.Build.Framework.

* LSP: Fix batch builds for file-based programs and fix `"dotnet.projects.binaryLogPath"` throwing an exception (#78644)

Co-authored-by: Joey Robichaud <[email protected]>

* Convert operand of `true`/`false` operator invoked as part of `&&`/`||`operator evaluation (#78629)

Fixes #78609.

* Extensions: rename syntax node (#78625)

* Simplify

* Use SolutionFileReader instead of calling out to BuildHost

Removes the GetProjectsInSolution BuildHost API. Instead we can use the SolutionPersister library in processes to parse the projects from a solution file.

* StaticLogMessage isn't pooling correctly

Noticed this when looking at a profile and was curious why there were 3 MB of allocations of the StaticLogMessage type.

LogMessage.Free sets _message to null before calling FreeCore, thus StaticLogMessage.FreeCore was never adding items back to it's pool. The ordering of LogMessage setting _message to null and calling FreeCore is important, so that can't be changed. Instead, add a flag to StaticLogMessage to indicate whether it's in a constructed state or not.

* Fixing symbol publishing (#78650)

These need to be manually listed given that they aren't published directly as a NuPkg

* Compare symbols by metadata name when searching for eqivalent symbols in FAR engine

* Extensions: disallow indexers (#78626)

* Add untriaged label to all new issues (#78658)

* Update resourceManagement.yml

* Update resourceManagement.yml

* Fixed crash in CSharpConvertToRecordRefactoringProvider.

* Return arity check for method symbols

* Fix doc comment

* Update src/Features/Core/Portable/SymbolSearch/SymbolSearchOptions.cs

* Fix

* Make solution file reading async

* Fix storage

* Hot Reload: Handle document deletes (#78516)

* Use SBRP version of M.CA for analyzers in the context of source build (#78668)

* fix symbol publishing? (#78671)

* Cleanup

* Use ThrowIfFalse when resolving paths

* Add test assertions

* Applied code review suggestions.

* Update Microsoft.Build.Tasks.CodeAnalysis.Sdk.csproj (#78681)

* Fix nullable crash for field keyword in partial property (#78672)

* Use VerifyDiagnostics format to report CompileAndVerify diagnostics (#78666)

* Revert "Update Microsoft.Build.Tasks.CodeAnalysis.Sdk.csproj (#78681)" (#78687)

This reverts commit 6d490de.

* Revert "fix symbol publishing? (#78671)" (#78686)

This reverts commit 2b6024e.

* Add comment

* [main] Source code updates from dotnet/dotnet (#78663)

* [VMR] Codeflow c3c7ad6-c3c7ad6

[[ commit created by automation ]]

* Update dependencies from https://github.com/dotnet/dotnet build 268973
No dependency updates to commit

* [VMR] Codeflow d219c4f-d219c4f

[[ commit created by automation ]]

* Update dependencies from https://github.com/dotnet/dotnet build 269082
No dependency updates to commit

---------

Co-authored-by: dotnet-maestro[bot] <dotnet-maestro[bot]@users.noreply.github.com>

* Add a test to verify that behavior is expected and bug no longer occurs

* [main] Source code updates from dotnet/dotnet (#78692)

* [VMR] Codeflow 2b6024e-2b6024e

[[ commit created by automation ]]

* Update dependencies from https://github.com/dotnet/dotnet build 269352
No dependency updates to commit

---------

Co-authored-by: dotnet-maestro[bot] <dotnet-maestro[bot]@users.noreply.github.com>

* Modify ABWQ.AddWork to take in a ROS instead of an IEnumerable (#78691)

I'm seeing this enumerator allocation as account for 50 MB (0.9%) in a customer trace for feedback ticket https://developercommunity.visualstudio.com/t/Razor-Development:-30-Second-CPU-Churn-A/10904811

This shows up a bit smaller in the C# editing speedometer test, but does show as 2.3 MB. Test insertion showed allocations were removed.

See PR for more numbers and speedometer test results.

* Add assert

* Add test

* Add support for 'fixed'

* Fix crash with introduce local within a compilation unit

* Sort

* Handle error case: static ref field (No NullReferenceException) (#78707)

* Organize

* Run on UI thread

* fix: RS2007 table borders

* [main] Source code updates from dotnet/dotnet (#78710)

* [VMR] Codeflow 533fde8-533fde8

[[ commit created by automation ]]

* Update dependencies from https://github.com/dotnet/dotnet build 269499
No dependency updates to commit

* Update dependencies from https://github.com/dotnet/dotnet build 269610
No dependency updates to commit

---------

Co-authored-by: dotnet-maestro[bot] <dotnet-maestro[bot]@users.noreply.github.com>

* Make operator glyph consistent witht eh rest of the members

* Fix

* avoid not needed compilationUnit clone (#78676)

* Always log PID

* Update MicrosoftVisualStudioExtensibilityTestingVersion (#78661)

* update extensibility version

* remove added package?

* update minversion

* test

* update vswhere version

* update versions.props vswhere

* update min version

* update coreeditor version ranges

* Add IsNull property to IAsyncToken (#78718)

* Extensions: nullability analysis for property access (#78646)

* Add helper for mapping between documents and hierarchy+itemids

* Updat epersistence version

* Do not convert tuple expression with error to a bad expression too early (#78645)

* [main] Source code updates from dotnet/dotnet (#78724)

* [VMR] Codeflow f5705c8-57b0396

[[ commit created by automation ]]

* Update dependencies from https://github.com/dotnet/dotnet build 269724
No dependency updates to commit

---------

Co-authored-by: dotnet-maestro[bot] <dotnet-maestro[bot]@users.noreply.github.com>

* [main] Update dependencies from dotnet/arcade (#78578)

* Update dependencies from https://github.com/dotnet/arcade build 20250513.5

Microsoft.DotNet.Arcade.Sdk , Microsoft.DotNet.Helix.Sdk , Microsoft.DotNet.XliffTasks
 From Version 9.0.0-beta.25263.2 -> To Version 9.0.0-beta.25263.5

* Update dependencies from https://github.com/dotnet/arcade build 20250516.2

Microsoft.DotNet.Arcade.Sdk , Microsoft.DotNet.Helix.Sdk , Microsoft.DotNet.XliffTasks
 From Version 9.0.0-beta.25263.2 -> To Version 9.0.0-beta.25266.2

* Update dependencies from https://github.com/dotnet/arcade build 2025052.1

Microsoft.DotNet.Arcade.Sdk , Microsoft.DotNet.Helix.Sdk , Microsoft.DotNet.XliffTasks
 From Version 9.0.0-beta.25263.2 -> To Version 9.0.0-beta.25271.1

---------

Co-authored-by: dotnet-maestro[bot] <dotnet-maestro[bot]@users.noreply.github.com>

* Invoke `dotnet run-api` to obtain virtual project (#78648)

Co-authored-by: David Barbet <[email protected]>

---------

Signed-off-by: Emmanuel Ferdman <[email protected]>
Co-authored-by: Deepak Rathore (ALLYIS INC) <[email protected]>
Co-authored-by: Emmanuel Ferdman <[email protected]>
Co-authored-by: Cyrus Najmabadi <[email protected]>
Co-authored-by: Cyrus Najmabadi <[email protected]>
Co-authored-by: Evgeny Tvorun <[email protected]>
Co-authored-by: Victor Pogor <[email protected]>
Co-authored-by: David Barbet <[email protected]>
Co-authored-by: Tomáš Matoušek <[email protected]>
Co-authored-by: Jason Malinowski <[email protected]>
Co-authored-by: Andrew Hall <[email protected]>
Co-authored-by: Todd Grunke <[email protected]>
Co-authored-by: Julien Couvreur <[email protected]>
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: Joey Robichaud <[email protected]>
Co-authored-by: Ella Hathaway <[email protected]>
Co-authored-by: Viktor Hofer <[email protected]>
Co-authored-by: Ankita Khera <[email protected]>
Co-authored-by: Rikki Gibson <[email protected]>
Co-authored-by: Jason Malinowski <[email protected]>
Co-authored-by: Jan Jones <[email protected]>
Co-authored-by: DoctorKrolic <[email protected]>
Co-authored-by: Matteo Prosperi <[email protected]>
Co-authored-by: Matteo Prosperi <[email protected]>
Co-authored-by: Jared Parsons <[email protected]>
Co-authored-by: Joey Robichaud <[email protected]>
Co-authored-by: AlekseyTs <[email protected]>
Co-authored-by: John Douglas Leitch <[email protected]>
Co-authored-by: Matt Thalman <[email protected]>
Co-authored-by: Bernd Baumanns <[email protected]>
Co-authored-by: Thomas Shephard <[email protected]>
Co-authored-by: DoctorKrolic <[email protected]>
Co-authored-by: David Barbet <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-Compilers untriaged Issues and PRs which have not yet been triaged by a lead VSCode
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants