Skip to content

Avoid boxing and unnecessary array allocations #12175

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.

Sign up for GitHub

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
rainersigwald merged 5 commits into from
Aug 19, 2025
Merged

Avoid boxing and unnecessary array allocations #12175

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
e3074de
Avoid boxing and unnecessary array allocations
Erarndt Jul 15, 2025
1a1d354
More fixes
Erarndt Jul 15, 2025
c273480
PR feedback
Erarndt Jul 16, 2025
33f3af0
Merge branch 'main' of https://github.com/dotnet/msbuild into dev/era…
Erarndt Aug 18, 2025
748bc1d
Merge branch 'main' into dev/erarndt/requestStatusBoxing
surayya-MS Aug 19, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
105 changes: 97 additions & 8 deletions src/Build/BackEnd/Components/BuildRequestEngine/BuildRequestEngine.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -343,7 +343,7 @@ public void SubmitBuildRequest(BuildRequest request)
QueueAction(
() =>
{
ErrorUtilities.VerifyThrow(_status != BuildRequestEngineStatus.Shutdown && _status != BuildRequestEngineStatus.Uninitialized, "Engine loop not yet started, status is {0}.", _status);
ErrorUtilities.VerifyThrow(_status != BuildRequestEngineStatus.Shutdown && _status != BuildRequestEngineStatus.Uninitialized, "Engine loop not yet started, status is {0}.", _status.Box());
TraceEngine("Request {0}({1}) (nr {2}) received and activated.", request.GlobalRequestId, request.ConfigurationId, request.NodeRequestId);

ErrorUtilities.VerifyThrow(!_requestsByGlobalRequestId.ContainsKey(request.GlobalRequestId), "Request {0} is already known to the engine.", request.GlobalRequestId);
Expand All @@ -363,7 +363,7 @@ public void SubmitBuildRequest(BuildRequest request)
config.RetrieveFromCache();
((IBuildResults)resultToReport).SavedCurrentDirectory = config.SavedCurrentDirectory;
((IBuildResults)resultToReport).SavedEnvironmentVariables = config.SavedEnvironmentVariables;
if (!request.BuildRequestDataFlags.HasFlag(BuildRequestDataFlags.IgnoreExistingProjectState))
if ((request.BuildRequestDataFlags & BuildRequestDataFlags.IgnoreExistingProjectState) != BuildRequestDataFlags.IgnoreExistingProjectState)
{
resultToReport.ProjectStateAfterBuild = config.Project;
}
Expand Down Expand Up @@ -414,7 +414,7 @@ public void UnblockBuildRequest(BuildRequestUnblocker unblocker)
QueueAction(
() =>
{
ErrorUtilities.VerifyThrow(_status != BuildRequestEngineStatus.Shutdown && _status != BuildRequestEngineStatus.Uninitialized, "Engine loop not yet started, status is {0}.", _status);
ErrorUtilities.VerifyThrow(_status != BuildRequestEngineStatus.Shutdown && _status != BuildRequestEngineStatus.Uninitialized, "Engine loop not yet started, status is {0}.", _status.Box());
ErrorUtilities.VerifyThrow(_requestsByGlobalRequestId.ContainsKey(unblocker.BlockedRequestId), "Request {0} is not known to the engine.", unblocker.BlockedRequestId);
BuildRequestEntry entry = _requestsByGlobalRequestId[unblocker.BlockedRequestId];

Expand Down Expand Up @@ -467,7 +467,11 @@ public void UnblockBuildRequest(BuildRequestUnblocker unblocker)
}
else
{
TraceEngine("Request {0}({1}) (nr {2}) is no longer waiting on nr {3} (UBR). Results are {4}.", entry.Request.GlobalRequestId, entry.Request.ConfigurationId, entry.Request.NodeRequestId, result.NodeRequestId, result.OverallResult);
// PERF: Explicitly check the debug flag here so that we don't pay the cost for getting OverallResult
if (_debugDumpState)
{
TraceEngine("Request {0}({1}) (nr {2}) is no longer waiting on nr {3} (UBR). Results are {4}.", entry.Request.GlobalRequestId, entry.Request.ConfigurationId, entry.Request.NodeRequestId, result.NodeRequestId, result.OverallResult);
}

// Update the configuration with targets information, if we received any and didn't already have it.
if (result.DefaultTargets != null)
Expand Down Expand Up @@ -515,7 +519,7 @@ public void ReportConfigurationResponse(BuildRequestConfigurationResponse respon
QueueAction(
() =>
{
ErrorUtilities.VerifyThrow(_status != BuildRequestEngineStatus.Shutdown && _status != BuildRequestEngineStatus.Uninitialized, "Engine loop not yet started, status is {0}.", _status);
ErrorUtilities.VerifyThrow(_status != BuildRequestEngineStatus.Shutdown && _status != BuildRequestEngineStatus.Uninitialized, "Engine loop not yet started, status is {0}.", _status.Box());

TraceEngine("Received configuration response for node config {0}, now global config {1}.", response.NodeConfigurationId, response.GlobalConfigurationId);
ErrorUtilities.VerifyThrow(_componentHost != null, "No host object set");
Expand Down Expand Up @@ -1449,9 +1453,94 @@ private void QueueAction(Action action, bool isLastTask)
}
}

/// <summary>
/// Method used for debugging purposes.
/// </summary>
private void TraceEngine(string format, ulong arg)
{
if (_debugDumpState)
{
TraceEngine(format, [arg]);
}
}

private void TraceEngine(string format, int arg)
{
if (_debugDumpState)
{
TraceEngine(format, [arg]);
}
}

private void TraceEngine(string format, int arg1, BuildRequestEngineStatus arg2)
{
if (_debugDumpState)
{
TraceEngine(format, [arg1, arg2.Box()]);
}
}

private void TraceEngine(string format, int arg1, int arg2)
{
if (_debugDumpState)
{
TraceEngine(format, [arg1, arg2]);
}
}

private void TraceEngine(string format, ulong arg1, ulong arg2)
{
if (_debugDumpState)
{
TraceEngine(format, [arg1, arg2]);
}
}

private void TraceEngine(string format, int arg1, int arg2, int arg3)
{
if (_debugDumpState)
{
TraceEngine(format, [arg1, arg2, arg3]);
}
}

private void TraceEngine(string format, int arg1, int arg2, string arg3)
{
if (_debugDumpState)
{
TraceEngine(format, [arg1, arg2, arg3]);
}
}

private void TraceEngine(string format, int arg1, int arg2, int arg3, string arg4)
{
if (_debugDumpState)
{
TraceEngine(format, [arg1, arg2, arg3, arg4]);
}
}

private void TraceEngine(string format, int arg1, int arg2, int arg3, BuildRequestEntryState arg4)
{
if (_debugDumpState)
{
TraceEngine(format, [arg1, arg2, arg3, arg4]);
}
}

private void TraceEngine(string format, int arg1, int arg2, int arg3, int arg4, int arg5)
{
if (_debugDumpState)
{
TraceEngine(format, [arg1, arg2, arg3, arg4, arg5]);
}
}

private void TraceEngine(string format, int arg1, int arg2, int arg3, int arg4, BuildResultCode arg5)
{
if (_debugDumpState)
{
TraceEngine(format, [arg1, arg2, arg3, arg4, arg5]);
}
}

private void TraceEngine(string format, params object[] stuff)
{
if (_debugDumpState)
Expand Down
32 changes: 32 additions & 0 deletions src/Build/BackEnd/Components/BuildRequestEngine/IBuildRequestEngine.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,38 @@ internal enum BuildRequestEngineStatus
Shutdown
}

/// <summary>
/// Provides boxed representations of the <see cref="BuildRequestEngineStatus"/> enumeration values.
/// </summary>
/// <remarks>This class offers pre-boxed objects for each status value to avoid repeated allocations due to boxing
/// when frequently accessing these status values. It also includes an extension method to retrieve the
/// boxed object corresponding to a given <see cref="BuildRequestEngineStatus"/>.</remarks>
internal static class BuildRequestEngineStatusBoxes
{
public static readonly object UninitializedBox = BuildRequestEngineStatus.Uninitialized;
public static readonly object IdleBox = BuildRequestEngineStatus.Idle;
public static readonly object ActiveBox = BuildRequestEngineStatus.Active;
public static readonly object WaitingBox = BuildRequestEngineStatus.Waiting;
public static readonly object ShutdownBox = BuildRequestEngineStatus.Shutdown;

/// <summary>
/// Gets the canonical boxed object for the specified <see cref="BuildRequestEngineStatus"/>.
/// </summary>
/// <param name="status">The desired <see cref="BuildRequestEngineStatus"/>.</param>
/// <returns>The boxed <see cref="BuildRequestEngineStatus"/>.</returns>
/// <exception cref="ArgumentOutOfRangeException">Thrown when <paramref name="status"/> is outside the range of
/// defined <see cref="BuildRequestEngineStatus"/> values.</exception>
public static object Box(this BuildRequestEngineStatus status) => status switch
{
BuildRequestEngineStatus.Uninitialized => UninitializedBox,
BuildRequestEngineStatus.Idle => IdleBox,
BuildRequestEngineStatus.Active => ActiveBox,
BuildRequestEngineStatus.Waiting => WaitingBox,
BuildRequestEngineStatus.Shutdown => ShutdownBox,
_ => throw new ArgumentOutOfRangeException(nameof(status)),
};
}

/// <summary>
/// Objects implementing this interface may be used by a Node to process build requests
/// and generate build results.
Expand Down
22 changes: 22 additions & 0 deletions src/Shared/ErrorUtilities.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -196,6 +196,17 @@ internal static void VerifyThrow([DoesNotReturnIf(false)] bool condition, string
}
}

/// <summary>
/// Overload for one string format argument.
/// </summary>
internal static void VerifyThrow([DoesNotReturnIf(false)] bool condition, string unformattedMessage, int arg0)
{
if (!condition)
{
ThrowInternalError(unformattedMessage, arg0);
}
}

/// <summary>
/// Overload for one string format argument.
/// </summary>
Expand All @@ -207,6 +218,17 @@ internal static void VerifyThrow([DoesNotReturnIf(false)] bool condition, string
}
}

/// <summary>
/// Overload for two string format arguments.
/// </summary>
internal static void VerifyThrow([DoesNotReturnIf(false)] bool condition, string unformattedMessage, int arg0, int arg1)
{
if (!condition)
{
ThrowInternalError(unformattedMessage, arg0, arg1);
}
}

/// <summary>
/// Overload for two string format arguments.
/// </summary>
Expand Down