Single hit

[sharwell]

📝 This pull request builds on #308

This pull request adds the <CoverletSingleHit> option (--single-hit for the global tool), which can be used to improve the performance by only recording a single hit at each location. Where #291 brought the time down to 15.5 seconds, this pull request further reduces the execution time to ~10.3 seconds.

Fixes #306

[codecov]

Codecov Report

Merging #309 into master will increase coverage by 0.07%.
The diff coverage is 96.15%.

@@            Coverage Diff            @@
##           master    #309      +/-   ##
=========================================
+ Coverage   87.03%   87.1%   +0.07%     
=========================================
  Files          31      31              
  Lines        3016    3033      +17     
=========================================
+ Hits         2625    2642      +17     
  Misses        391     391

[sharwell]

📝 The main value here is avoiding this write invalidating cache lines. OpenCover uses a configurable threshold value. I could rewrite it to use a configurable threshold, though most uses I've seen in practice either omit this value or set it to 1.

[MarcoRossignoli]

Why don't pass _singleHit as parameter or use static var and have less methods?Inlining?

[sharwell]

It's always a possibility to come back and do this if it proves to be equally fast. Since the expanded form is at least that fast and not difficult to work with, it seemed like the straightforward initial approach.

[sharwell]

@tonerdo Note that OpenCover uses -threshold for this feature. I didn't retain the same name because it appears Coverlet uses that term with a different meaning, and I didn't want to confuse things.

[tonerdo]

@sharwell @petli @MarcoRossignoli What are your thoughts on making this option an environment variable? It seems like an optimization detail that shouldn't be at the forefront of the primary options, kinda like how we can turn on/off tiered compilation in .NET Core

[MarcoRossignoli]

I'm for parameter --single-hit my impression is that this parameter will be used a lot and hiding it's a shame, I could be wrong but the vast majority of coverage usage is check final percentage of coverage, and in case of issue people check how many tests(hits) cover particular code section. I don't know how many teams check hits amount regularly.

[sharwell]

@tonerdo I lean towards what @MarcoRossignoli said, but I'm happy to implement it either way as long as I can use it. Keep in mind the impact is dramatic - more than 50% reduction in execution overhead for coverage runs. This translates to many minutes of build time savings for CI of large projects.

[MarcoRossignoli]

Not strictly related only to this PR,but in general, don't you think that we've too few unit/perf testing for these core part?(I'm aware of rush to unblock scenarios)