Skip to content

Conversation

ccastanedaucf
Copy link
Contributor

Context

Fixes an allocation regression in a VS C++ scenario due to allocating ProjectMetadataInstance when retrieving metadata values in BatchingEngine.

Here's the trace pulled from VcxprojReader when loading the UE52 test solution in VS:
image

Changes Made

This refactors various methods so that ProjectItemInstance.GetMetadataEscaped can directly retrieve the escaped string value without intermediate ProjectMetadataInstance allocations.

@Copilot Copilot AI review requested due to automatic review settings October 3, 2025 02:43
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR optimizes performance by eliminating unnecessary ProjectMetadataInstance allocations when retrieving metadata values in MSBuild's BatchingEngine. The changes refactor metadata retrieval methods to directly return escaped string values instead of creating intermediate ProjectMetadataInstance objects.

  • Refactored GetMetadataEscaped to use string-based methods instead of metadata instance objects
  • Updated GetItemDefinitionMetadata to return strings directly as GetItemDefinitionMetadataEscaped
  • Optimized IMetadataTable.GetEscapedValueIfPresent to avoid unnecessary metadata object creation

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
src/Build/Instance/ProjectItemInstance.cs Refactored metadata retrieval methods to work with escaped strings directly instead of allocating ProjectMetadataInstance objects
src/Build/Instance/ProjectItemDefinitionInstance.cs Optimized GetEscapedValueIfPresent to return string values directly from metadata dictionary

@ccastanedaucf
Copy link
Contributor Author

FYI the test failure is unrelated

�[37m    Microsoft.Build.Engine.UnitTests.BackEnd.TaskHostFactory_Tests.TaskNodesDieAfterBuild(taskHostFactorySpecified: False, envVariableSpecified: True) [PASS]
�[m�[30;1m      Output:
�[m�[37m        [02:55:29.980] Attached to Sidecar (PID 8174)
�[mUnhandled exception. System.InvalidOperationException: Process was not started by this object, so requested information cannot be determined.
   at System.Diagnostics.Process.EnsureState(State state)
   at System.Diagnostics.Process.get_ExitCode()
   at Microsoft.Build.Engine.UnitTests.BackEnd.TaskHostFactory_Tests.ProcessTracker.<>c__DisplayClass1_1.<AttachToProcess>b__0(Object sender, EventArgs e) in /home/vsts/work/1/s/src/Build.UnitTests/BackEnd/TaskHostFactory_Tests.cs:line 366
   at System.Diagnostics.Process.RaiseOnExited()
   at System.Diagnostics.Process.CompletionCallback(Object waitHandleContext, Boolean wasSignaled)
   at System.Threading.ExecutionContext.RunInternal(ExecutionContext executionContext, ContextCallback callback, Object state)
--- End of stack trace from previous location ---
   at System.Threading.ExecutionContext.RunInternal(ExecutionContext executionContext, ContextCallback callback, Object state)
   at System.Threading.CompleteWaitThreadPoolWorkItem.System.Threading.IThreadPoolWorkItem.Execute()
   at System.Threading.ThreadPoolWorkQueue.Dispatch()
   at System.Threading.PortableThreadPool.WorkerThread.WorkerThreadStart()
   at System.Threading.Thread.StartCallback()

@JanProvaznik JanProvaznik self-assigned this Oct 3, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants