Skip to content

Commit ff5dbe6

Browse files
authored
Don't hold onto references in Consumer (#2191)
* Don't store references in Consumer. Don't box nullable value types. * Removed reference check as it's no longer necessary. * Overwrite field to null instead of calling DeadCodeEliminationHelper.KeepAliveWithoutBoxing.
1 parent d3fbc03 commit ff5dbe6

File tree

1 file changed

+21
-17
lines changed

1 file changed

+21
-17
lines changed

src/BenchmarkDotNet/Engines/Consumer.cs

Lines changed: 21 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ private static readonly HashSet<Type> SupportedTypes
1818
.Where(field => !field.IsStatic) // exclude this HashSet itself
1919
.Select(field => field.FieldType));
2020

21+
#pragma warning disable IDE0052 // Remove unread private members
2122
private volatile byte byteHolder;
2223
private volatile sbyte sbyteHolder;
2324
private volatile short shortHolder;
@@ -30,10 +31,10 @@ private static readonly HashSet<Type> SupportedTypes
3031
private double doubleHolder;
3132
private long longHolder;
3233
private ulong ulongHolder;
33-
private string stringHolder;
34-
private object objectHolder;
35-
private IntPtr ptrHolder;
36-
private UIntPtr uptrHolder;
34+
private volatile object objectHolder;
35+
private volatile IntPtr ptrHolder;
36+
private volatile UIntPtr uptrHolder;
37+
#pragma warning restore IDE0052 // Remove unread private members
3738

3839
[MethodImpl(MethodImplOptions.AggressiveInlining)]
3940
[PublicAPI]
@@ -84,11 +85,11 @@ private static readonly HashSet<Type> SupportedTypes
8485

8586
[MethodImpl(MethodImplOptions.AggressiveInlining)]
8687
[PublicAPI]
87-
public void Consume(IntPtr intPtrValue) => Volatile.Write(ref ptrHolder, intPtrValue);
88+
public void Consume(IntPtr intPtrValue) => ptrHolder = intPtrValue;
8889

8990
[MethodImpl(MethodImplOptions.AggressiveInlining)]
9091
[PublicAPI]
91-
public void Consume(UIntPtr uintPtrValue) => Volatile.Write(ref uptrHolder, uintPtrValue);
92+
public void Consume(UIntPtr uintPtrValue) => uptrHolder = uintPtrValue;
9293

9394
[CLSCompliant(false)]
9495
[MethodImpl(MethodImplOptions.AggressiveInlining)]
@@ -97,22 +98,28 @@ private static readonly HashSet<Type> SupportedTypes
9798

9899
[MethodImpl(MethodImplOptions.AggressiveInlining)]
99100
[PublicAPI]
100-
public void Consume(string stringValue) => Volatile.Write(ref stringHolder, stringValue);
101+
public void Consume(string stringValue) => Consume((object) stringValue);
101102

102103
[MethodImpl(MethodImplOptions.AggressiveInlining)]
103104
[PublicAPI]
104-
public void Consume(object objectValue) => Volatile.Write(ref objectHolder, objectValue);
105+
public void Consume(object objectValue)
106+
{
107+
// Write to volatile field to prevent dead code elimination and out-of-order execution.
108+
objectHolder = objectValue;
109+
// Overwrite field to null so we aren't holding onto references to affect GC behavior. (#1942)
110+
objectHolder = null;
111+
}
105112

106113
[MethodImpl(MethodImplOptions.AggressiveInlining)]
107114
[PublicAPI]
108115
public void Consume<T>(T objectValue) where T : class // class constraint prevents from boxing structs
109-
=> Volatile.Write(ref objectHolder, objectValue);
116+
=> Consume((object) objectValue);
110117

111118
[MethodImpl(MethodImplOptions.AggressiveInlining)]
112-
public unsafe void Consume<T>(T* ptrValue) where T: unmanaged => Volatile.Write(ref ptrHolder, (IntPtr)ptrValue);
119+
public unsafe void Consume<T>(T* ptrValue) where T : unmanaged => ptrHolder = (IntPtr) ptrValue;
113120

114121
[MethodImpl(MethodImplOptions.AggressiveInlining)]
115-
public unsafe void Consume(void* ptrValue) => Volatile.Write(ref ptrHolder, (IntPtr)ptrValue);
122+
public unsafe void Consume(void* ptrValue) => ptrHolder = (IntPtr) ptrValue;
116123

117124
[MethodImpl(MethodImplOptions.AggressiveInlining)]
118125
public void Consume<T>(in T value)
@@ -141,15 +148,12 @@ public void Consume<T>(in T value)
141148
Volatile.Write(ref longHolder, (long)(object)value);
142149
else if (typeof(T) == typeof(ulong))
143150
Volatile.Write(ref ulongHolder, (ulong)(object)value);
144-
else if (default(T) == null)
145-
objectHolder = (object) value;
151+
else if (default(T) == null && !typeof(T).IsValueType)
152+
Consume((object) value);
146153
else
147-
ValueTypesConsumer(value); // non-primitive value types
154+
DeadCodeEliminationHelper.KeepAliveWithoutBoxingReadonly(value); // non-primitive and nullable value types
148155
}
149156

150-
[MethodImpl(MethodImplOptions.NoInlining)]
151-
private void ValueTypesConsumer<T>(in T _) { }
152-
153157
internal static bool IsConsumable(Type type)
154158
=> SupportedTypes.Contains(type) || type.GetTypeInfo().IsClass || type.GetTypeInfo().IsInterface;
155159

0 commit comments

Comments
 (0)