DefaultDynamicLazyFieldInterceptor does not handle generic methods correctly

https://nhibernate.jira.com/browse/NH-3244#icft=NH-3244 is somehow related, yes. But it will most likely still need an additional fix like the one I already started in the pull request. This issue here handles a "simple" generic method whereas https://nhibernate.jira.com/browse/NH-3244#icft=NH-3244 handles generic methods with type constraints/attributes (which need even more processing when emitting the IL code).

this seems to be a related issue https://nhibernate.jira.com/browse/NH-3244 with it's own pull request 🙂

In .Net4, for a large number of iterations, calling through the interceptor (with MakeGenericType()) takes about twice as long as in .Net2, where we already have the closed generic method. Given that an interceptor is in most cases expected to call the original method, I agree with cremor that having the closed generic method in InvocationInfo,TargetMethod is most useful.

The problem in our current method body emitter is that it refers directly to the open generic method. By what might be a bug in .Net 3.5, this oddly retrieves the method instantiated with the surrounding method's generic parameters. To improve the code, we should (when building the proxy) create a closed generic method, and as type arguments use the GenericTypeParameterBuilder instances that we got when we defined the generated method. Then emit LdToken with that constructed type.

I have reverted the previous fix and committed a change to EmitMethodBody() instead (69e44756c686d5a24d02b4e86aab38407eb65946).

I vote for fixing EmitMethodBody() for two reasons:
First because it's way more logical if both runtimes behave the same. Just remember the thread in the Google group. Fabio said it's fixed, but it didn't work for me. Took a while until I had the idea to run the test in the .NET 4.0 runtime.
Second because calling MakeGenericMethod() in the Interceptor needs to be done for each call. Even if it's only a tiny performance affect, there is no reason that doing it multiple times during runtime is better than doing it only once during startup.

I've been digging a bit more. It now seems like the behaviour on .Net 3.5 is actually a bug in the framework, which just happens to be useful. EmitMethodBody() will emit the following construct to access the intercepted method:
IL.Emit(OpCodes.Ldtoken, method); // Here, method is an open generic method.

The generated code is:
2.0 code line:
L_0014: ldtoken instance !

!T0>()
4.0 code line:
L_0014: ldtoken instance !!0 [NHibernate.Test]NHibernate.Test.DynamicProxyTests.GenericMethodsTests.GenericMethodShouldBeProxied/MyClass::As()
(note the missing generic type parameter at the end)

Given that we pass a MethodInfo for an open generic type to the Emit() call, the code generated on .Net 4.0 actually seems more logical. The 2.0 code is in fact reusing the type argument given to the generated method - which for dynamic proxy cases just happen to work and be useful because the generated method has the same number of type parameters as the intercepted method.

So really, the question is what API do we want? Attempt to preserve InvocationInfo.TargetMethod as a closed generic method even on .Net 4, or leave it as an open generic method and let the interceptor handle it, as the committed patch already does? I wonder how the calls to MakeGenericMethod() might affect performance.