Move GrowableArray tests to a proper unit test

[sharwell]

Blocked on #290.

📝 Please make sure to not rebase or squash this change during the merge process.

[vancem]

Most of this is the change to eliminate float (making it double) in most if not all uses in PerfView.

I know that Sam has noticed that various statistics do not 'add up' properly in some scenarios which is what prompted the change. Generally speaking I don't have a problem with using double instead of float in any place where size is not an issue (which is many places).

The place that gives me pause, are the values in CallTreeNodeBase and HIstogram as TreeNodes are very common and this change is likely to make these nodes increase in size by a substantial factor (I am guessing 60-80%). We KNOW that we allocate GB of this kind of stuff (since we get out of Memory exceptions on 32 bit), so size really SHOULD matter, and increasing it by substantial amount is a worrisome prospect.

We could try to measure this impact, but it is not clear to me when we decide the perf loss is too much and we live with the inaccuracy. Clearly we want both.

So I have an alternate suggestion. Change this checkin so that you use double for all the CONTRACTs and frankly anything that is not perf-critical (which I am saying is anything generated in call trees including CallTreeNodeBase, and Histogram). Leave those as floats for now (you can put in an #ifdef to flip it, but leave it as float for now).

That can be checked in now, and gets rid of most of the possible merge conflicts (which are undoubtedly with the signatures not with the fields).

This also lets us do side-by-side comparisons to determine if size really does matter.

Assuming that we do want to optimize size, what I would LIKE is to see what we can do to get the precision AND the smaller size . Maybe just leaving the Histogram as float, or encoding some of the fields as deltas from other fields can work. I could imagine all sorts of things that probably would work (e.g. having two versions of CallTree that have more precision). I don't really know what the right answer is.

However we don't need the right answer to move forward. I am OK making interfaces use double, and having an #ifdef to do the flip. We can then explore to either find a way to get both small size and good precision, or gather enough data to make use comfortable making the policy that we swallow the larger size for the sake of accuracy.

This first step, however should be a reasonably small change from this PR.

[sharwell]

@vancem As mentioned in the summary, I wasn't expecting this to be reviewed prior to dealing with #290 (which is the change from float->double). I replied to the issue regarding call tree nodes in the other issue but haven't heard back yet.

[codecov-io]

Codecov Report

Merging #433 into master will increase coverage by 0.15%. The diff coverage is 43.8%.

@@            Coverage Diff             @@
##           master     #433      +/-   ##
==========================================
+ Coverage   17.44%   17.59%   +0.15%     
==========================================
  Files         213      214       +1     
  Lines      123790   123835      +45     
  Branches    11971    11977       +6     
==========================================
+ Hits        21590    21790     +200     
+ Misses     101189   101186       -3     
+ Partials     1011      859     -152

Flag	Coverage Δ
#2017	17.59% <43.8%> (+0.15%)	:arrow_up:
#Debug	17.59% <43.8%> (+0.15%)	:arrow_up:
#Release	100% <ø> (ø)	:arrow_up:

Impacted Files	Coverage Δ
src/TraceEvent/TraceEventSession.cs	2.45% <ø> (ø)	:arrow_up:
src/FastSerialization/GrowableArray.cs	61.45% <ø> (ø)	:arrow_up:
src/PerfView/memory/ClrProfilerParser.cs	0% <0%> (ø)	:arrow_up:
src/PerfView/Triggers.cs	0% <0%> (ø)	:arrow_up:
src/TraceEvent/Computers/TraceManagedProcess.cs	0% <0%> (ø)	:arrow_up:
src/PerfView/OtherSources/XMLStackSource.cs	18.46% <0%> (ø)	:arrow_up:
...eMachineFramework/ComputingResourceStateMachine.cs	0% <0%> (ø)	:arrow_up:
src/TraceEvent/TraceEventStacks.cs	0% <0%> (ø)	:arrow_up:
src/TraceEvent/Computers/ThreadTimeComputer.cs	0% <0%> (ø)	:arrow_up:
src/PerfView/OverweightReport.cs	0% <0%> (ø)	:arrow_up:
... and 33 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update ed9cc8d...d130fcc. Read the comment docs.

[sharwell]

Updated after repository rewrite

[brianrob]

@sharwell is it possible to split this into two PRs? I see two distinct pieces of this PR:

1. FastSerializationTests
2. Changes from float to double for the stack viewer statistics.

It seems that 1 is fairly straightforward and non-controversial, but 2 is not. I'd love to merge 1 and then consider 2 separately.

[sharwell]

@brianrob This pull request is blocked on #290