Skip to content

Commit e0ef001

Browse files
Perf: Use struct for WorkUnitResult (#12403)
### Context `TargetEntry.ExecuteTarget()` + `TaskBuilder.ExecuteTask()` allocate a large number of `WorkUnitResult` objects. Nearly all of these allocations are a result of aggregating results across item buckets, as this produces a new `WorkUnitResult` instance on every single merge. This looks like an easy case for switching to a struct, given these are already treated as immutable and used as value containers (apart from 1 diff here). Overall net ~40MB in this trace (after subtracting the small increase in types seen below + `TargetResult` + extra lock object). Before <img width="1307" height="396" alt="image" src="https://github.com/user-attachments/assets/199fe6cc-b05f-4310-a17b-01d64ce22ae1" /> After <img width="1319" height="425" alt="image" src="https://github.com/user-attachments/assets/fa5aa79c-4ff4-4e25-8cb1-34230f942e88" />
1 parent 8501997 commit e0ef001

4 files changed

Lines changed: 35 additions & 26 deletions

File tree

src/Build/BackEnd/Components/RequestBuilder/TargetEntry.cs

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -457,7 +457,6 @@ internal async Task ExecuteTarget(ITaskBuilder taskBuilder, BuildRequestEntry re
457457

458458
targetLoggingContext = projectLoggingContext.LogTargetBatchStarted(projectFullPath, _target, parentTargetName, _buildReason);
459459
bucket.Initialize(targetLoggingContext);
460-
WorkUnitResult bucketResult = null;
461460
targetSuccess = false;
462461

463462
Lookup.Scope entryForInference = null;
@@ -522,7 +521,7 @@ internal async Task ExecuteTarget(ITaskBuilder taskBuilder, BuildRequestEntry re
522521
}
523522

524523
// We either have some work to do or at least we need to infer outputs from inputs.
525-
bucketResult = await ProcessBucket(taskBuilder, targetLoggingContext, GetTaskExecutionMode(dependencyResult), lookupForInference, lookupForExecution);
524+
WorkUnitResult bucketResult = await ProcessBucket(taskBuilder, targetLoggingContext, GetTaskExecutionMode(dependencyResult), lookupForInference, lookupForExecution);
526525

527526
// Now aggregate the result with the existing known results. There are four rules, assuming the target was not
528527
// skipped due to being up-to-date:
@@ -553,7 +552,7 @@ internal async Task ExecuteTarget(ITaskBuilder taskBuilder, BuildRequestEntry re
553552
entryForInference = null;
554553
entryForExecution.LeaveScope();
555554
entryForExecution = null;
556-
targetSuccess = (bucketResult?.ResultCode == WorkUnitResultCode.Success);
555+
targetSuccess = bucketResult.ResultCode == WorkUnitResultCode.Success;
557556
break;
558557

559558
case DependencyAnalysisResult.SkipNoInputs:
@@ -784,7 +783,7 @@ internal void MarkForStop()
784783
ErrorUtilities.VerifyThrow(_targetResult.ResultCode == TargetResultCode.Skipped, "ResultCode must be Skipped. ResultCode is {0}.", _state);
785784
ErrorUtilities.VerifyThrow(_targetResult.WorkUnitResult.ActionCode == WorkUnitActionCode.Continue, "ActionCode must be Continue. ActionCode is {0}.", _state);
786785

787-
_targetResult.WorkUnitResult.ActionCode = WorkUnitActionCode.Stop;
786+
_targetResult.WorkUnitResult = new WorkUnitResult(_targetResult.WorkUnitResult.ResultCode, WorkUnitActionCode.Stop, _targetResult.WorkUnitResult.Exception);
788787
}
789788

790789
/// <summary>

src/Build/BackEnd/Components/RequestBuilder/TaskBuilder.cs

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -669,11 +669,7 @@ private async Task<WorkUnitResult> InitializeAndExecuteTask(TaskLoggingContext t
669669
{
670670
// UNDONE: Move this and the task host.
671671
taskHost.LoggingContext = taskLoggingContext;
672-
WorkUnitResult executionResult = await ExecuteInstantiatedTask(_taskExecutionHost, taskLoggingContext, taskHost, bucket, howToExecuteTask);
673-
674-
ErrorUtilities.VerifyThrow(executionResult != null, "Unexpected null execution result");
675-
676-
return executionResult;
672+
return await ExecuteInstantiatedTask(_taskExecutionHost, taskLoggingContext, taskHost, bucket, howToExecuteTask);
677673
}
678674
finally
679675
{

src/Build/BackEnd/Shared/TargetResult.cs

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,11 @@ namespace Microsoft.Build.Execution
2121
/// </summary>
2222
public class TargetResult : ITargetResult, ITranslatable
2323
{
24+
/// <summary>
25+
/// Lock for cache file access.
26+
/// </summary>
27+
private readonly object _lock = new();
28+
2429
/// <summary>
2530
/// The result for this target.
2631
/// </summary>
@@ -100,7 +105,7 @@ public ITaskItem[] Items
100105
[DebuggerStepThrough]
101106
get
102107
{
103-
lock (_result)
108+
lock (_lock)
104109
{
105110
if (_items == null)
106111
{
@@ -167,6 +172,9 @@ internal WorkUnitResult WorkUnitResult
167172
{
168173
[DebuggerStepThrough]
169174
get => _result;
175+
176+
[DebuggerStepThrough]
177+
set => _result = value;
170178
}
171179

172180
/// <summary>
@@ -214,7 +222,7 @@ void ITranslatable.Translate(ITranslator translator)
214222
{
215223
if (translator.Mode == TranslationDirection.WriteToStream)
216224
{
217-
lock (_result)
225+
lock (_lock)
218226
{
219227
// Should we have cached these items but now want to send them to another node, we need to
220228
// ensure they are loaded before doing so.
@@ -271,7 +279,7 @@ internal static string GetCacheDirectory(int configId, string targetToCache)
271279
/// </summary>
272280
internal void CacheItems(int configId, string targetName)
273281
{
274-
lock (_result)
282+
lock (_lock)
275283
{
276284
if (_items == null)
277285
{

src/Build/BackEnd/Shared/WorkUnitResult.cs

Lines changed: 20 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -52,27 +52,27 @@ internal enum WorkUnitActionCode
5252
/// <summary>
5353
/// A result of executing a target or task.
5454
/// </summary>
55-
internal class WorkUnitResult : ITranslatable
55+
internal readonly struct WorkUnitResult : ITranslatable
5656
{
5757
/// <summary>
5858
/// The result.
5959
/// </summary>
60-
private WorkUnitResultCode _resultCode;
60+
private readonly WorkUnitResultCode _resultCode;
6161

6262
/// <summary>
6363
/// The next action to take.
6464
/// </summary>
65-
private WorkUnitActionCode _actionCode;
65+
private readonly WorkUnitActionCode _actionCode;
6666

6767
/// <summary>
6868
/// The exception from the failure, if any.
6969
/// </summary>
70-
private Exception _exception;
70+
private readonly Exception _exception;
7171

7272
/// <summary>
7373
/// Creates a new work result ready for aggregation during batches.
7474
/// </summary>
75-
internal WorkUnitResult()
75+
public WorkUnitResult()
7676
{
7777
_resultCode = WorkUnitResultCode.Skipped;
7878
_actionCode = WorkUnitActionCode.Continue;
@@ -94,7 +94,9 @@ internal WorkUnitResult(WorkUnitResultCode resultCode, WorkUnitActionCode action
9494
/// </summary>
9595
private WorkUnitResult(ITranslator translator)
9696
{
97-
((ITranslatable)this).Translate(translator);
97+
translator.TranslateEnum(ref _resultCode, (int)_resultCode);
98+
translator.TranslateEnum(ref _actionCode, (int)_actionCode);
99+
translator.TranslateException(ref _exception);
98100
}
99101

100102
/// <summary>
@@ -105,11 +107,7 @@ private WorkUnitResult(ITranslator translator)
105107
/// <summary>
106108
/// Get the action code.
107109
/// </summary>
108-
internal WorkUnitActionCode ActionCode
109-
{
110-
get => _actionCode;
111-
set => _actionCode = value;
112-
}
110+
internal WorkUnitActionCode ActionCode => _actionCode;
113111

114112
/// <summary>
115113
/// Get the exception
@@ -123,9 +121,17 @@ internal WorkUnitActionCode ActionCode
123121
/// </summary>
124122
public void Translate(ITranslator translator)
125123
{
126-
translator.TranslateEnum(ref _resultCode, (int)_resultCode);
127-
translator.TranslateEnum(ref _actionCode, (int)_actionCode);
128-
translator.TranslateException(ref _exception);
124+
if (translator.Mode == TranslationDirection.ReadFromStream)
125+
{
126+
throw new InvalidOperationException("Readonly struct - use the factory method for deserialization.");
127+
}
128+
129+
WorkUnitResultCode resultCode = _resultCode;
130+
WorkUnitActionCode actionCode = _actionCode;
131+
Exception exception = _exception;
132+
translator.TranslateEnum(ref resultCode, (int)_resultCode);
133+
translator.TranslateEnum(ref actionCode, (int)_actionCode);
134+
translator.TranslateException(ref exception);
129135
}
130136

131137
#endregion

0 commit comments

Comments
 (0)