Welcome Guest! To enable all features please Login or Register.

Notification

Icon
Error
When NCRunch adds instrumentation, it sometimes changes the signature of class members
Options
Previous Topic Next Topic
abelb
#1 Posted : Wednesday, September 20, 2017 7:11:03 PM(UTC)
Rank: Advanced Member

Groups: Registered
Joined: 9/12/2014(UTC)
Posts: 155
Location: Netherlands

Thanks: 19 times
Was thanked: 11 time(s) in 11 post(s)
(as discussed before)

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:

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:

Code:
    [<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.
Back to top
Remco
#2 Posted : Thursday, September 21, 2017 1:16:58 AM(UTC)
Rank: NCrunch Developer

Groups: Administrators
Joined: 4/16/2011(UTC)
Posts: 6,976

Thanks: 930 times
Was thanked: 1257 time(s) in 1170 post(s)
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.
Back to top
abelb
#3 Posted : Thursday, September 21, 2017 11:11:06 AM(UTC)
Rank: Advanced Member

Groups: Registered
Joined: 9/12/2014(UTC)
Posts: 155
Location: Netherlands

Thanks: 19 times
Was thanked: 11 time(s) in 11 post(s)
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.
    		•
    Back to top
    Remco
    #4 Posted : Thursday, September 21, 2017 11:43:53 AM(UTC)
    Rank: NCrunch Developer

    Groups: Administrators
    Joined: 4/16/2011(UTC)
    Posts: 6,976

    Thanks: 930 times
    Was thanked: 1257 time(s) in 1170 post(s)
    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.
    Back to top
    abelb
    #5 Posted : Thursday, September 21, 2017 12:43:50 PM(UTC)
    Rank: Advanced Member

    Groups: Registered
    Joined: 9/12/2014(UTC)
    Posts: 155
    Location: Netherlands

    Thanks: 19 times
    Was thanked: 11 time(s) in 11 post(s)
    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.c...-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.co...custom-instrumentation, in short, it uses attributes and/or XML config files.
    Back to top
     |  Edit by user
    1 user thanked abelb for this useful post.
    Remco on 9/21/2017(UTC)
    Users browsing this topic
    Guest
    Forum Jump  
    You cannot post new topics in this forum.
    You cannot reply to topics in this forum.
    You cannot delete your posts in this forum.
    You cannot edit your posts in this forum.
    You cannot create polls in this forum.
    You cannot vote in polls in this forum.

    YAF | YAF © 2003-2011, Yet Another Forum.NET
    This page was generated in 0.049 seconds.
    Trial NCrunch
    Take NCrunch for a spin
    Do your fingers a favour and supercharge your testing workflow
    Free Download