I have seen this with F#, but it is possible it can occur with other languages too. The signature change is unobtrusive, it adds the generics type, and any uses of that method do not have to change. However, as you can see, it *does* change the method signature and as a result it *does* look different in reflection (or similarly with tests that are written to make sure API functions are available), which is a problem.
To reproduce this, simple create a default F# library project and type in this code:
[<CompilationRepresentation(CompilationRepresentationFlags.UseNullAsTrueValue)>]
type MyOption<'T> =
| MyNone
| MySome of 'T
member x.IsNone = match x with MyNone -> true | _ -> false
member x.IsSome = match x with MySome _ -> true | _ -> false
Then use it from any test project. For instance, with NUnit it would look like this:
[<Test>]
member __.``Test MyOption.IsSome``() =
let opt = MyOption.MySome 12
Assert.IsTrue(opt.IsSome)
I had to select "Copy referenced assemblies to workspace", as otherwise I don't know where to find the assemblies that are instrumented (they don't seem to appear in the _ncrunchreferences folder in the workspace). Open it in Reflector or similar to see the differences.
If you remove the UseNullAsTrueValue attribute, the problem goes away. Unfortunately, that is not an option that can be considered in this case. I tested some other scenarios, but I didn't yet find other scenarios that caused this behavior.
Thanks for sharing this problem. I think this is happening on writeback of the instrumented assembly. Probably there is some metadata here that is quite specific to F# and goes a bit wrong somewhere in the processing.
Issues like this one can be quite hard to fix, so unfortunately I can't provide an immediate fix to this one. I hope you can find a way to work around it. I've added a task to investigate this in detail and implement a fix if possible.
The workaround I currently have is to *not* use instrumentation. It is an important test, it checks the surface area of available API functions, to make sure nobody changes the declarations, or adds a new public API function without proper discussion or adding documentation. If I would change the test (that is, change the expected type), it would fail in test runs on the CI platforms, or in the NUnit test runner, and if I remove the check, the test would fail as it will report a public function that is available, but not listed as one.
But no rush, I can use the code coverage and accept the failing tests for now, and run without instrumentation on other runs.
As an aside, I have found a few other behaviors of adding instrumentation:
On very small methods/functions, the overhead is significant, esp. the line-timings, this leads to the JIT not inlining such methods. On trivial functions (i.e. without loop constructs, or calls to other methods) it seems to add more instrumentation than perhaps necessary (I realize that changing this would be complex).
On recursive functions, it seems that you loose tailcall optimization opportunities. After the added instrumentation, tailcall optimization seems not to happen on some methods (educated guess, taken from a difference in runtime of certain recursive functions from 10 sec without, and 7 minutes with instrumentation).
It would be interesting whether it is possible to do method-level instrumentation, as opposed to line level
Even more interesting: disable instrumentation through an external config file (I am aware of the method with comments, but that is not an option here as different programmers use different tools and adding comments like these would litter the code too much)
Obviously, these are improvement suggestions and considering the complexity of this subject I would understand that, even if you consider them, they wouldn't have a high priority for now.
Because it involves heavy code manipulation and detailed tracking, instrumentation does come with technical limitations. It is unfortunately not possible to measure the performance of software without at some level affecting the performance itself. Because NCrunch is line-by-line, the instrumentation does add considerable weight to heavily executed code. For this reason, it's better to use the code coverage suppression comments to disable instrumentation for areas of code that are executed very heavily.
Turning off the analyse line execution times setting will change NCrunch's instrumentation structure so that it adds less weight to the code and does not 'warp' the code as heavily. The natural trade-off here is that you won't get performance data for the code.
Unfortunately there are some very strict limitations when engineering this area of the software, because:
1. Compatibility is very important. The instrumentation needs to be able to deal with unforeseeable use cases arising from new language features and less heavily used languages like F#.
2. Instrumentation is performance critical, not only during execution of code, but the actual placement of instrumentation. It would not be useful if a project took 10 times as long to instrument as it did to build.
3. Some of the structures necessary for reliable instrumentation at times conflict with undocumented security rules in the CLR that can cause the program to crash without any meaningful error information.
4. Code needs to behave logically the same when instrumented as when not, as otherwise people won't trust the product.
5. Instrumentation must behave consistently when the software behaves badly (i.e. unhandled exceptions, stack overflows, etc).
Method-level instrumentation (as opposed to line-level) can be achieved through selective use of code coverage suppression comments for method body code. It's messy, but it can be used this way.
Thanks for the insights, I expected that it is tricky. On an 180k lines project, introducing comments to exclude certain lines is not only messy, it would disrupt the way people code. I understand there is currently no other way, but being able to exclude a class of methods, something like `exclude FSharp.Code.Unsafe.*` or `exculdeByRegex Namespace\.Foo\.Bar\.(get_|set_).*` would seem very helpful.
When timing performance with, say, ANTS, you get more options (but of course, that product was built for a different use-case). Microsoft's profiling API (not sure whether you use a third-party tool for this) accepts many configuration options to suppress instrumentation for certain functions, see for instance: https://docs.microsoft.com/en-us/visualstudio/profiling/how-to-limit-instrumentation-to-specific-functions. I don't think that would put a big dent in the performance of adding instrumentation itself, in fact, it probably improves it.
Another approach is taken by New Relic: https://docs.newrelic.com/docs/agents/net-agent/custom-instrumentation/introduction-net-custom-instrumentation, in short, it uses attributes and/or XML config files.
