From f7d7bc832b603e47a162ed999aaceba5e666d61c Mon Sep 17 00:00:00 2001 From: Jared Parsons Date: Thu, 19 Sep 2024 10:09:53 -0700 Subject: [PATCH 1/2] Test allocations Continuing to work on unnecessary allocations in our unit tests --- .../Test/Semantic/Semantics/NativeIntegerTests.cs | 2 +- .../Test/Core/Platform/Desktop/Extensions.cs | 13 +++++++------ src/Compilers/Test/Core/TargetFrameworkUtil.cs | 15 ++++++++++++++- 3 files changed, 22 insertions(+), 8 deletions(-) diff --git a/src/Compilers/CSharp/Test/Semantic/Semantics/NativeIntegerTests.cs b/src/Compilers/CSharp/Test/Semantic/Semantics/NativeIntegerTests.cs index 71bb638bbda91..2f54e0e1b624a 100644 --- a/src/Compilers/CSharp/Test/Semantic/Semantics/NativeIntegerTests.cs +++ b/src/Compilers/CSharp/Test/Semantic/Semantics/NativeIntegerTests.cs @@ -12384,7 +12384,7 @@ void binaryOperator(string op, string leftType, string rightType, string expecte if (expectedDiagnostics.Length == 0) { - CompileAndVerify(comp); + CompileAndVerify(comp, emitOptions: EmitOptions.Default); } static bool useUnsafe(string type) => type == "void*"; diff --git a/src/Compilers/Test/Core/Platform/Desktop/Extensions.cs b/src/Compilers/Test/Core/Platform/Desktop/Extensions.cs index f46a3c5f4a0be..61e12e8f230e8 100644 --- a/src/Compilers/Test/Core/Platform/Desktop/Extensions.cs +++ b/src/Compilers/Test/Core/Platform/Desktop/Extensions.cs @@ -13,6 +13,7 @@ using System.IO; using System.Linq; using System.Reflection; +using System.Runtime.InteropServices; using System.Runtime.Serialization; using System.Text; using System.Threading; @@ -30,13 +31,13 @@ public static void AddArray(this SerializationInfo info, string name, Immutab // we will copy the content into an array and serialize the copy // we could serialize element-wise, but that would require serializing // name and type for every serialized element which seems worse than creating a copy. - info.AddValue(name, value.IsDefault ? null : value.ToArray(), typeof(T[])); + info.AddValue(name, value.IsDefault ? null : ImmutableCollectionsMarshal.AsArray(value), typeof(T[])); } public static ImmutableArray GetArray(this SerializationInfo info, string name) where T : class { - var arr = (T[])info.GetValue(name, typeof(T[])); - return ImmutableArray.Create(arr); + var array = (T[])info.GetValue(name, typeof(T[])); + return ImmutableCollectionsMarshal.AsImmutableArray(array); } public static void AddByteArray(this SerializationInfo info, string name, ImmutableArray value) @@ -44,13 +45,13 @@ public static void AddByteArray(this SerializationInfo info, string name, Immuta // we will copy the content into an array and serialize the copy // we could serialize element-wise, but that would require serializing // name and type for every serialized element which seems worse than creating a copy. - info.AddValue(name, value.IsDefault ? null : value.ToArray(), typeof(byte[])); + info.AddValue(name, value.IsDefault ? null : ImmutableCollectionsMarshal.AsArray(value), typeof(byte[])); } public static ImmutableArray GetByteArray(this SerializationInfo info, string name) { - var arr = (byte[])info.GetValue(name, typeof(byte[])); - return ImmutableArray.Create(arr); + var array = (byte[])info.GetValue(name, typeof(byte[])); + return ImmutableCollectionsMarshal.AsImmutableArray(array); } } } diff --git a/src/Compilers/Test/Core/TargetFrameworkUtil.cs b/src/Compilers/Test/Core/TargetFrameworkUtil.cs index f73c0fc7d4aca..2b7ed086309ed 100644 --- a/src/Compilers/Test/Core/TargetFrameworkUtil.cs +++ b/src/Compilers/Test/Core/TargetFrameworkUtil.cs @@ -113,13 +113,26 @@ public static class NetCoreApp /// public static class NetFramework { + private static ImmutableArray s_references; + /// /// This is the full set of references provided by default on the .NET Framework TFM /// /// /// Need to special case tuples until we move to net472 /// - public static ImmutableArray References { get; } = [.. Net461.References.All, Net461.ExtraReferences.SystemValueTuple]; + public static ImmutableArray References + { + get + { + if (s_references.IsDefault) + { + s_references = [.. Net461.References.All, Net461.ExtraReferences.SystemValueTuple]; + } + + return s_references; + } + } /// /// This is a limited set of references on this .NET Framework TFM. This should be avoided in new code From 8fe85953c37162522931747028d5d04345dfb204 Mon Sep 17 00:00:00 2001 From: Jared Parsons Date: Thu, 19 Sep 2024 11:01:12 -0700 Subject: [PATCH 2/2] PR feedback --- .../Test/Core/Platform/Desktop/Extensions.cs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/Compilers/Test/Core/Platform/Desktop/Extensions.cs b/src/Compilers/Test/Core/Platform/Desktop/Extensions.cs index 61e12e8f230e8..14621ca6a0e99 100644 --- a/src/Compilers/Test/Core/Platform/Desktop/Extensions.cs +++ b/src/Compilers/Test/Core/Platform/Desktop/Extensions.cs @@ -28,9 +28,9 @@ internal static class SerializationInfoExtensions { public static void AddArray(this SerializationInfo info, string name, ImmutableArray value) where T : class { - // we will copy the content into an array and serialize the copy - // we could serialize element-wise, but that would require serializing - // name and type for every serialized element which seems worse than creating a copy. + // This will store the underlying T[] directly into the SerializationInfo. That is safe because it + // only ever reads from the array. This is done instead of creating a copy because it is a + // significant source of allocations in our unit tests. info.AddValue(name, value.IsDefault ? null : ImmutableCollectionsMarshal.AsArray(value), typeof(T[])); } @@ -42,9 +42,9 @@ public static ImmutableArray GetArray(this SerializationInfo info, string public static void AddByteArray(this SerializationInfo info, string name, ImmutableArray value) { - // we will copy the content into an array and serialize the copy - // we could serialize element-wise, but that would require serializing - // name and type for every serialized element which seems worse than creating a copy. + // This will store the underlying byte[] directly into the SerializationInfo. That is safe because it + // only ever reads from the array. This is done instead of creating a copy because it is a + // significant source of allocations in our unit tests. info.AddValue(name, value.IsDefault ? null : ImmutableCollectionsMarshal.AsArray(value), typeof(byte[])); }