Extract a ViewModelHelper that will be useful with static view models

[marner2]

Waiting on #524

Compare without #524

This should be a behavior-preserving transform that extracts some behavior from DynamicViewModel into a ViewModelHelper which can then be shared with a future ViewModelBase implemented in #522

[TysonMN]

The diff has huge blocks of + and -, which is very difficult to review on my phone. Is it possible to extract smaller pieces in separate PRs?

[marner2]

@TysonMN can you view the diffs one commit at a time? A lot of these changes don't make much sense on their own.

[marner2]

Also the majority of the diff issue happens when I split the class in two at the end, and I don't think I can help that because the diff view doesn't track code reordering (which must be done at that time and can't be reduced any more that I can see).

[TysonMN]

@TysonMN can you view the diffs one commit at a time? A lot of these changes don't make much sense on their own.

The GitHub mobile app supports expanding the lines shown around a charge when reviewing a PR but not when looking at a specific commit.

[marner2]

@TysonMN Do you have a set of performance tests or techniques that you used for performance testing?

[TysonMN]

I used Benchmark .NET when implementing the linear-time merge. I once used Visual Studio's profiler to investigate a reported performance issue.

If you want to add performance testing, I highly recommend Benchmark .NET.

[marner2]

Test DynamicViewModel.UpdateModel() with various numbers of get bindings and consecutive calls to update.

Before this PR:

Method	BindingCount	UpdateCount	Mean	Error	StdDev
Update	1	1	129.7 ns	2.63 ns	4.16 ns
Update	10	1	976.5 ns	17.36 ns	36.62 ns
Update	100	1	9,807.6 ns	205.11 ns	604.76 ns
Update	1	100	26,035.6 ns	519.15 ns	1,349.33 ns
Update	10	100	231,474.9 ns	4,588.53 ns	8,618.37 ns
Update	100	100	2,316,700.6 ns	42,886.60 ns	109,934.76 ns
Update	1	10000	2,548,893.3 ns	50,655.13 ns	113,297.46 ns
Update	10	10000	22,980,704.4 ns	445,873.29 ns	920,805.66 ns
Update	100	10000	226,083,178.9 ns	4,509,962.45 ns	11,479,294.41 ns

After this PR:

Method	BindingCount	UpdateCount	Mean	Error	StdDev
Update	1	1	168.6 ns	3.86 ns	11.32 ns
Update	10	1	1,148.8 ns	20.23 ns	29.01 ns
Update	100	1	9,657.5 ns	197.42 ns	579.00 ns
Update	1	100	31,116.9 ns	607.03 ns	1,063.16 ns
Update	10	100	251,177.5 ns	4,735.05 ns	8,416.55 ns
Update	100	100	2,208,949.0 ns	53,163.00 ns	156,752.31 ns
Update	1	10000	2,978,675.0 ns	58,545.28 ns	65,072.92 ns
Update	10	10000	24,010,168.1 ns	477,667.63 ns	798,075.31 ns
Update	100	10000	224,637,806.2 ns	4,476,978.11 ns	12,627,396.18 ns

[marner2]

@TysonMN do you want a PR with a benchmarks project? It's currently here: https://github.com/elmish/Elmish.WPF/compare/master...marner2:Elmish.WPF:feature/add_benchmark_project?expand=1

[TysonMN]

@TysonMN do you want a PR with a benchmarks project?

Sure.

Test DynamicViewModel.UpdateModel() with various numbers of get bindings and consecutive calls to update.

So the conclusion is that the performance is essentially the same before and after this PR, right?

[marner2]

So the conclusion is that the performance is essentially the same before and after this PR, right?

Yes. Seems to be mostly within margin of error, or mixed results. It probably would be good to write more benchmarks too.

[marner2]

The PR with the benchmark project is #538