Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Providing Generic of Return Type to Enable Async Support #89

Open
mesheets opened this issue Oct 13, 2021 · 11 comments
Open

Providing Generic of Return Type to Enable Async Support #89

mesheets opened this issue Oct 13, 2021 · 11 comments

Comments

@mesheets
Copy link

mesheets commented Oct 13, 2021

Is it feasible to update the method signature of the On{Event}() methods to include a generic of the return type in the method signature?

  • OnEntry<TReturnType>(MethodExecutionArgs args)
  • OnExit<TReturnType>(MethodExecutionArgs args)
  • OnException<TReturnType>(MethodExecutionArgs args)

At least to the extent needed for the use case in question, I have been able to get a workable solution for async support; however, it relies heavily on reflection. If it were possible to update the method signature of the On{Event}() methods to include a generic of the return type, the performance hit due to using reflection could be eliminated.

There have been other use cases I've encountered where having a generic of the return type would have been very helpful, so its usefulness would seem to extend beyond just this async scenario.

Below is a sample code snippet that uses reflection. Reflection is necessary due to generic type variance rules (including for Task<T>), so having the return type as a generic was necessary in order to properly assign args.ReturnValue without an error. Eliminate the availability of the return type generic, and the assigning of the .ContinueWith().Unwrap() Task<T> to args.ReturnValue doesn't work.

public class OnAsyncMethodBoundaryAspect : OnMethodBoundaryAspect
{
    public override void OnExit(MethodExecutionArgs args)
    {
        base.OnExit(args);

        if (((MethodInfo)args.Method).ReturnType.IsSubclassOf(typeof(Task)))
        {
            // Handle async methods
            if (null != args.ReturnValue)
            {
                if (args.ReturnValue.GetType().IsGenericType)
                {
                    // We are dealing with a return type of type Task<T>
                    // TODO: Error handling and optimization
                    MethodInfo method = typeof(OnAsyncMethodBoundaryAspect).GetMethod("InvokeApiCall", new Type[] { typeof(MethodExecutionArgs) });
                    Type taskResultType = ((MethodInfo)args.Method).ReturnType.GenericTypeArguments[0];
                    MethodInfo genericMethod = method.MakeGenericMethod(taskResultType);
                    genericMethod.Invoke(this, new object[] { args });
                }
                else
                {
                    // TODO: async method with no result type (non-generic Task)
                }
            }
            else
            {
                // TODO: Return value is of type Task, but the task is null
            }
        }
        else
        {
            // Handle non-async methods
            // Application-specific code here
            // args.ReturnValue = Call<T>(args.Instance, (MethodInfo)args.Method, args.Arguments);
        }
    }

    public void InvokeApiCall<T>(MethodExecutionArgs args)
    {
        // Without unwrapping the .ContinueWith() task and then assigning to args.ReturnValue, the method caller receives the original result instead of the modified result
        // c.f. https://stackoverflow.com/q/22797340
        // The above link does not reference Unwrap(), but it does describe the reason for the need
        Task<T> continuedTask =
            ((Task<T>)args.ReturnValue).ContinueWith(t =>
                {
                    // Application-specific code here; example:
                    //  CallAsync<T>(args.Instance, (MethodInfo)args.Method, args.Arguments)
                }
            ).Unwrap();

        args.ReturnValue = continuedTask;
    }
}
@Ralf1108
Copy link
Collaborator

I think part of the performance problems are because of #85

In your code you could try to cache the created generic method.
I played with your code and using expression tree + compile increased the performance a little bit:

namespace BoundaryAspectTestCore
{
    class Program
    {
        private MethodBase _currentMethod;

        public static async Task Main(string[] args)
        {
            var p = new Program();
            var result = await p.TestCall(1, 2, 3);
            Console.WriteLine("1. intercepted result was: " + result);

            result = await p.TestCall(1, 2, 3);
            Console.WriteLine("2. intercepted result was: " + result);

            Console.WriteLine("Benchmark invocation");
            var sw = Stopwatch.StartNew();
            var count = 3000000;
            for (var i = 0; i < count; i++) 
                result += await p.TestCallWithoutOutput(1, 2, 3);
            sw.Stop();

            var durationPerInvocation = (double) sw.ElapsedMilliseconds / count * 1000;
            Console.WriteLine($"Duration per 1000 invocation: {durationPerInvocation}ms");
            Console.WriteLine($"Result was: {result}");
        }

        [OnAsyncMethodBoundaryAspect]
        public Task<int> TestCall(int i1, int i2, int i3)
        {
            var result = i1 + i2 + i3;
            Console.WriteLine("Real result was: " + result);
            return Task.FromResult(result);
        }

        [OnAsyncMethodBoundaryAspect]
        public Task<int> TestCallWithoutOutput(int i1, int i2, int i3)
        {
            var result = 0;
            for (var i = 0; i < 1000; i++)
                result += i1 + i2 + i3;

            return Task.FromResult(result);
        }

        public Task<int> Direct(int i1, int i2, int i3)
        {
            _currentMethod ??= MethodBase.GetCurrentMethod();
            var args = new MethodExecutionArgs
            {
                Instance = this,
                Method = _currentMethod,
                Arguments = new object[] {i1, i2, i3},
                ReturnValue = Task.FromResult(44)
            };

            var aspect = new OnAsyncMethodBoundaryAspect();
            aspect.OnExit(args);

            return (Task<int>)args.ReturnValue;
        }
    }

    public class OnAsyncMethodBoundaryAspect : OnMethodBoundaryAspect
    {
        private static readonly MethodInfo MethodInvokeApiCall;
        private static readonly MethodInfo MethodInvokeApiCallUntyped;

        private static readonly ConcurrentDictionary<Type, MethodInfo> MethodCacheReflection =
            new ConcurrentDictionary<Type, MethodInfo>();

        private static readonly ConcurrentDictionary<Type, LateBoundFunc> MethodCacheCompiled =
            new ConcurrentDictionary<Type, LateBoundFunc>();

        private static readonly ConcurrentDictionary<Type, Func<OnAsyncMethodBoundaryAspect, object, object>> MethodCacheCompiledUntyped =
            new ConcurrentDictionary<Type, Func<OnAsyncMethodBoundaryAspect, object, object>>();

        private delegate object LateBoundFunc(object target, params object[] arguments);

        static OnAsyncMethodBoundaryAspect()
        {
            MethodInvokeApiCall =
                typeof(OnAsyncMethodBoundaryAspect).GetMethod("InvokeApiCall", new[] {typeof(MethodExecutionArgs)});
            MethodInvokeApiCallUntyped =
                typeof(OnAsyncMethodBoundaryAspect).GetMethod("InvokeApiCallUntyped", new[] {typeof(object)});
        }

        public override void OnExit(MethodExecutionArgs args)
        {
            base.OnExit(args);

            var methodInfo = (MethodInfo) args.Method;
            var returnType = methodInfo.ReturnType;
            if (returnType.IsSubclassOf(typeof(Task)))
            {
                // Handle async methods
                if (args.ReturnValue != null)
                {
                    if (returnType.IsGenericType) // Task<T>
                    {
                        // We are dealing with a return type of type Task<T>
                        var taskResultType = returnType.GenericTypeArguments[0];

                        ////CallViaReflection(args, taskResultType);
                        ////CallViaCompiled(args, taskResultType);
                        CallViaCompiledUntyped(args, taskResultType);
                    }
                    else // Task
                    {
                        // TODO: async method with no result type (non-generic Task)
                    }
                }
                else
                {
                    // TODO: Return value is of type Task, but the task is null
                }
            }
            else
            {
                // Handle non-async methods
                // Application-specific code here
                // args.ReturnValue = Call<T>(args.Instance, (MethodInfo)args.Method, args.Arguments);
            }
        }

        public Task<T> InvokeApiCall<T>(MethodExecutionArgs args)
        {
            var argsReturnValue = (Task<T>) args.ReturnValue;
            var continuedTask = argsReturnValue
                .ContinueWith(
                    t =>
                    {
                        // Application-specific code here; example:
                        //  CallAsync<T>(args.Instance, (MethodInfo)args.Method, args.Arguments)
                        return Task.FromResult((T)(object)42); // dummy value
                    }
                ).Unwrap();

            return continuedTask;
        }

        public object InvokeApiCallUntyped<T>(object args)
        {
            return InvokeApiCall<T>((MethodExecutionArgs) args);
        }

        private void CallViaCompiledUntyped(MethodExecutionArgs args, Type taskResultType)
        {
            if (!MethodCacheCompiledUntyped.TryGetValue(taskResultType, out var genericMethod))
            {
                // ensure that it is created once
                Console.WriteLine("Created generic method for: " + taskResultType);
                var genericMethodInfo2 = MethodInvokeApiCallUntyped.MakeGenericMethod(taskResultType);

                ParameterExpression expression;
                ParameterExpression expression2;

                genericMethod = Expression.Lambda<Func<OnAsyncMethodBoundaryAspect, object, object>>(
                        Expression.Call(
                            expression = Expression.Parameter(typeof(OnAsyncMethodBoundaryAspect), "target"),
                            genericMethodInfo2,
                            expression2 = Expression.Parameter(typeof(object), "args"))
                        , expression, expression2)
                    .Compile();

                MethodCacheCompiledUntyped.TryAdd(taskResultType, genericMethod);
            }

            args.ReturnValue = genericMethod(this, args);
        }

        private void CallViaCompiled(MethodExecutionArgs args, Type taskResultType)
        {
            if (!MethodCacheCompiled.TryGetValue(taskResultType, out var genericMethod))
            {
                // ensure that it is created once
                Console.WriteLine("Created generic method for: " + taskResultType);
                var genericMethodInfo2 = MethodInvokeApiCall.MakeGenericMethod(taskResultType);

                // compile to mitigate reflection invocation costs
                ParameterExpression expression;
                ParameterExpression expression2;
                genericMethod = Expression.Lambda<LateBoundFunc>(
                        Expression.Convert(
                            Expression.Call(
                                Expression.Convert(expression = Expression.Parameter(typeof(object), "target"),
                                    genericMethodInfo2.DeclaringType),
                                genericMethodInfo2,
                                CreateParameterExpressions(genericMethodInfo2,
                                    expression2 = Expression.Parameter(typeof(object[]), "arguments"))),
                            typeof(object)), expression, expression2)
                    .Compile();
                MethodCacheCompiled.TryAdd(taskResultType, genericMethod);
            }

            args.ReturnValue = genericMethod(this, args);
        }

        private void CallViaReflection(MethodExecutionArgs args, Type taskResultType)
        {
            if (!MethodCacheReflection.TryGetValue(taskResultType, out var genericMethodInfo))
            {
                // ensure that it is created once
                Console.WriteLine("Created generic method info for: " + taskResultType);
                genericMethodInfo = MethodInvokeApiCall.MakeGenericMethod(taskResultType);
                MethodCacheReflection.TryAdd(taskResultType, genericMethodInfo);
            }

            args.ReturnValue = genericMethodInfo.Invoke(this, new object[] {args});
        }

        public static UnaryExpression[] CreateParameterExpressions(MethodInfo method, Expression argumentsParameter)
        {
            return
                method.GetParameters().Select(
                        (parameter, index) =>
                            Expression.Convert(
                                Expression.ArrayIndex(argumentsParameter, Expression.Constant(index)),
                                parameter.ParameterType))
                    .ToArray();
        }
    }

@Ralf1108
Copy link
Collaborator

#85 is now solved.
please check and report if you observe other performance problems

@mesheets
Copy link
Author

mesheets commented Oct 18, 2021

Thank you, I will check the updates committed with #85 and report back.

EDIT:
Linking to this method for future reference.

@Ralf1108
Copy link
Collaborator

Please checkout v2.0.144 because this release includes performance optimizations also for open generic methods.

@mesheets
Copy link
Author

mesheets commented Jan 28, 2022

In building this out, I have attempted to refactor the code into something more usable; however, I have been encountering issues that might appear to relate to how MethodBoundaryAspect.Fody is implemented.

My approach is to split this into two parts:

  1. Handle generic OnEntry<T>() / OnExit<T>() / OnException<T>()
  2. Handle setting arg.ReturnValue when the decorated method is async.

For the generic part, there are few particular sticking points

  1. The methods OnEntry() / OnExit() / OnException() must be defined before the methods OnEntry<T>() / OnExit<T>() / OnException<T>(); otherwise, a nebulous "System.InvalidProgramException: 'Common Language Runtime detected an invalid program.'" error will occur during runtime.
  2. If I create a class OnTypedMethodBoundaryAspect that inherits OnMethodBoundaryAspect and implements all of the details necessary for handling generic OnEntry<T>() / OnExit<T>() / OnException<T>() methods, I cannot in turn create and use a "MyTypedMethodBoundaryAttribute" class for implementing application-specific details, as the presence of any method decorated with [MyTypedMethodBoundary] will cause a build to fail with the following error:

Fody: An unhandled exception occurred:
Exception:
Failed to execute weaver C:\Users\766722.nuget\packages\methodboundaryaspect.fody\2.0.144\build..\weaver\MethodBoundaryAspect.Fody.dll
Type:
System.Exception
StackTrace:
at InnerWeaver.ExecuteWeavers() in C:\projects\fody\FodyIsolated\InnerWeaver.cs:line 210
at InnerWeaver.Execute() in C:\projects\fody\FodyIsolated\InnerWeaver.cs:line 111
Source:
FodyIsolated
TargetSite:
Void ExecuteWeavers()
Sequence contains more than one matching element
Type:
System.InvalidOperationException
StackTrace:
at System.Linq.Enumerable.Single[TSource](IEnumerable`1 source, Func`2 predicate)
at MethodBoundaryAspect.Fody.InstructionBlockChainCreator.CreateMethodExecutionArgsInstance(NamedInstructionBlockChain argumentsArrayChain, TypeReference anyAspectTypeDefinition, MethodDefinition method, MethodInfoCompileTimeWeaver methodInfoCompileTimeWeaver)
at MethodBoundaryAspect.Fody.MethodWeaver.WeaveMethodExecutionArgs(NamedInstructionBlockChain arguments)
at MethodBoundaryAspect.Fody.MethodWeaver.Weave()
at MethodBoundaryAspect.Fody.ModuleWeaver.WeaveMethod(ModuleDefinition module, MethodDefinition method, List`1 aspectInfos, MethodInfoCompileTimeWeaver methodInfoCompileTimeWeaver)
at MethodBoundaryAspect.Fody.ModuleWeaver.WeaveType(ModuleDefinition module, TypeDefinition type, Collection`1 assemblyMethodBoundaryAspects)
at MethodBoundaryAspect.Fody.ModuleWeaver.WeaveTypeAndNestedTypes(ModuleDefinition module, TypeDefinition type, Collection`1 assemblyMethodBoundaryAspects)
at MethodBoundaryAspect.Fody.ModuleWeaver.Execute(ModuleDefinition module)
at InnerWeaver.ExecuteWeavers() in C:\projects\fody\FodyIsolated\InnerWeaver.cs:line 198
Source:
System.Core
TargetSite:
TSource Single[TSource](System.Collections.Generic.IEnumerable`1[TSource], System.Func`2[TSource,System.Boolean])

An additional apparent problem is that if a particular OnEntry() / OnExit() / OnException() override is not used, optimizations occur that avoid calling virtual OnEntry() / OnExit() / OnException() methods that are not overridden. For existing practices, that is fine; however, as OnEntry() / OnExit() / OnException() overrides are used to implement OnEntry<T>() / OnExit<T>() / OnException<T>(), that means the generic form of the method would always be called regardless of whether an override exists for a given generic form of the method (OnEntry<T>() / OnExit<T>() / OnException<T>()).

What would be the best way to address these issues and facilitate a reusable implementation?

Example Code

using System.Reflection;
using MethodBoundaryAspect.Fody.Attributes;

namespace EnhancedMethodBoundaryAspect
{
    class Program
    {
        private MethodBase _currentMethod;

        public static async Task Main(string[] args)
        {
            var p = new Program();
            var result = 0;

            result = p.TestGenericCall();
            Console.WriteLine("0. intercepted result was: " + result);
        }

        // This attribute represents a more reusable form but triggers a build error [MyTypedMethodBoundary]
        [OnTypedMethodBoundaryAspect]
        public int TestGenericCall()
        {
            int result = 100;
            return result;
        }
    }

    public class MyTypedMethodBoundaryAttribute : OnTypedMethodBoundaryAspect
    {
        public override void OnEntry<T>(MethodExecutionArgs arg)
        {
            base.OnEntry<T>(arg);
            arg.ReturnValue = 1;
        }

        public override void OnExit<T>(MethodExecutionArgs arg)
        {
            base.OnExit<T>(arg);
            arg.ReturnValue = 21;
        }

        public override void OnException<T>(MethodExecutionArgs arg)
        {
            base.OnException<T>(arg);
            arg.ReturnValue = -1;
        }
    }

    public class OnTypedMethodBoundaryAspect : OnMethodBoundaryAspect
    {
        private class TypedMethodBoundaryData
        {
            public readonly MethodInfo UntypedMethod;
            public readonly ConcurrentDictionary<Type, Action<OnTypedMethodBoundaryAspect, object>> CompiledUntypedMethodCache =
                new ConcurrentDictionary<Type, Action<OnTypedMethodBoundaryAspect, object>>();

            public TypedMethodBoundaryData(string methodName)
            {
                UntypedMethod =
                    typeof(OnTypedMethodBoundaryAspect).GetMethod(methodName + "Untyped"
                        , BindingFlags.NonPublic | BindingFlags.Instance, new[] { typeof(object) });
            }
        }

        private TypedMethodBoundaryData MethodOnEntryUntyped = new TypedMethodBoundaryData("OnEntry");
        private TypedMethodBoundaryData MethodOnExitUntyped = new TypedMethodBoundaryData("OnExit");
        private TypedMethodBoundaryData MethodOnExceptionUntyped = new TypedMethodBoundaryData("OnException");


        public override void OnEntry(MethodExecutionArgs arg)
        {
            base.OnEntry(arg);
            OnMethodBoundary(MethodOnEntryUntyped, arg);
        }

        public override void OnExit(MethodExecutionArgs arg)
        {
            base.OnExit(arg);
            OnMethodBoundary(MethodOnExitUntyped, arg);
        }

        public override void OnException(MethodExecutionArgs arg)
        {
            base.OnException(arg);
            OnMethodBoundary(MethodOnExceptionUntyped, arg);
        }


        // Must not be defined before OnEntry()
        public virtual void OnEntry<T>(MethodExecutionArgs arg) { }

        // Must not be defined before OnExit()
        public virtual void OnExit<T>(MethodExecutionArgs arg) { arg.ReturnValue = 1024; }

        // Must not be defined before OnException()
        public virtual void OnException<T>(MethodExecutionArgs arg) { }


        private void OnMethodBoundary(TypedMethodBoundaryData methodData, MethodExecutionArgs arg)
        {
            MethodInfo methodInfoOfDecoratedMethod = (MethodInfo)arg.Method;
            Type returnTypeOfDecoratedMethod = methodInfoOfDecoratedMethod.ReturnType;

            if (returnTypeOfDecoratedMethod == null || returnTypeOfDecoratedMethod == typeof(void))
            {
                // A valid Type is needed for the Generic <T>, so default to object
                returnTypeOfDecoratedMethod = typeof(object);
            }

            CallViaCompiledUntyped(methodData, arg, returnTypeOfDecoratedMethod);
        }

        private void CallViaCompiledUntyped(TypedMethodBoundaryData methodData, MethodExecutionArgs arg, Type returnType)
        {
            if (!methodData.CompiledUntypedMethodCache.TryGetValue(returnType, out var genericMethod))
            {
                // ensure that it is created once
                Console.WriteLine("Created generic method for: " + returnType.Name);
                MethodInfo genericMethodInfo2 = methodData.UntypedMethod.MakeGenericMethod(returnType);

                ParameterExpression expression1 = Expression.Parameter(typeof(OnTypedMethodBoundaryAspect), "target");
                ParameterExpression expression2 = Expression.Parameter(typeof(object), "arg");

                genericMethod = Expression.Lambda<Action<OnTypedMethodBoundaryAspect, object>>(
                        Expression.Call(expression1, genericMethodInfo2, expression2)
                        , expression1, expression2)
                    .Compile();

                methodData.CompiledUntypedMethodCache.TryAdd(returnType, genericMethod);
            }

            genericMethod(this, arg);
        }


        private void OnEntryUntyped<T>(object arg)
        {
            OnEntry<T>((MethodExecutionArgs)arg);
        }

        private void OnExitUntyped<T>(object arg)
        {
            OnExit<T>((MethodExecutionArgs)arg);
        }

        private void OnExceptionUntyped<T>(object arg)
        {
            OnException<T>((MethodExecutionArgs)arg);
        }
    }
}

@mesheets
Copy link
Author

Following on the above post (including the noted issues that still exist), here is an update that incorporates async capabilities. In addition to the existing three overrides [ OnEntry(), OnExit(), OnException() ], this proposes to add the following:

  • OnEntry<T>()
  • OnExit<T>()
  • OnExitWithTask()
  • OnExitWithTask<T>()
  • OnException<T>()

Updated code is below. If there is a way to work out the inheritance problem referenced in the post above, it would be nice to hide these implementation details similar to how OnMethodBoundaryAspect does.

using System.Collections.Concurrent;
using System.Diagnostics;
using System.Linq.Expressions;
using System.Reflection;
using MethodBoundaryAspect.Fody.Attributes;

namespace EnhancedMethodBoundaryAspect
{
    class Program
    {
        private MethodBase _currentMethod;

        public static async Task Main(string[] args)
        {
            var p = new Program();
            var result = 0;

            result = p.TestGenericCall();
            Console.WriteLine("0. intercepted result was: " + result);

            await p.TestVoidCall(1, 2, 3);
            Console.WriteLine("1. task completed; no result to intercept");

            result = await p.TestCall(1, 2, 3);
            Console.WriteLine("2. intercepted result was: " + result);

            result = await p.TestCall(1, 2, 3);
            Console.WriteLine("3. intercepted result was: " + result);

            //Console.WriteLine("Benchmark invocation");
            //var sw = Stopwatch.StartNew();
            //var count = 3000000;
            //for (var i = 0; i < count; i++)
            //    result += await p.TestCallWithoutOutput(1, 2, 3);
            //sw.Stop();

            //var durationPerInvocation = (double)sw.ElapsedMilliseconds / count * 1000;
            //Console.WriteLine($"Duration per 1000 invocation: {durationPerInvocation}ms");
            //Console.WriteLine($"Result was: {result}");
        }

        //[MyTypedMethodBoundary]
        [OnTypedMethodBoundaryAspect]
        public int TestGenericCall()
        {
            int result = 100;
            return result;
        }

        [OnTypedMethodBoundaryAspect]
        public Task TestVoidCall(int i1, int i2, int i3)
        {
            int result = i1 + i2 + i3;
            Console.WriteLine("Real result was: " + result);
            return Task.CompletedTask;
        }

        [OnTypedMethodBoundaryAspect]
        public Task<int> TestCall(int i1, int i2, int i3)
        {
            int result = i1 + i2 + i3;
            Console.WriteLine("Real result was: " + result);
            return Task.FromResult<int>(result);
        }

        [OnTypedMethodBoundaryAspect]
        public Task<int> TestCallWithoutOutput(int i1, int i2, int i3)
        {
            var result = 0;
            for (var i = 0; i < 1000; i++)
            {
                result += i1 + i2 + i3;
            }

            return Task.FromResult(result);
        }

        //public Task<int> Direct(int i1, int i2, int i3)
        //{
        //    _currentMethod ??= MethodBase.GetCurrentMethod();
        //    var args = new MethodExecutionArgs
        //    {
        //        Instance = this,
        //        Method = _currentMethod,
        //        Arguments = new object[] { i1, i2, i3 },
        //        ReturnValue = Task.FromResult(44)
        //    };

        //    var aspect = new OnAsyncMethodBoundaryAspect();
        //    aspect.OnExit(args);

        //    return (Task<int>)args.ReturnValue;
        //}
    }

    public class OnTypedMethodBoundaryAspect : OnMethodBoundaryAspect
    {
        private class TypedMethodBoundaryData
        {
            public readonly MethodInfo UntypedMethod;
            public readonly ConcurrentDictionary<Type, Action<OnTypedMethodBoundaryAspect, object>> CompiledUntypedMethodCache =
                new ConcurrentDictionary<Type, Action<OnTypedMethodBoundaryAspect, object>>();

            public TypedMethodBoundaryData(string methodName)
            {
                UntypedMethod =
                    typeof(OnTypedMethodBoundaryAspect).GetMethod(methodName + "Untyped"
                        , BindingFlags.NonPublic | BindingFlags.Instance, null, new[] { typeof(object) }, null);
            }
        }

        private TypedMethodBoundaryData MethodOnEntryUntyped = new TypedMethodBoundaryData("OnEntry");
        private TypedMethodBoundaryData MethodOnExitUntyped = new TypedMethodBoundaryData("OnExit");
        private TypedMethodBoundaryData MethodOnExceptionUntyped = new TypedMethodBoundaryData("OnException");


        public override void OnEntry(MethodExecutionArgs arg)
        {
            base.OnEntry(arg);
            OnMethodBoundary(MethodOnEntryUntyped, arg);
        }

        public override void OnExit(MethodExecutionArgs arg)
        {
            base.OnExit(arg);
            OnMethodBoundary(MethodOnExitUntyped, arg);
        }

        public override void OnException(MethodExecutionArgs arg)
        {
            base.OnException(arg);
            OnMethodBoundary(MethodOnExceptionUntyped, arg);
        }


        public virtual void OnEntry<T>(MethodExecutionArgs arg) { }

        public virtual void OnExit<T>(MethodExecutionArgs arg) { }

        /// <summary>
        /// Executes in a continuation of a decorated asynchronous method with a return type of Task.
        /// </summary>
        /// <param name="arg"></param>
        /// <returns>
        /// The Task that will be returned by the Task continuation.
        /// </returns>
        /// <remarks>
        /// This method executes asynchronously as a continuation within a ContinueWith().
        /// </remarks>
        public virtual Task OnExitWithTask(MethodExecutionArgs arg, Task originalTask, Task continuationTask)
        {
            return Task.CompletedTask;
        }

        /// <summary>
        /// Executes in a continuation of a decorated asynchronous method with a return type of Task<T>.
        /// The value returned by this method will replace the value that the decorated method returns.
        /// To preserve the result of the original task, simply set it as the new result.
        /// </summary>
        /// <typeparam name="T"></typeparam>
        /// <param name="arg"></param>
        /// <returns>
        /// The value that will be returned by the decorated method.
        /// NOTE: If not changing the return value, simply "return Task<T>.FromResult<T>(originalTask.Result);"
        ///    or call and return the value returned by the base method (e.g. "return base.OnExitWithTask<T>();").
        /// </returns>
        /// <remarks>
        /// This method executes asynchronously as a continuation within a ContinueWith().
        /// </remarks>
        public virtual Task<T> OnExitWithTask<T>(MethodExecutionArgs arg, Task<T> originalTask, Task<T> continuationTask)
        {
            return Task<T>.FromResult<T>(originalTask.Result);
            //object temp = (int)65535;
            //return Task<T>.FromResult<T>((T)temp);
        }

        public virtual void OnException<T>(MethodExecutionArgs arg) { }


        private bool HasReturnType(MethodExecutionArgs arg)
        {
            Type returnType = ((MethodInfo)arg.Method).ReturnType;
            bool hasReturnType = (returnType != null && returnType != typeof(void));
            return hasReturnType;
        }

        private void OnMethodBoundary(TypedMethodBoundaryData methodData, MethodExecutionArgs arg)
        {
            // No point having a generic OnMethodBoundary<T>() overload
            //    if the return type of the decorated method is void.
            if (HasReturnType(arg))
            {
                // The return type of the decorated method
                Type returnType = ((MethodInfo)arg.Method).ReturnType;

                if (returnType.IsSubclassOf(typeof(Task)) && returnType.IsGenericType)
                {
                    Type taskResultType = returnType.GenericTypeArguments[0];
                    CallViaCompiledUntyped(methodData, arg, taskResultType);
                }
                else // Non-Task or non-generic Task
                {
                    CallViaCompiledUntyped(methodData, arg, returnType);
                }
            }
        }

        private void CallViaCompiledUntyped(TypedMethodBoundaryData methodData, MethodExecutionArgs arg, Type returnType)
        {
            if (!methodData.CompiledUntypedMethodCache.TryGetValue(returnType, out var genericMethod))
            {
                // ensure that it is created once
                Console.WriteLine("Created generic method for: " + returnType.Name);
                MethodInfo genericMethodInfo2 = methodData.UntypedMethod.MakeGenericMethod(returnType);

                ParameterExpression expression1 = Expression.Parameter(typeof(OnTypedMethodBoundaryAspect), "target");
                ParameterExpression expression2 = Expression.Parameter(typeof(object), "arg");

                genericMethod = Expression.Lambda<Action<OnTypedMethodBoundaryAspect, object>>(
                        Expression.Call(expression1, genericMethodInfo2, expression2)
                        , expression1, expression2)
                    .Compile();

                methodData.CompiledUntypedMethodCache.TryAdd(returnType, genericMethod);
            }

            genericMethod(this, arg);
        }


        private void OnExitInvokeTask(MethodExecutionArgs arg)
        {
            // Without unwrapping the .ContinueWith() task and then assigning to args.ReturnValue, the method caller receives the original result instead of the modified result
            // c.f. https://stackoverflow.com/q/22797340
            // The above link does not reference Unwrap(), but it does describe the reason for the need
            Task originalTask = (Task)arg.ReturnValue;
            Task continuedTask = originalTask
                .ContinueWith(
                    t =>
                    {
                        return OnExitWithTask(arg, originalTask, t);
                    }
                ).Unwrap();

            arg.ReturnValue = continuedTask;
        }

        private void OnExitInvokeTask<T>(MethodExecutionArgs arg)
        {
            // Without unwrapping the .ContinueWith() task and then assigning to args.ReturnValue, the method caller receives the original result instead of the modified result
            // c.f. https://stackoverflow.com/q/22797340
            // The above link does not reference Unwrap(), but it does describe the reason for the need
            Task<T> originalTask = (Task<T>)arg.ReturnValue;
            Task<T> continuedTask = originalTask
                .ContinueWith(
                    t =>
                    {
                        return OnExitWithTask<T>(arg, originalTask, t);
                    }
                ).Unwrap();

            arg.ReturnValue = continuedTask;
        }

        private void OnEntryUntyped<T>(object argObj)
        {
            OnEntry<T>((MethodExecutionArgs)argObj);
        }

        private void OnExitUntyped<T>(object argObj)
        {
            MethodExecutionArgs arg = (MethodExecutionArgs)argObj;

            if (HasReturnType(arg))
            {
                // The return type of the decorated method
                Type returnType = ((MethodInfo)arg.Method).ReturnType;

                if (returnType.IsSubclassOf(typeof(Task)) && returnType.IsGenericType)
                {
                    OnExit<Task<T>>(arg);
                    OnExitInvokeTask<T>(arg);
                }
                else
                {
                    OnExit<T>(arg);

                    if (returnType == typeof(Task))
                    {
                        OnExitInvokeTask(arg);
                    }
                }
            }
        }

        private void OnExceptionUntyped<T>(object argObj)
        {
            OnException<T>((MethodExecutionArgs)argObj);
        }


        #region Alternate Implementations

        private static readonly MethodInfo MethodOnEntry =
            typeof(OnTypedMethodBoundaryAspect).GetMethod("OnEntry", new[] { typeof(MethodExecutionArgs) });
        private static readonly MethodInfo MethodOnExit =
            typeof(OnTypedMethodBoundaryAspect).GetMethod("OnExit", new[] { typeof(MethodExecutionArgs) });
        private static readonly MethodInfo MethodOnException =
            typeof(OnTypedMethodBoundaryAspect).GetMethod("OnException", new[] { typeof(MethodExecutionArgs) });

        #region Reflection

        private static readonly ConcurrentDictionary<Type, MethodInfo> MethodCacheReflection =
           new ConcurrentDictionary<Type, MethodInfo>();

        private void CallViaReflection(MethodInfo methodInfoTyped, MethodExecutionArgs arg, Type taskResultType)
        {
            if (!MethodCacheReflection.TryGetValue(taskResultType, out var genericMethodInfo))
            {
                // ensure that it is created once
                Console.WriteLine("Created generic method info for: " + taskResultType);
                genericMethodInfo = methodInfoTyped.MakeGenericMethod(taskResultType);
                MethodCacheReflection.TryAdd(taskResultType, genericMethodInfo);
            }

            arg.ReturnValue = genericMethodInfo.Invoke(this, new object[] { arg });
        }

        #endregion

        #region Compiled

        private delegate object LateBoundFunc(object target, params object[] arguments);

        private static readonly ConcurrentDictionary<Type, LateBoundFunc> MethodCacheCompiled =
            new ConcurrentDictionary<Type, LateBoundFunc>();

        private void CallViaCompiled(MethodInfo methodInfoTyped, MethodExecutionArgs args, Type taskResultType)
        {
            if (!MethodCacheCompiled.TryGetValue(taskResultType, out var genericMethod))
            {
                // ensure that it is created once
                Console.WriteLine("Created generic method for: " + taskResultType);
                var genericMethodInfo2 = methodInfoTyped.MakeGenericMethod(taskResultType);

                // compile to mitigate reflection invocation costs
                ParameterExpression expression;
                ParameterExpression expression2;
                genericMethod = Expression.Lambda<LateBoundFunc>(
                        Expression.Convert(
                            Expression.Call(
                                Expression.Convert(expression = Expression.Parameter(typeof(object), "target"),
                                    genericMethodInfo2.DeclaringType),
                                genericMethodInfo2,
                                CreateParameterExpressions(genericMethodInfo2,
                                    expression2 = Expression.Parameter(typeof(object[]), "arguments"))),
                            typeof(object)), expression, expression2)
                    .Compile();
                MethodCacheCompiled.TryAdd(taskResultType, genericMethod);
            }

            args.ReturnValue = genericMethod(this, args);
        }

        private static UnaryExpression[] CreateParameterExpressions(MethodInfo method, Expression argumentsParameter)
        {
            return
                method.GetParameters().Select(
                        (parameter, index) =>
                            Expression.Convert(
                                Expression.ArrayIndex(argumentsParameter, Expression.Constant(index)),
                                parameter.ParameterType))
                    .ToArray();
        }

        #endregion

        #endregion
    }

    public class MyTypedMethodBoundaryAttribute : OnTypedMethodBoundaryAspect
    {
        public override void OnEntry<T>(MethodExecutionArgs arg)
        {
            base.OnEntry<T>(arg);
            arg.ReturnValue = 1;
        }

        public override void OnExit<T>(MethodExecutionArgs arg)
        {
            base.OnExit<T>(arg);
            arg.ReturnValue = 21;
        }

        public override void OnException<T>(MethodExecutionArgs arg)
        {
            base.OnException<T>(arg);
            arg.ReturnValue = -1;
        }
    }

}

@Ralf1108
Copy link
Collaborator

Regarding the problem of resolving the OnEntry() method I think the weaver will throw the exception because it finds 2 OnEntry() methods, one which is parametrized and the non parametrized. This can be fixed easily in the weaver.
It can also be fixed by you by renaming your OnEntry() method.

Regarding the optimization to remove unused aspect methods.
If you override them in your derived "OnTypedMethodBoundaryAspect" then they should count as "used".
So this didn't work out?

@mesheets
Copy link
Author

Regarding the problem of resolving the OnEntry() method I think the weaver will throw the exception because it finds 2 OnEntry() methods, one which is parametrized and the non parametrized.

OK, if it is not differentiating on generic parameters and then just picks the first one it finds, that seems to to follow why order matters. When combined with the other items, though, a challenge appears to be a cascading set of changes that might be required?

Regarding the optimization to remove unused aspect methods.
If you override them in your derived "OnTypedMethodBoundaryAspect" then they should count as "used".
So this didn't work out?

The problem right now is that for a class intended to be a reusable base class (perhaps even potentially as part of OnMethodBoundaryAspect itself?), they are always counting as used. For this extended OnMethodBoundaryAspect functionality to work, it needs to hook into OnEntry(), OnExit(), and OnException(). If, however, for example, even if neither OnEntry() nor OnEntry<T>() are not overridden in a class that would inherit from OnTypedMethodBoundaryAspect, OnEntry() still gets called because that is the hook that is currently used.

Perhaps at least some of the code in question might be in the method below?

Right now, though, I'm not even really getting that far yet, as inheritance of the OnMethodBoundaryAspect class appears to only work if the inheriting class is a direct descendant of OnMethodBoundaryAspect (c.f. the error with stack track from above), so right now it does not seem possible to use OnTypedMethodBoundaryAspect itself as a base class:

If I create a class OnTypedMethodBoundaryAspect that inherits OnMethodBoundaryAspect and implements all of the details necessary for handling generic OnEntry<T>() / OnExit<T>() / OnException<T>() methods, I cannot in turn create and use a "MyTypedMethodBoundaryAttribute" class for implementing application-specific details, as the presence of any method decorated with [MyTypedMethodBoundary] will cause a build to fail with the [stack trace] error.

OnMethodBoundaryAspect doesn't contain any implementation code, so perhaps it would be better to implement the hooks at a lower level, independent of the virtual methods exposed by OnMethodBoundaryAspect, and then include the additional virtual methods directly in OnMethodBoundaryAspect itself?

@Ralf1108
Copy link
Collaborator

@mesheets your problem regarding the duplicate method names should be fixed in v2.0.145 with commit 3608d6b.

Then your code using your derived aspect "MyTypedMethodBoundary" compiles and runs.
But in your aspect you use the "ReturnValue" for storing the aspect task and also setting it in the "MyTypedMethodBoundary".
Was this by intent? If yes you should use a result class to hold multiple values.

@mesheets
Copy link
Author

mesheets commented Feb 3, 2022

Thank you! I will test further, but this update appears to have addressed the items in question.

I think some of the ReturnValue usages noted were some artifacts left over from earlier testing and troubleshooting.

@Cricle
Copy link

Cricle commented Sep 22, 2022

I think it will a solution in async intercept, implement list in below.

  • Task<T>
  • Task
  • ValueTask
  • ValueTask<T>

The result is

image

using MethodBoundaryAspect.Fody.Attributes;
using System;
using System.Collections.Generic;
using System.Linq.Expressions;
using System.Reflection;
using System.Threading.Tasks;

namespace Hello
{
    class Program
    {
        static void Main(string[] args)
        {
            var c = new MyClass();
            var res = c.Add(1, 2).GetAwaiter().GetResult();

            Console.WriteLine(res);
        }
    }
    public class MyClass
    {
        [AMyMethodBoundaryAspect]
        public Task<int> Add(int a, int b)
        {
            return Task.FromResult(a + b);
        }
    }
    public class AMyMethodBoundaryAspect : OnMethodBoundaryAspect, IAsyncMethodHandle
    {
        public override void OnExit(MethodExecutionArgs arg)
        {
            MethodBoundaryAspectHelper.AsyncIntercept(arg, this, MethodBoundaryMethods.Exit);
        }
        public override void OnEntry(MethodExecutionArgs arg)
        {
            MethodBoundaryAspectHelper.AsyncIntercept(arg, this, MethodBoundaryMethods.Entry);
        }
        public override void OnException(MethodExecutionArgs arg)
        {
            MethodBoundaryAspectHelper.AsyncIntercept(arg, this, MethodBoundaryMethods.Exception);
        }

        public Task<T> HandleEntryAsync<T>(MethodExecutionArgs arg, T old)
        {
            return Task.FromResult(old);
        }

        public async Task<T> HandleExitAsync<T>(MethodExecutionArgs arg, T old)
        {
            Console.WriteLine("Intercept begin");
            await Task.Delay(1000);
            Console.WriteLine("Intercept after");
            return old;
        }

        public Task<T> HandleExceptionAsync<T>(MethodExecutionArgs arg, T old)
        {
            return Task.FromResult(old);
        }
    }
    public interface IAsyncMethodHandle
    {
        Task<T> HandleEntryAsync<T>(MethodExecutionArgs arg, T old);

        Task<T> HandleExitAsync<T>(MethodExecutionArgs arg, T old);

        Task<T> HandleExceptionAsync<T>(MethodExecutionArgs arg, T old);
    }
    public static class MethodBoundaryAspectHelper
    {
        class HandleMerge
        {
            public Delegate Entry;

            public Delegate Exit;

            public Delegate Exception;
        }
        enum MethodReturnCase
        {
            Other = 0,
            Task = 1,
            TaskResult = 2
        }
        class MethodReturnInfo
        {
            public MethodReturnCase Case;

            public Type ReturnGenericType;
        }
        private static readonly MethodInfo entryMethodInfo =
            typeof(IAsyncMethodHandle).GetMethod(nameof(IAsyncMethodHandle.HandleEntryAsync));
        private static readonly MethodInfo exitMethodInfo =
            typeof(IAsyncMethodHandle).GetMethod(nameof(IAsyncMethodHandle.HandleExitAsync));
        private static readonly MethodInfo exceptionMethodInfo =
            typeof(IAsyncMethodHandle).GetMethod(nameof(IAsyncMethodHandle.HandleExceptionAsync));

        private static readonly Dictionary<Type, HandleMerge> handleDelegate = new Dictionary<Type, HandleMerge>();
        private static readonly Dictionary<MethodInfo, MethodReturnInfo> isTaskWithSouce = new Dictionary<MethodInfo, MethodReturnInfo>();
        private static readonly object syncRootHandleDelegate = new object();
        private static readonly object syncRootisTaskWithSouce = new object();

        private static MethodReturnInfo GetTaskResultType(MethodInfo method)
        {
            if (!isTaskWithSouce.TryGetValue(method, out var ifo))
            {
                lock (syncRootisTaskWithSouce)
                {
                    if (!isTaskWithSouce.TryGetValue(method, out ifo))
                    {
                        var @case = MethodReturnCase.Other;
                        if (method.ReturnType == typeof(Task))
                        {
                            @case = MethodReturnCase.Task;
                        }
                        else if (typeof(Task).IsAssignableFrom(method.ReturnType) &&
                            method.ReturnType.IsGenericType)
                        {
                            @case = MethodReturnCase.TaskResult;
                        }
                        else
                        {
                            @case = MethodReturnCase.Other;
                        }
                        ifo = new MethodReturnInfo
                        {
                            Case = @case,
                            ReturnGenericType = @case == MethodReturnCase.TaskResult ? method.ReturnType.GenericTypeArguments[0] : null
                        };
                        isTaskWithSouce[method] = ifo;
                    }
                }
            }
            return ifo;
        }

        private static HandleMerge GetHandleDelegate(Type type)
        {
            if (!handleDelegate.TryGetValue(type, out var merge))
            {
                lock (syncRootHandleDelegate)
                {
                    if (!handleDelegate.TryGetValue(type, out merge))
                    {
                        merge = new HandleMerge
                        {
                            Entry = CreateHandleDelegate(type, entryMethodInfo),
                            Exit = CreateHandleDelegate(type, exitMethodInfo),
                            Exception = CreateHandleDelegate(type, exceptionMethodInfo),
                        };
                        handleDelegate[type] = merge;
                    }
                }
            }
            return merge;
        }
        private static Delegate CreateHandleDelegate(Type type, MethodInfo method)
        {
            var genMethod = method.MakeGenericMethod(type);
            var par0 = Expression.Parameter(typeof(IAsyncMethodHandle));
            var par1 = Expression.Parameter(typeof(MethodExecutionArgs));
            var par2 = Expression.Parameter(type);
            return Expression.Lambda(Expression.Call(par0, genMethod, par1, par2),
                par0, par1, par2)
                .Compile();
        }

        public static bool AsyncIntercept(MethodExecutionArgs arg, IAsyncMethodHandle handle, MethodBoundaryMethods method)
        {
            var methodInfo = (MethodInfo)arg.Method;
            var rt = GetTaskResultType(methodInfo);
            if (rt.Case == MethodReturnCase.TaskResult)
            {
                var merge = GetHandleDelegate(rt.ReturnGenericType);
                var del = DynamicTaskProxy.GetDelegate(rt.ReturnGenericType);
                var mergeDel = merge.Entry;
                switch (method)
                {
                    case MethodBoundaryMethods.Exit:
                        mergeDel = merge.Exit;
                        break;
                    case MethodBoundaryMethods.Exception:
                        mergeDel = merge.Exception;
                        break;
                    case MethodBoundaryMethods.Entry:
                    default:
                        break;
                }
                arg.ReturnValue = del.DynamicInvoke(handle,
                    arg,
                    (Task<int>)arg.ReturnValue,
                    mergeDel);
                return true;
            }
            return false;
        }
    }
    public enum MethodBoundaryMethods
    {
        Entry,
        Exit,
        Exception
    }
    public class MyMethodBoundaryAspect : OnMethodBoundaryAspect, IAsyncMethodHandle
    {
        public override void OnExit(MethodExecutionArgs arg)
        {
            MethodBoundaryAspectHelper.AsyncIntercept(arg, this, MethodBoundaryMethods.Exit);
        }
        public override void OnEntry(MethodExecutionArgs arg)
        {
            MethodBoundaryAspectHelper.AsyncIntercept(arg, this, MethodBoundaryMethods.Entry);
        }
        public override void OnException(MethodExecutionArgs arg)
        {
            MethodBoundaryAspectHelper.AsyncIntercept(arg, this, MethodBoundaryMethods.Exception);
        }

        public Task<T> HandleEntryAsync<T>(MethodExecutionArgs arg, T old)
        {
            return Task.FromResult(old);
        }

        public Task<T> HandleExitAsync<T>(MethodExecutionArgs arg, T old)
        {
            return Task.FromResult(old);
        }

        public Task<T> HandleExceptionAsync<T>(MethodExecutionArgs arg, T old)
        {
            return Task.FromResult(old);
        }
    }
    public static class DynamicTaskProxy
    {
        private static readonly MethodInfo withResultMethodInfo = typeof(DynamicTaskProxy).GetMethod(nameof(WithResult), BindingFlags.Static | BindingFlags.Public);
        private static readonly Dictionary<Type, Delegate> g = new Dictionary<Type, Delegate>();
        private static readonly object syncRoot = new object();

        public static Delegate GetDelegate(Type type)
        {
            if (!g.TryGetValue(type, out var del))
            {
                lock (syncRoot)
                {
                    if (!g.TryGetValue(type, out del))
                    {
                        var t = typeof(Func<,,,,>).MakeGenericType(
                            typeof(IAsyncMethodHandle),
                            typeof(MethodExecutionArgs),
                            typeof(Task<>).MakeGenericType(type),
                            typeof(Func<,,,>).MakeGenericType(
                                typeof(IAsyncMethodHandle),
                                typeof(MethodExecutionArgs),
                                type,
                                typeof(Task<>).MakeGenericType(type)),
                                typeof(Task<>).MakeGenericType(type));
                        del = Delegate.CreateDelegate(t,
                            withResultMethodInfo.MakeGenericMethod(type));
                        g.Add(type, del);
                    }
                }
            }
            return del;
        }

        public static async Task<T> WithResult<T>(IAsyncMethodHandle handle,
            MethodExecutionArgs arg,
            Task<T> left,
            Func<IAsyncMethodHandle, MethodExecutionArgs, T, Task<T>> right)
        {
            var old = default(T);
            if (left != null)
            {
                old = await left;
            }
            return await right(handle, arg, old);
        }
    }
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants