Skip to content

Commit e278ebc

Browse files
timcasselltimcassell
authored
Update codegen for forward and backward compatibility with C# unsafe evolution and simplify some generated code. (#3134)
1 parent cbbfe6d commit e278ebc

11 files changed

Lines changed: 183 additions & 150 deletions

File tree

‎src/BenchmarkDotNet/Code/CodeGenerator.cs‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -194,11 +194,10 @@ private static string GetDeclareFieldsContainer(BenchmarkCase benchmarkCase, Ben
194194
return string.Empty;
195195
}
196196

197-
// Wrapper struct is necessary because of error CS4004: Cannot await in an unsafe context
198197
var sb = new StringBuilder();
199198
sb.AppendLine("""
200199
[global::System.Runtime.InteropServices.StructLayout(global::System.Runtime.InteropServices.LayoutKind.Auto)]
201-
private unsafe struct FieldsContainer
200+
private struct FieldsContainer
202201
{
203202
""");
204203
foreach (var field in fields)

‎src/BenchmarkDotNet/Code/DeclarationsProvider.cs‎

Lines changed: 49 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -107,48 +107,60 @@ protected override SmartStringBuilder ReplaceCore(SmartStringBuilder smartString
107107
string passArguments = GetPassArguments();
108108
string workloadMethodCall = GetWorkloadMethodCall(passArguments);
109109
string coreImpl = $$"""
110-
private unsafe {{CoreReturnType}} OverheadActionUnroll({{CoreParameters}})
110+
private {{CoreReturnType}} OverheadActionUnroll({{CoreParameters}})
111111
{
112-
{{loadArguments}}
113-
{{StartClockSyncCode}}
114-
while (--invokeCount >= 0)
112+
unsafe
115113
{
116-
this.__Overhead({{passArguments}});@Unroll@
114+
{{loadArguments}}
115+
{{StartClockSyncCode}}
116+
while (--invokeCount >= 0)
117+
{
118+
this.__Overhead({{passArguments}});@Unroll@
119+
}
120+
{{ReturnSyncCode}}
117121
}
118-
{{ReturnSyncCode}}
119122
}
120123
121-
private unsafe {{CoreReturnType}} OverheadActionNoUnroll({{CoreParameters}})
124+
private {{CoreReturnType}} OverheadActionNoUnroll({{CoreParameters}})
122125
{
123-
{{loadArguments}}
124-
{{StartClockSyncCode}}
125-
while (--invokeCount >= 0)
126+
unsafe
126127
{
127-
this.__Overhead({{passArguments}});
128+
{{loadArguments}}
129+
{{StartClockSyncCode}}
130+
while (--invokeCount >= 0)
131+
{
132+
this.__Overhead({{passArguments}});
133+
}
134+
{{ReturnSyncCode}}
128135
}
129-
{{ReturnSyncCode}}
130136
}
131137
132-
private unsafe {{CoreReturnType}} WorkloadActionUnroll({{CoreParameters}})
138+
private {{CoreReturnType}} WorkloadActionUnroll({{CoreParameters}})
133139
{
134-
{{loadArguments}}
135-
{{StartClockSyncCode}}
136-
while (--invokeCount >= 0)
140+
unsafe
137141
{
138-
{{workloadMethodCall}}@Unroll@
142+
{{loadArguments}}
143+
{{StartClockSyncCode}}
144+
while (--invokeCount >= 0)
145+
{
146+
{{workloadMethodCall}}@Unroll@
147+
}
148+
{{ReturnSyncCode}}
139149
}
140-
{{ReturnSyncCode}}
141150
}
142151
143-
private unsafe {{CoreReturnType}} WorkloadActionNoUnroll({{CoreParameters}})
152+
private {{CoreReturnType}} WorkloadActionNoUnroll({{CoreParameters}})
144153
{
145-
{{loadArguments}}
146-
{{StartClockSyncCode}}
147-
while (--invokeCount >= 0)
154+
unsafe
148155
{
149-
{{workloadMethodCall}}
156+
{{loadArguments}}
157+
{{StartClockSyncCode}}
158+
while (--invokeCount >= 0)
159+
{
160+
{{workloadMethodCall}}
161+
}
162+
{{ReturnSyncCode}}
150163
}
151-
{{ReturnSyncCode}}
152164
}
153165
""";
154166

@@ -184,19 +196,19 @@ internal abstract class AsyncDeclarationsProviderBase(BenchmarkCase benchmark) :
184196

185197
public override string[] GetExtraFields() =>
186198
[
187-
$"public {typeof(WorkloadValueTaskSource).GetCorrectCSharpTypeName()} workloadContinuerAndValueTaskSource;",
199+
$"public {typeof(WorkloadValueTaskSource).GetCorrectCSharpTypeName()} workloadValueTaskSource;",
188200
$"public {typeof(IClock).GetCorrectCSharpTypeName()} clock;",
189201
"public long invokeCount;"
190202
];
191203

192204
protected override string GetExtraGlobalSetupImpl()
193205
=> $$"""
194-
this.__fieldsContainer.workloadContinuerAndValueTaskSource = new {{typeof(WorkloadValueTaskSource).GetCorrectCSharpTypeName()}}();
206+
this.__fieldsContainer.workloadValueTaskSource = new {{typeof(WorkloadValueTaskSource).GetCorrectCSharpTypeName()}}();
195207
this.__StartWorkload();
196208
""";
197209

198210
protected override string GetExtraGlobalCleanupImpl()
199-
=> "this.__fieldsContainer.workloadContinuerAndValueTaskSource.Complete();";
211+
=> "this.__fieldsContainer.workloadValueTaskSource.Complete();";
200212

201213
protected bool TryGetAsyncMethodBuilderAttribute(out string asyncMethodBuilderAttribute)
202214
{
@@ -283,7 +295,7 @@ protected override SmartStringBuilder ReplaceCore(SmartStringBuilder smartString
283295
this.__fieldsContainer.clock = clock;
284296
// The source is allocated and the workload loop started in __GlobalSetup,
285297
// so this hot path is branchless and allocation-free.
286-
return this.__fieldsContainer.workloadContinuerAndValueTaskSource.Continue();
298+
return this.__fieldsContainer.workloadValueTaskSource.Continue();
287299
}
288300
289301
private async void __StartWorkload()
@@ -296,7 +308,7 @@ private async void __StartWorkload()
296308
{
297309
try
298310
{
299-
if (await this.__fieldsContainer.workloadContinuerAndValueTaskSource.GetIsComplete())
311+
if (await this.__fieldsContainer.workloadValueTaskSource.GetIsComplete())
300312
{
301313
{{finalReturn}}
302314
}
@@ -307,15 +319,15 @@ private async void __StartWorkload()
307319
{
308320
{{GetCallAndConsumeImpl(workloadMethodCall)}}
309321
}
310-
if (await this.__fieldsContainer.workloadContinuerAndValueTaskSource.SetResultAndGetIsComplete(startedClock.GetElapsed()))
322+
if (await this.__fieldsContainer.workloadValueTaskSource.SetResultAndGetIsComplete(startedClock.GetElapsed()))
311323
{
312324
{{finalReturn}}
313325
}
314326
}
315327
}
316328
catch (global::System.Exception e)
317329
{
318-
__fieldsContainer.workloadContinuerAndValueTaskSource.SetException(e);
330+
__fieldsContainer.workloadValueTaskSource.SetException(e);
319331
{{finalReturn}}
320332
}
321333
}
@@ -332,24 +344,14 @@ internal class AsyncDeclarationsProvider(BenchmarkCase benchmark, Type resultTyp
332344
{
333345
protected override string GetCallAndConsumeImpl(string workloadMethodCall)
334346
{
335-
string awaitStatement;
336347
if (resultType == typeof(void))
337348
{
338-
awaitStatement = "await awaitable;";
349+
return $"await {workloadMethodCall}";
339350
}
340-
else
341-
{
342-
var resultTypeName = resultType.GetCorrectCSharpTypeName();
343-
awaitStatement = $"""
344-
{resultTypeName} result = await awaitable;
345-
{typeof(DeadCodeEliminationHelper).GetCorrectCSharpTypeName()}.KeepAliveWithoutBoxing<{resultTypeName}>(in result);
346-
""";
347-
}
348-
return $$"""
349-
// Necessary because of error CS4004: Cannot await in an unsafe context
350-
{{Descriptor.WorkloadMethod.ReturnType.GetCorrectCSharpTypeName()}} awaitable;
351-
unsafe { awaitable = {{workloadMethodCall}} }
352-
{{awaitStatement}}
351+
var resultTypeName = resultType.GetCorrectCSharpTypeName();
352+
return $"""
353+
{resultTypeName} result = await {workloadMethodCall}
354+
{typeof(DeadCodeEliminationHelper).GetCorrectCSharpTypeName()}.KeepAliveWithoutBoxing<{resultTypeName}>(in result);
353355
""";
354356
}
355357
}
@@ -362,10 +364,7 @@ protected override string GetCallAndConsumeImpl(string workloadMethodCall)
362364
{
363365
string itemTypeName = itemType.GetCorrectCSharpTypeName();
364366
return $$"""
365-
// Necessary because of error CS4004: Cannot await in an unsafe context
366-
{{Descriptor.WorkloadMethod.ReturnType.GetCorrectCSharpTypeName()}} enumerable;
367-
unsafe { enumerable = {{workloadMethodCall}} }
368-
await foreach ({{itemTypeName}} item in enumerable)
367+
await foreach ({{itemTypeName}} item in {{workloadMethodCall.TrimEnd(';')}})
369368
{
370369
{{typeof(DeadCodeEliminationHelper).GetCorrectCSharpTypeName()}}.KeepAliveWithoutBoxing<{{itemTypeName}}>(in item);
371370
}

‎src/BenchmarkDotNet/Templates/BenchmarkType.txt‎

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
// Type name must be in sync with DisassemblyDiagnoser.BuildClrMdArgs.
22
[global::BenchmarkDotNet.Attributes.CompilerServices.AggressivelyOptimizeMethods]
3-
public sealed partial class Runnable_$ID$ : $WorkloadTypeName$
3+
public sealed class Runnable_$ID$ : $WorkloadTypeName$
44
{
55
public static async global::System.Threading.Tasks.ValueTask Run(global::BenchmarkDotNet.Engines.IHost host, global::System.String benchmarkName, global::BenchmarkDotNet.Diagnosers.RunMode diagnoserRunMode)
66
{
@@ -62,7 +62,7 @@
6262
}
6363

6464
[global::System.Diagnostics.CodeAnalysis.SetsRequiredMembers]
65-
public unsafe Runnable_$ID$(global::System.Threading.CancellationToken cancellationToken)
65+
public Runnable_$ID$(global::System.Threading.CancellationToken cancellationToken)
6666
{
6767
$InitializeArgumentFields$
6868
$ParamsContent$
@@ -93,20 +93,21 @@
9393

9494
// this method is used only for the disassembly diagnoser purposes
9595
// the goal is to get this and the benchmarked method jitted, but without executing the benchmarked method itself
96-
public global::System.Int32 NotEleven;
9796
[global::System.Runtime.CompilerServices.MethodImpl(global::System.Runtime.CompilerServices.MethodImplOptions.NoOptimization | global::System.Runtime.CompilerServices.MethodImplOptions.NoInlining)]
9897
public void __TrickTheJIT__()
9998
{
100-
this.NotEleven = new global::System.Random(123).Next(0, 10);
101-
$DisassemblerEntryMethodName$();
99+
$DisassemblerEntryMethodName$(new global::System.Random(123).Next(0, 10));
102100
}
103101

104102
[global::System.Runtime.CompilerServices.MethodImpl(global::System.Runtime.CompilerServices.MethodImplOptions.NoOptimization | global::System.Runtime.CompilerServices.MethodImplOptions.NoInlining)]
105-
public unsafe void $DisassemblerEntryMethodName$()
103+
public void $DisassemblerEntryMethodName$(global::System.Int32 notEleven)
106104
{
107-
if (this.NotEleven == 11)
105+
if (notEleven == 11)
108106
{
109-
$DisassemblerEntryMethodImpl$
107+
unsafe
108+
{
109+
$DisassemblerEntryMethodImpl$
110+
}
110111
}
111112
}
112113

‎src/BenchmarkDotNet/Toolchains/InProcess/Emit/Implementation/Emitters/AsyncCoreEmitter.cs‎

Lines changed: 29 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -31,11 +31,16 @@ protected override void EmitWorkloadCore()
3131
typeof(ValueTaskAwaiter<bool>),
3232
FieldAttributes.Private
3333
);
34-
var benchmarkAwaiterField = asyncStateMachineTypeBuilder.DefineField(
35-
"<>u__2",
36-
awaitableInfo.AwaiterType,
37-
FieldAttributes.Private
38-
);
34+
// Roslyn dedupes hoisted awaiter fields by type — when the workload's awaiter matches the
35+
// workloadValueTaskSource awaiter (e.g., the workload itself returns ValueTask<bool>) only
36+
// `<>u__1` is emitted, and both awaits reuse it.
37+
var benchmarkAwaiterField = awaitableInfo.AwaiterType == workloadContinuerAwaiterField.FieldType
38+
? workloadContinuerAwaiterField
39+
: asyncStateMachineTypeBuilder.DefineField(
40+
"<>u__2",
41+
awaitableInfo.AwaiterType,
42+
FieldAttributes.Private
43+
);
3944
EmitMoveNextImpl();
4045
var asyncStateMachineType = CompleteAsyncStateMachineType(asyncMethodBuilderType, builderInfo);
4146

@@ -48,19 +53,27 @@ void EmitMoveNextImpl()
4853
var resultType = awaitableInfo.ResultType;
4954
var isCompleteAwaiterLocal = ilBuilder.DeclareLocal(typeof(ValueTaskAwaiter<bool>));
5055
var isCompleteAwaitableLocal = ilBuilder.DeclareLocal(typeof(ValueTask<bool>));
51-
// The value-type awaitable spill local (Roslyn declares one for ValueTask<T> et al. so it
52-
// can take its address for GetAwaiter — reference-type awaitables stay on the stack).
53-
var benchmarkAwaitableLocal = Descriptor.WorkloadMethod.ReturnType.IsValueType
54-
? ilBuilder.DeclareLocal(Descriptor.WorkloadMethod.ReturnType)
55-
: null;
56-
// Source local for `T result = await awaitable;` — declared only when the awaiter's
56+
// Source local for `T result = await workloadCall;` — declared only when the awaiter's
5757
// GetResult returns non-void, in which case the template captures the value and pipes
5858
// it through DeadCodeEliminationHelper so the JIT can't elide the producer's work.
59-
// Roslyn places this AFTER the (optional) awaitable spill and BEFORE the awaiter temp.
59+
// Roslyn places this BEFORE the awaiter temp and the (optional) awaitable spill.
6060
var resultLocal = resultType == typeof(void)
6161
? null
6262
: ilBuilder.DeclareLocal(resultType);
63-
var benchmarkAwaiterLocal = ilBuilder.DeclareLocal(benchmarkAwaiterField.FieldType);
63+
// Roslyn dedupes locals by type — when the workload awaiter matches the isComplete
64+
// awaiter (e.g., the workload itself returns ValueTask<bool>), it reuses the existing
65+
// local rather than declaring a new one.
66+
var benchmarkAwaiterLocal = benchmarkAwaiterField.FieldType == isCompleteAwaiterLocal.LocalType
67+
? isCompleteAwaiterLocal
68+
: ilBuilder.DeclareLocal(benchmarkAwaiterField.FieldType);
69+
// The value-type awaitable spill local (Roslyn declares one for ValueTask<T> et al. so it
70+
// can take its address for GetAwaiter — reference-type awaitables stay on the stack).
71+
// Roslyn places this AFTER the awaiter temp, and dedupes by type just like the awaiter.
72+
var benchmarkAwaitableLocal = Descriptor.WorkloadMethod.ReturnType.IsValueType
73+
? (Descriptor.WorkloadMethod.ReturnType == isCompleteAwaitableLocal.LocalType
74+
? isCompleteAwaitableLocal
75+
: ilBuilder.DeclareLocal(Descriptor.WorkloadMethod.ReturnType))
76+
: null;
6477
var invokeCountLocal = ilBuilder.DeclareLocal(typeof(long));
6578
var exceptionLocal = ilBuilder.DeclareLocal(typeof(Exception));
6679

@@ -92,7 +105,7 @@ void EmitMoveNextImpl()
92105
// var awaitable = workloadValueTaskSource.GetIsComplete();
93106
ilBuilder.EmitLdloc(thisLocal!);
94107
ilBuilder.Emit(OpCodes.Ldflda, fieldsContainerField);
95-
ilBuilder.Emit(OpCodes.Ldfld, workloadContinuerAndValueTaskSourceField);
108+
ilBuilder.Emit(OpCodes.Ldfld, workloadValueTaskSourceField);
96109
ilBuilder.Emit(OpCodes.Callvirt, typeof(WorkloadValueTaskSource).GetMethod(nameof(WorkloadValueTaskSource.GetIsComplete), BindingFlags.Public | BindingFlags.Instance)!);
97110
ilBuilder.EmitStloc(isCompleteAwaitableLocal);
98111
// var awaiter = awaitable.GetAwaiter();
@@ -260,7 +273,7 @@ void EmitMoveNextImpl()
260273
// var awaitable = workloadValueTaskSource.SetResultAndGetIsComplete(startedClock.GetElapsed());
261274
ilBuilder.EmitLdloc(thisLocal!);
262275
ilBuilder.Emit(OpCodes.Ldflda, fieldsContainerField);
263-
ilBuilder.Emit(OpCodes.Ldfld, workloadContinuerAndValueTaskSourceField);
276+
ilBuilder.Emit(OpCodes.Ldfld, workloadValueTaskSourceField);
264277
ilBuilder.Emit(OpCodes.Ldarg_0);
265278
ilBuilder.Emit(OpCodes.Ldflda, startedClockField);
266279
ilBuilder.Emit(OpCodes.Call, typeof(StartedClock).GetMethod(nameof(StartedClock.GetElapsed), BindingFlags.Public | BindingFlags.Instance)!);
@@ -333,7 +346,7 @@ void EmitMoveNextImpl()
333346
// workloadValueTaskSource.SetException(exception);
334347
ilBuilder.EmitLdloc(thisLocal!);
335348
ilBuilder.Emit(OpCodes.Ldflda, fieldsContainerField);
336-
ilBuilder.Emit(OpCodes.Ldfld, workloadContinuerAndValueTaskSourceField);
349+
ilBuilder.Emit(OpCodes.Ldfld, workloadValueTaskSourceField);
337350
ilBuilder.EmitLdloc(exceptionLocal);
338351
ilBuilder.Emit(OpCodes.Callvirt, typeof(WorkloadValueTaskSource).GetMethod(nameof(WorkloadValueTaskSource.SetException), [typeof(Exception)])!);
339352
// result = default;

0 commit comments

Comments
 (0)