Merge pull request #4034 from lahma/test-case-perf-improvements-v3

Improve method discovery and filtering performance

src/NUnitFramework/framework/Internal/AttributeProviderWrapper.cs

@@ -23,12 +23,11 @@
 
 using System;
 using System.Collections.Generic;
-using System.Linq;
 using System.Reflection;
 
 namespace NUnit.Framework.Internal
 {
-    internal class AttributeProviderWrapper<T> : ICustomAttributeProvider
+    internal sealed class AttributeProviderWrapper<T> : ICustomAttributeProvider
         where T : Attribute
     {
         private readonly ICustomAttributeProvider _innerProvider;
@@ -52,14 +51,27 @@ public object[] GetCustomAttributes(bool inherit)
 
         public bool IsDefined(Type attributeType, bool inherit)
         {
-            return GetCustomAttributes(attributeType, inherit).Any();
+            return GetCustomAttributes(attributeType, inherit).Length > 0;
         }
 
-        private static T[] GetFiltered(IEnumerable<object> attributes)
+        private static T[] GetFiltered(object[] attributes)
         {
-            return attributes
-                   .OfType<T>()
-                   .ToArray();
+            List<T> filtered = null;
+            foreach (var attribute in attributes)
+            {
+                if (attribute is T t)
+                {
+                    filtered ??= new List<T>();
+                    filtered.Add(t);
+                }
+            }
+
+            return filtered?.ToArray() ??
+#if NETSTANDARD2_0_OR_GREATER
+                Array.Empty<T>();
+#else
+                new T[0];
+#endif
         }
     }
 }

src/NUnitFramework/framework/Internal/Builders/DefaultTestCaseBuilder.cs

@@ -105,8 +105,9 @@ public Test BuildFrom(IMethodInfo method, Test? parentSuite)
         {
             var tests = new List<TestMethod>();
 
-            List<ITestBuilder> builders = new List<ITestBuilder>(
-                method.GetCustomAttributes<ITestBuilder>(false));
+            var metadata = MethodInfoCache.Get(method);
+
+            List<ITestBuilder> builders = new List<ITestBuilder>(metadata.TestBuilderAttributes);
 
             // See if we need to add a CombinatorialAttribute for parameterized data
             if (method.MethodInfo.GetParameters().Any(param => param.HasAttribute<IParameterDataSource>(false))

src/NUnitFramework/framework/Internal/Builders/MethodInfoCache.cs

@@ -0,0 +1,76 @@
+// Copyright (c) Charlie Poole, Rob Prouse and Contributors. MIT License - see LICENSE.txt
+
+using System.Collections.Concurrent;
+using System.Collections.Generic;
+using NUnit.Framework.Interfaces;
+
+namespace NUnit.Framework.Internal.Builders
+{
+    /// <summary>
+    /// Caches static information for IMethodInfo to reduce re-calculations and memory allocations from reflection.
+    /// </summary>
+    internal static class MethodInfoCache
+    {
+        // we would otherwise do a lot of attribute allocations when repeatedly checking the same method for example
+        // in case of building TestCaseAttribute-based tests
+#if NET35
+        private static readonly Dictionary<IMethodInfo, TestMethodMetadata> MethodMetadataCache = new Dictionary<IMethodInfo, TestMethodMetadata>();
+#else
+        private static readonly ConcurrentDictionary<IMethodInfo, TestMethodMetadata> MethodMetadataCache = new ConcurrentDictionary<IMethodInfo, TestMethodMetadata>();
+#endif
+
+        /// <summary>
+        /// Returns cached metadata for method instance.
+        /// </summary>
+        internal static TestMethodMetadata Get(IMethodInfo method)
+        {
+#if NET35
+            lock (MethodMetadataCache)
+            {
+                if (!MethodMetadataCache.TryGetValue(method, out var metadata))
+                    MethodMetadataCache.Add(method, metadata = new TestMethodMetadata(method));
+
+                return metadata;
+            }
+#else
+            return MethodMetadataCache.GetOrAdd(method, m => new TestMethodMetadata(m));
+#endif
+        }
+
+        /// <summary>
+        /// Memoization of TestMethod information to reduce subsequent allocations from parameter and attribute information.
+        /// </summary>
+        internal sealed class TestMethodMetadata
+        {
+            public TestMethodMetadata(IMethodInfo method)
+            {
+                Parameters = method.GetParameters();
+                IsAsyncOperation = AsyncToSyncAdapter.IsAsyncOperation(method.MethodInfo);
+                IsVoidOrUnit = Reflect.IsVoidOrUnit(method.ReturnType.Type);
+
+                // TODO could probably go trough inherited and non inherited in two passes instead of multiple
+
+                // inherited
+                RepeatTestAttributes = method.GetCustomAttributes<IRepeatTest>(true);
+                WrapTestMethodAttributes = method.GetCustomAttributes<IWrapTestMethod>(true);
+                WrapSetupTearDownAttributes = method.GetCustomAttributes<IWrapSetUpTearDown>(true);
+                ApplyToContextAttributes = method.GetCustomAttributes<IApplyToContext>(true);
+
+                // non-inherited
+                TestBuilderAttributes = method.GetCustomAttributes<ITestBuilder>(false);
+                TestActionAttributes = method.GetCustomAttributes<ITestAction>(false);
+            }
+
+            public IParameterInfo[] Parameters { get; }
+            public bool IsAsyncOperation { get; }
+            public bool IsVoidOrUnit { get; }
+
+            public IRepeatTest[] RepeatTestAttributes { get; }
+            public ITestBuilder[] TestBuilderAttributes { get; }
+            public IWrapTestMethod[] WrapTestMethodAttributes { get; }
+            public ITestAction[] TestActionAttributes { get; }
+            public IWrapSetUpTearDown[] WrapSetupTearDownAttributes { get; }
+            public IApplyToContext[] ApplyToContextAttributes { get; }
+        }
+    }
+}

src/NUnitFramework/framework/Internal/Builders/NUnitTestCaseBuilder.cs

@@ -58,9 +58,11 @@ public TestMethod BuildTestMethod(IMethodInfo method, Test? parentSuite, TestCas
                 Seed = _randomizer.Next()
             };
 
-            CheckTestMethodAttributes(testMethod);
+            var metadata = MethodInfoCache.Get(method);
 
-            CheckTestMethodSignature(testMethod, parms);
+            CheckTestMethodAttributes(testMethod, metadata);
+
+            CheckTestMethodSignature(testMethod, metadata, parms);
 
             if (parms == null || parms.Arguments.Length == 0)
                 testMethod.ApplyAttributesToTest(method.MethodInfo);
@@ -112,10 +114,11 @@ public TestMethod BuildTestMethod(IMethodInfo method, Test? parentSuite, TestCas
         /// </summary>
         /// <param name="testMethod">The TestMethod to be checked. If it
         /// is found to be non-runnable, it will be modified.</param>
+        /// <param name="metadata">Metadata for this TestMethod.</param>
         /// <returns>True if the method signature is valid, false if not</returns>
-        private static bool CheckTestMethodAttributes(TestMethod testMethod)
+        private static bool CheckTestMethodAttributes(TestMethod testMethod, MethodInfoCache.TestMethodMetadata metadata)
         {
-            if (testMethod.Method.GetCustomAttributes<IRepeatTest>(true).Length > 1)
+            if (metadata.RepeatTestAttributes.Length > 1)
                 return MarkAsNotRunnable(testMethod, "Multiple attributes that repeat a test may cause issues.");
 
             return true;
@@ -135,22 +138,22 @@ private static bool CheckTestMethodAttributes(TestMethod testMethod)
         /// </summary>
         /// <param name="testMethod">The TestMethod to be checked. If it
         /// is found to be non-runnable, it will be modified.</param>
+        /// <param name="metadata">Metadata for this TestMethod.</param>
         /// <param name="parms">Parameters to be used for this test, or null</param>
         /// <returns>True if the method signature is valid, false if not</returns>
         /// <remarks>
         /// The return value is no longer used internally, but is retained
         /// for testing purposes.
         /// </remarks>
-        private static bool CheckTestMethodSignature(TestMethod testMethod, TestCaseParameters? parms)
+        private static bool CheckTestMethodSignature(TestMethod testMethod, MethodInfoCache.TestMethodMetadata metadata, TestCaseParameters? parms)
         {
             if (testMethod.Method.IsAbstract)
                 return MarkAsNotRunnable(testMethod, "Method is abstract");
 
             if (!testMethod.Method.IsPublic)
                 return MarkAsNotRunnable(testMethod, "Method is not public");
 
-            IParameterInfo[] parameters;
-            parameters = testMethod.Method.GetParameters();
+            IParameterInfo[] parameters = metadata.Parameters;
             int minArgsNeeded = 0;
             foreach (var parameter in parameters)
             {
@@ -180,7 +183,7 @@ private static bool CheckTestMethodSignature(TestMethod testMethod, TestCasePara
 
             var returnType = testMethod.Method.ReturnType.Type;
 
-            if (AsyncToSyncAdapter.IsAsyncOperation(testMethod.Method.MethodInfo))
+            if (metadata.IsAsyncOperation)
             {
                 if (returnType == typeof(void))
                     return MarkAsNotRunnable(testMethod, "Async test method must have non-void return type");
@@ -195,7 +198,7 @@ private static bool CheckTestMethodSignature(TestMethod testMethod, TestCasePara
                     return MarkAsNotRunnable(testMethod,
                         "Async test method must return an awaitable with a non-void result when a result is expected");
             }
-            else if (Reflect.IsVoidOrUnit(returnType))
+            else if (metadata.IsVoidOrUnit)
             {
                 if (parms != null && parms.HasExpectedResult)
                     return MarkAsNotRunnable(testMethod, "Method returning void cannot have an expected result");

src/NUnitFramework/framework/Internal/Commands/SetUpTearDownItem.cs

@@ -8,10 +8,10 @@
 // distribute, sublicense, and/or sell copies of the Software, and to
 // permit persons to whom the Software is furnished to do so, subject to
 // the following conditions:
-// 
+//
 // The above copyright notice and this permission notice shall be
 // included in all copies or substantial portions of the Software.
-// 
+//
 // THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
 // EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
 // MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
@@ -25,6 +25,7 @@
 using System.Collections.Generic;
 using System.Reflection;
 using NUnit.Framework.Interfaces;
+using NUnit.Framework.Internal.Builders;
 using NUnit.Framework.Internal.Execution;
 
 namespace NUnit.Framework.Internal.Commands
@@ -47,8 +48,8 @@ public class SetUpTearDownItem
         /// <param name="tearDownMethods">A list teardown methods for this level</param>
         /// <param name="methodValidator">A method validator to validate each method before calling.</param>
         public SetUpTearDownItem(
-            IList<IMethodInfo> setUpMethods, 
-            IList<IMethodInfo> tearDownMethods, 
+            IList<IMethodInfo> setUpMethods,
+            IList<IMethodInfo> tearDownMethods,
             IMethodValidator methodValidator = null)
         {
             _setUpMethods = setUpMethods;
@@ -113,7 +114,9 @@ private void RunSetUpOrTearDownMethod(TestExecutionContext context, IMethodInfo
             Guard.ArgumentNotAsyncVoid(method.MethodInfo, nameof(method));
             _methodValidator?.Validate(method.MethodInfo);
 
-            if (AsyncToSyncAdapter.IsAsyncOperation(method.MethodInfo))
+            var methodInfo = MethodInfoCache.Get(method);
+
+            if (methodInfo.IsAsyncOperation)
                 AsyncToSyncAdapter.Await(() => InvokeMethod(method, context));
             else
                 InvokeMethod(method, context);

src/NUnitFramework/framework/Internal/Commands/TestMethodCommand.cs

@@ -23,6 +23,7 @@
 
 using System;
 using NUnit.Framework.Interfaces;
+using NUnit.Framework.Internal.Builders;
 
 namespace NUnit.Framework.Internal.Commands
 {
@@ -76,7 +77,9 @@ public override TestResult Execute(TestExecutionContext context)
 
         private object RunTestMethod(TestExecutionContext context)
         {
-            if (AsyncToSyncAdapter.IsAsyncOperation(testMethod.Method.MethodInfo))
+            var methodInfo = MethodInfoCache.Get(testMethod.Method);
+
+            if (methodInfo.IsAsyncOperation)
             {
                 return AsyncToSyncAdapter.Await(() => InvokeTestMethod(context));
             }

src/NUnitFramework/framework/Internal/Execution/SimpleWorkItem.cs

@@ -25,6 +25,7 @@
 using System.Threading;
 using NUnit.Framework.Interfaces;
 using NUnit.Framework.Internal.Abstractions;
+using NUnit.Framework.Internal.Builders;
 using NUnit.Framework.Internal.Commands;
 using NUnit.Framework.Internal.Extensions;
 
@@ -111,10 +112,10 @@ private TestCommand MakeTestCommand()
                 // Command to execute test
                 TestCommand command = new TestMethodCommand(_testMethod);
 
-                var method = _testMethod.Method;
+                var method = MethodInfoCache.Get(_testMethod.Method);
 
                 // Add any wrappers to the TestMethodCommand
-                foreach (IWrapTestMethod wrapper in method.GetCustomAttributes<IWrapTestMethod>(true))
+                foreach (IWrapTestMethod wrapper in method.WrapTestMethodAttributes)
                     command = wrapper.Wrap(command);
 
                 // Create TestActionCommands using attributes of the method
@@ -158,11 +159,11 @@ private TestCommand MakeTestCommand()
                 }
 
                 // Add wrappers that apply before setup and after teardown
-                foreach (ICommandWrapper decorator in method.GetCustomAttributes<IWrapSetUpTearDown>(true))
+                foreach (ICommandWrapper decorator in method.WrapSetupTearDownAttributes)
                     command = decorator.Wrap(command);
 
                 // Add command to set up context using attributes that implement IApplyToContext
-                foreach (var attr in method.GetCustomAttributes<IApplyToContext>(true))
+                foreach (var attr in method.ApplyToContextAttributes)
                     command = new ApplyChangesToContextCommand(command, attr);
 
                 // Add a construct command and optionally a dispose command in case of instance per test case.
@@ -182,7 +183,7 @@ private TestCommand MakeTestCommand()
                     command = new TimeoutCommand(command, timeout, _debugger);
 
                 // Add wrappers for repeatable tests after timeout so the timeout is reset on each repeat
-                foreach (var repeatableAttribute in method.GetCustomAttributes<IRepeatTest>(true))
+                foreach (var repeatableAttribute in method.RepeatTestAttributes)
                     command = repeatableAttribute.Wrap(command);
 
                 return command;

src/NUnitFramework/framework/Internal/Filters/AndFilter.cs

@@ -21,18 +21,16 @@
 // WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
 // ***********************************************************************
 
-using System;
-using System.Collections.Generic;
 using System.Linq;
 using NUnit.Framework.Interfaces;
 
 namespace NUnit.Framework.Internal.Filters
 {
     /// <summary>
-    /// Combines multiple filters so that a test must pass all 
+    /// Combines multiple filters so that a test must pass all
     /// of them in order to pass this filter.
     /// </summary>
-    internal class AndFilter : CompositeFilter
+    internal sealed class AndFilter : CompositeFilter
     {
         /// <summary>
         /// Constructs an empty AndFilter

src/NUnitFramework/framework/Internal/Filters/CategoryFilter.cs

@@ -21,10 +21,7 @@
 // WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
 // ***********************************************************************
 
-using System;
 using System.Collections;
-using System.Collections.Generic;
-using System.Text;
 using NUnit.Framework.Interfaces;
 
 namespace NUnit.Framework.Internal.Filters
@@ -33,7 +30,7 @@ namespace NUnit.Framework.Internal.Filters
     /// CategoryFilter is able to select or exclude tests
     /// based on their categories.
     /// </summary>
-    internal class CategoryFilter : ValueMatchFilter
+    internal sealed class CategoryFilter : ValueMatchFilter
     {
         /// <summary>
         /// Construct a CategoryFilter using a single category name

src/NUnitFramework/framework/Internal/Filters/ClassNameFilter.cs

@@ -21,16 +21,17 @@
 // WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
 // ***********************************************************************
 
-using System;
 using NUnit.Framework.Interfaces;
 
 namespace NUnit.Framework.Internal.Filters
 {
     /// <summary>
     /// ClassName filter selects tests based on the class FullName
     /// </summary>
-    internal class ClassNameFilter : ValueMatchFilter
+    internal sealed class ClassNameFilter : ValueMatchFilter
     {
+        internal const string XmlElementName = "class";
+
         /// <summary>
         /// Construct a FullNameFilter for a single name
         /// </summary>
@@ -54,9 +55,6 @@ public override bool Match(ITest test)
         /// Gets the element name
         /// </summary>
         /// <value>Element name</value>
-        protected override string ElementName
-        {
-            get { return "class"; }
-        }
+        protected override string ElementName => XmlElementName;
     }
 }

src/NUnitFramework/framework/Internal/Filters/CompositeFilter.cs

@@ -21,7 +21,6 @@
 // WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
 // ***********************************************************************
 
-using System;
 using System.Collections.Generic;
 using System.Collections.ObjectModel;
 using NUnit.Framework.Interfaces;