Skip to content

Commit 0f8b66d

Browse files
authored
Avoid boxing and unnecessary array allocations (#12175)
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.
1 parent d07a4e8 commit 0f8b66d

File tree

3 files changed

+151
-8
lines changed

3 files changed

+151
-8
lines changed

src/Build/BackEnd/Components/BuildRequestEngine/BuildRequestEngine.cs

Lines changed: 97 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -343,7 +343,7 @@ public void SubmitBuildRequest(BuildRequest request)
343343
QueueAction(
344344
() =>
345345
{
346-
ErrorUtilities.VerifyThrow(_status != BuildRequestEngineStatus.Shutdown && _status != BuildRequestEngineStatus.Uninitialized, "Engine loop not yet started, status is {0}.", _status);
346+
ErrorUtilities.VerifyThrow(_status != BuildRequestEngineStatus.Shutdown && _status != BuildRequestEngineStatus.Uninitialized, "Engine loop not yet started, status is {0}.", _status.Box());
347347
TraceEngine("Request {0}({1}) (nr {2}) received and activated.", request.GlobalRequestId, request.ConfigurationId, request.NodeRequestId);
348348

349349
ErrorUtilities.VerifyThrow(!_requestsByGlobalRequestId.ContainsKey(request.GlobalRequestId), "Request {0} is already known to the engine.", request.GlobalRequestId);
@@ -363,7 +363,7 @@ public void SubmitBuildRequest(BuildRequest request)
363363
config.RetrieveFromCache();
364364
((IBuildResults)resultToReport).SavedCurrentDirectory = config.SavedCurrentDirectory;
365365
((IBuildResults)resultToReport).SavedEnvironmentVariables = config.SavedEnvironmentVariables;
366-
if (!request.BuildRequestDataFlags.HasFlag(BuildRequestDataFlags.IgnoreExistingProjectState))
366+
if ((request.BuildRequestDataFlags & BuildRequestDataFlags.IgnoreExistingProjectState) != BuildRequestDataFlags.IgnoreExistingProjectState)
367367
{
368368
resultToReport.ProjectStateAfterBuild = config.Project;
369369
}
@@ -414,7 +414,7 @@ public void UnblockBuildRequest(BuildRequestUnblocker unblocker)
414414
QueueAction(
415415
() =>
416416
{
417-
ErrorUtilities.VerifyThrow(_status != BuildRequestEngineStatus.Shutdown && _status != BuildRequestEngineStatus.Uninitialized, "Engine loop not yet started, status is {0}.", _status);
417+
ErrorUtilities.VerifyThrow(_status != BuildRequestEngineStatus.Shutdown && _status != BuildRequestEngineStatus.Uninitialized, "Engine loop not yet started, status is {0}.", _status.Box());
418418
ErrorUtilities.VerifyThrow(_requestsByGlobalRequestId.ContainsKey(unblocker.BlockedRequestId), "Request {0} is not known to the engine.", unblocker.BlockedRequestId);
419419
BuildRequestEntry entry = _requestsByGlobalRequestId[unblocker.BlockedRequestId];
420420

@@ -467,7 +467,11 @@ public void UnblockBuildRequest(BuildRequestUnblocker unblocker)
467467
}
468468
else
469469
{
470-
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);
470+
// PERF: Explicitly check the debug flag here so that we don't pay the cost for getting OverallResult
471+
if (_debugDumpState)
472+
{
473+
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);
474+
}
471475

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

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

1452-
/// <summary>
1453-
/// Method used for debugging purposes.
1454-
/// </summary>
1456+
private void TraceEngine(string format, ulong arg)
1457+
{
1458+
if (_debugDumpState)
1459+
{
1460+
TraceEngine(format, [arg]);
1461+
}
1462+
}
1463+
1464+
private void TraceEngine(string format, int arg)
1465+
{
1466+
if (_debugDumpState)
1467+
{
1468+
TraceEngine(format, [arg]);
1469+
}
1470+
}
1471+
1472+
private void TraceEngine(string format, int arg1, BuildRequestEngineStatus arg2)
1473+
{
1474+
if (_debugDumpState)
1475+
{
1476+
TraceEngine(format, [arg1, arg2.Box()]);
1477+
}
1478+
}
1479+
1480+
private void TraceEngine(string format, int arg1, int arg2)
1481+
{
1482+
if (_debugDumpState)
1483+
{
1484+
TraceEngine(format, [arg1, arg2]);
1485+
}
1486+
}
1487+
1488+
private void TraceEngine(string format, ulong arg1, ulong arg2)
1489+
{
1490+
if (_debugDumpState)
1491+
{
1492+
TraceEngine(format, [arg1, arg2]);
1493+
}
1494+
}
1495+
1496+
private void TraceEngine(string format, int arg1, int arg2, int arg3)
1497+
{
1498+
if (_debugDumpState)
1499+
{
1500+
TraceEngine(format, [arg1, arg2, arg3]);
1501+
}
1502+
}
1503+
1504+
private void TraceEngine(string format, int arg1, int arg2, string arg3)
1505+
{
1506+
if (_debugDumpState)
1507+
{
1508+
TraceEngine(format, [arg1, arg2, arg3]);
1509+
}
1510+
}
1511+
1512+
private void TraceEngine(string format, int arg1, int arg2, int arg3, string arg4)
1513+
{
1514+
if (_debugDumpState)
1515+
{
1516+
TraceEngine(format, [arg1, arg2, arg3, arg4]);
1517+
}
1518+
}
1519+
1520+
private void TraceEngine(string format, int arg1, int arg2, int arg3, BuildRequestEntryState arg4)
1521+
{
1522+
if (_debugDumpState)
1523+
{
1524+
TraceEngine(format, [arg1, arg2, arg3, arg4]);
1525+
}
1526+
}
1527+
1528+
private void TraceEngine(string format, int arg1, int arg2, int arg3, int arg4, int arg5)
1529+
{
1530+
if (_debugDumpState)
1531+
{
1532+
TraceEngine(format, [arg1, arg2, arg3, arg4, arg5]);
1533+
}
1534+
}
1535+
1536+
private void TraceEngine(string format, int arg1, int arg2, int arg3, int arg4, BuildResultCode arg5)
1537+
{
1538+
if (_debugDumpState)
1539+
{
1540+
TraceEngine(format, [arg1, arg2, arg3, arg4, arg5]);
1541+
}
1542+
}
1543+
14551544
private void TraceEngine(string format, params object[] stuff)
14561545
{
14571546
if (_debugDumpState)

src/Build/BackEnd/Components/BuildRequestEngine/IBuildRequestEngine.cs

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -86,6 +86,38 @@ internal enum BuildRequestEngineStatus
8686
Shutdown
8787
}
8888

89+
/// <summary>
90+
/// Provides boxed representations of the <see cref="BuildRequestEngineStatus"/> enumeration values.
91+
/// </summary>
92+
/// <remarks>This class offers pre-boxed objects for each status value to avoid repeated allocations due to boxing
93+
/// when frequently accessing these status values. It also includes an extension method to retrieve the
94+
/// boxed object corresponding to a given <see cref="BuildRequestEngineStatus"/>.</remarks>
95+
internal static class BuildRequestEngineStatusBoxes
96+
{
97+
public static readonly object UninitializedBox = BuildRequestEngineStatus.Uninitialized;
98+
public static readonly object IdleBox = BuildRequestEngineStatus.Idle;
99+
public static readonly object ActiveBox = BuildRequestEngineStatus.Active;
100+
public static readonly object WaitingBox = BuildRequestEngineStatus.Waiting;
101+
public static readonly object ShutdownBox = BuildRequestEngineStatus.Shutdown;
102+
103+
/// <summary>
104+
/// Gets the canonical boxed object for the specified <see cref="BuildRequestEngineStatus"/>.
105+
/// </summary>
106+
/// <param name="status">The desired <see cref="BuildRequestEngineStatus"/>.</param>
107+
/// <returns>The boxed <see cref="BuildRequestEngineStatus"/>.</returns>
108+
/// <exception cref="ArgumentOutOfRangeException">Thrown when <paramref name="status"/> is outside the range of
109+
/// defined <see cref="BuildRequestEngineStatus"/> values.</exception>
110+
public static object Box(this BuildRequestEngineStatus status) => status switch
111+
{
112+
BuildRequestEngineStatus.Uninitialized => UninitializedBox,
113+
BuildRequestEngineStatus.Idle => IdleBox,
114+
BuildRequestEngineStatus.Active => ActiveBox,
115+
BuildRequestEngineStatus.Waiting => WaitingBox,
116+
BuildRequestEngineStatus.Shutdown => ShutdownBox,
117+
_ => throw new ArgumentOutOfRangeException(nameof(status)),
118+
};
119+
}
120+
89121
/// <summary>
90122
/// Objects implementing this interface may be used by a Node to process build requests
91123
/// and generate build results.

src/Shared/ErrorUtilities.cs

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -196,6 +196,17 @@ internal static void VerifyThrow([DoesNotReturnIf(false)] bool condition, string
196196
}
197197
}
198198

199+
/// <summary>
200+
/// Overload for one string format argument.
201+
/// </summary>
202+
internal static void VerifyThrow([DoesNotReturnIf(false)] bool condition, string unformattedMessage, int arg0)
203+
{
204+
if (!condition)
205+
{
206+
ThrowInternalError(unformattedMessage, arg0);
207+
}
208+
}
209+
199210
/// <summary>
200211
/// Overload for one string format argument.
201212
/// </summary>
@@ -207,6 +218,17 @@ internal static void VerifyThrow([DoesNotReturnIf(false)] bool condition, string
207218
}
208219
}
209220

221+
/// <summary>
222+
/// Overload for two string format arguments.
223+
/// </summary>
224+
internal static void VerifyThrow([DoesNotReturnIf(false)] bool condition, string unformattedMessage, int arg0, int arg1)
225+
{
226+
if (!condition)
227+
{
228+
ThrowInternalError(unformattedMessage, arg0, arg1);
229+
}
230+
}
231+
210232
/// <summary>
211233
/// Overload for two string format arguments.
212234
/// </summary>

0 commit comments

Comments
 (0)