Update Agent API invocations to use a Func or Action delegate instead…

… of reflection.

src/Agent/NewRelic/Agent/Core/AgentShim.cs

@@ -109,11 +109,11 @@ public static object GetFinishTracerDelegateFunc()
         }
 
         /// <summary>
-        /// This method is used to work around .net framework ad .net < 6.0 not supporting generic delegates
+        /// This method is used to work around .net framework and .net < 6.0 not supporting generic delegates
         /// with 11 parameters.
         /// </summary>
         /// <param name="parameters">An array of boxed parameters for the GetFinishTracerDelegate method.</param>
-        /// <returns><The delegate to invoke when a method completes or results in an exception./returns>
+        /// <returns>The delegate to invoke when a method completes or results in an exception.</returns>
         public static Action<object, Exception> GetFinishTracerDelegateParameterWrapper(object[] parameters)
         {
             return GetFinishTracerDelegate(

src/Agent/NewRelic/Agent/Core/ProfilerAgentMethodCallCache.cs

@@ -1,9 +1,10 @@
-// Copyright 2020 New Relic, Inc. All rights reserved.
+// Copyright 2020 New Relic, Inc. All rights reserved.
 // SPDX-License-Identifier: Apache-2.0
 
 using System;
 using System.Collections.Concurrent;
-using System.Reflection;
+using System.Collections.Generic;
+using System.Linq.Expressions;
 
 namespace NewRelic.Agent.Core
 {
@@ -12,50 +13,90 @@ namespace NewRelic.Agent.Core
     /// </summary>
     public class ProfilerAgentMethodCallCache
     {
-        private static readonly ConcurrentDictionary<string, MethodInfo> _methodInfoCache = new ConcurrentDictionary<string, MethodInfo>();
+        private static readonly ConcurrentDictionary<string, Func<object[], object>> _invokerCache = new ConcurrentDictionary<string, Func<object[], object>>();
 
         /// <summary>
         /// This method is invoked once using reflection by the byte-code injected by the profiler. This method is used to get a 
-        /// reference to the method that is used to cache reflection lookups for Agent methods. These methods are either
-        /// Public Agent API methods, or AgentShim methods.
+        /// reference to the method that is used to cache reflection lookups and Expression compilation for Agent methods.
+        /// This is currently used for Agent API methods.
         /// </summary>
         /// <returns>
-        /// A reference to the method that can access the method info cache. The result is treated as an object
+        /// A reference to the method that is used to invoked the requrested agent method. The result is treated as an object
         /// to simplify the type definition injected by the profiler to store this reference.
         /// </returns>
-        public static object GetMethodCacheFunc()
+        public static object GetInvokerFromCache()
         {
-            return (Func<string, string, string, Type[], MethodInfo>)GetAgentMethodInfoFromCache;
+            return (Func<string, string, string, Type[], Type, object[], object>)GetAndInvokeMethodFromCache;
         }
 
-        private static MethodInfo GetAgentMethodInfoFromCache(string key, string className, string methodName, Type[] types)
+        /// <summary>
+        /// This is the method that is invoked by the injected byte code for the Agent API.
+        /// It uses a cache so that the more expensive operations are only executed once.
+        /// This method will invoke the requested method using the specified parameters.
+        /// The method will always return a value. This value will either be the result of the
+        /// requested method call, or null if the requested method has a void return type.
+        /// </summary>
+        /// <param name="key">The cache key generated by the profiler.</param>
+        /// <param name="className">The name of the type containing the static method to execute.</param>
+        /// <param name="methodName">The name of the static method to execute.</param>
+        /// <param name="originalTypes">An array of types that represent the type for each parameter in the method to invoke.</param>
+        /// <param name="returnType">The type returned by the method to invoke. A null is used to represent a void return type.</param>
+        /// <param name="originalParameters">The parameters to invoke the method with.</param>
+        /// <returns>The value returned from invoking the requested method, or null if the method has a void return type.</returns>
+        public static object GetAndInvokeMethodFromCache(string key, string className, string methodName, Type[] originalTypes, Type returnType, object[] originalParameters)
         {
-            var methodInfoFactory = new MethodInfoCacheItemFactory(className, methodName, types);
-            return _methodInfoCache!.GetOrAdd(key, methodInfoFactory.GetMethodInfo);
+            var types = originalTypes ?? Array.Empty<Type>();
+            var parameters = originalParameters ?? Array.Empty<object>();
+
+            if (!_invokerCache.TryGetValue(key, out var invoker))
+            {
+                var type = Type.GetType(className);
+                var mi = type.GetMethod(methodName, types);
+                var delegateType = returnType == null ? Expression.GetActionType(types) : Expression.GetFuncType(GetTypesForFunc(types, returnType));
+                var methodDelegate = mi.CreateDelegate(delegateType);
+
+                /* Create a function similar to the following code
+                 * object InovkeMethod(object[] parameters) {
+                 *   return methodDelegate.Invoke((types[0])parameters[0]);
+                 * }
+                 */
+                var parametersParam = Expression.Parameter(typeof(object[]), "parameters");
+                var methodToCall = Expression.Constant(methodDelegate, delegateType);
+                IEnumerable<Expression> parameterExpressions = GetInvokerParameterExpressions(parametersParam, types);
+                var invocationExpression = Expression.Invoke(methodToCall, parameterExpressions);
+
+                var lambdaExpression = returnType != null ?
+                    Expression.Lambda<Func<object[], object>>(invocationExpression, parametersParam) :
+                    Expression.Lambda<Func<object[], object>>(Expression.Block(invocationExpression, Expression.Constant(null)), parametersParam);
+
+                invoker = lambdaExpression.Compile();
+                _invokerCache.TryAdd(key, invoker);
+            }
+
+            return invoker(parameters);
         }
 
-        // Using a struct to try to take advantage of stack allocation to reduce overall allocations.
-        private struct MethodInfoCacheItemFactory
+        private static IEnumerable<Expression> GetInvokerParameterExpressions(Expression parametersExpression, Type[] types)
         {
-            private string _className;
-            private string _methodName;
-            private Type[] _types;
-
-            public MethodInfoCacheItemFactory(string className, string methodName, Type[] types)
+            for (var i = 0; i < types.Length; i++)
             {
-                _className = className;
-                _methodName = methodName;
-                _types = types;
+                var getParameterAtIndex = Expression.ArrayAccess(parametersExpression, Expression.Constant(i));
+                yield return Expression.Convert(getParameterAtIndex, types[i]);
             }
+        }
 
-            public MethodInfo GetMethodInfo(string _)
+        private static Type[] GetTypesForFunc(Type[] parameterTypes, Type returnType)
+        {
+            if (parameterTypes.Length == 0)
             {
-                var type = Type.GetType(_className);
+                return new Type[] { returnType };
+            }
 
-                var method = _types != null ? type.GetMethod(_methodName, _types) : type.GetMethod(_methodName);
+            var types = new Type[parameterTypes.Length + 1];
+            Array.Copy(parameterTypes, types, parameterTypes.Length);
+            types[parameterTypes.Length] = returnType;
 
-                return method;
-            }
+            return types;
         }
     }
 }

src/Agent/NewRelic/Profiler/MethodRewriter/ApiFunctionManipulator.h

@@ -45,21 +45,68 @@ namespace NewRelic { namespace Profiler { namespace MethodRewriter
             TryCatch(
                 [&]()
                 {
-                    // delegate = System.CannotUnloadAppDomainException.GetMethodFromAppDomainStorageOrReflectionOrThrow("NewRelic_Delegate_API_<function name><function signature>", "C:\path\to\NewRelic.Agent.Core", "NewRelic.Core.AgentApi", "<function name>", new object[] { <method parameter types> })
-                    LoadMethodInfo(_instrumentationSettings->GetCorePath(), _X("NewRelic.Agent.Core.AgentApi"), _function->GetFunctionName(), _function->GetFunctionId(), GetArrayOfTypeParametersLamdba());
-
-                    _instructions->Append(_X("ldnull"));
-                    BuildObjectArrayOfParameters();
-                    _instructions->Append(_X("call   instance object [") + _instructions->GetCoreLibAssemblyName() + _X("]System.Reflection.MethodBase::Invoke(object, object[])"));
-
-                    if (_methodSignature->_returnType->_kind == SignatureParser::ReturnType::Kind::VOID_RETURN_TYPE)
+                    if (_agentCallStrategy == AgentCallStyle::Strategy::InAgentCache)
                     {
-                        _instructions->Append(_X("pop"));
+                        // Ensure that the managed agent is loaded
+                        _instructions->AppendString(_instrumentationSettings->GetCorePath());
+                        _instructions->Append(_X("call void [") + _instructions->GetCoreLibAssemblyName() + _X("]System.CannotUnloadAppDomainException::EnsureInitialized(string)"));
+
+                        // Get the Func holding a reference to the ProfilerAgentMethodCallCache.GetAndInvokeMethodFromCache method
+                        _instructions->Append(_X("call object [") + _instructions->GetCoreLibAssemblyName() + _X("]System.CannotUnloadAppDomainException::GetMethodCacheLookupMethod()"));
+                        _instructions->Append(CEE_CASTCLASS, _X("class [") + _instructions->GetCoreLibAssemblyName() + _X("]System.Func`7<string, string, string, class [") + _instructions->GetCoreLibAssemblyName() + _X("]System.Type[], class [") + _instructions->GetCoreLibAssemblyName() + _X("]System.Type, object[], object>"));
+
+                        xstring_t className = _X("NewRelic.Agent.Core.AgentApi");
+                        xstring_t methodName = _function->GetFunctionName();
+                        xstring_t keyName = className + _X(".") + methodName + _X("_") + to_xstring((unsigned long)_function->GetFunctionId());
+                        auto argumentTypesLambda = GetArrayOfTypeParametersLamdba();
+
+                        _instructions->AppendString(keyName);
+                        _instructions->AppendString(className);
+                        _instructions->AppendString(methodName);
+
+                        if (argumentTypesLambda == NULL)
+                        {
+                            _instructions->Append(CEE_LDNULL);
+                        }
+                        else
+                        {
+                            argumentTypesLambda();
+                        }
+
+                        _instructions->AppendTypeOfArgument(_methodSignature->_returnType);
+
+                        BuildObjectArrayOfParameters();
+
+                        _instructions->Append(CEE_CALLVIRT, _X("instance !6 class [") + _instructions->GetCoreLibAssemblyName() + _X("]System.Func`7<string, string, string, class [") + _instructions->GetCoreLibAssemblyName() + _X("]System.Type[], class [") + _instructions->GetCoreLibAssemblyName() + _X("]System.Type, object[], object>::Invoke(!0, !1, !2, !3, !4, !5)"));
+
+                        if (_methodSignature->_returnType->_kind == SignatureParser::ReturnType::Kind::VOID_RETURN_TYPE)
+                        {
+                            _instructions->Append(_X("pop"));
+                        }
+                        else {
+                            // we can't leave an object on the stack and CEE_LEAVE a protected block.
+                            // we have to store it in a local and reload it outside of the try..catch.
+                            _instructions->AppendStoreLocal(resultLocalIndex);
+                        }
                     }
-                    else {
-                        // we can't leave an object on the stack and CEE_LEAVE a protected block.
-                        // we have to store it in a local and reload it outside of the try..catch.
-                        _instructions->AppendStoreLocal(resultLocalIndex);
+                    else
+                    {
+                        // delegate = System.CannotUnloadAppDomainException.GetMethodFromAppDomainStorageOrReflectionOrThrow("NewRelic_Delegate_API_<function name><function signature>", "C:\path\to\NewRelic.Agent.Core", "NewRelic.Core.AgentApi", "<function name>", new object[] { <method parameter types> })
+                        LoadMethodInfo(_instrumentationSettings->GetCorePath(), _X("NewRelic.Agent.Core.AgentApi"), _function->GetFunctionName(), _function->GetFunctionId(), GetArrayOfTypeParametersLamdba());
+
+                        _instructions->Append(_X("ldnull"));
+                        BuildObjectArrayOfParameters();
+                        _instructions->Append(_X("call   instance object [") + _instructions->GetCoreLibAssemblyName() + _X("]System.Reflection.MethodBase::Invoke(object, object[])"));
+
+                        if (_methodSignature->_returnType->_kind == SignatureParser::ReturnType::Kind::VOID_RETURN_TYPE)
+                        {
+                            _instructions->Append(_X("pop"));
+                        }
+                        else {
+                            // we can't leave an object on the stack and CEE_LEAVE a protected block.
+                            // we have to store it in a local and reload it outside of the try..catch.
+                            _instructions->AppendStoreLocal(resultLocalIndex);
+                        }
                     }
                 },
                 [&]()

src/Agent/NewRelic/Profiler/MethodRewriter/FunctionManipulator.h

@@ -320,25 +320,7 @@ namespace NewRelic { namespace Profiler { namespace MethodRewriter
         // The function id is used as a tie-breaker for overloaded methods when computing the key name for the cache.
         void LoadMethodInfo(xstring_t assemblyPath, xstring_t className, xstring_t methodName, uintptr_t functionId, std::function<void()> argumentTypesLambda)
         {
-            if (_agentCallStrategy == AgentCallStyle::Strategy::InAgentCache)
-            {
-                auto keyName = className + _X(".") + methodName + _X("_") + to_xstring((unsigned long)functionId);
-                _instructions->AppendString(keyName);
-                _instructions->AppendString(assemblyPath);
-                _instructions->AppendString(className);
-                _instructions->AppendString(methodName);
-                if (argumentTypesLambda == NULL)
-                {
-                    _instructions->Append(CEE_LDNULL);
-                }
-                else
-                {
-                    argumentTypesLambda();
-                }
-
-                _instructions->Append(CEE_CALL, _X("class [") + _instructions->GetCoreLibAssemblyName() + _X("]System.Reflection.MethodInfo [") + _instructions->GetCoreLibAssemblyName() + _X("]System.CannotUnloadAppDomainException::GetMethodInfoFromAgentCache(string,string,string,string,class [") + _instructions->GetCoreLibAssemblyName() + _X("]System.Type[])"));
-            }
-            else if (_agentCallStrategy == AgentCallStyle::Strategy::AppDomainCache)
+            if (_agentCallStrategy == AgentCallStyle::Strategy::AppDomainCache)
             {
                 auto keyName = className + _X(".") + methodName + _X("_") + to_xstring((unsigned long)functionId);
                 _instructions->AppendString(keyName);
@@ -356,11 +338,16 @@ namespace NewRelic { namespace Profiler { namespace MethodRewriter
 
                 _instructions->Append(CEE_CALL, _X("class [") + _instructions->GetCoreLibAssemblyName() + _X("]System.Reflection.MethodInfo [") + _instructions->GetCoreLibAssemblyName() + _X("]System.CannotUnloadAppDomainException::GetMethodFromAppDomainStorageOrReflectionOrThrow(string,string,string,string,class [") + _instructions->GetCoreLibAssemblyName() + _X("]System.Type[])"));
             }
-            else
+            else if (_agentCallStrategy == AgentCallStyle::Strategy::Reflection)
             {
                 LoadType(assemblyPath, className);
                 LoadMethodInfoFromType(methodName, argumentTypesLambda);
             }
+            else
+            {
+                LogError(L"Attempting to call LoadMethodInfo when an AgentCallStyle::Strategy that does not support loading method info is configured.");
+                throw FunctionManipulatorException();
+            }
         }
 
         // Creates an array of elementLoadLanbdas.size() and loads the elements into the array

src/Agent/NewRelic/Profiler/MethodRewriter/HelperFunctionManipulator.h

@@ -211,7 +211,7 @@ namespace NewRelic { namespace Profiler { namespace MethodRewriter
         {
             _instructions->Append(CEE_LDARG_0);
             _instructions->AppendString(_X("NewRelic.Agent.Core.ProfilerAgentMethodCallCache"));
-            _instructions->AppendString(_X("GetMethodCacheFunc"));
+            _instructions->AppendString(_X("GetInvokerFromCache"));
             _instructions->Append(CEE_LDNULL);
             _instructions->Append(CEE_CALL, _X("class System.Reflection.MethodInfo System.CannotUnloadAppDomainException::GetMethodViaReflectionOrThrow(string,string,string,class System.Type[])"));
 

src/Agent/NewRelic/Profiler/MethodRewriter/InstructionSet.h

@@ -346,6 +346,22 @@ namespace NewRelic { namespace Profiler { namespace MethodRewriter
             }
         }
 
+        void AppendTypeOfArgument(SignatureParser::ReturnTypePtr returnType)
+        {
+            auto typeToken = GetTypeTokenForReturnType(returnType);
+
+            if (typeToken != 0)
+            {
+                Append(CEE_LDTOKEN, typeToken);
+                Append(_X("call class [") + _coreLibAssemblyName + _X("]System.Type[") + _coreLibAssemblyName + _X("]System.Type::GetTypeFromHandle(valuetype[") + _coreLibAssemblyName + _X("]System.RuntimeTypeHandle)"));
+            }
+            else
+            {
+                // void return type
+                Append(CEE_LDNULL);
+            }
+        }
+
         // append a load argument instruction
         void AppendLoadArgument(uint16_t argumentIndex)
         {
@@ -544,6 +560,32 @@ namespace NewRelic { namespace Profiler { namespace MethodRewriter
             }
         }
 
+        uint32_t GetTypeTokenForReturnType(SignatureParser::ReturnTypePtr returnType)
+        {
+            switch (returnType->_kind)
+            {
+                case SignatureParser::ReturnType::Kind::VOID_RETURN_TYPE:
+                {
+                    return 0;
+                }
+                case SignatureParser::ReturnType::Kind::TYPED_BY_REF_RETURN_TYPE:
+                {
+                    auto token = _tokenizer->GetTypeRefToken(_coreLibAssemblyName, _X("System.TypedReference"));
+                    return token;
+                }
+                case SignatureParser::ReturnType::Kind::TYPED_RETURN_TYPE:
+                {
+                    auto typedReturnType = std::static_pointer_cast<SignatureParser::TypedReturnType>(returnType);
+                    return GetTypeTokenForType(typedReturnType->_type);
+                }
+                default:
+                {
+                    LogError(L"Unhandled return type encountered in InstructionSet::GetTypeTokenForReturnType. Kind: ", std::hex, std::showbase, returnType->_kind, std::resetiosflags(std::ios_base::basefield | std::ios_base::showbase));
+                    throw InstructionSetException();
+                }
+            }
+        }
+
         uint32_t GetTypeTokenForType(SignatureParser::TypePtr type)
         {
             switch (type->_kind)