insert without sidecar #12430
Merged
insert without sidecar #12430
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
Co-authored-by: dotnet-maestro[bot] <dotnet-maestro[bot]@users.noreply.github.com>
Updating binding redirects for workflow editor tools in VS. This allows building of workflow projects in VS. Updated binding redirects to 18.0. Applied fix locally and verified WF projects could build.
### Context On Windows, apps targeting the console subsystem, like `MSBuild.exe`, get [launched by default with a console](https://learn.microsoft.com/windows/console/creation-of-a-console), which on modern Windows means that it gets an attached `conhost.exe` process. This has been happening with our worker nodes, but isn't necessary and uses some resources. <img width="203" height="533" alt="Screenshot of Task Explorer showing a bunch of dotnet processes each with a conhost process child" src="https://github.com/user-attachments/assets/df753c95-4a3d-4c05-bbec-bbf8b8e4fd1a" /> ### Changes Made Pass `DETACHED_PROCESS` when we are already jumping through hoops to avoid creating console windows on Windows. ### Testing <img width="151" height="421" alt="Screenshot of Task Explorer showing a bunch of dotnet processes with no conhost children (and a couple of other processes)" src="https://github.com/user-attachments/assets/c028e050-b51f-4472-851c-bcfb616947cf" /> --------- Co-authored-by: Copilot <[email protected]>
Fixes # ### Context The existing implementation of `ExtractFunctionArguments()` can be streamlined to avoid some unnecessary allocations. Before: <img width="1413" height="133" alt="image" src="https://github.com/user-attachments/assets/31acb799-c193-4582-a953-763f7b500a59" /> After: <img width="1395" height="182" alt="image" src="https://github.com/user-attachments/assets/db8cf223-9942-4709-8358-799a8b1e51ed" /> ### Changes Made ### Testing ### Notes ---------
There are several delegates called during build that cause some sneaky allocations. For instance, there is boxing of the `BuildRequestEngineStatus` enum. The culprit is the `ErrorUtilities.VerifyThrow()` calls that take `object` as parameters for the formatted message. To make the call, the enum gets boxed even when the condition check returns `true`. Adding additional overloads that take the specific type lets multiple callers benefit, and I did some additional cleanup in the delegates to remove some allocations.
Just noticed that a recent change duplicates this instruction, so removing the duplicate.
Co-authored-by: Jan Provazník <[email protected]>
The implementation of `ConcurrentStack<T>` creates a new wrapper node
each time an item is pushed onto the stack
public void Push(T item)
{
Node node = new Node(item);
node.m_next = m_head;
if (Interlocked.CompareExchange(ref m_head, node, node.m_next) !=
node.m_next)
{
PushCore(node, node);
}
}
The creates an appreciable amount of allocations during build.
These allocations can almost entirely be eliminated by using a vanilla
generic `Stack<T>` with a lock.
Fixes # ### Context The usage pattern of `Dictionary<TKey, TValue>` in `Lookup` can cause a large number of allocations due to resizing. The dictionaries are often created in paths that add items one at a time. We can use available context to make better guesses about the desired size to reduce the number of allocations. Before: <img width="1318" height="120" alt="image" src="https://github.com/user-attachments/assets/7656b202-7439-4e32-9c64-28d0bf738740" /> After: <img width="1166" height="294" alt="image" src="https://github.com/user-attachments/assets/6935a568-4837-4412-8662-0149b19f5fc1" /> Where the allocations happen shift around a bit, but the total allocations drop roughly 100MB ### Changes Made ### Testing ### Notes ---------
Fixes # ### Context Part of GC cost is the amount of surviving objects during a collection. During solution load, this stack showed up as currently rooted items: <img width="1198" height="259" alt="image" src="https://github.com/user-attachments/assets/b524dded-4941-43b9-95e7-a0bf7d6fcab4" /> You can see that there are roughly 30MB worth of `object` instances being held onto by the tables collection inside of `ConcurrentDictionary`. Additionally, `ConcurrentDictionary` objects are significantly larger than a `Dictionary`. For instance: 10k of them when empty: <img width="1200" height="89" alt="image" src="https://github.com/user-attachments/assets/38d2cda3-5a27-4a27-b902-d0a8d20e07f4" /> 10k of them with one item: <img width="1258" height="158" alt="image" src="https://github.com/user-attachments/assets/6dd439ad-6689-4a09-9d1d-9f4fd8adda75" /> This is a size difference between 5x and 15x. The existing implementation can be swapped to just use a dictionary with a lock. ### Changes Made ### Testing ### Notes
Fixes # ### Context We can avoid several allocations by changing the usage of `ItemExpressionCapture`. Switching from a class to a struct avoids a large number of allocations, and we can further reduce the allocations using a struct-based enumerator rather than constructing a list. Before: <img width="830" height="79" alt="image" src="https://github.com/user-attachments/assets/d010528f-5cb2-4af5-8d7f-dd0d077ab3e3" /> After: <img width="832" height="65" alt="image" src="https://github.com/user-attachments/assets/44f5beef-f508-4bb3-a508-42a7044539a6" /> ### Changes Made ### Testing ### Notes
…12170) Fixes # ### Context The `BackingCollection` property checks the backing collection for `ICollection<T>`. <img width="638" height="241" alt="image" src="https://github.com/user-attachments/assets/9012ec98-7ddf-4ad4-8c5f-30c5e4520666" /> This implementation creates a list copy if the underlying backing type doesn't implement `ICollection<T>`, and `PropertyDictionary` and `ItemDictionary` are two types that fall into this copying behavior. <img width="899" height="218" alt="image" src="https://github.com/user-attachments/assets/049fd288-c1cb-4920-8e6d-a9bbd46a8a76" /> This can be fixed by having them implement `ICollection<T>`. I don't see any allocations in this path after the change. ### Changes Made ### Testing ### Notes
Co-authored-by: dotnet-maestro[bot] <dotnet-maestro[bot]@users.noreply.github.com>
Co-authored-by: dotnet-maestro[bot] <dotnet-maestro[bot]@users.noreply.github.com> Co-authored-by: Jan Provazník <[email protected]>
Co-authored-by: dotnet-maestro[bot] <dotnet-maestro[bot]@users.noreply.github.com>
…: Build ID 12255236 (#12395) This is the pull request automatically created by the OneLocBuild task in the build process to check-in localized files generated based upon translation source files (.lcl files) handed-back from the downstream localization pipeline. If there are issues in translations, visit https://aka.ms/icxLocBug and log bugs for fixes. The OneLocBuild wiki is https://aka.ms/onelocbuild and the localization process in general is documented at https://aka.ms/AllAboutLoc.
Fixes : Issue with darc publishing
…: Build ID 12264231 (#12421)
Contributor
There was a problem hiding this comment.
Pull Request Overview
This PR removes the deprecated "serial console logger" functionality from MSBuild and refactors to always use the parallel console logger. It also includes numerous performance optimizations for string handling, collections, and expression parsing throughout the codebase.
Key changes:
- Removes the serial console logger entirely and always uses the parallel console logger
- Optimizes string and collection handling for better performance
- Reduces memory allocations in various hot paths
- Improves telemetry and debugging capabilities
Reviewed Changes
Copilot reviewed 62 out of 62 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/Build/Logging/SerialConsoleLogger.cs | Complete removal of the serial console logger implementation |
| src/Build/Logging/ConsoleLogger.cs | Simplified to always use parallel logger, removed serial logger option |
| src/StringTools/WeakStringCache.cs | Refactored to eliminate dual string handling strategy |
| src/Build/Evaluation/ExpressionShredder.cs | Changed from List to struct-based enumerator for performance |
| src/Build/Evaluation/Expander.cs | Updated to work with new enumerator pattern |
| src/Shared/SolutionConfiguration.cs | Pre-sized dictionaries for better performance |
| Multiple xlf files | Removed references to DisableMPLogging parameter from localized strings |
| src/Framework/Telemetry/LoggingConfigurationTelemetry.cs | Removed serial/parallel logger type tracking |
Comments suppressed due to low confidence (2)
Member
|
please include this change |
YuliiaKovalova
approved these changes
Aug 27, 2025
rainersigwald
approved these changes
Aug 27, 2025
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.
Fixes #
Context
Changes Made
Testing
https://devdiv.visualstudio.com/DevDiv/_git/VS/pullrequest/665253
Notes