Revert "Merge pull request #276 from petli/memory-mapped-hit-counts" #322
Conversation
ViktorHofer
force-pushed
the
RevertMMF
branch
from
8ed8807 to
72b212d
Compare
ViktorHofer
force-pushed
the
RevertMMF
branch
from
72b212d to
c71b7d8
Compare
Codecov Report
@@ Coverage Diff @@
## master #322 +/- ##
==========================================
- Coverage 87.53% 86.57% -0.96%
==========================================
Files 16 16
Lines 2045 2011 -34
==========================================
- Hits 1790 1741 -49
- Misses 255 270 +15 |
1 similar comment
Codecov Report
@@ Coverage Diff @@
## master #322 +/- ##
==========================================
- Coverage 87.53% 86.57% -0.96%
==========================================
Files 16 16
Lines 2045 2011 -34
==========================================
- Hits 1790 1741 -49
- Misses 255 270 +15 |
| { | ||
| var t = new Thread(HitIndex); | ||
| t.Start(i); | ||
| t.Join(); |
There was a problem hiding this comment.
This line should be kept, because not waiting for the test threads to finish running makes the test unreliable. Then the Skip on the test can probably be removed.
There was a problem hiding this comment.
Maybe better track with issue and fix with a PR after merge.
There was a problem hiding this comment.
I generally prefer to do a full revert if possible, and then re-apply the part(s) that are still desired, but I wouldn't be opposed to either approach.
There was a problem hiding this comment.
Agreed, that is better. Optionally, the code this test is testing is no longer in the code base after changing to Interlocked.Increment in RecordHit so it could be removed altogether.
This reverts commit 0d285b1
Reasoning: #276 (comment)
cc @petli @sharwell @tonerdo