Optimize conversion of MouseGesture from/to string, reduce allocations

[h3xds1nz]

Description

Parsing optimization for MouseGesture and its conversion from/to string. Also adds documentation for whole class.
I've additionally removed the duplicated functionality in MouseGesture to check for MouseAction validity.

ConvertFrom Benchmarks

Method	source	Mean [ns]	Error [ns]	StdDev [ns]	Gen0	Allocated [B]
Original	Ctr(...)ick [29]	238.962 ns	3.9509 ns	3.6956 ns	0.0238	400 B
PR__EDIT	Ctr(...)ick [29]	30.909 ns	0.3631 ns	0.3396 ns	0.0019	32 B
Original	Ctrl (...)Click [31]	271.305 ns	3.0945 ns	2.7432 ns	0.0310	520 B
PR__EDIT	Ctrl (...)Click [30]	38.710 ns	0.4011 ns	0.3556 ns	0.0019	32 B
Original	Ctrl+LeftClick	203.858 ns	1.2445 ns	1.1641 ns	0.0119	200 B
PR__EDIT	Ctrl+LeftClick	16.343 ns	0.1529 ns	0.1430 ns	0.0019	32 B
Original	LeftClick	100.071 ns	0.4549 ns	0.4033 ns	0.0057	96 B
PR__EDIT	LeftClick	9.861 ns	0.1375 ns	0.1219 ns	0.0019	32 B

[Benchmark]
[Arguments("Ctrl + Alt +  MiddleDoubleClick")]
[Arguments("  Ctrl +  MiddleDoubleClick  ")]
[Arguments("Ctrl+LeftClick")]
[Arguments("LeftClick")]
public MouseGesture Original(object source)
{
   return (MouseGesture)oldMouseGestureConverter.ConvertFrom(null, null, source);
}

ConvertTo Benchmarks

Method	source	Mean [ns]	Error [ns]	StdDev [ns]	Gen0	Allocated [B]
Original	Syste(...)sture [33]	183.925 ns	1.9113 ns	1.7878 ns	0.0153	256 B
PR__EDIT	Syste(...)sture [33]	23.399 ns	0.4599 ns	0.6296 ns	0.0062	104 B
Original	Syste(...)sture [33]	158.744 ns	0.6850 ns	0.6073 ns	0.0029	48 B
PR__EDIT	Syste(...)sture [33]	7.592 ns	0.0938 ns	0.0832 ns	0.0014	24 B
Original	Syste(...)sture [33]	173.100 ns	1.2477 ns	1.1060 ns	0.0086	144 B
PR__EDIT	Syste(...)sture [33]	21.988 ns	0.4355 ns	0.4074 ns	0.0052	88 B
Original	Syste(...)sture [33]	172.577 ns	1.1510 ns	0.9611 ns	0.0029	48 B
PR__EDIT	Syste(...)sture [33]	7.650 ns	0.1691 ns	0.1582 ns	0.0014	24 B

public static IEnumerable<object> GestureData
{
   get
   {
       yield return new MouseGesture(MouseAction.LeftDoubleClick, ModifierKeys.Control | ModifierKeys.Shift);
       yield return new MouseGesture(MouseAction.LeftClick, ModifierKeys.None);
       yield return new MouseGesture(MouseAction.RightDoubleClick, ModifierKeys.Control);
       yield return new MouseGesture(MouseAction.None, ModifierKeys.None);
   }
}

[Benchmark]
[ArgumentsSource(nameof(GestureData))]
public string Original(object source)
{
   return (string)oldMouseGestureConverter.ConvertTo(null, null, source, typeof(string));
}

Customer Impact

Increased performance, decreased allocations.

Regression

No.

Testing

Local build, backed with #10263.

Risk

Low, changes have been tested extensively.

There is a behavioral change related to the fact that type converters of MouseAction and ModifierKeys are no longer retrieved via TypeDescriptor.GetConverter but rather used statically, which is aligned with how KeyGestureConverter and most of other converters using other types that have converters are implemented. This provides around 80-90% of the performance benefit, allows use of ReadOnlySpan<char> to pass slices to the converter methods (as you cannot box it in object), etc.

I haven't found any evidence of people registering custom type converters for those enums, needless to say that you may just register one for MouseGesture if you wish to influence the output for this type directly.

[ThomasGoulet73]

There is a behavioral change related to the fact that type converters of MouseAction and ModifierKeys are no longer retrieved via TypeDescriptor.GetConverter but rather used statically, which is aligned with how KeyGestureConverter and most of other converters using other types that have converters are implemented. This provides around 80-90% of the performance benefit, allows use of ReadOnlySpan<char> to pass slices to the converter methods (as you cannot box it in object), etc.

Would it be possible to keep the existing behavior by first checking that the type converters registered for MouseAction and ModifierKeys is the default one ? You could then call the normal methods when there's a custom type converter and call the span methods when it's the default type converter. It would be a bit slower due to the fact that we need to check the registered type converters but at least this change wouldn't be potentially breaking.