Remove allocations on all base converters, improve TokenizerHelper

[h3xds1nz]

Description

This is probably one of the impactful PRs I've posted; optimizes conversion/parsing of all the base-generated types but also ThicknessConverter (once more), CornerRadiusConverter, KeySplineConverter and VirtualizationCacheLengthConverter.

Completely removes any allocations caused by the original Tokenizer by using ReadOnlySpan<char> instead of substrings.
I have also introduced a ref struct variant that can be used in any code that doesn't compile for net472.

MilCodeGen has also been adjusted to support this.

Single Thickness parsing (4 units)

Method	Mean [ns]	Error [ns]	StdDev [ns]	Code Size [B]	Gen0	Allocated [B]
Original	237.4 ns	4.64 ns	4.34 ns	555 B	0.0105	176 B
PR_EDITREF	191.6 ns	1.62 ns	1.51 ns	761 B	-	-

Parsing Thickness 10 times (4 units)

Method	Mean [ns]	Error [ns]	StdDev [ns]	Code Size [B]	Gen0	Allocated [B]
Original	2,434.0 ns	17.02 ns	14.08 ns	608 B	0.1030	1760 B
PR_EDITREF	1,885.1 ns	5.68 ns	4.44 ns	814 B	-	-

Creation of 10x Matrix3D via Parse

Method	Mean [ns]	Error [ns]	StdDev [ns]	Gen0	Code Size [B]	Allocated [B]
Original	7,643.3 ns	26.76 ns	20.89 ns	0.3052	6,876 B	5200 B
PR_EDIT	5,885.4 ns	67.98 ns	60.27 ns	-	7,177 B	-

Customer Impact

Improved performance by around 20% on base types, zeroed out allocations.

Regression

No.

Testing

Local build, basic testing of the tokenizer with span and some of the converters.

Risk

Low, the scope of the PR is big but with a simple change, hence any mistakes shall be easy to figure out on a DRT run.

Microsoft Reviewers: Open in CodeFlow

[h3xds1nz]

Resolved merge conflicts 12, there are better ways to spent an hour and a half.

[ThomasGoulet73]

Side note: @h3xds1nz when there's a review on a PR I recommend adding new commits and doing merges instead of rebasing. It makes it much easier to track what changed since the last review.

[ThomasGoulet73]

Could this be replaced with this:

Suggested change

	DefaultInterpolatedStringHandler handler = new(3, 4, cultureInfo, stackalloc char[64]);
	handler.AppendFormatted(keySpline.ControlPoint1.X);
	handler.AppendLiteral(listSeparator);
	handler.AppendFormatted(keySpline.ControlPoint1.Y);
	handler.AppendLiteral(listSeparator);
	handler.AppendFormatted(keySpline.ControlPoint2.X);
	handler.AppendLiteral(listSeparator);
	handler.AppendFormatted(keySpline.ControlPoint2.Y);
	return handler.ToStringAndClear();
	return string.Create(cultureInfo, stackalloc char[64], $"{keySpline.ControlPoint1.X}{listSeparator}{keySpline.ControlPoint1.Y}{listSeparator}{keySpline.ControlPoint2.X}{listSeparator}{keySpline.ControlPoint2.Y}");

I ran a quick benchmark and it seems to be as fast and doesn't require the direct use of DefaultInterpolatedStringHandler (Which isn't its intended use BTW):

Method	Mean	Error	StdDev	Ratio	RatioSD	Code Size	Gen0	Allocated	Alloc Ratio
ToString	149.9 ns	2.74 ns	2.57 ns	1.00	0.02	4,723 B	0.0024	40 B	1.00
ToStringWithStringFormat	150.2 ns	1.93 ns	1.71 ns	1.00	0.02	3,589 B	0.0024	40 B	1.00

[h3xds1nz]

I chose readibility in this case, I know it will end up in the same code generation (so your bench is correct).

It would be wiser to introduce a ValueStringBuilder variant but that's for another PR; right now I'd prefer to keep it as it is and change it later along with other places its already used.

[ThomasGoulet73]

But the fact remains that DefaultInterpolatedStringHandler is not supposed to be called directly, it is supposed to be called by the compiler (See the remarks here). We should avoid using it where possible, even if the syntax is less pretty. There are cases which are "fine" IMO like when the format specifier is not a constant or when we want to do custom formatting to the value (Like you did here: #9363).

It would be wiser to introduce a ValueStringBuilder variant but that's for another PR; right now I'd prefer to keep it as it is and change it later along with other places its already used.

If you already plan on changing it in the future I think it would be best to use DefaultInterpolatedStringHandler how it's intended (By letting the compiler do it) for now and to change it in the future.

[ThomasGoulet73]

Is the || or && at the start of the line instead of at the end of the line wrong according to the coding style ? Both seems to be used a lot and if one isn't better than the other I don't see a reason to change it.

[ThomasGoulet73]

When I wrote this I put it on its own line on purpose because I think it's cleaner when the variable is used multiple times. Is there a reason you changed it ? I know there's an IDE0018 suggestion to change it but projects like winforms or runtime keep it as a suggestion and reason might be that sometimes it's cleaner not to do it.

[h3xds1nz]

I will revert the stylistic changes to your code before merge if you think it's cleaner, as it is your code. I just auto formatted here (which does for me a tad more than pure .editorconfig).

[ThomasGoulet73]

Could you double check your using changes in this PR ? There's a couple places where the changes don't make sense to me, like this one where you change unsorts the usings (It might be caused by a bad merge conflict resolution).

[h3xds1nz]

The usings are a mess thanks to that other PR, I will have to take a look onto the files once this PR is closer to review. I was hoping it could be before P1 but maybe P2 or P3 😀