From 60b21b0b1da717bc54207d79201f6aba4d0c8e3a Mon Sep 17 00:00:00 2001 From: Tim Date: Thu, 27 Oct 2022 09:01:34 -0400 Subject: [PATCH 1/3] Don't store references in Consumer. Don't box nullable value types. --- src/BenchmarkDotNet/Engines/Consumer.cs | 17 ++++++----------- 1 file changed, 6 insertions(+), 11 deletions(-) diff --git a/src/BenchmarkDotNet/Engines/Consumer.cs b/src/BenchmarkDotNet/Engines/Consumer.cs index 015b3a952d..fc973064b9 100644 --- a/src/BenchmarkDotNet/Engines/Consumer.cs +++ b/src/BenchmarkDotNet/Engines/Consumer.cs @@ -30,8 +30,6 @@ private static readonly HashSet SupportedTypes private double doubleHolder; private long longHolder; private ulong ulongHolder; - private string stringHolder; - private object objectHolder; private IntPtr ptrHolder; private UIntPtr uptrHolder; @@ -97,16 +95,16 @@ private static readonly HashSet SupportedTypes [MethodImpl(MethodImplOptions.AggressiveInlining)] [PublicAPI] - public void Consume(string stringValue) => Volatile.Write(ref stringHolder, stringValue); + public void Consume(string stringValue) => DeadCodeEliminationHelper.KeepAliveWithoutBoxing(stringValue); [MethodImpl(MethodImplOptions.AggressiveInlining)] [PublicAPI] - public void Consume(object objectValue) => Volatile.Write(ref objectHolder, objectValue); + public void Consume(object objectValue) => DeadCodeEliminationHelper.KeepAliveWithoutBoxing(objectValue); [MethodImpl(MethodImplOptions.AggressiveInlining)] [PublicAPI] public void Consume(T objectValue) where T : class // class constraint prevents from boxing structs - => Volatile.Write(ref objectHolder, objectValue); + => DeadCodeEliminationHelper.KeepAliveWithoutBoxing(objectValue); [MethodImpl(MethodImplOptions.AggressiveInlining)] public unsafe void Consume(T* ptrValue) where T: unmanaged => Volatile.Write(ref ptrHolder, (IntPtr)ptrValue); @@ -141,15 +139,12 @@ public void Consume(in T value) Volatile.Write(ref longHolder, (long)(object)value); else if (typeof(T) == typeof(ulong)) Volatile.Write(ref ulongHolder, (ulong)(object)value); - else if (default(T) == null) - objectHolder = (object) value; + else if (default(T) == null && !typeof(T).IsValueType) + DeadCodeEliminationHelper.KeepAliveWithoutBoxing(value); else - ValueTypesConsumer(value); // non-primitive value types + DeadCodeEliminationHelper.KeepAliveWithoutBoxingReadonly(value); // non-primitive value types } - [MethodImpl(MethodImplOptions.NoInlining)] - private void ValueTypesConsumer(in T _) { } - internal static bool IsConsumable(Type type) => SupportedTypes.Contains(type) || type.GetTypeInfo().IsClass || type.GetTypeInfo().IsInterface; From df1029476b30f225ef3a0c56b2d242d9a770e29e Mon Sep 17 00:00:00 2001 From: Tim Date: Thu, 27 Oct 2022 09:08:13 -0400 Subject: [PATCH 2/3] Removed reference check as it's no longer necessary. --- src/BenchmarkDotNet/Engines/Consumer.cs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/BenchmarkDotNet/Engines/Consumer.cs b/src/BenchmarkDotNet/Engines/Consumer.cs index fc973064b9..f35c466c63 100644 --- a/src/BenchmarkDotNet/Engines/Consumer.cs +++ b/src/BenchmarkDotNet/Engines/Consumer.cs @@ -139,10 +139,8 @@ public void Consume(in T value) Volatile.Write(ref longHolder, (long)(object)value); else if (typeof(T) == typeof(ulong)) Volatile.Write(ref ulongHolder, (ulong)(object)value); - else if (default(T) == null && !typeof(T).IsValueType) - DeadCodeEliminationHelper.KeepAliveWithoutBoxing(value); else - DeadCodeEliminationHelper.KeepAliveWithoutBoxingReadonly(value); // non-primitive value types + DeadCodeEliminationHelper.KeepAliveWithoutBoxingReadonly(value); // non-primitive and nullable value types } internal static bool IsConsumable(Type type) From bdb40379f4bc2d7109bcc0b3456f89bbf77df3f8 Mon Sep 17 00:00:00 2001 From: Tim Date: Sat, 29 Oct 2022 21:06:16 -0400 Subject: [PATCH 3/3] Overwrite field to null instead of calling DeadCodeEliminationHelper.KeepAliveWithoutBoxing. --- src/BenchmarkDotNet/Engines/Consumer.cs | 29 +++++++++++++++++-------- 1 file changed, 20 insertions(+), 9 deletions(-) diff --git a/src/BenchmarkDotNet/Engines/Consumer.cs b/src/BenchmarkDotNet/Engines/Consumer.cs index f35c466c63..1abedec0d6 100644 --- a/src/BenchmarkDotNet/Engines/Consumer.cs +++ b/src/BenchmarkDotNet/Engines/Consumer.cs @@ -18,6 +18,7 @@ private static readonly HashSet SupportedTypes .Where(field => !field.IsStatic) // exclude this HashSet itself .Select(field => field.FieldType)); +#pragma warning disable IDE0052 // Remove unread private members private volatile byte byteHolder; private volatile sbyte sbyteHolder; private volatile short shortHolder; @@ -30,8 +31,10 @@ private static readonly HashSet SupportedTypes private double doubleHolder; private long longHolder; private ulong ulongHolder; - private IntPtr ptrHolder; - private UIntPtr uptrHolder; + private volatile object objectHolder; + private volatile IntPtr ptrHolder; + private volatile UIntPtr uptrHolder; +#pragma warning restore IDE0052 // Remove unread private members [MethodImpl(MethodImplOptions.AggressiveInlining)] [PublicAPI] @@ -82,11 +85,11 @@ private static readonly HashSet SupportedTypes [MethodImpl(MethodImplOptions.AggressiveInlining)] [PublicAPI] - public void Consume(IntPtr intPtrValue) => Volatile.Write(ref ptrHolder, intPtrValue); + public void Consume(IntPtr intPtrValue) => ptrHolder = intPtrValue; [MethodImpl(MethodImplOptions.AggressiveInlining)] [PublicAPI] - public void Consume(UIntPtr uintPtrValue) => Volatile.Write(ref uptrHolder, uintPtrValue); + public void Consume(UIntPtr uintPtrValue) => uptrHolder = uintPtrValue; [CLSCompliant(false)] [MethodImpl(MethodImplOptions.AggressiveInlining)] @@ -95,22 +98,28 @@ private static readonly HashSet SupportedTypes [MethodImpl(MethodImplOptions.AggressiveInlining)] [PublicAPI] - public void Consume(string stringValue) => DeadCodeEliminationHelper.KeepAliveWithoutBoxing(stringValue); + public void Consume(string stringValue) => Consume((object) stringValue); [MethodImpl(MethodImplOptions.AggressiveInlining)] [PublicAPI] - public void Consume(object objectValue) => DeadCodeEliminationHelper.KeepAliveWithoutBoxing(objectValue); + public void Consume(object objectValue) + { + // Write to volatile field to prevent dead code elimination and out-of-order execution. + objectHolder = objectValue; + // Overwrite field to null so we aren't holding onto references to affect GC behavior. (#1942) + objectHolder = null; + } [MethodImpl(MethodImplOptions.AggressiveInlining)] [PublicAPI] public void Consume(T objectValue) where T : class // class constraint prevents from boxing structs - => DeadCodeEliminationHelper.KeepAliveWithoutBoxing(objectValue); + => Consume((object) objectValue); [MethodImpl(MethodImplOptions.AggressiveInlining)] - public unsafe void Consume(T* ptrValue) where T: unmanaged => Volatile.Write(ref ptrHolder, (IntPtr)ptrValue); + public unsafe void Consume(T* ptrValue) where T : unmanaged => ptrHolder = (IntPtr) ptrValue; [MethodImpl(MethodImplOptions.AggressiveInlining)] - public unsafe void Consume(void* ptrValue) => Volatile.Write(ref ptrHolder, (IntPtr)ptrValue); + public unsafe void Consume(void* ptrValue) => ptrHolder = (IntPtr) ptrValue; [MethodImpl(MethodImplOptions.AggressiveInlining)] public void Consume(in T value) @@ -139,6 +148,8 @@ public void Consume(in T value) Volatile.Write(ref longHolder, (long)(object)value); else if (typeof(T) == typeof(ulong)) Volatile.Write(ref ulongHolder, (ulong)(object)value); + else if (default(T) == null && !typeof(T).IsValueType) + Consume((object) value); else DeadCodeEliminationHelper.KeepAliveWithoutBoxingReadonly(value); // non-primitive and nullable value types }