Optimize PixelFormat creation and PixelFormatConverter conversion speed

[h3xds1nz]

Description

PixelFormat is a mutable struct, so we cannot really make it readonly if we don't want to incur performance degradation during creation of one; however several non-mutating method and members shall be made readonly to prevent defensive copies in those cases, and to improve code quality. This is not a breaking change.

• Improved performance of Guid and string internal constructors up to two times.
• Removed all allocations during creation and use of those constructors (+ some private methods).
• I've converted all those big switch statements into switch expressions, increasing readibility and shaving off over 300 lines.
• Since the original ctor was unsafe and I like unsafe, I kept it yet improved the performance.
• I've removed the PartialList wrapper since it doesn't really make sense because the array ownership goes to the caller.
  • Same as Redefine PixelFormatChannelMask as readonly struct, optimize methods #9860, the difference being that the IList<PixelFormatChannelMask> wrapper now will allow setting/mutating the array via indexer, while PartialList<PixelFormatChannelMask> would throw an exception previously, so you would have to explicitly go and create new array if needed.
  • I do not think it matters, the array is owned by the caller as we make a private copy upon retrieval.
  • PixelFormatChannelMask[] cannot be NULL when returned from the getter, so the behavior is also unchanged.
  • We will swap to ReadOnlyCollection<T> as the more lightweight and universal wrapper.

"Cmyk32" (string ctor)

Method	Mean [ns]	Error [ns]	StdDev [ns]	Gen0	Code Size [B]	Allocated [B]
Original	16.926 ns	0.2629 ns	0.2459 ns	0.0024	1,701 B	40 B
PR__EDIT3	8.937 ns	0.1564 ns	0.1463 ns	-	1,819 B	-

Prgba128Float (string ctor)

Method	Mean [ns]	Error [ns]	StdDev [ns]	Gen0	Code Size [B]	Allocated [B]
Original	22.524 ns	0.2015 ns	0.1786 ns	0.0029	1,695 B	48 B
PR__EDIT3	9.165 ns	0.0451 ns	0.0377 ns	-	1,798 B	-

WICPixelFormat128bppPRGBAFloat (Guid ctor)

Method	Mean [ns]	Error [ns]	StdDev [ns]	Code Size [B]	Allocated [B]
Original	12.871 ns	0.1142 ns	0.1069 ns	240 B	-
PR__EDIT3	8.647 ns	0.0772 ns	0.0685 ns	245 B	-

WICPixelFormat64bppRGBHalf (Guid ctor)

Method	Mean [ns]	Error [ns]	StdDev [ns]	Gen0	Code Size [B]	Allocated [B]
Original	18.91 ns	0.228 ns	0.213 ns	0.0024	241 B	40 B
PR__EDIT3	10.95 ns	0.069 ns	0.061 ns	-	246 B	-

Customer Impact

Improved performance, decreased allocations, cleaner codebase.

Regression

No.

Testing

Local build, playing with the struct.

Risk

Low, don't think anyone would dare to depend on the behavior of an internal type - PartialList that wraps the array, which is to throw an exception when trying to set any members of the underlying array. Such use case feels rather weird.

Microsoft Reviewers: Open in CodeFlow

[h3xds1nz]

Resolved merge conflicts 03

[harshit7962]

Overall LGTM.
@h3xds1nz, can you please address the minor comments and resolve the conflicts.

[h3xds1nz]

@harshit7962 Conflicts resolved, convos addressed; one more thing:

Per my last point in description (now strikethrough) and our conversation with Thomas in #9860, I've decided to wrap the masks returned in Masks property in ReadOnlyCollection so it matches 100% the previous behaviour but with less allocation (originally it used the PartialList)

[harshit7962]

Re-approving

[harshit7962]

Thanks @h3xds1nz for the contribution and quick follow-ups.

[h3xds1nz]

@harshit7962 Thank you, happy to help!